diff --git a/src/wrapper.rs b/src/wrapper.rs index 58acaf29..02e3c5b2 100644 --- a/src/wrapper.rs +++ b/src/wrapper.rs @@ -1669,6 +1669,28 @@ fn rewrite_depinfo_outputs(artifacts: &ArtifactSet, anchor: &Path, mode: link::D } } +/// How to materialize one restored artifact. +/// +/// `kind` comes from the compile context, which does not always identify an +/// executable. A `[[test]] harness = false` target supplies its own `main`, so +/// cargo invokes rustc with neither `--test` nor `--crate-type`; its +/// extensionless output classifies as `Other("rustc:unknown")`, whose strategy +/// is `Hardlink` — no `0o755` on restore, and cargo then fails the run with +/// "Permission denied (os error 13)". +/// +/// The executable bit recorded at insert time is the reliable signal, and the +/// insert side already trusts it over the filename (`store::hardlink_eligible` +/// refuses to hardlink anything carrying a mode bit). Restore trusts it the +/// same way, which also keeps executables on the independent-inode path so a +/// post-build `strip` or codesign cannot reach back into the shared blob. +fn restore_link_strategy(kind: ArtifactKind, executable: bool) -> link::LinkStrategy { + if executable { + link::LinkStrategy::Copy + } else { + kind.link_strategy() + } +} + /// Materialize one cached blob at its invocation-specific output path. /// /// The caller owns target-path resolution because that is compiler-specific @@ -1736,21 +1758,21 @@ fn materialize_cached_artifact( } }; + let strategy = restore_link_strategy(kind, cached_file.executable); + match transformed { Some(content) => { - link::write_restored(target_path, &content, kind.link_strategy()) + link::write_restored(target_path, &content, strategy) .with_context(|| format!("{context}: writing {}", target_path.display()))?; } None => { - link::link_to_target(&store_path, target_path, kind.link_strategy()).with_context( - || { - format!( - "{context}: linking {} -> {}", - store_path.display(), - target_path.display() - ) - }, - )?; + link::link_to_target(&store_path, target_path, strategy).with_context(|| { + format!( + "{context}: linking {} -> {}", + store_path.display(), + target_path.display() + ) + })?; } } @@ -3853,6 +3875,90 @@ mod tests { ); } + /// A `[[test]] harness = false` target is compiled without `--test` and + /// without `--crate-type`, so its extensionless output classifies as + /// `Other("rustc:unknown")` — the compile context simply never says + /// "executable". The mode bit recorded at insert time does, and restore + /// must honour it: otherwise the restored test binary comes back 0o644 and + /// cargo fails the run with "Permission denied (os error 13)". + #[cfg(unix)] + #[test] + fn materialize_cached_artifact_restores_executable_bit_recorded_at_insert() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let config = test_config(dir.path().join("cache")); + let store = Store::open(&config).unwrap(); + let hash = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + create_blob(&store, hash, b"\x7fELF harness=false test binary"); + // Store blobs are read-only and carry no executable bit, so a restore + // that never chmods cannot produce a runnable file. + std::fs::set_permissions( + store.blob_path(hash), + std::fs::Permissions::from_mode(0o444), + ) + .unwrap(); + + let mut cached = cached_file("harness-a1b2c3d4e5f60718", hash); + cached.executable = true; + let target = dir.path().join("target").join("harness-a1b2c3d4e5f60718"); + let platform = platform::current(); + + materialize_cached_artifact( + &BlobSource::Store(&store), + &cached, + &target, + ArtifactKind::Other("rustc:unknown"), + dir.path(), + &*platform, + "test restore", + ) + .unwrap(); + + let mode = std::fs::metadata(&target).unwrap().permissions().mode(); + assert_ne!( + mode & 0o111, + 0, + "restored test binary must stay executable, got {mode:o}" + ); + } + + /// The converse: an artifact that was not executable at insert time must + /// not acquire the bit on restore. + #[cfg(unix)] + #[test] + fn materialize_cached_artifact_leaves_non_executable_artifacts_unexecutable() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let config = test_config(dir.path().join("cache")); + let store = Store::open(&config).unwrap(); + let hash = "cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"; + create_blob(&store, hash, b"rlib bytes"); + + let cached = cached_file("libfoo.rlib", hash); + let target = dir.path().join("target").join("libfoo.rlib"); + let platform = platform::current(); + + materialize_cached_artifact( + &BlobSource::Store(&store), + &cached, + &target, + ArtifactKind::Library, + dir.path(), + &*platform, + "test restore", + ) + .unwrap(); + + let mode = std::fs::metadata(&target).unwrap().permissions().mode(); + assert_eq!( + mode & 0o111, + 0, + "library must not become executable, got {mode:o}" + ); + } + // ── fallback wrapper ───────────────────────────────────────────── #[test] diff --git a/test-projects/custom-harness/Cargo.lock b/test-projects/custom-harness/Cargo.lock new file mode 100644 index 00000000..a768de3b --- /dev/null +++ b/test-projects/custom-harness/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "custom-harness" +version = "0.1.0" diff --git a/test-projects/custom-harness/Cargo.toml b/test-projects/custom-harness/Cargo.toml new file mode 100644 index 00000000..857546fc --- /dev/null +++ b/test-projects/custom-harness/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "custom-harness" +version = "0.1.0" +edition = "2021" + +# Empty `[workspace]` — see test-projects/multi-dep/Cargo.toml for why. +[workspace] + +# The point of this fixture: a test target that supplies its own `main`. +# Cargo compiles it with neither `--test` nor `--crate-type`, so the +# extensionless output carries no "this is an executable" signal in the rustc +# argv — only the mode bit on the file itself. +[[test]] +name = "harness" +harness = false diff --git a/test-projects/custom-harness/src/lib.rs b/test-projects/custom-harness/src/lib.rs new file mode 100644 index 00000000..43259336 --- /dev/null +++ b/test-projects/custom-harness/src/lib.rs @@ -0,0 +1,3 @@ +pub fn answer() -> u32 { + 42 +} diff --git a/test-projects/custom-harness/tests/harness.rs b/test-projects/custom-harness/tests/harness.rs new file mode 100644 index 00000000..3d8f699a --- /dev/null +++ b/test-projects/custom-harness/tests/harness.rs @@ -0,0 +1,7 @@ +//! A test target with `harness = false`: cargo runs this binary directly, so +//! it must come back from the cache executable. + +fn main() { + assert_eq!(custom_harness::answer(), 42); + println!("custom harness ok"); +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index f74067ca..f029e493 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -97,3 +97,22 @@ fn bootstrap_kache() -> Result { pub fn isolated_config_path(cache_dir: &Path) -> PathBuf { cache_dir.join("config.toml") } + +/// A scratch directory on the *repository's* filesystem, for tests whose +/// subject is how artifacts are materialized. +/// +/// `TempDir::new()` lands in `TMPDIR`, so a cache placed there sits on a +/// different filesystem from the build — or on tmpfs, which supports no +/// reflink. Either way the store ingest and the restore take different paths +/// than they would in a real build, and those paths differ in what file mode +/// they leave behind. Anchoring here keeps such tests on whatever filesystem +/// the developer or CI actually builds on, and `cargo clean` reclaims it. +/// +/// Only the tests that materialize artifacts need this, and each integration +/// test binary compiles this module separately, so the rest see it as dead. +#[allow(dead_code)] +pub fn scratch_dir() -> PathBuf { + let dir = bootstrap_target_dir().with_file_name("kache-test-scratch"); + std::fs::create_dir_all(&dir).expect("creating scratch dir"); + dir +} diff --git a/tests/custom_harness_test.rs b/tests/custom_harness_test.rs new file mode 100644 index 00000000..8b9e0b17 --- /dev/null +++ b/tests/custom_harness_test.rs @@ -0,0 +1,127 @@ +//! End-to-end guard for restoring `[[test]] harness = false` binaries. +//! +//! A custom-harness test target supplies its own `main`, so cargo compiles it +//! with neither `--test` nor `--crate-type`. Its extensionless output therefore +//! classifies as `Other("rustc:unknown")`, whose link strategy is `Hardlink` — +//! and the restore path used to derive the output's permissions from that +//! strategy alone. The restored binary came back without its executable bit and +//! cargo failed the run with "Permission denied (os error 13)". +//! +//! The build here is a real one through the real wrapper: warm the cache, throw +//! the target directory away so every artifact has to come back from the cache, +//! then let cargo execute the restored binary. +//! +//! The restored mode depends on which materialization path runs, and only one +//! of them preserves the bit by accident: +//! +//! | path | when | mode | +//! |---|---|---| +//! | reflink | cache and target on one CoW filesystem | fresh file at the umask | +//! | hardlink | one filesystem, blob copy-ingested | blob's `0o555` — survives | +//! | hardlink | one filesystem, blob reflink-ingested | blob's `0o444` | +//! | copy | cache and target on different filesystems | `0o644`, hardcoded | +//! +//! So the only configuration that accidentally works is a same-filesystem +//! hardlink from a copy-ingested blob — which is what a plain ext4 checkout +//! with `TMPDIR` alongside it produces, and why this went unnoticed. +//! +//! Running under `TMPDIR` would land in exactly that blind spot, so this test +//! uses the repository's filesystem instead. `wrapper::tests` asserts the same +//! contract directly, without depending on any of it. + +use std::path::Path; + +use tempfile::TempDir; + +mod common; +use common::{build_kache, isolated_config_path, kache_binary, scratch_dir}; + +fn copy_dir(src: &Path, dst: &Path) { + std::fs::create_dir_all(dst).unwrap(); + for entry in std::fs::read_dir(src).unwrap() { + let entry = entry.unwrap(); + let dest_path = dst.join(entry.file_name()); + if entry.file_type().unwrap().is_dir() { + copy_dir(&entry.path(), &dest_path); + } else { + std::fs::copy(entry.path(), &dest_path).unwrap(); + } + } +} + +fn copy_fixture() -> TempDir { + let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("test-projects/custom-harness"); + let tmp = TempDir::new_in(scratch_dir()).unwrap(); + copy_dir(&fixture, tmp.path()); + tmp +} + +/// Runs `cargo test --test harness` through the kache wrapper. +fn cargo_test(project: &Path, cache_dir: &Path, target_dir: &Path) -> std::process::Output { + std::process::Command::new("cargo") + .args(["test", "--test", "harness"]) + .current_dir(project) + .env("RUSTC_WRAPPER", kache_binary()) + .env("KACHE_CACHE_DIR", cache_dir) + .env("KACHE_CONFIG", isolated_config_path(cache_dir)) + .env("CARGO_TARGET_DIR", target_dir) + .env("CARGO_INCREMENTAL", "0") + .output() + .expect("failed to run cargo test") +} + +/// The restored test binary must still be runnable. +#[test] +fn custom_harness_test_binary_is_executable_after_restore() { + build_kache(); + let project = copy_fixture(); + let cache = TempDir::new_in(scratch_dir()).unwrap(); + let target = TempDir::new_in(scratch_dir()).unwrap(); + + let cold = cargo_test(project.path(), cache.path(), target.path()); + assert!( + cold.status.success(), + "cold build failed.\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&cold.stdout), + String::from_utf8_lossy(&cold.stderr), + ); + + // Drop every build output so the second run has to restore the test binary + // from the cache rather than reuse the one the linker just produced. + std::fs::remove_dir_all(target.path()).unwrap(); + std::fs::create_dir_all(target.path()).unwrap(); + + let warm = cargo_test(project.path(), cache.path(), target.path()); + assert!( + warm.status.success(), + "restored harness=false test binary could not be run.\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&warm.stdout), + String::from_utf8_lossy(&warm.stderr), + ); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + + let binary = std::fs::read_dir(target.path().join("debug").join("deps")) + .unwrap() + .filter_map(Result::ok) + .map(|entry| entry.path()) + .find(|path| { + path.extension().is_none() + && path + .file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.starts_with("harness-")) + }) + .expect("restored harness test binary"); + + let mode = std::fs::metadata(&binary).unwrap().permissions().mode(); + assert_ne!( + mode & 0o111, + 0, + "{} was restored without its executable bit (mode {mode:o})", + binary.display() + ); + } +}