From d07ce9928421bbdd12adb254ec010bac307d98cf Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 14 Nov 2025 15:35:24 +0100 Subject: [PATCH 1/3] Add new `rename-from` field to ensure checks when `source` URL file differs from `name` --- README.md | 4 ++ src/main.rs | 10 ++++ src/manifest.rs | 127 +++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 117 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 72e7bb2..5846806 100644 --- a/README.md +++ b/README.md @@ -31,6 +31,10 @@ the TOML files in the `files/` directory. Each entry has the following schema: artifacts built from open source code you should put the license identifier, for everything else you should put a link to the licensing terms. +* **`rename-from`**: in case the `source` file has a different name than `name`, + you need to add this field to explicitly mark that this is expected with the + file name from `source`. + You can add a new entry either by manually modifying a TOML file in the `files` directory, or by using the following command: diff --git a/src/main.rs b/src/main.rs index c46f476..90a71db 100644 --- a/src/main.rs +++ b/src/main.rs @@ -167,11 +167,21 @@ async fn add_file(args: AddFileArgs) -> anyhow::Result<()> { .append(true) .open(&args.toml_file)?; + let rename_from = if let Some(file_name) = args.url.path().split('/').last() + && let Some(path_name) = args.path.split('/').last() + && file_name != path_name + { + Some(file_name.to_string()) + } else { + None + }; + let entry = ManifestFileManaged::new( args.path, to_hex(&hash), args.url, args.license.unwrap_or(String::new()), + rename_from, ); let entry = toml::to_string(&entry)?; diff --git a/src/manifest.rs b/src/manifest.rs index 329d0a1..5bfb3ce 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -44,44 +44,106 @@ struct LocationCache { pub(crate) fn load_manifests(load_from: &Path) -> Result<(Vec, Vec), Error> { let mut result = Vec::new(); let mut cache = LocationCache::default(); + let mut errors = Vec::new(); fn load_inner( load_from: &Path, result: &mut Vec, cache: &mut LocationCache, + errors: &mut Vec, ) -> anyhow::Result<()> { for entry in load_from.read_dir()? { let path = entry?.path(); if path.is_file() && path.extension().and_then(|s| s.to_str()) == Some("toml") { - let manifest = std::fs::read_to_string(&path) + let file_source = std::fs::read_to_string(&path) + .map_err(Error::from) + .with_context(|| format!("failed to read {}", path.display()))?; + let manifest = toml::from_str::(&file_source) .map_err(Error::from) - .and_then(|raw| toml::from_str::(&raw).map_err(Error::from)) .with_context(|| format!("failed to read {}", path.display()))?; record_locations(&path, &manifest, cache); for file in manifest.files { - result.push(match file.into_inner() { + let mirror_file = match file.into_inner() { ManifestFile::Legacy(legacy) => MirrorFile { name: legacy.name, sha256: legacy.sha256, source: Source::Legacy, + rename_from: None, }, ManifestFile::Managed(managed) => MirrorFile { name: managed.name, sha256: managed.sha256, source: Source::Url(managed.source), + rename_from: managed.rename_from, }, - }); + }; + if let Source::Url(ref source) = mirror_file.source { + if let Some(file_name) = source.path().split('/').last() + && let Some(path_name) = mirror_file.name.split('/').last() + { + match mirror_file.rename_from { + Some(ref rename_from) => { + if path_name == file_name { + let location = cache + .seen_paths + .get(&mirror_file.name) + .unwrap() + .first() + .unwrap(); + let (src_line, snippet) = span_info(&file_source, location); + errors.push(format!( + "`rename-from` field isn't needed since `source` and `name` field have the same file name (`{file_name}`):\n\ + # {} (line {src_line})\n{snippet}\n", + location.file.display() + )); + } else if path_name != file_name && rename_from != file_name { + let location = cache + .seen_paths + .get(&mirror_file.name) + .unwrap() + .first() + .unwrap(); + let (src_line, snippet) = span_info(&file_source, location); + errors.push(format!( + "`rename-from` field value doesn't match name from the URL `{source}` (`{file_name}` != `{rename_from}`):\n\ + # {} (line {src_line})\n{snippet}\n", + location.file.display() + )); + } + } + None => { + if path_name != file_name { + let location = cache + .seen_paths + .get(&mirror_file.name) + .unwrap() + .first() + .unwrap(); + let (src_line, snippet) = span_info(&file_source, location); + errors.push(format!( + "The name from the URL `{source}` doesn't match the `name` field (`{file_name}` != `{path_name}`). \ + Add `rename-from = {file_name:?}` to fix this error:\n\ + # {} (line {src_line})\n{snippet}\n", + location.file.display() + )); + } + } + } + } + } + result.push(mirror_file); } } else if path.is_dir() { - load_inner(&path, result, cache)?; + load_inner(&path, result, cache, errors)?; } } Ok(()) } - load_inner(load_from, &mut result, &mut cache)?; - Ok((result, find_errors(cache))) + load_inner(load_from, &mut result, &mut cache, &mut errors)?; + find_errors(cache, &mut errors); + Ok((result, errors)) } fn record_locations(toml_path: &Path, manifest: &Manifest, cache: &mut LocationCache) { @@ -114,12 +176,32 @@ fn record_locations(toml_path: &Path, manifest: &Manifest, cache: &mut LocationC .or_default() .insert(location.clone()); if let Some(url) = url { - cache.seen_urls.entry(url).or_default().insert(location); + cache + .seen_urls + .entry(url) + .or_default() + .insert(location.clone()); + } + } +} + +fn span_info<'a>(content: &'a str, location: &Location) -> (usize, &'a str) { + // Find the corresponding line number + let mut accumulated_chars = 0; + let mut src_line = 0; + for (index, line) in content.lines().enumerate() { + accumulated_chars += line.len() + 1; // +1 for newline + if accumulated_chars > location.span.0.start { + src_line = index + 1; + break; } } + + let snippet = &content[location.span.0.start..location.span.0.end]; + (src_line, snippet) } -fn find_errors(cache: LocationCache) -> Vec { +fn find_errors(cache: LocationCache, errors: &mut Vec) { let mut file_cache: HashMap = HashMap::new(); fn format_locations( @@ -136,18 +218,7 @@ fn find_errors(cache: LocationCache) -> Vec { }) }); - // Find the corresponding line number - let mut accumulated_chars = 0; - let mut src_line = 0; - for (index, line) in content.lines().enumerate() { - accumulated_chars += line.len() + 1; // +1 for newline - if accumulated_chars > location.span.0.start { - src_line = index + 1; - break; - } - } - - let snippet = &content[location.span.0.start..location.span.0.end]; + let (src_line, snippet) = span_info(&content, location); writeln!( output, "# {} (line {src_line})\n{snippet}\n", @@ -159,7 +230,6 @@ fn find_errors(cache: LocationCache) -> Vec { output } - let mut errors = Vec::new(); for (path, locations) in cache.seen_paths { if locations.len() > 1 { errors.push(format!( @@ -184,13 +254,13 @@ fn find_errors(cache: LocationCache) -> Vec { )); } } - errors } pub(crate) struct MirrorFile { pub(crate) name: String, pub(crate) sha256: String, pub(crate) source: Source, + pub(crate) rename_from: Option, } pub(crate) enum Source { @@ -233,15 +303,24 @@ pub struct ManifestFileManaged { // This field is not considered at all by the automation, we just enforce its presence so that // people adding new entries think about the licensing implications. license: String, + #[serde(default, rename = "rename-from")] + rename_from: Option, } impl ManifestFileManaged { - pub fn new(name: String, sha256: String, source: Url, license: String) -> Self { + pub fn new( + name: String, + sha256: String, + source: Url, + license: String, + rename_from: Option, + ) -> Self { Self { name, sha256, source, license, + rename_from, } } } From af169da0e1689992aa5d85a92a179dd40180ac50 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 17 Nov 2025 12:13:45 +0100 Subject: [PATCH 2/3] Move common error code into an inner function --- src/manifest.rs | 88 ++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 38 deletions(-) diff --git a/src/manifest.rs b/src/manifest.rs index 5bfb3ce..5647389 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -46,6 +46,27 @@ pub(crate) fn load_manifests(load_from: &Path) -> Result<(Vec, Vec, + ) { + let location = cache + .seen_paths + .get(&mirror_file.name) + .unwrap() + .first() + .unwrap(); + let (src_line, snippet) = span_info(&file_source, location); + errors.push(format!( + "{error}:\n\ + # {} (line {src_line})\n{snippet}\n", + location.file.display() + )); + } + fn load_inner( load_from: &Path, result: &mut Vec, @@ -85,48 +106,39 @@ pub(crate) fn load_manifests(load_from: &Path) -> Result<(Vec, Vec { if path_name == file_name { - let location = cache - .seen_paths - .get(&mirror_file.name) - .unwrap() - .first() - .unwrap(); - let (src_line, snippet) = span_info(&file_source, location); - errors.push(format!( - "`rename-from` field isn't needed since `source` and `name` field have the same file name (`{file_name}`):\n\ - # {} (line {src_line})\n{snippet}\n", - location.file.display() - )); - } else if path_name != file_name && rename_from != file_name { - let location = cache - .seen_paths - .get(&mirror_file.name) - .unwrap() - .first() - .unwrap(); - let (src_line, snippet) = span_info(&file_source, location); - errors.push(format!( - "`rename-from` field value doesn't match name from the URL `{source}` (`{file_name}` != `{rename_from}`):\n\ - # {} (line {src_line})\n{snippet}\n", - location.file.display() - )); + emit_error( + format!( + "`rename-from` field isn't needed since `source` and `name` field have the same file name (`{file_name}`)" + ), + &mirror_file, + &file_source, + cache, + errors, + ); + } else if rename_from != file_name { + emit_error( + format!( + "`rename-from` field value doesn't match name from the URL `{source}` (`{file_name}` != `{rename_from}`)" + ), + &mirror_file, + &file_source, + cache, + errors, + ); } } None => { if path_name != file_name { - let location = cache - .seen_paths - .get(&mirror_file.name) - .unwrap() - .first() - .unwrap(); - let (src_line, snippet) = span_info(&file_source, location); - errors.push(format!( - "The name from the URL `{source}` doesn't match the `name` field (`{file_name}` != `{path_name}`). \ - Add `rename-from = {file_name:?}` to fix this error:\n\ - # {} (line {src_line})\n{snippet}\n", - location.file.display() - )); + emit_error( + format!( + "The name from the URL `{source}` doesn't match the `name` field (`{file_name}` != `{path_name}`). \ + Add `rename-from = {file_name:?}` to fix this error" + ), + &mirror_file, + &file_source, + cache, + errors, + ); } } } From b08da67c21db665c085bbac36a05488b04309d6f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 14 Nov 2025 15:35:39 +0100 Subject: [PATCH 3/3] Fix new error in `files/gha-self-hosted.toml` --- files/gha-self-hosted.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/files/gha-self-hosted.toml b/files/gha-self-hosted.toml index 37ae37c..35fbd24 100644 --- a/files/gha-self-hosted.toml +++ b/files/gha-self-hosted.toml @@ -3,3 +3,4 @@ name = "gha-self-hosted/qemu-efi-aarch64_2022.11-6.deb" source = "http://ftp.debian.org/debian/pool/main/e/edk2/qemu-efi-aarch64_2022.11-6+deb12u2_all.deb" sha256 = "9a55c7b94fdf13a28928359a77d42e5364aa3ae2e558bd1fd5361955bf479d81" license = "BSD" +rename-from = "qemu-efi-aarch64_2022.11-6+deb12u2_all.deb"