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/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" 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..5647389 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -44,44 +44,118 @@ 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 emit_error( + error: String, + mirror_file: &MirrorFile, + file_source: &str, + cache: &LocationCache, + errors: &mut 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, 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 { + 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 { + 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, + ); + } + } + } + } + } + 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 +188,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 find_errors(cache: LocationCache) -> Vec { +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, errors: &mut Vec) { let mut file_cache: HashMap = HashMap::new(); fn format_locations( @@ -136,18 +230,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 +242,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 +266,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 +315,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, } } }