diff --git a/xtask/src/assets.rs b/xtask/src/assets.rs index d0c825da0..8c9b14080 100644 --- a/xtask/src/assets.rs +++ b/xtask/src/assets.rs @@ -23,13 +23,23 @@ pub fn missing(files: &[String], assets_md: &str) -> Vec { files.iter().filter(|f| is_asset(f) && !assets_md.contains(&format!("`{f}`"))).cloned().collect() } +/// The paths in `git ls-files -z` output: NUL-separated and never quoted, so a name with +/// non-ASCII letters (`"assets/\303\251.png"` without `-z`) or a newline is still checked. +fn git_paths(stdout: &[u8]) -> Vec { + stdout.split(|b| *b == 0).filter(|p| !p.is_empty()).map(|p| String::from_utf8_lossy(p).into_owned()).collect() +} + pub fn run(root: &Path) -> Result<(), String> { let out = Command::new("git") .current_dir(root) - .args(["ls-files", "--cached", "--others", "--exclude-standard"]) + .args(["ls-files", "-z", "--cached", "--others", "--exclude-standard"]) .output() .map_err(|e| format!("git ls-files: {e}"))?; - let files: Vec = String::from_utf8_lossy(&out.stdout).lines().filter(|l| root.join(l).exists()).map(str::to_owned).collect(); + // A failed listing is no listing: read as empty, it would pass with nothing checked (#1364). + if !out.status.success() { + return Err(format!("git ls-files failed ({}): {}", out.status, String::from_utf8_lossy(&out.stderr).trim())); + } + let files: Vec = git_paths(&out.stdout).into_iter().filter(|l| root.join(l).exists()).collect(); let md = std::fs::read_to_string(root.join("ATTRIBUTION.md")).map_err(|e| format!("ATTRIBUTION.md: {e}"))?; let miss = missing(&files, &md); if miss.is_empty() { @@ -52,4 +62,26 @@ mod tests { assert!(!is_asset("crates/x/src/lib.rs")); assert!(is_asset("assets/fonts/OFL.txt")); } + + #[test] + fn reads_raw_nul_separated_paths() { + assert_eq!(git_paths(b"assets/\xc3\xa9 b.png\0docs/images/x.svg\0"), vec!["assets/é b.png".to_string(), "docs/images/x.svg".into()]); + assert!(git_paths(b"").is_empty()); + } + + #[test] + fn a_failed_file_listing_fails_the_check() { + // #1364: outside a git checkout `git ls-files` fails, and its empty output read as "no + // files" passed the check. + let dir = std::env::temp_dir().join(format!("wordcraft-xtask-assets-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("ATTRIBUTION.md"), "").unwrap(); + let inside_git = Command::new("git").current_dir(&dir).args(["rev-parse", "--git-dir"]).output().is_ok_and(|o| o.status.success()); + let result = run(&dir); + let _ = std::fs::remove_dir_all(&dir); + if inside_git { + return; // the temporary folder is in a checkout here: nothing to test + } + assert!(result.as_ref().is_err_and(|e| e.starts_with("git ls-files failed")), "{result:?}"); + } }