fix(mission_fs): drop build output when collecting work back from a container
`pack_dir` (host -> container) skips `transport_excludes`; `copy_out`
(container -> host) is the raw Docker archive API and carries the whole tree,
`target/` included. The asymmetry was invisible for as long as the runtime
image had no cmake — nothing could compile, so no `target/` existed.
The moment missions could actually build, every collection died on a build
artifact:
failed to unpack `…/repo/target/debug/build/ahash-…/build_script_build-…`
`phase_runner` then correctly refused to capture, rather than record a stale
tree as an empty diff — so mission 01a00c57's coding phase, which had done the
work, delivered nothing and retried forever. A fix that let missions compile
created a delivery failure one layer down.
`unpack_into` now skips excluded entries by NAME at any depth (a workspace has
a `target/` per crate) and logs how many it dropped.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
99dd29cc8a
commit
d341640255
@@ -128,8 +128,52 @@ pub fn unpack_into(archive: &[u8], dest: &Path) -> Result<(), String> {
|
||||
// Ownership in the archive is the container's root; re-applying it on the
|
||||
// host would recreate the very uid split this module exists to remove.
|
||||
ar.set_preserve_permissions(false);
|
||||
ar.unpack(dest)
|
||||
.map_err(|e| format!("unpack into {}: {e}", dest.display()))
|
||||
|
||||
// Filter on the way OUT as well as on the way in.
|
||||
//
|
||||
// `pack_dir` (host -> container) skips `transport_excludes`, but `copy_out`
|
||||
// (container -> host) is the raw Docker archive API, which carries the whole
|
||||
// tree — `target/` included. The asymmetry was invisible for as long as the
|
||||
// runtime image had no `cmake`, because nothing could compile and no
|
||||
// `target/` existed. The moment missions could build, every collection
|
||||
// failed on a build artifact:
|
||||
//
|
||||
// failed to unpack `…/repo/target/debug/build/ahash-…/build_script_build-…`
|
||||
//
|
||||
// and `phase_runner` correctly refused to capture a stale tree — so a
|
||||
// coding phase that HAD done the work delivered nothing, retrying forever.
|
||||
//
|
||||
// Entries are skipped by NAME at any depth, the same rule `is_excluded`
|
||||
// uses, because a workspace has a `target/` per crate.
|
||||
let mut skipped = 0usize;
|
||||
for entry in ar
|
||||
.entries()
|
||||
.map_err(|e| format!("read archive for {}: {e}", dest.display()))?
|
||||
{
|
||||
let mut entry = entry.map_err(|e| format!("read entry for {}: {e}", dest.display()))?;
|
||||
let path = entry
|
||||
.path()
|
||||
.map_err(|e| format!("entry path for {}: {e}", dest.display()))?
|
||||
.into_owned();
|
||||
if path
|
||||
.components()
|
||||
.any(|c| is_excluded(&c.as_os_str().to_string_lossy()))
|
||||
{
|
||||
skipped += 1;
|
||||
continue;
|
||||
}
|
||||
entry
|
||||
.unpack_in(dest)
|
||||
.map_err(|e| format!("unpack into {}: {e}", dest.display()))?;
|
||||
}
|
||||
if skipped > 0 {
|
||||
eprintln!(
|
||||
"mission_fs: unpack into {} skipped {skipped} excluded entr{} (build output)",
|
||||
dest.display(),
|
||||
if skipped == 1 { "y" } else { "ies" }
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Copy a host directory into a running container at [`CONTAINER_MISSION_DIR`].
|
||||
@@ -327,6 +371,44 @@ mod tests {
|
||||
std::fs::write(root.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap();
|
||||
}
|
||||
|
||||
/// Build output must be dropped on the way BACK, not only on the way out.
|
||||
///
|
||||
/// `copy_out` uses the raw Docker archive API, which carries `target/`
|
||||
/// whatever `pack_dir` did. Unpacking it failed on a build-script binary
|
||||
/// and took the whole collection down with it, so a coding phase that had
|
||||
/// really done the work delivered nothing.
|
||||
#[test]
|
||||
fn unpacking_drops_build_output_but_keeps_the_source() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let src = tmp.path().join("repo");
|
||||
std::fs::create_dir_all(src.join("src")).unwrap();
|
||||
std::fs::create_dir_all(src.join("target/debug/build")).unwrap();
|
||||
std::fs::create_dir_all(src.join("crates/inner/target")).unwrap();
|
||||
std::fs::write(src.join("src/lib.rs"), "pub fn x() {}\n").unwrap();
|
||||
std::fs::write(src.join("target/debug/build/script"), "ELF").unwrap();
|
||||
std::fs::write(src.join("crates/inner/target/blob"), "ELF").unwrap();
|
||||
|
||||
// Built WITHOUT the filter, the way the Docker API hands it to us.
|
||||
let mut buf = Vec::new();
|
||||
{
|
||||
let mut b = tar::Builder::new(&mut buf);
|
||||
b.append_dir_all("repo", &src).unwrap();
|
||||
b.finish().unwrap();
|
||||
}
|
||||
|
||||
let dest = tmp.path().join("out");
|
||||
unpack_into(&buf, &dest).expect("must not fail on build output");
|
||||
assert!(dest.join("repo/src/lib.rs").is_file(), "source must survive");
|
||||
assert!(
|
||||
!dest.join("repo/target").exists(),
|
||||
"root target/ must be dropped"
|
||||
);
|
||||
assert!(
|
||||
!dest.join("repo/crates/inner/target").exists(),
|
||||
"a per-crate target/ must be dropped too — matched by NAME at any depth"
|
||||
);
|
||||
}
|
||||
|
||||
/// A checkout must survive the round trip intact — including `.git`,
|
||||
/// without which the whole delivery path (diff, commit, push) is dead.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user