xtask assets: fail when git ls-files fails, read raw NUL-separated paths (#1364) (#1401)

The attribution check spawned `git ls-files` but never looked at its
exit status: outside a usable checkout, or on a git error, the empty
output read as "no files" and the check passed with nothing checked.
A non-zero status is now an error with git's message.

The list is read NUL-separated (`-z`): without it git quotes a name
with non-ASCII letters ("assets/\303\251.png"), which matched no file
on disk and was silently left out of the check.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

xtask/src/assets.rs

@@ -23,13 +23,23 @@ pub fn missing(files: &[String], assets_md: &str) -> Vec<String> {
     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<String> {
+    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> = 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<String> = 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:?}");
+    }
 }