Эх сурвалжийг харах

build: compute_dir_checksum hashes symlinks by target text (EISDIR on dir-links killed sm121 build on the vendored kindling tree: vllm -> .submodules/spark-agent/vllm); upload tar dereferences separately

Alexander Korolev 3 долоо хоног өмнө
parent
commit
94b400a0be

+ 25 - 8
src/docker/build_dir.rs

@@ -248,11 +248,12 @@ fn glob_match(pat: &str, name: &str) -> bool {
 
 pub fn compute_dir_checksum(dir: &Path) -> Result<String> {
     let mut hasher = Sha256::new();
-    let mut entries: Vec<_> = Vec::new();
+    let mut entries: Vec<(String, Vec<u8>)> = Vec::new();
 
     for entry in walkdir::WalkDir::new(dir).sort_by_file_name() {
         let entry = entry?;
-        if entry.file_type().is_dir() {
+        let file_type = entry.file_type();
+        if file_type.is_dir() {
             continue;
         }
         let rel = entry
@@ -261,17 +262,33 @@ pub fn compute_dir_checksum(dir: &Path) -> Result<String> {
             .unwrap_or(entry.path())
             .to_string_lossy()
             .to_string();
-        entries.push((rel, entry.path().to_path_buf()));
+        if file_type.is_symlink() {
+            // A symlink hashes as its dotted target text, never followed:
+            // build contexts may link a directory (e.g. an upstream `vllm ->
+            // .submodules/spark-agent/vllm`), and opening it reads a
+            // directory (EISDIR) or drags a whole tree into the digest.
+            // The upload tar dereferences links, so the shipped bytes and
+            // the digest stay independent concerns.
+            let target = std::fs::read_link(entry.path())?;
+            entries.push((
+                rel,
+                format!("symlink\0{}", target.to_string_lossy()).into_bytes(),
+            ));
+            continue;
+        }
+        if !file_type.is_file() {
+            continue;
+        }
+        let mut buf = Vec::new();
+        std::fs::File::open(entry.path())?.read_to_end(&mut buf)?;
+        entries.push((rel, buf));
     }
     entries.sort_by(|a, b| a.0.cmp(&b.0));
 
-    for (rel, path) in &entries {
-        let mut file = std::fs::File::open(path)?;
-        let mut buf = Vec::new();
-        file.read_to_end(&mut buf)?;
+    for (rel, buf) in &entries {
         hasher.update(rel.as_bytes());
         hasher.update(b"\0");
-        hasher.update(&buf);
+        hasher.update(buf);
     }
 
     let hash = hasher.finalize();

+ 42 - 0
src/docker/tests.rs

@@ -441,3 +441,45 @@ mod solo_tests;
 
 #[path = "sglang_tests.rs"]
 mod sglang_tests;
+
+/// Symlinks in a build context (upstream repos may link a directory, e.g.
+/// `vllm -> .submodules/spark-agent/vllm`) must not be dereferenced: reading
+/// one as a file returns EISDIR and killed `sm121 build` before the first
+/// byte. They hash as their dotted target text instead — the upload tar
+/// dereferences separately (`tar --dereference`), so the digest tracks the
+/// tree shape, not the copied bytes.
+#[test]
+fn dir_checksum_hashes_symlinks_by_target_text() {
+    use super::build_dir::compute_dir_checksum;
+
+    let root = std::env::temp_dir().join(format!(
+        "sm121-chksum-link-{}",
+        std::process::id()
+    ));
+    let dir = root.join("ctx");
+    let _ = std::fs::remove_dir_all(&root);
+    // Linked target lives OUTSIDE the hashed tree (like an upstream submodule
+    // checkout outside the build context's flattened Docker view).
+    std::fs::create_dir_all(root.join("linked")).unwrap();
+    std::fs::write(root.join("linked/status-server.py"), b"# py").unwrap();
+    std::fs::create_dir_all(&dir).unwrap();
+    std::fs::write(dir.join("top.txt"), b"top").unwrap();
+    std::os::unix::fs::symlink("../linked", dir.join("vllm")).unwrap();
+
+    // Regression: previously File::open on the directory link -> EISDIR.
+    let first = compute_dir_checksum(&dir).unwrap();
+    assert_eq!(first, compute_dir_checksum(&dir).unwrap(), "deterministic");
+
+    // The target lives outside and is never followed: edits there do not
+    // move the digest.
+    std::fs::write(root.join("linked/status-server.py"), b"# changed")
+        .unwrap();
+    assert_eq!(first, compute_dir_checksum(&dir).unwrap());
+
+    // The link's own text is part of the digest: retarget -> different hash.
+    std::fs::remove_file(dir.join("vllm")).unwrap();
+    std::os::unix::fs::symlink("../top.txt", dir.join("vllm")).unwrap();
+    assert_ne!(first, compute_dir_checksum(&dir).unwrap());
+
+    let _ = std::fs::remove_dir_all(&root);
+}