From 59221f99197edd32582de232afcdffe12ccdef9a Mon Sep 17 00:00:00 2001 From: Matt Mastracci Date: Thu, 2 Apr 2026 10:21:34 -0600 Subject: [PATCH 01/11] feat: strip basedirs from Rust hash key for cross-machine cache hits SCCACHE_BASEDIRS now normalizes cwd, CARGO_MANIFEST_DIR, CARGO_WORKSPACE_DIR, CARGO_TARGET_TMPDIR, CARGO_MANIFEST_PATH, CARGO_BIN_EXE_*, dep-info env var values, and the concatenated argument string in the Rust compiler's hash key computation. This enables cache hits when the same crate is compiled from different absolute paths on different machines (e.g., CI runners with different checkout roots). strip_basedir_prefix now also matches when the value equals the basedir minus its trailing '/', so `cwd == basedir` strips to the empty string rather than passing through. Without this, two machines with different checkout paths produced different hashes even with matching basedirs -- the feature's central claim. --- src/cache/readonly.rs | 9 +- src/compiler/clang.rs | 2 +- src/compiler/compiler.rs | 10 +- src/compiler/diab.rs | 2 +- src/compiler/gcc.rs | 6 +- src/compiler/msvc.rs | 4 +- src/compiler/rust.rs | 459 +++++++++++++++++++++++++++++++++++-- src/compiler/tasking_vx.rs | 4 +- src/test/mock_storage.rs | 32 ++- tests/helpers/mod.rs | 12 +- tests/sccache_cargo.rs | 89 +++++++ 11 files changed, 585 insertions(+), 44 deletions(-) diff --git a/src/cache/readonly.rs b/src/cache/readonly.rs index 40f9873f5f..c82dd7277d 100644 --- a/src/cache/readonly.rs +++ b/src/cache/readonly.rs @@ -110,7 +110,7 @@ mod test { #[test] fn readonly_storage_is_readonly() { - let storage = ReadOnlyStorage(Arc::new(MockStorage::new(None, false))); + let storage = ReadOnlyStorage(Arc::new(MockStorage::default())); assert_eq!( storage.check().now_or_never().unwrap().unwrap(), CacheMode::ReadOnly @@ -119,8 +119,7 @@ mod test { #[test] fn readonly_storage_forwards_preprocessor_cache_mode_config() { - let storage_no_preprocessor_cache = - ReadOnlyStorage(Arc::new(MockStorage::new(None, false))); + let storage_no_preprocessor_cache = ReadOnlyStorage(Arc::new(MockStorage::default())); assert!( !storage_no_preprocessor_cache .preprocessor_cache_mode_config() @@ -128,7 +127,7 @@ mod test { ); let storage_with_preprocessor_cache = - ReadOnlyStorage(Arc::new(MockStorage::new(None, true))); + ReadOnlyStorage(Arc::new(MockStorage::new(None, true, vec![]))); assert!( storage_with_preprocessor_cache .preprocessor_cache_mode_config() @@ -178,7 +177,7 @@ mod test { .build() .unwrap(); - let storage = ReadOnlyStorage(Arc::new(MockStorage::new(None, true))); + let storage = ReadOnlyStorage(Arc::new(MockStorage::new(None, true, vec![]))); runtime.block_on(async move { assert_eq!( storage diff --git a/src/compiler/clang.rs b/src/compiler/clang.rs index bd7a980092..852a567cb4 100644 --- a/src/compiler/clang.rs +++ b/src/compiler/clang.rs @@ -1379,7 +1379,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 78bb5a4332..332e4f66d3 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -2374,7 +2374,7 @@ LLVM version: 6.0", false, pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode)), + Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), CacheControl::Default, ) .wait() @@ -2442,7 +2442,7 @@ LLVM version: 6.0", false, pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode)), + Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), CacheControl::Default, ) .wait() @@ -2508,7 +2508,7 @@ LLVM version: 6.0", false, pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode)), + Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), CacheControl::Default, ) .wait() @@ -2810,7 +2810,7 @@ LLVM version: 6.0", let gcc = f.mk_bin("gcc").unwrap(); let runtime = Runtime::new().unwrap(); let pool = runtime.handle().clone(); - let storage = MockStorage::new(None, preprocessor_cache_mode); + let storage = MockStorage::new(None, preprocessor_cache_mode, vec![]); let storage: Arc = Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage.clone(), pool.clone()); @@ -2903,7 +2903,7 @@ LLVM version: 6.0", std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap(); // Make our storage wait 2ms for each get/put operation. let storage_delay = Duration::from_millis(2); - let storage = MockStorage::new(Some(storage_delay), preprocessor_cache_mode); + let storage = MockStorage::new(Some(storage_delay), preprocessor_cache_mode, vec![]); let storage: Arc = Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage.clone(), pool.clone()); // Pretend to be GCC. diff --git a/src/compiler/diab.rs b/src/compiler/diab.rs index a11e578606..53d3ad7823 100644 --- a/src/compiler/diab.rs +++ b/src/compiler/diab.rs @@ -792,7 +792,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 8a832b5d68..01dd01a54c 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -2605,7 +2605,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -2666,7 +2666,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -2725,7 +2725,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/msvc.rs b/src/compiler/msvc.rs index 900501f6a6..b3fd99ceca 100644 --- a/src/compiler/msvc.rs +++ b/src/compiler/msvc.rs @@ -2852,7 +2852,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -2942,7 +2942,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 975f4346b1..15dc89046e 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -62,6 +62,138 @@ use std::time; use crate::errors::*; +/// Strip a basedir prefix from a byte slice, returning the relative portion. +/// +/// Basedirs are pre-normalized with trailing `/` (see config.rs), so the +/// result is a clean relative path. Iteration is in the order basedirs are +/// listed in config; the first match wins. A value that equals a basedir +/// minus the trailing `/` (e.g. `cwd == basedir`) also matches and strips +/// to the empty byte string. +/// +/// On Windows the value is normalized (lowercased with forward slashes) +/// before comparison since basedirs are stored in that form; a match there +/// returns owned bytes because the borrow would point into the normalized +/// buffer rather than `value`. +fn strip_basedir_prefix<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + if basedirs.is_empty() { + return Cow::Borrowed(value); + } + strip_basedir_prefix_impl(value, basedirs) +} + +#[cfg(not(windows))] +fn strip_basedir_prefix_impl<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + for basedir in basedirs { + if value.starts_with(basedir) { + return Cow::Borrowed(&value[basedir.len()..]); + } + if is_basedir_minus_slash(value, basedir) { + return Cow::Borrowed(b""); + } + } + Cow::Borrowed(value) +} + +#[cfg(windows)] +fn strip_basedir_prefix_impl<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + let normalized = crate::util::normalize_win_path(value); + for basedir in basedirs { + if normalized.starts_with(basedir) { + return Cow::Owned(normalized[basedir.len()..].to_vec()); + } + if is_basedir_minus_slash(&normalized, basedir) { + return Cow::Owned(Vec::new()); + } + } + Cow::Borrowed(value) +} + +/// Returns true if `value` is `basedir` with the trailing `/` removed. +/// Handles the `cwd == basedir` case where a subpath `starts_with` check +/// would otherwise miss. +fn is_basedir_minus_slash(value: &[u8], basedir: &[u8]) -> bool { + basedir.last() == Some(&b'/') && value.len() + 1 == basedir.len() && basedir.starts_with(value) +} + +/// Strip every basedir occurrence from a single rustc argument. +/// +/// A match is any basedir that appears at the start of `arg` or immediately +/// after an arg-internal separator (`=`, `,`). Covers patterns like: +/// * `/abs/path/src.rs` (source file path) +/// * `--remap-path-prefix=/abs/path=/new` (rust-lang/cargo#12137) +/// * `-Clinker=/abs/path` +/// * `-Clink-arg=-Wl,-rpath,/abs/path` (sccache#2652 comment) +/// +/// Overlapping matches are resolved longest-first at each position. On +/// Windows the value is first normalized (lowercased with forward slashes) +/// so it can be compared against basedirs stored in that canonical form. +fn strip_basedirs_in_arg<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + if basedirs.is_empty() { + return Cow::Borrowed(arg); + } + strip_basedirs_in_arg_impl(arg, basedirs) +} + +#[cfg(not(windows))] +fn strip_basedirs_in_arg_impl<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + find_and_strip_basedirs(arg, basedirs) +} + +#[cfg(windows)] +fn strip_basedirs_in_arg_impl<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + let normalized = crate::util::normalize_win_path(arg); + match find_and_strip_basedirs(&normalized, basedirs) { + // No match: return the original arg (mirrors strip_basedir_prefix). + Cow::Borrowed(_) => Cow::Borrowed(arg), + // Match: the slice points into the local normalized buffer. + Cow::Owned(v) => Cow::Owned(v), + } +} + +/// Core matcher used by `strip_basedirs_in_arg_impl`: look up every basedir +/// in `haystack` at start-of-string / post-`=` / post-`,` boundaries, then +/// elide the matched ranges. +fn find_and_strip_basedirs<'a>(haystack: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + let mut matches: Vec<(usize, usize)> = Vec::new(); + for basedir in basedirs { + let b = basedir.as_slice(); + if b.is_empty() || b.len() > haystack.len() { + continue; + } + for start in memchr::memmem::find_iter(haystack, b) { + let is_boundary = start == 0 || matches!(haystack[start - 1], b'=' | b','); + if is_boundary { + matches.push((start, start + b.len())); + } + } + } + if matches.is_empty() { + return Cow::Borrowed(haystack); + } + // Sort by start ascending; break ties by length descending so the longest + // match at a given position wins (e.g. `/a/b/` before `/a/` when both are + // in the basedirs list). + matches.sort_by(|x, y| x.0.cmp(&y.0).then(y.1.cmp(&x.1))); + // Coalesce overlapping matches, keeping the first (longest) at each + // position. + let mut filtered: Vec<(usize, usize)> = Vec::new(); + let mut last_end = 0; + for (s, e) in matches { + if s >= last_end { + filtered.push((s, e)); + last_end = e; + } + } + let mut out = Vec::with_capacity(haystack.len()); + let mut pos = 0; + for (s, e) in filtered { + out.extend_from_slice(&haystack[pos..s]); + pos = e; + } + out.extend_from_slice(&haystack[pos..]); + Cow::Owned(out) +} + #[cfg(feature = "dist-client")] const RLIB_PREFIX: &str = "lib"; #[cfg(feature = "dist-client")] @@ -1392,10 +1524,11 @@ where _may_dist: bool, pool: &tokio::runtime::Handle, _rewrite_includes_only: bool, - _storage: Arc, + storage: Arc, _cache_control: CacheControl, ) -> Result> { trace!("[{}]: generate_hash_key", self.parsed_args.crate_name); + let basedirs = storage.basedirs(); // TODO: this doesn't produce correct arguments if they should be concatenated - should use iter_os_strings let os_string_arguments: Vec<(OsString, Option)> = self .parsed_args @@ -1510,7 +1643,13 @@ where // A few argument types are not passed in a deterministic order // by cargo: --extern, -L, --cfg. We'll filter those out, sort them, // and append them to the rest of the arguments. - let args = { + // Strip basedir occurrences per-argument before hashing. Handles both + // the common source-file-path arg (`/abs/path/src.rs`) and patterns + // that embed paths after `=` or `,`: `--remap-path-prefix=/abs/path`, + // `-Clinker=/abs/path`, `-Clink-arg=-Wl,-rpath,/abs/path`. See + // mozilla/sccache#2652. + let mut args_bytes = Vec::new(); + { let (mut sortables, rest): (Vec<_>, Vec<_>) = os_string_arguments .iter() // We exclude a few arguments from the hash: @@ -1534,15 +1673,16 @@ where // out, sort them, and append them to the rest of the arguments. .partition(|&(arg, _)| arg == "--cfg"); sortables.sort(); - rest.into_iter() + for arg in rest + .into_iter() .chain(sortables) .flat_map(|(arg, val)| iter::once(arg).chain(val.as_ref())) - .fold(OsString::new(), |mut a, b| { - a.push(b); - a - }) - }; - args.hash(&mut HashToDigest { digest: &mut m }); + { + args_bytes + .extend_from_slice(&strip_basedirs_in_arg(arg.as_encoded_bytes(), basedirs)); + } + } + args_bytes.hash(&mut HashToDigest { digest: &mut m }); // 4. The digest of all source files (this includes src file from cmdline). // 5. The digest of all files listed on the commandline (self.externs). // 6. The digest of all static libraries listed on the commandline (self.staticlibs). @@ -1562,7 +1702,10 @@ where for (var, val) in env_deps.iter() { var.hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); - val.hash(&mut HashToDigest { digest: &mut m }); + // Strip basedir prefixes from dep-info env var values (e.g. OUT_DIR) + // to enable cross-machine cache hits. + let val_bytes = val.as_encoded_bytes(); + strip_basedir_prefix(val_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); } let mut env_vars: Vec<_> = env_vars .iter() @@ -1593,10 +1736,18 @@ where var.hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); - val.hash(&mut HashToDigest { digest: &mut m }); + // Strip any basedir prefix from every CARGO_* env var value. + // Stripping is a no-op for values that don't start with a basedir, + // so this is safe to apply to non-path vars too and avoids the + // whitelist-maintenance bug class (CARGO_TARGET_DIR, CARGO_HOME, + // future additions, etc.). + let val_bytes = val.as_encoded_bytes(); + strip_basedir_prefix(val_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); } // 9. The cwd of the compile. This will wind up in the rlib. - cwd.hash(&mut HashToDigest { digest: &mut m }); + // Strip basedir prefix for cross-machine cache portability. + let cwd_bytes = cwd.as_os_str().as_encoded_bytes(); + strip_basedir_prefix(cwd_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); // 10. The version of the compiler. self.version.hash(&mut HashToDigest { digest: &mut m }); @@ -3242,7 +3393,7 @@ abc def.rs: #[cfg(not(windows))] #[test] - fn test_parse_dep_info_cwd() { + fn test_parse_dep_info_cwd_unix() { let deps = "foo: baz.rs abc.rs bar.rs baz.rs: @@ -3264,7 +3415,7 @@ bar.rs: #[cfg(not(windows))] #[test] - fn test_parse_dep_info_abs_paths() { + fn test_parse_dep_info_abs_paths_unix() { let deps = "/foo/foo: /foo/baz.rs /foo/abc.rs /foo/bar.rs /foo/baz.rs: @@ -3281,7 +3432,7 @@ bar.rs: #[cfg(windows)] #[test] - fn test_parse_dep_info_cwd() { + fn test_parse_dep_info_cwd_windows() { let deps = "foo: baz.rs abc.rs bar.rs baz.rs: @@ -3307,7 +3458,7 @@ bar.rs: #[cfg(windows)] #[test] - fn test_parse_dep_info_abs_paths() { + fn test_parse_dep_info_abs_paths_windows() { let deps = "c:/foo/foo: c:/foo/baz.rs c:/foo/abc.rs c:/foo/bar.rs c:/foo/baz.rs: c:/foo/bar.rs @@ -3575,7 +3726,7 @@ proc_macro false false, &pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode)), + Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), CacheControl::Default, ) .wait() @@ -3589,7 +3740,10 @@ proc_macro false // sysroot shlibs digests. m.update(FAKE_DIGEST.as_bytes()); // Arguments, with cfgs sorted at the end. - OsStr::new("ab--cfgabc--cfgxyz").hash(&mut HashToDigest { digest: &mut m }); + let args_str = OsStr::new("ab--cfgabc--cfgxyz"); + args_str + .as_encoded_bytes() + .hash(&mut HashToDigest { digest: &mut m }); // bar.rs (source file, from dep-info) m.update(empty_digest.as_bytes()); // foo.rs (source file, from dep-info) @@ -3602,11 +3756,18 @@ proc_macro false // Env vars OsStr::new("CARGO_BLAH").hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); - OsStr::new("abc").hash(&mut HashToDigest { digest: &mut m }); + OsStr::new("abc") + .as_encoded_bytes() + .hash(&mut HashToDigest { digest: &mut m }); OsStr::new("CARGO_PKG_NAME").hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); OsStr::new("foo").hash(&mut HashToDigest { digest: &mut m }); - f.tempdir.path().hash(&mut HashToDigest { digest: &mut m }); + // cwd + f.tempdir + .path() + .as_os_str() + .as_encoded_bytes() + .hash(&mut HashToDigest { digest: &mut m }); TEST_RUSTC_VERSION.hash(&mut HashToDigest { digest: &mut m }); let digest = m.finish(); assert_eq!(res.key, digest); @@ -3621,6 +3782,7 @@ proc_macro false env_vars: &[(OsString, OsString)], pre_func: F, preprocessor_cache_mode: bool, + basedirs: Vec>, ) -> String where F: Fn(&Path) -> Result<()>, @@ -3667,7 +3829,7 @@ proc_macro false false, &pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode)), + Arc::new(MockStorage::new(None, preprocessor_cache_mode, basedirs)), CacheControl::Default, ) .wait() @@ -3712,6 +3874,7 @@ proc_macro false &[], mk_files, preprocessor_cache_mode, + vec![], ), hash_key( &f, @@ -3733,6 +3896,7 @@ proc_macro false &[], mk_files, preprocessor_cache_mode, + vec![], ) ); } @@ -3762,6 +3926,7 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ), hash_key( &f, @@ -3783,6 +3948,7 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ) ); } @@ -3815,6 +3981,7 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ), hash_key( &f, @@ -3838,6 +4005,7 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ) ); } @@ -3865,6 +4033,7 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ), hash_key( &f, @@ -3886,6 +4055,7 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ) ); } @@ -3915,6 +4085,7 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ), hash_key( &f, @@ -3936,10 +4107,256 @@ proc_macro false &[], nothing, preprocessor_cache_mode, + vec![], ) ); } + #[test] + fn test_basedirs_strips_cwd_and_cargo_manifest_dir() { + let f = TestFixture::new(); + let cwd = f.tempdir.path().to_string_lossy().into_owned(); + + let args = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + ]; + + let manifest_dir = format!("{}/some/pkg", cwd); + let env_vars = vec![ + ( + OsString::from("CARGO_MANIFEST_DIR"), + OsString::from(&manifest_dir), + ), + (OsString::from("CARGO_PKG_NAME"), OsString::from("foo")), + ]; + + let key_without = hash_key(&f, args, &env_vars, nothing, false, vec![]); + + // Basedirs are normalized at config time (forward slashes, lowercase + // on Windows, trailing slash); replicate that here. + let basedir = cwd.into_bytes(); + #[cfg(windows)] + let basedir = crate::util::normalize_win_path(&basedir); + let mut basedir = basedir; + basedir.push(b'/'); + let key_with = hash_key(&f, args, &env_vars, nothing, false, vec![basedir]); + + assert_ne!(key_without, key_with, "basedirs should change the hash key"); + } + + /// Build the canonical basedir byte-string for a given tempdir path: + /// normalize on Windows, append a trailing `/`. Matches how + /// `Config` stores basedirs at runtime. + fn basedir_for(path: &Path) -> Vec { + let bytes = path.to_string_lossy().into_owned().into_bytes(); + #[cfg(windows)] + let bytes = crate::util::normalize_win_path(&bytes); + let mut bytes = bytes; + bytes.push(b'/'); + bytes + } + + #[test] + fn test_basedirs_stable_across_absolute_paths() { + // The central guarantee of this feature: when two machines build the + // same crate from different absolute checkout paths, supplying each + // side's checkout root as a basedir produces identical hash keys, so + // one machine's cache entry is a hit on the other. + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + assert_ne!( + f1.tempdir.path(), + f2.tempdir.path(), + "fixtures must be at different absolute paths" + ); + + let args = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + ]; + + // Matching CARGO_MANIFEST_DIR under each fixture exercises the env-var + // basedir-stripping path in addition to the cwd-stripping path. + let manifest1 = format!("{}/some/pkg", f1.tempdir.path().display()); + let manifest2 = format!("{}/some/pkg", f2.tempdir.path().display()); + let env1 = vec![ + ( + OsString::from("CARGO_MANIFEST_DIR"), + OsString::from(&manifest1), + ), + (OsString::from("CARGO_PKG_NAME"), OsString::from("foo")), + ]; + let env2 = vec![ + ( + OsString::from("CARGO_MANIFEST_DIR"), + OsString::from(&manifest2), + ), + (OsString::from("CARGO_PKG_NAME"), OsString::from("foo")), + ]; + + let k1 = hash_key( + &f1, + args, + &env1, + nothing, + false, + vec![basedir_for(f1.tempdir.path())], + ); + let k2 = hash_key( + &f2, + args, + &env2, + nothing, + false, + vec![basedir_for(f2.tempdir.path())], + ); + assert_eq!( + k1, k2, + "basedir stripping must produce identical hashes across different checkout paths" + ); + } + + #[test] + + fn test_strip_basedir_prefix_no_match() { + let out = super::strip_basedir_prefix(b"/other/path", &[b"/home/runner/".to_vec()]); + assert_eq!( + &*out, b"/other/path", + "no match should return value unchanged" + ); + } + + #[test] + + fn test_strip_basedir_prefix_first_match_wins() { + // Documents the current contract: iteration is in config order and the + // first matching basedir wins. Listing a more-specific basedir before a + // less-specific one is the caller's responsibility. + let basedirs = vec![b"/home/".to_vec(), b"/home/runner/".to_vec()]; + let out = super::strip_basedir_prefix(b"/home/runner/src/foo.rs", &basedirs); + assert_eq!(&*out, b"runner/src/foo.rs"); + } + + // strip_basedirs_in_arg covers embedded-path arg patterns (mozilla/sccache#2652). + + #[test] + + fn test_strip_basedirs_in_arg_prefix() { + let out = + super::strip_basedirs_in_arg(b"/home/user/src/lib.rs", &[b"/home/user/".to_vec()]); + assert_eq!(&*out, b"src/lib.rs"); + } + + #[test] + + fn test_strip_basedirs_in_arg_after_equals() { + // `--remap-path-prefix=/abs/path=/new` -- basedir appears after `=`. + let out = super::strip_basedirs_in_arg( + b"--remap-path-prefix=/home/user/a=/new", + &[b"/home/user/".to_vec()], + ); + assert_eq!(&*out, b"--remap-path-prefix=a=/new"); + } + + // Preserves the real `-Clink-arg=-Wl,...` rustc flag capitalization; that + // uppercase gets lowercased by `normalize_win_path` on Windows, so the + // assertion only holds on non-Windows. Windows exercises the same + // code path via `test_strip_basedirs_in_arg_after_comma_lowercase`. + #[cfg(not(windows))] + #[test] + fn test_strip_basedirs_in_arg_after_comma() { + let out = super::strip_basedirs_in_arg( + b"-Clink-arg=-Wl,-rpath,/home/user/lib", + &[b"/home/user/".to_vec()], + ); + assert_eq!(&*out, b"-Clink-arg=-Wl,-rpath,lib"); + } + + // Lowercase-only mirror of `test_strip_basedirs_in_arg_after_comma` so the + // after-`,` boundary match is exercised on every platform. + #[test] + fn test_strip_basedirs_in_arg_after_comma_lowercase() { + let out = super::strip_basedirs_in_arg( + b"-clink-arg=-wl,-rpath,/home/user/lib", + &[b"/home/user/".to_vec()], + ); + assert_eq!(&*out, b"-clink-arg=-wl,-rpath,lib"); + } + + #[test] + + fn test_strip_basedirs_in_arg_multiple_in_one() { + let out = + super::strip_basedirs_in_arg(b"/home/user/a,/home/user/b", &[b"/home/user/".to_vec()]); + assert_eq!(&*out, b"a,b"); + } + + #[test] + + fn test_strip_basedirs_in_arg_no_match_inside() { + // Basedir preceded by a non-boundary byte: no strip. + let out = + super::strip_basedirs_in_arg(b"prefix/home/user/suffix", &[b"/home/user/".to_vec()]); + assert_eq!(&*out, b"prefix/home/user/suffix"); + } + + #[test] + + fn test_strip_basedirs_in_arg_longest_at_same_position() { + // When `/a/b/` and `/a/` both match at the same position, the longer + // one wins (sort is by start asc, length desc). + let basedirs = vec![b"/a/".to_vec(), b"/a/b/".to_vec()]; + let out = super::strip_basedirs_in_arg(b"/a/b/x", &basedirs); + assert_eq!(&*out, b"x"); + } + + #[test] + fn test_basedirs_deterministic() { + // Running the same compilation with the same basedirs twice should + // produce the same hash, and it should differ from no-basedirs. + let f = TestFixture::new(); + let cwd = f.tempdir.path().to_string_lossy().into_owned(); + + let args = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + ]; + let env_vars = vec![(OsString::from("CARGO_PKG_NAME"), OsString::from("foo"))]; + + let basedir = cwd.into_bytes(); + #[cfg(windows)] + let basedir = crate::util::normalize_win_path(&basedir); + let mut basedir = basedir; + basedir.push(b'/'); + + let key1 = hash_key(&f, args, &env_vars, nothing, false, vec![basedir.clone()]); + let key2 = hash_key(&f, args, &env_vars, nothing, false, vec![basedir]); + + assert_eq!(key1, key2, "Same basedir should produce deterministic hash"); + } + #[test] fn test_parse_unstable_profile_flag() { let h = parses!( diff --git a/src/compiler/tasking_vx.rs b/src/compiler/tasking_vx.rs index b3fff8238a..26ac7676f4 100644 --- a/src/compiler/tasking_vx.rs +++ b/src/compiler/tasking_vx.rs @@ -730,7 +730,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -784,7 +784,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::new(None, false); + let storage = MockStorage::default(); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/test/mock_storage.rs b/src/test/mock_storage.rs index d4260e79e2..4d4faa23bc 100644 --- a/src/test/mock_storage.rs +++ b/src/test/mock_storage.rs @@ -28,17 +28,33 @@ pub struct MockStorage { tx: mpsc::UnboundedSender>, delay: Option, preprocessor_cache_mode: bool, + basedirs: Vec>, } impl MockStorage { - /// Create a new `MockStorage`. if `delay` is `Some`, wait for that amount of time before returning from operations. - pub(crate) fn new(delay: Option, preprocessor_cache_mode: bool) -> MockStorage { + /// Construct a `MockStorage`. + /// + /// # Arguments + /// + /// * `delay` — if `Some`, every `get`/`put` sleeps this long before + /// returning, to simulate slow storage. + /// * `preprocessor_cache_mode` — value returned from + /// [`Storage::preprocessor_cache_mode_config`]. + /// * `basedirs` — the list reported by [`Storage::basedirs`], used when + /// tests need to exercise basedir-prefix stripping in + /// `generate_hash_key`. + pub(crate) fn new( + delay: Option, + preprocessor_cache_mode: bool, + basedirs: Vec>, + ) -> MockStorage { let (tx, rx) = mpsc::unbounded(); Self { tx, rx: Arc::new(Mutex::new(rx)), delay, preprocessor_cache_mode, + basedirs, } } @@ -48,6 +64,15 @@ impl MockStorage { } } +impl Default for MockStorage { + /// Zero-delay mock with preprocessor cache mode disabled and no basedirs + /// configured -- the usual choice for tests that don't care about those + /// knobs. + fn default() -> Self { + Self::new(None, false, vec![]) + } +} + #[async_trait] impl Storage for MockStorage { async fn get(&self, _key: &str) -> Result { @@ -75,6 +100,9 @@ impl Storage for MockStorage { async fn max_size(&self) -> Result> { Ok(None) } + fn basedirs(&self) -> &[Vec] { + &self.basedirs + } fn preprocessor_cache_mode_config(&self) -> PreprocessorCacheModeConfig { PreprocessorCacheModeConfig { use_preprocessor_cache_mode: self.preprocessor_cache_mode, diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index db0e978a5a..e954ff629e 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -64,9 +64,17 @@ impl SccacheTest<'_> { trace!("sccache --start-server"); - Command::new(SCCACHE_BIN.as_os_str()) + let mut server_cmd = Command::new(SCCACHE_BIN.as_os_str()); + server_cmd .arg("--start-server") - .env("SCCACHE_DIR", &cache_dir) + .env("SCCACHE_DIR", &cache_dir); + // Forward `additional_envs` to the server too: some config (e.g. + // `SCCACHE_BASEDIRS`) is only read at server startup, so passing it + // here avoids a `restart_sccache` dance in each test. + if let Some(vec) = additional_envs { + server_cmd.envs(vec.iter().cloned()); + } + server_cmd .assert() .try_success() .context("Failed to start sccache server")?; diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index 6734a33f5b..8918984211 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -37,6 +37,95 @@ fn test_rust_cargo_build() -> Result<()> { test_rust_cargo_cmd("build", SccacheTest::new(None)?) } +#[test] +#[serial] +fn test_rust_cargo_basedirs_cross_dir_cache_hit() -> Result<()> { + // Copy tests/test-crate to two different absolute paths, then build in + // each with SCCACHE_BASEDIRS covering both roots. Without basedir + // stripping the two builds would compute different cache keys (cwd + + // CARGO_MANIFEST_DIR differ); with it, keys converge and the second + // build hits the first build's cache entries. + let work = tempfile::Builder::new() + .prefix("sccache_basedirs_xdir") + .tempdir() + .context("tempdir")?; + // On macOS `/var/...` is a symlink to `/private/var/...` and cargo reports + // the resolved target for CARGO_MANIFEST_DIR. Basedirs are compared by + // byte prefix, so the user-supplied path must be in the same canonical + // form. Windows `fs::canonicalize` returns `\\?\`-prefixed UNC paths that + // cargo does not emit, so only canonicalize on Unix. + #[cfg(unix)] + let work_root = fs::canonicalize(work.path())?; + #[cfg(not(unix))] + let work_root = work.path().to_path_buf(); + let root_a = work_root.join("machine_a"); + let root_b = work_root.join("machine_b"); + let crate_a = root_a.join("project"); + let crate_b = root_b.join("project"); + copy_crate(&CRATE_DIR, &crate_a)?; + copy_crate(&CRATE_DIR, &crate_b)?; + + // Basedir separator: `:` on Unix, `;` on Windows (matches config.rs). + let sep = if cfg!(windows) { ';' } else { ':' }; + let basedirs = format!("{}{sep}{}", root_a.display(), root_b.display()); + let test = SccacheTest::new(Some(&[( + "SCCACHE_BASEDIRS", + std::ffi::OsString::from(basedirs), + )]))?; + + run_cargo_build(&test, &crate_a, &crate_a.join("target"))?; + run_cargo_build(&test, &crate_b, &crate_b.join("target"))?; + + // After the second build, sccache must report Rust cache hits. The exact + // count matches the existing `test_rust_cargo_cmd` baseline (2), which + // exercises the same crate in a single directory with `cargo clean` + // between runs; if basedirs works, a cross-directory run should behave + // identically. + test.show_stats()? + .try_stdout(predicates::str::contains(r#""cache_hits":{"counts":{"Rust":2}"#).from_utf8())? + .try_success()?; + Ok(()) +} + +fn copy_crate(src: &Path, dst: &Path) -> Result<()> { + use walkdir::WalkDir; + fs::create_dir_all(dst)?; + for entry in WalkDir::new(src) { + let entry = entry.context("walkdir")?; + let rel = entry.path().strip_prefix(src).unwrap(); + let target = dst.join(rel); + if entry.file_type().is_dir() { + fs::create_dir_all(&target)?; + } else if entry.file_type().is_file() { + fs::copy(entry.path(), &target)?; + } + } + Ok(()) +} + +fn run_cargo_build(test: &SccacheTest, cwd: &Path, target_dir: &Path) -> Result<()> { + // The harness's default CARGO_TARGET_DIR is shared across invocations, + // which would let cargo short-circuit recompiles. Override per-build so + // each `cargo build` actually invokes rustc for every crate. + let env: Vec<_> = test + .env + .iter() + .filter(|(k, _)| *k != "CARGO_TARGET_DIR") + .cloned() + .chain(std::iter::once(( + "CARGO_TARGET_DIR", + target_dir.as_os_str().to_owned(), + ))) + .collect(); + Command::new(CARGO.as_os_str()) + .arg("build") + .envs(env) + .current_dir(cwd) + .assert() + .try_success()?; + Ok(()) +} + #[test] #[serial] fn test_rust_cargo_build_readonly() -> Result<()> { From abdf1a11a80b21cb6f00352e61b5be3bb33c8cd1 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Fri, 31 Jul 2026 18:29:37 +0200 Subject: [PATCH 02/11] rust: harden basedir cache key normalization #2652 #2678 Keep rustc-reported environment dependencies location-sensitive and hash PGO data and LLVM plugins before normalizing their paths. Preserve non-path argument bytes on Windows and use conservative matching for non-ASCII input. Handle exact and overlapping basedirs consistently, bump the Rust cache-key version, and document that embedded paths still require compiler remapping. --- docs/Rust.md | 1 + src/compiler/rust.rs | 436 +++++++++++++++++++++++++++++++++------ src/test/mock_storage.rs | 6 +- 3 files changed, 380 insertions(+), 63 deletions(-) diff --git a/docs/Rust.md b/docs/Rust.md index 5d6f98c3cf..f53cc5885c 100644 --- a/docs/Rust.md +++ b/docs/Rust.md @@ -9,5 +9,6 @@ sccache includes support for caching Rust compilation. This includes many caveat * Procedural macros that read files from the filesystem may not be cached properly. * `rustc`'s incremental compilation needs to be disabled. See [The Cargo Book](https://doc.rust-lang.org/cargo/reference/profiles.html#incremental) * Crates that invoke the system linker cannot be cached. Examples are `bin`, `dylib`, `cdylib`, and `proc-macro` crates. +* `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys, but does not rewrite paths embedded in compiler outputs. Use rustc's `--remap-path-prefix` when reproducible embedded paths are required. If you are using Rust 1.18 or later, you can ask cargo to wrap all compilation with sccache by setting `RUSTC_WRAPPER=sccache` in your build environment. diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 15dc89046e..d14018afa3 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -65,8 +65,8 @@ use crate::errors::*; /// Strip a basedir prefix from a byte slice, returning the relative portion. /// /// Basedirs are pre-normalized with trailing `/` (see config.rs), so the -/// result is a clean relative path. Iteration is in the order basedirs are -/// listed in config; the first match wins. A value that equals a basedir +/// result is a clean relative path. When multiple basedirs match, the longest +/// prefix wins. A value that equals a basedir /// minus the trailing `/` (e.g. `cwd == basedir`) also matches and strips /// to the empty byte string. /// @@ -83,29 +83,37 @@ fn strip_basedir_prefix<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u #[cfg(not(windows))] fn strip_basedir_prefix_impl<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - for basedir in basedirs { - if value.starts_with(basedir) { - return Cow::Borrowed(&value[basedir.len()..]); - } - if is_basedir_minus_slash(value, basedir) { - return Cow::Borrowed(b""); - } + let Some(basedir) = basedirs + .iter() + .filter(|basedir| value.starts_with(basedir) || is_basedir_minus_slash(value, basedir)) + .max_by_key(|basedir| basedir.len()) + else { + return Cow::Borrowed(value); + }; + if value.starts_with(basedir) { + Cow::Borrowed(&value[basedir.len()..]) + } else { + Cow::Borrowed(b"") } - Cow::Borrowed(value) } #[cfg(windows)] fn strip_basedir_prefix_impl<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { let normalized = crate::util::normalize_win_path(value); - for basedir in basedirs { - if normalized.starts_with(basedir) { - return Cow::Owned(normalized[basedir.len()..].to_vec()); - } - if is_basedir_minus_slash(&normalized, basedir) { - return Cow::Owned(Vec::new()); - } + let Some(basedir) = basedirs + .iter() + .filter(|basedir| { + normalized.starts_with(basedir) || is_basedir_minus_slash(&normalized, basedir) + }) + .max_by_key(|basedir| basedir.len()) + else { + return Cow::Borrowed(value); + }; + if normalized.starts_with(basedir) { + Cow::Owned(normalized[basedir.len()..].to_vec()) + } else { + Cow::Owned(Vec::new()) } - Cow::Borrowed(value) } /// Returns true if `value` is `basedir` with the trailing `/` removed. @@ -141,19 +149,18 @@ fn strip_basedirs_in_arg_impl<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a #[cfg(windows)] fn strip_basedirs_in_arg_impl<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - let normalized = crate::util::normalize_win_path(arg); - match find_and_strip_basedirs(&normalized, basedirs) { - // No match: return the original arg (mirrors strip_basedir_prefix). - Cow::Borrowed(_) => Cow::Borrowed(arg), - // Match: the slice points into the local normalized buffer. - Cow::Owned(v) => Cow::Owned(v), - } + strip_windows_basedirs_in_arg(arg, basedirs) } /// Core matcher used by `strip_basedirs_in_arg_impl`: look up every basedir /// in `haystack` at start-of-string / post-`=` / post-`,` boundaries, then /// elide the matched ranges. fn find_and_strip_basedirs<'a>(haystack: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + let matches = find_basedir_matches(haystack, basedirs); + strip_matches(haystack, &matches) +} + +fn find_basedir_matches(haystack: &[u8], basedirs: &[Vec]) -> Vec<(usize, usize)> { let mut matches: Vec<(usize, usize)> = Vec::new(); for basedir in basedirs { let b = basedir.as_slice(); @@ -161,14 +168,31 @@ fn find_and_strip_basedirs<'a>(haystack: &'a [u8], basedirs: &[Vec]) -> Cow< continue; } for start in memchr::memmem::find_iter(haystack, b) { - let is_boundary = start == 0 || matches!(haystack[start - 1], b'=' | b','); - if is_boundary { + if is_arg_path_boundary(haystack, start) { matches.push((start, start + b.len())); } } + if let Some(basedir_without_slash) = b.strip_suffix(b"/") { + for start in memchr::memmem::find_iter(haystack, basedir_without_slash) { + let end = start + basedir_without_slash.len(); + if is_arg_path_boundary(haystack, start) + && (end == haystack.len() || matches!(haystack[end], b'=' | b',')) + { + matches.push((start, end)); + } + } + } } + filter_overlapping_matches(matches) +} + +fn is_arg_path_boundary(value: &[u8], start: usize) -> bool { + start == 0 || matches!(value[start - 1], b'=' | b',') +} + +fn filter_overlapping_matches(mut matches: Vec<(usize, usize)>) -> Vec<(usize, usize)> { if matches.is_empty() { - return Cow::Borrowed(haystack); + return matches; } // Sort by start ascending; break ties by length descending so the longest // match at a given position wins (e.g. `/a/b/` before `/a/` when both are @@ -184,16 +208,37 @@ fn find_and_strip_basedirs<'a>(haystack: &'a [u8], basedirs: &[Vec]) -> Cow< last_end = e; } } - let mut out = Vec::with_capacity(haystack.len()); + filtered +} + +fn strip_matches<'a>(value: &'a [u8], matches: &[(usize, usize)]) -> Cow<'a, [u8]> { + if matches.is_empty() { + return Cow::Borrowed(value); + } + let mut out = Vec::with_capacity(value.len()); let mut pos = 0; - for (s, e) in filtered { - out.extend_from_slice(&haystack[pos..s]); - pos = e; + for &(start, end) in matches { + out.extend_from_slice(&value[pos..start]); + pos = end; } - out.extend_from_slice(&haystack[pos..]); + out.extend_from_slice(&value[pos..]); Cow::Owned(out) } +/// Match Windows basedirs case-insensitively while preserving the original +/// non-path bytes in the cache key. Non-ASCII arguments remain location-sensitive +/// because Unicode case folding can change byte offsets. +#[cfg(any(windows, test))] +fn strip_windows_basedirs_in_arg<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + if !value.is_ascii() { + return Cow::Borrowed(value); + } + let normalized = crate::util::normalize_win_path(value); + debug_assert_eq!(normalized.len(), value.len()); + let matches = find_basedir_matches(&normalized, basedirs); + strip_matches(value, &matches) +} + #[cfg(feature = "dist-client")] const RLIB_PREFIX: &str = "lib"; #[cfg(feature = "dist-client")] @@ -302,6 +347,8 @@ pub struct ParsedArguments { /// /// For more information, see https://doc.rust-lang.org/rustc/profile-guided-optimization.html profile: Option, + /// Profile data and compiler plugins whose contents affect code generation. + codegen_input_files: Vec, /// If `-Z profile` has been enabled, we will use a GCC-compatible, gcov-based /// coverage implementation. /// @@ -367,7 +414,7 @@ static ALLOWED_EMIT: LazyLock> = LazyLock::new(|| ["link", "metadata", "dep-info"].iter().copied().collect()); /// Version number for cache key. -const CACHE_VERSION: &[u8] = b"6"; +const CACHE_VERSION: &[u8] = b"7"; /// Get absolute paths for all source files and env-deps listed in rustc's dep-info output. async fn get_source_files_and_env_deps( @@ -1271,6 +1318,7 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments CompilerArguments extra_filename = Some(value.to_owned()), ("extra-filename", None) => cannot_cache!("extra-filename"), - ("profile-use", Some(v)) => profile = Some(v.clone()), + ("profile-use", Some(v)) => { + profile = Some(v.clone()); + codegen_input_files.push(v.into()); + } // Incremental compilation makes a mess of sccache's entire world // view. It produces additional compiler outputs that we don't cache, // and just letting rustc do its work in incremental mode is likely @@ -1344,10 +1395,16 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments (), } } - Some(Unstable(ArgUnstable { opt, value })) => match value.as_deref() { - Some("y") | Some("yes") | Some("on") | None if opt == "profile" => { + Some(Unstable(ArgUnstable { opt, value })) => match (opt.as_ref(), value.as_deref()) { + ("profile", Some("y") | Some("yes") | Some("on") | None) => { gcno = true; } + ("profile-sample-use", Some(path)) => { + codegen_input_files.push(path.into()); + } + ("llvm-plugins", Some(paths)) => { + codegen_input_files.extend(paths.split_whitespace().map(PathBuf::from)); + } _ => (), }, Some(Color(value)) => { @@ -1502,6 +1559,7 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments>(); + let codegen_input_hashes = hash_all(&codegen_input_files, pool); + // Perform all hashing operations on the files. let ( (source_files, source_hashes, mut env_deps), extern_hashes, staticlib_hashes, target_json_hash, + codegen_input_hashes, ) = futures::try_join!( source_files_and_hashes_and_env_deps, extern_hashes, staticlib_hashes, - target_json_hash + target_json_hash, + codegen_input_hashes )?; // If you change any of the inputs to the hash, you should change `CACHE_VERSION`. @@ -1687,25 +1757,26 @@ where // 5. The digest of all files listed on the commandline (self.externs). // 6. The digest of all static libraries listed on the commandline (self.staticlibs). // 7. The digest of the content of the target json file specified via `--target` (if any). + // 8. The digest of PGO data and LLVM plugins (if any). for h in source_hashes .into_iter() .chain(extern_hashes) .chain(staticlib_hashes) .chain(target_json_hash) + .chain(codegen_input_hashes) { m.update(h.as_bytes()); } - // 8. Environment variables: Hash all environment variables listed in the rustc dep-info + // 9. Environment variables: Hash all environment variables listed in the rustc dep-info // output. Additionally also has all environment variables starting with `CARGO_`, // since those are not listed in dep-info but affect cacheability. env_deps.sort(); for (var, val) in env_deps.iter() { var.hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); - // Strip basedir prefixes from dep-info env var values (e.g. OUT_DIR) - // to enable cross-machine cache hits. - let val_bytes = val.as_encoded_bytes(); - strip_basedir_prefix(val_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); + // rustc reports variables read by env! and option_env! here. Their values can be + // embedded verbatim in the artifact, so they must remain location-sensitive. + val.hash(&mut HashToDigest { digest: &mut m }); } let mut env_vars: Vec<_> = env_vars .iter() @@ -1744,11 +1815,11 @@ where let val_bytes = val.as_encoded_bytes(); strip_basedir_prefix(val_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); } - // 9. The cwd of the compile. This will wind up in the rlib. + // 10. The cwd of the compile. This will wind up in the rlib. // Strip basedir prefix for cross-machine cache portability. let cwd_bytes = cwd.as_os_str().as_encoded_bytes(); strip_basedir_prefix(cwd_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); - // 10. The version of the compiler. + // 11. The version of the compiler. self.version.hash(&mut HashToDigest { digest: &mut m }); // Turn arguments into a simple Vec to calculate outputs. @@ -3592,7 +3663,11 @@ proc_macro false ); } - fn mock_dep_info(creator: &Arc>, dep_srcs: &[&str]) { + fn mock_dep_info( + creator: &Arc>, + dep_srcs: &[&str], + env_deps: &[(&str, &str)], + ) { // Mock the `rustc --emit=dep-info` process by writing // a dep-info file. let mut sorted_deps = dep_srcs @@ -3600,6 +3675,10 @@ proc_macro false .map(|s| (*s).to_string()) .collect::>(); sorted_deps.sort(); + let env_deps = env_deps + .iter() + .map(|(var, val)| ((*var).to_string(), (*val).to_string())) + .collect::>(); next_command_calls(creator, move |args| { let mut dep_info_path = None; let mut it = args.iter(); @@ -3615,6 +3694,9 @@ proc_macro false for d in sorted_deps.iter() { writeln!(f, "{}:", d)?; } + for (var, val) in &env_deps { + writeln!(f, "# env-dep:{var}={val}")?; + } Ok(MockChild::new(exit_status(0), "", "")) }); } @@ -3696,12 +3778,13 @@ proc_macro false color_mode: ColorMode::Auto, has_json: false, profile: None, + codegen_input_files: vec![], gcno: None, target_json: None, }, }); let creator = new_creator(); - mock_dep_info(&creator, &["foo.rs", "bar.rs"]); + mock_dep_info(&creator, &["foo.rs", "bar.rs"], &[]); mock_file_names(&creator, &["foo.rlib", "foo.a"]); let runtime = single_threaded_runtime(); let pool = runtime.handle().clone(); @@ -3784,6 +3867,29 @@ proc_macro false preprocessor_cache_mode: bool, basedirs: Vec>, ) -> String + where + F: Fn(&Path) -> Result<()>, + { + hash_key_with_env_deps( + f, + args, + env_vars, + pre_func, + preprocessor_cache_mode, + basedirs, + &[], + ) + } + + fn hash_key_with_env_deps( + f: &TestFixture, + args: &[&'static str], + env_vars: &[(OsString, OsString)], + pre_func: F, + preprocessor_cache_mode: bool, + basedirs: Vec>, + env_deps: &[(&str, &str)], + ) -> String where F: Fn(&Path) -> Result<()>, { @@ -3819,7 +3925,7 @@ proc_macro false let runtime = single_threaded_runtime(); let pool = runtime.handle().clone(); - mock_dep_info(&creator, &["foo.rs"]); + mock_dep_info(&creator, &["foo.rs"], env_deps); mock_file_names(&creator, &["foo.rlib"]); hasher .generate_hash_key( @@ -4231,6 +4337,203 @@ proc_macro false ); } + #[test] + fn test_basedirs_preserve_path_sensitive_env_dependencies() { + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + let args = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + ]; + let manifest1 = f1.tempdir.path().to_string_lossy().into_owned(); + let manifest2 = f2.tempdir.path().to_string_lossy().into_owned(); + let env1 = vec![( + OsString::from("CARGO_MANIFEST_DIR"), + OsString::from(&manifest1), + )]; + let env2 = vec![( + OsString::from("CARGO_MANIFEST_DIR"), + OsString::from(&manifest2), + )]; + + let k1 = hash_key_with_env_deps( + &f1, + args, + &env1, + nothing, + false, + vec![basedir_for(f1.tempdir.path())], + &[("CARGO_MANIFEST_DIR", &manifest1)], + ); + let k2 = hash_key_with_env_deps( + &f2, + args, + &env2, + nothing, + false, + vec![basedir_for(f2.tempdir.path())], + &[("CARGO_MANIFEST_DIR", &manifest2)], + ); + + assert_ne!( + k1, k2, + "env! values reported by rustc must remain location-sensitive" + ); + } + + #[test] + fn test_basedirs_hash_profile_use_contents() { + fn write_profile_a(path: &Path) -> Result<()> { + fs::write(path.join("profile.profdata"), b"profile-a")?; + Ok(()) + } + + fn write_profile_b(path: &Path) -> Result<()> { + fs::write(path.join("profile.profdata"), b"profile-b")?; + Ok(()) + } + + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + let args = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + "-C", + "profile-use=profile.profdata", + ]; + let k1 = hash_key( + &f1, + args, + &[], + write_profile_a, + false, + vec![basedir_for(f1.tempdir.path())], + ); + let k2 = hash_key( + &f2, + args, + &[], + write_profile_b, + false, + vec![basedir_for(f2.tempdir.path())], + ); + + assert_ne!(k1, k2, "different profile data must produce different keys"); + } + + #[test] + fn test_basedirs_hash_sample_profile_contents() { + fn write_profile_a(path: &Path) -> Result<()> { + fs::write(path.join("sample.prof"), b"sample-a")?; + Ok(()) + } + + fn write_profile_b(path: &Path) -> Result<()> { + fs::write(path.join("sample.prof"), b"sample-b")?; + Ok(()) + } + + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + let args = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + "-Z", + "profile-sample-use=sample.prof", + ]; + let k1 = hash_key( + &f1, + args, + &[], + write_profile_a, + false, + vec![basedir_for(f1.tempdir.path())], + ); + let k2 = hash_key( + &f2, + args, + &[], + write_profile_b, + false, + vec![basedir_for(f2.tempdir.path())], + ); + + assert_ne!( + k1, k2, + "different sample profiles must produce different keys" + ); + } + + #[test] + fn test_basedirs_hash_all_llvm_plugins() { + fn write_plugins_a(path: &Path) -> Result<()> { + fs::write(path.join("first.so"), b"first")?; + fs::write(path.join("second.so"), b"second-a")?; + Ok(()) + } + + fn write_plugins_b(path: &Path) -> Result<()> { + fs::write(path.join("first.so"), b"first")?; + fs::write(path.join("second.so"), b"second-b")?; + Ok(()) + } + + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + let args = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + "-Z", + "llvm-plugins=first.so second.so", + ]; + let k1 = hash_key( + &f1, + args, + &[], + write_plugins_a, + false, + vec![basedir_for(f1.tempdir.path())], + ); + let k2 = hash_key( + &f2, + args, + &[], + write_plugins_b, + false, + vec![basedir_for(f2.tempdir.path())], + ); + + assert_ne!(k1, k2, "all LLVM plugin contents must affect the key"); + } + #[test] fn test_strip_basedir_prefix_no_match() { @@ -4243,13 +4546,10 @@ proc_macro false #[test] - fn test_strip_basedir_prefix_first_match_wins() { - // Documents the current contract: iteration is in config order and the - // first matching basedir wins. Listing a more-specific basedir before a - // less-specific one is the caller's responsibility. + fn test_strip_basedir_prefix_longest_match_wins() { let basedirs = vec![b"/home/".to_vec(), b"/home/runner/".to_vec()]; let out = super::strip_basedir_prefix(b"/home/runner/src/foo.rs", &basedirs); - assert_eq!(&*out, b"runner/src/foo.rs"); + assert_eq!(&*out, b"src/foo.rs"); } // strip_basedirs_in_arg covers embedded-path arg patterns (mozilla/sccache#2652). @@ -4273,11 +4573,29 @@ proc_macro false assert_eq!(&*out, b"--remap-path-prefix=a=/new"); } - // Preserves the real `-Clink-arg=-Wl,...` rustc flag capitalization; that - // uppercase gets lowercased by `normalize_win_path` on Windows, so the - // assertion only holds on non-Windows. Windows exercises the same - // code path via `test_strip_basedirs_in_arg_after_comma_lowercase`. - #[cfg(not(windows))] + #[test] + fn test_strip_basedirs_in_arg_exact_basedir() { + let out = super::strip_basedirs_in_arg( + b"--remap-path-prefix=/home/user=/new", + &[b"/home/user/".to_vec()], + ); + assert_eq!(&*out, b"--remap-path-prefix==/new"); + } + + #[test] + fn test_windows_arg_matching_preserves_non_path_case() { + let arg = b"--remap-path-prefix=C:\\Work\\Repo=VIRTUAL"; + let out = super::strip_windows_basedirs_in_arg(arg, &[b"c:/work/repo/".to_vec()]); + assert_eq!(&*out, b"--remap-path-prefix==VIRTUAL"); + } + + #[test] + fn test_windows_non_ascii_arg_remains_location_sensitive() { + let arg = b"--remap-path-prefix=C:\\Work\\Repo=\xc4\xb0"; + let out = super::strip_windows_basedirs_in_arg(arg, &[b"c:/work/repo/".to_vec()]); + assert_eq!(&*out, arg); + } + #[test] fn test_strip_basedirs_in_arg_after_comma() { let out = super::strip_basedirs_in_arg( @@ -4287,8 +4605,6 @@ proc_macro false assert_eq!(&*out, b"-Clink-arg=-Wl,-rpath,lib"); } - // Lowercase-only mirror of `test_strip_basedirs_in_arg_after_comma` so the - // after-`,` boundary match is exercised on every platform. #[test] fn test_strip_basedirs_in_arg_after_comma_lowercase() { let out = super::strip_basedirs_in_arg( diff --git a/src/test/mock_storage.rs b/src/test/mock_storage.rs index 4d4faa23bc..122993d2e4 100644 --- a/src/test/mock_storage.rs +++ b/src/test/mock_storage.rs @@ -36,11 +36,11 @@ impl MockStorage { /// /// # Arguments /// - /// * `delay` — if `Some`, every `get`/`put` sleeps this long before + /// * `delay` - if `Some`, every `get`/`put` sleeps this long before /// returning, to simulate slow storage. - /// * `preprocessor_cache_mode` — value returned from + /// * `preprocessor_cache_mode` - value returned from /// [`Storage::preprocessor_cache_mode_config`]. - /// * `basedirs` — the list reported by [`Storage::basedirs`], used when + /// * `basedirs` - the list reported by [`Storage::basedirs`], used when /// tests need to exercise basedir-prefix stripping in /// `generate_hash_key`. pub(crate) fn new( From 8f6836a6e2a4fc5f6d03ac94fa72f58c80a2985d Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Mon, 3 Aug 2026 18:23:12 +0200 Subject: [PATCH 03/11] rust: simplify basedir key normalization https://github.com/mozilla/sccache/issues/2652 https://github.com/mozilla/sccache/pull/2678 Replace broad Rust argument scanning with normalization of explicit remap prefixes and path-valued Cargo variables. Hash the actual path produced by rustc remapping, honoring scope and mapping precedence, while keeping unremapped working directories location-sensitive. Frame normalized values to prevent absolute and relative inputs from sharing a key. Reduce the mock and integration-test changes, and leave profile and plugin paths location-sensitive. --- docs/Rust.md | 2 +- src/cache/readonly.rs | 9 +- src/compiler/clang.rs | 2 +- src/compiler/compiler.rs | 10 +- src/compiler/diab.rs | 2 +- src/compiler/gcc.rs | 6 +- src/compiler/msvc.rs | 4 +- src/compiler/rust.rs | 913 +++++++++++-------------------------- src/compiler/tasking_vx.rs | 4 +- src/test/mock_storage.rs | 34 +- tests/helpers/mod.rs | 12 +- tests/sccache_cargo.rs | 95 ++-- 12 files changed, 332 insertions(+), 761 deletions(-) diff --git a/docs/Rust.md b/docs/Rust.md index f53cc5885c..1f2c6c8ad1 100644 --- a/docs/Rust.md +++ b/docs/Rust.md @@ -9,6 +9,6 @@ sccache includes support for caching Rust compilation. This includes many caveat * Procedural macros that read files from the filesystem may not be cached properly. * `rustc`'s incremental compilation needs to be disabled. See [The Cargo Book](https://doc.rust-lang.org/cargo/reference/profiles.html#incremental) * Crates that invoke the system linker cannot be cached. Examples are `bin`, `dylib`, `cdylib`, and `proc-macro` crates. -* `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys, but does not rewrite paths embedded in compiler outputs. Use rustc's `--remap-path-prefix` when reproducible embedded paths are required. +* `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys when rustc's `--remap-path-prefix` covers the working directory with the default or `all` remap scope. It does not rewrite compiler outputs itself. If you are using Rust 1.18 or later, you can ask cargo to wrap all compilation with sccache by setting `RUSTC_WRAPPER=sccache` in your build environment. diff --git a/src/cache/readonly.rs b/src/cache/readonly.rs index c82dd7277d..40f9873f5f 100644 --- a/src/cache/readonly.rs +++ b/src/cache/readonly.rs @@ -110,7 +110,7 @@ mod test { #[test] fn readonly_storage_is_readonly() { - let storage = ReadOnlyStorage(Arc::new(MockStorage::default())); + let storage = ReadOnlyStorage(Arc::new(MockStorage::new(None, false))); assert_eq!( storage.check().now_or_never().unwrap().unwrap(), CacheMode::ReadOnly @@ -119,7 +119,8 @@ mod test { #[test] fn readonly_storage_forwards_preprocessor_cache_mode_config() { - let storage_no_preprocessor_cache = ReadOnlyStorage(Arc::new(MockStorage::default())); + let storage_no_preprocessor_cache = + ReadOnlyStorage(Arc::new(MockStorage::new(None, false))); assert!( !storage_no_preprocessor_cache .preprocessor_cache_mode_config() @@ -127,7 +128,7 @@ mod test { ); let storage_with_preprocessor_cache = - ReadOnlyStorage(Arc::new(MockStorage::new(None, true, vec![]))); + ReadOnlyStorage(Arc::new(MockStorage::new(None, true))); assert!( storage_with_preprocessor_cache .preprocessor_cache_mode_config() @@ -177,7 +178,7 @@ mod test { .build() .unwrap(); - let storage = ReadOnlyStorage(Arc::new(MockStorage::new(None, true, vec![]))); + let storage = ReadOnlyStorage(Arc::new(MockStorage::new(None, true))); runtime.block_on(async move { assert_eq!( storage diff --git a/src/compiler/clang.rs b/src/compiler/clang.rs index 852a567cb4..bd7a980092 100644 --- a/src/compiler/clang.rs +++ b/src/compiler/clang.rs @@ -1379,7 +1379,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 332e4f66d3..78bb5a4332 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -2374,7 +2374,7 @@ LLVM version: 6.0", false, pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), + Arc::new(MockStorage::new(None, preprocessor_cache_mode)), CacheControl::Default, ) .wait() @@ -2442,7 +2442,7 @@ LLVM version: 6.0", false, pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), + Arc::new(MockStorage::new(None, preprocessor_cache_mode)), CacheControl::Default, ) .wait() @@ -2508,7 +2508,7 @@ LLVM version: 6.0", false, pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), + Arc::new(MockStorage::new(None, preprocessor_cache_mode)), CacheControl::Default, ) .wait() @@ -2810,7 +2810,7 @@ LLVM version: 6.0", let gcc = f.mk_bin("gcc").unwrap(); let runtime = Runtime::new().unwrap(); let pool = runtime.handle().clone(); - let storage = MockStorage::new(None, preprocessor_cache_mode, vec![]); + let storage = MockStorage::new(None, preprocessor_cache_mode); let storage: Arc = Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage.clone(), pool.clone()); @@ -2903,7 +2903,7 @@ LLVM version: 6.0", std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap(); // Make our storage wait 2ms for each get/put operation. let storage_delay = Duration::from_millis(2); - let storage = MockStorage::new(Some(storage_delay), preprocessor_cache_mode, vec![]); + let storage = MockStorage::new(Some(storage_delay), preprocessor_cache_mode); let storage: Arc = Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage.clone(), pool.clone()); // Pretend to be GCC. diff --git a/src/compiler/diab.rs b/src/compiler/diab.rs index 53d3ad7823..a11e578606 100644 --- a/src/compiler/diab.rs +++ b/src/compiler/diab.rs @@ -792,7 +792,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 01dd01a54c..8a832b5d68 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -2605,7 +2605,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -2666,7 +2666,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -2725,7 +2725,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/msvc.rs b/src/compiler/msvc.rs index b3fd99ceca..900501f6a6 100644 --- a/src/compiler/msvc.rs +++ b/src/compiler/msvc.rs @@ -2852,7 +2852,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -2942,7 +2942,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index d14018afa3..851f05164e 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -62,181 +62,118 @@ use std::time; use crate::errors::*; -/// Strip a basedir prefix from a byte slice, returning the relative portion. -/// -/// Basedirs are pre-normalized with trailing `/` (see config.rs), so the -/// result is a clean relative path. When multiple basedirs match, the longest -/// prefix wins. A value that equals a basedir -/// minus the trailing `/` (e.g. `cwd == basedir`) also matches and strips -/// to the empty byte string. -/// -/// On Windows the value is normalized (lowercased with forward slashes) -/// before comparison since basedirs are stored in that form; a match there -/// returns owned bytes because the borrow would point into the normalized -/// buffer rather than `value`. -fn strip_basedir_prefix<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - if basedirs.is_empty() { - return Cow::Borrowed(value); - } - strip_basedir_prefix_impl(value, basedirs) -} - #[cfg(not(windows))] -fn strip_basedir_prefix_impl<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - let Some(basedir) = basedirs +fn strip_basedir_prefix<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + let prefix_len = basedirs .iter() - .filter(|basedir| value.starts_with(basedir) || is_basedir_minus_slash(value, basedir)) - .max_by_key(|basedir| basedir.len()) - else { - return Cow::Borrowed(value); - }; - if value.starts_with(basedir) { - Cow::Borrowed(&value[basedir.len()..]) - } else { - Cow::Borrowed(b"") - } + .filter_map(|basedir| { + value + .starts_with(basedir) + .then_some(basedir.len()) + .or_else(|| (basedir.strip_suffix(b"/") == Some(value)).then_some(value.len())) + }) + .max() + .unwrap_or(0); + Cow::Borrowed(&value[prefix_len..]) } #[cfg(windows)] -fn strip_basedir_prefix_impl<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { +fn strip_basedir_prefix<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { + if !value.is_ascii() { + return Cow::Borrowed(value); + } let normalized = crate::util::normalize_win_path(value); - let Some(basedir) = basedirs + let prefix_len = basedirs .iter() - .filter(|basedir| { - normalized.starts_with(basedir) || is_basedir_minus_slash(&normalized, basedir) + .filter_map(|basedir| { + normalized + .starts_with(basedir) + .then_some(basedir.len()) + .or_else(|| { + (basedir.strip_suffix(b"/") == Some(normalized.as_slice())) + .then_some(normalized.len()) + }) }) - .max_by_key(|basedir| basedir.len()) - else { - return Cow::Borrowed(value); - }; - if normalized.starts_with(basedir) { - Cow::Owned(normalized[basedir.len()..].to_vec()) + .max() + .unwrap_or(0); + if prefix_len == 0 { + Cow::Borrowed(value) } else { - Cow::Owned(Vec::new()) + Cow::Borrowed(&value[prefix_len..]) } } -/// Returns true if `value` is `basedir` with the trailing `/` removed. -/// Handles the `cwd == basedir` case where a subpath `starts_with` check -/// would otherwise miss. -fn is_basedir_minus_slash(value: &[u8], basedir: &[u8]) -> bool { - basedir.last() == Some(&b'/') && value.len() + 1 == basedir.len() && basedir.starts_with(value) -} - -/// Strip every basedir occurrence from a single rustc argument. -/// -/// A match is any basedir that appears at the start of `arg` or immediately -/// after an arg-internal separator (`=`, `,`). Covers patterns like: -/// * `/abs/path/src.rs` (source file path) -/// * `--remap-path-prefix=/abs/path=/new` (rust-lang/cargo#12137) -/// * `-Clinker=/abs/path` -/// * `-Clink-arg=-Wl,-rpath,/abs/path` (sccache#2652 comment) -/// -/// Overlapping matches are resolved longest-first at each position. On -/// Windows the value is first normalized (lowercased with forward slashes) -/// so it can be compared against basedirs stored in that canonical form. -fn strip_basedirs_in_arg<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - if basedirs.is_empty() { - return Cow::Borrowed(arg); +fn normalize_arg_value<'a>( + flag: &OsString, + value: &'a OsString, + basedirs: &[Vec], +) -> Cow<'a, [u8]> { + let value = value.as_encoded_bytes(); + if flag == "--remap-path-prefix" { + let Some(separator) = value.iter().rposition(|byte| *byte == b'=') else { + return Cow::Borrowed(value); + }; + let stripped = strip_basedir_prefix(&value[..separator], basedirs); + if stripped.len() == separator { + return Cow::Borrowed(value); + } + let mut result = stripped.into_owned(); + result.extend_from_slice(&value[separator..]); + return Cow::Owned(result); } - strip_basedirs_in_arg_impl(arg, basedirs) -} - -#[cfg(not(windows))] -fn strip_basedirs_in_arg_impl<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - find_and_strip_basedirs(arg, basedirs) -} - -#[cfg(windows)] -fn strip_basedirs_in_arg_impl<'a>(arg: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - strip_windows_basedirs_in_arg(arg, basedirs) + Cow::Borrowed(value) } -/// Core matcher used by `strip_basedirs_in_arg_impl`: look up every basedir -/// in `haystack` at start-of-string / post-`=` / post-`,` boundaries, then -/// elide the matched ranges. -fn find_and_strip_basedirs<'a>(haystack: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - let matches = find_basedir_matches(haystack, basedirs); - strip_matches(haystack, &matches) -} - -fn find_basedir_matches(haystack: &[u8], basedirs: &[Vec]) -> Vec<(usize, usize)> { - let mut matches: Vec<(usize, usize)> = Vec::new(); - for basedir in basedirs { - let b = basedir.as_slice(); - if b.is_empty() || b.len() > haystack.len() { - continue; - } - for start in memchr::memmem::find_iter(haystack, b) { - if is_arg_path_boundary(haystack, start) { - matches.push((start, start + b.len())); - } +fn remap_scope_is_all(arguments: &[(OsString, Option)]) -> bool { + let mut scopes = arguments.iter().filter_map(|(flag, value)| { + if flag == "--remap-path-scope" { + return value.as_ref()?.to_str(); } - if let Some(basedir_without_slash) = b.strip_suffix(b"/") { - for start in memchr::memmem::find_iter(haystack, basedir_without_slash) { - let end = start + basedir_without_slash.len(); - if is_arg_path_boundary(haystack, start) - && (end == haystack.len() || matches!(haystack[end], b'=' | b',')) - { - matches.push((start, end)); - } - } + if flag == "-Z" { + return value.as_ref()?.to_str()?.strip_prefix("remap-path-scope="); } - } - filter_overlapping_matches(matches) -} - -fn is_arg_path_boundary(value: &[u8], start: usize) -> bool { - start == 0 || matches!(value[start - 1], b'=' | b',') + flag.to_str()?.strip_prefix("--remap-path-scope=") + }); + scopes.next().is_none_or(|first| { + iter::once(first) + .chain(scopes) + .any(|scope| scope.split(',').any(|scope| scope == "all")) + }) } -fn filter_overlapping_matches(mut matches: Vec<(usize, usize)>) -> Vec<(usize, usize)> { - if matches.is_empty() { - return matches; +fn remap_path(path: &Path, arguments: &[(OsString, Option)]) -> Option { + if !remap_scope_is_all(arguments) { + return None; } - // Sort by start ascending; break ties by length descending so the longest - // match at a given position wins (e.g. `/a/b/` before `/a/` when both are - // in the basedirs list). - matches.sort_by(|x, y| x.0.cmp(&y.0).then(y.1.cmp(&x.1))); - // Coalesce overlapping matches, keeping the first (longest) at each - // position. - let mut filtered: Vec<(usize, usize)> = Vec::new(); - let mut last_end = 0; - for (s, e) in matches { - if s >= last_end { - filtered.push((s, e)); - last_end = e; + for (flag, value) in arguments.iter().rev() { + if flag != "--remap-path-prefix" { + continue; + } + let (prefix, replacement) = value.as_ref()?.to_str()?.rsplit_once('=')?; + if let Ok(suffix) = path.strip_prefix(prefix) { + let remapped = if suffix.as_os_str().is_empty() { + PathBuf::from(replacement) + } else { + Path::new(replacement).join(suffix) + }; + return Some(remapped.into_os_string()); } } - filtered -} - -fn strip_matches<'a>(value: &'a [u8], matches: &[(usize, usize)]) -> Cow<'a, [u8]> { - if matches.is_empty() { - return Cow::Borrowed(value); - } - let mut out = Vec::with_capacity(value.len()); - let mut pos = 0; - for &(start, end) in matches { - out.extend_from_slice(&value[pos..start]); - pos = end; - } - out.extend_from_slice(&value[pos..]); - Cow::Owned(out) + None } -/// Match Windows basedirs case-insensitively while preserving the original -/// non-path bytes in the cache key. Non-ASCII arguments remain location-sensitive -/// because Unicode case folding can change byte offsets. -#[cfg(any(windows, test))] -fn strip_windows_basedirs_in_arg<'a>(value: &'a [u8], basedirs: &[Vec]) -> Cow<'a, [u8]> { - if !value.is_ascii() { - return Cow::Borrowed(value); - } - let normalized = crate::util::normalize_win_path(value); - debug_assert_eq!(normalized.len(), value.len()); - let matches = find_basedir_matches(&normalized, basedirs); - strip_matches(value, &matches) +fn is_path_cargo_env(var: &OsString) -> bool { + matches!( + var.to_str(), + Some( + "CARGO_HOME" + | "CARGO_MANIFEST_DIR" + | "CARGO_MANIFEST_PATH" + | "CARGO_TARGET_DIR" + | "CARGO_TARGET_TMPDIR" + | "CARGO_WORKSPACE_DIR" + ) + ) || var.as_encoded_bytes().starts_with(b"CARGO_BIN_EXE_") } #[cfg(feature = "dist-client")] @@ -347,8 +284,6 @@ pub struct ParsedArguments { /// /// For more information, see https://doc.rust-lang.org/rustc/profile-guided-optimization.html profile: Option, - /// Profile data and compiler plugins whose contents affect code generation. - codegen_input_files: Vec, /// If `-Z profile` has been enabled, we will use a GCC-compatible, gcov-based /// coverage implementation. /// @@ -1318,7 +1253,6 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments CompilerArguments extra_filename = Some(value.to_owned()), ("extra-filename", None) => cannot_cache!("extra-filename"), - ("profile-use", Some(v)) => { - profile = Some(v.clone()); - codegen_input_files.push(v.into()); - } + ("profile-use", Some(v)) => profile = Some(v.clone()), // Incremental compilation makes a mess of sccache's entire world // view. It produces additional compiler outputs that we don't cache, // and just letting rustc do its work in incremental mode is likely @@ -1395,16 +1326,10 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments (), } } - Some(Unstable(ArgUnstable { opt, value })) => match (opt.as_ref(), value.as_deref()) { - ("profile", Some("y") | Some("yes") | Some("on") | None) => { + Some(Unstable(ArgUnstable { opt, value })) => match value.as_deref() { + Some("y") | Some("yes") | Some("on") | None if opt == "profile" => { gcno = true; } - ("profile-sample-use", Some(path)) => { - codegen_input_files.push(path.into()); - } - ("llvm-plugins", Some(paths)) => { - codegen_input_files.extend(paths.split_whitespace().map(PathBuf::from)); - } _ => (), }, Some(Color(value)) => { @@ -1559,7 +1484,6 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments>(); - let codegen_input_hashes = hash_all(&codegen_input_files, pool); - // Perform all hashing operations on the files. let ( (source_files, source_hashes, mut env_deps), extern_hashes, staticlib_hashes, target_json_hash, - codegen_input_hashes, ) = futures::try_join!( source_files_and_hashes_and_env_deps, extern_hashes, staticlib_hashes, - target_json_hash, - codegen_input_hashes + target_json_hash )?; // If you change any of the inputs to the hash, you should change `CACHE_VERSION`. @@ -1708,17 +1619,9 @@ where } let weak_toolchain_key = m.clone().finish(); // 3. The full commandline (self.arguments) - // TODO: there will be full paths here, it would be nice to - // normalize them so we can get cross-machine cache hits. // A few argument types are not passed in a deterministic order // by cargo: --extern, -L, --cfg. We'll filter those out, sort them, // and append them to the rest of the arguments. - // Strip basedir occurrences per-argument before hashing. Handles both - // the common source-file-path arg (`/abs/path/src.rs`) and patterns - // that embed paths after `=` or `,`: `--remap-path-prefix=/abs/path`, - // `-Clinker=/abs/path`, `-Clink-arg=-Wl,-rpath,/abs/path`. See - // mozilla/sccache#2652. - let mut args_bytes = Vec::new(); { let (mut sortables, rest): (Vec<_>, Vec<_>) = os_string_arguments .iter() @@ -1743,31 +1646,40 @@ where // out, sort them, and append them to the rest of the arguments. .partition(|&(arg, _)| arg == "--cfg"); sortables.sort(); - for arg in rest - .into_iter() - .chain(sortables) - .flat_map(|(arg, val)| iter::once(arg).chain(val.as_ref())) - { - args_bytes - .extend_from_slice(&strip_basedirs_in_arg(arg.as_encoded_bytes(), basedirs)); + let mut hash_arg = |normalized: bool, value: &[u8]| { + normalized.hash(&mut HashToDigest { digest: &mut m }); + value.hash(&mut HashToDigest { digest: &mut m }); + }; + for (arg, value) in rest.into_iter().chain(sortables) { + let arg_bytes = arg.as_encoded_bytes(); + if value.is_none() + && let Some(remapped) = remap_path(Path::new(arg), &os_string_arguments) + { + hash_arg(true, remapped.as_encoded_bytes()); + } else { + hash_arg(false, arg_bytes); + } + if let Some(value) = value { + let value_bytes = value.as_encoded_bytes(); + let normalized = normalize_arg_value(arg, value, basedirs); + let normalized_bytes: &[u8] = &normalized; + hash_arg(normalized_bytes != value_bytes, normalized_bytes); + } } } - args_bytes.hash(&mut HashToDigest { digest: &mut m }); // 4. The digest of all source files (this includes src file from cmdline). // 5. The digest of all files listed on the commandline (self.externs). // 6. The digest of all static libraries listed on the commandline (self.staticlibs). // 7. The digest of the content of the target json file specified via `--target` (if any). - // 8. The digest of PGO data and LLVM plugins (if any). for h in source_hashes .into_iter() .chain(extern_hashes) .chain(staticlib_hashes) .chain(target_json_hash) - .chain(codegen_input_hashes) { m.update(h.as_bytes()); } - // 9. Environment variables: Hash all environment variables listed in the rustc dep-info + // 8. Environment variables: Hash all environment variables listed in the rustc dep-info // output. Additionally also has all environment variables starting with `CARGO_`, // since those are not listed in dep-info but affect cacheability. env_deps.sort(); @@ -1807,19 +1719,28 @@ where var.hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); - // Strip any basedir prefix from every CARGO_* env var value. - // Stripping is a no-op for values that don't start with a basedir, - // so this is safe to apply to non-path vars too and avoids the - // whitelist-maintenance bug class (CARGO_TARGET_DIR, CARGO_HOME, - // future additions, etc.). let val_bytes = val.as_encoded_bytes(); - strip_basedir_prefix(val_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); + let normalized = if is_path_cargo_env(var) { + strip_basedir_prefix(val_bytes, basedirs) + } else { + Cow::Borrowed(val_bytes) + }; + let normalized_bytes: &[u8] = &normalized; + (normalized_bytes != val_bytes).hash(&mut HashToDigest { digest: &mut m }); + normalized.hash(&mut HashToDigest { digest: &mut m }); } - // 10. The cwd of the compile. This will wind up in the rlib. - // Strip basedir prefix for cross-machine cache portability. + // 9. The cwd of the compile. This will wind up in the rlib. let cwd_bytes = cwd.as_os_str().as_encoded_bytes(); - strip_basedir_prefix(cwd_bytes, basedirs).hash(&mut HashToDigest { digest: &mut m }); - // 11. The version of the compiler. + if let Some(remapped) = remap_path(&cwd, &os_string_arguments) { + true.hash(&mut HashToDigest { digest: &mut m }); + remapped + .as_encoded_bytes() + .hash(&mut HashToDigest { digest: &mut m }); + } else { + false.hash(&mut HashToDigest { digest: &mut m }); + cwd_bytes.hash(&mut HashToDigest { digest: &mut m }); + } + // 10. The version of the compiler. self.version.hash(&mut HashToDigest { digest: &mut m }); // Turn arguments into a simple Vec to calculate outputs. @@ -3464,7 +3385,7 @@ abc def.rs: #[cfg(not(windows))] #[test] - fn test_parse_dep_info_cwd_unix() { + fn test_parse_dep_info_cwd() { let deps = "foo: baz.rs abc.rs bar.rs baz.rs: @@ -3486,7 +3407,7 @@ bar.rs: #[cfg(not(windows))] #[test] - fn test_parse_dep_info_abs_paths_unix() { + fn test_parse_dep_info_abs_paths() { let deps = "/foo/foo: /foo/baz.rs /foo/abc.rs /foo/bar.rs /foo/baz.rs: @@ -3503,7 +3424,7 @@ bar.rs: #[cfg(windows)] #[test] - fn test_parse_dep_info_cwd_windows() { + fn test_parse_dep_info_cwd() { let deps = "foo: baz.rs abc.rs bar.rs baz.rs: @@ -3529,7 +3450,7 @@ bar.rs: #[cfg(windows)] #[test] - fn test_parse_dep_info_abs_paths_windows() { + fn test_parse_dep_info_abs_paths() { let deps = "c:/foo/foo: c:/foo/baz.rs c:/foo/abc.rs c:/foo/bar.rs c:/foo/baz.rs: c:/foo/bar.rs @@ -3778,7 +3699,6 @@ proc_macro false color_mode: ColorMode::Auto, has_json: false, profile: None, - codegen_input_files: vec![], gcno: None, target_json: None, }, @@ -3809,7 +3729,7 @@ proc_macro false false, &pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode, vec![])), + Arc::new(MockStorage::new(None, preprocessor_cache_mode)), CacheControl::Default, ) .wait() @@ -3823,10 +3743,10 @@ proc_macro false // sysroot shlibs digests. m.update(FAKE_DIGEST.as_bytes()); // Arguments, with cfgs sorted at the end. - let args_str = OsStr::new("ab--cfgabc--cfgxyz"); - args_str - .as_encoded_bytes() - .hash(&mut HashToDigest { digest: &mut m }); + for arg in ["a", "b", "--cfg", "abc", "--cfg", "xyz"] { + false.hash(&mut HashToDigest { digest: &mut m }); + arg.as_bytes().hash(&mut HashToDigest { digest: &mut m }); + } // bar.rs (source file, from dep-info) m.update(empty_digest.as_bytes()); // foo.rs (source file, from dep-info) @@ -3839,13 +3759,14 @@ proc_macro false // Env vars OsStr::new("CARGO_BLAH").hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); - OsStr::new("abc") - .as_encoded_bytes() - .hash(&mut HashToDigest { digest: &mut m }); + false.hash(&mut HashToDigest { digest: &mut m }); + b"abc".as_slice().hash(&mut HashToDigest { digest: &mut m }); OsStr::new("CARGO_PKG_NAME").hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); - OsStr::new("foo").hash(&mut HashToDigest { digest: &mut m }); + false.hash(&mut HashToDigest { digest: &mut m }); + b"foo".as_slice().hash(&mut HashToDigest { digest: &mut m }); // cwd + false.hash(&mut HashToDigest { digest: &mut m }); f.tempdir .path() .as_os_str() @@ -3861,11 +3782,10 @@ proc_macro false fn hash_key( f: &TestFixture, - args: &[&'static str], + args: &[&str], env_vars: &[(OsString, OsString)], pre_func: F, preprocessor_cache_mode: bool, - basedirs: Vec>, ) -> String where F: Fn(&Path) -> Result<()>, @@ -3876,14 +3796,27 @@ proc_macro false env_vars, pre_func, preprocessor_cache_mode, - basedirs, + vec![], &[], ) } + fn hash_key_with_basedirs( + f: &TestFixture, + args: &[&str], + env_vars: &[(OsString, OsString)], + pre_func: F, + basedirs: Vec>, + ) -> String + where + F: Fn(&Path) -> Result<()>, + { + hash_key_with_env_deps(f, args, env_vars, pre_func, false, basedirs, &[]) + } + fn hash_key_with_env_deps( f: &TestFixture, - args: &[&'static str], + args: &[&str], env_vars: &[(OsString, OsString)], pre_func: F, preprocessor_cache_mode: bool, @@ -3935,7 +3868,7 @@ proc_macro false false, &pool, false, - Arc::new(MockStorage::new(None, preprocessor_cache_mode, basedirs)), + Arc::new(MockStorage::new(None, preprocessor_cache_mode).with_basedirs(basedirs)), CacheControl::Default, ) .wait() @@ -3980,7 +3913,6 @@ proc_macro false &[], mk_files, preprocessor_cache_mode, - vec![], ), hash_key( &f, @@ -4002,7 +3934,6 @@ proc_macro false &[], mk_files, preprocessor_cache_mode, - vec![], ) ); } @@ -4032,7 +3963,6 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ), hash_key( &f, @@ -4054,7 +3984,6 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ) ); } @@ -4087,7 +4016,6 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ), hash_key( &f, @@ -4111,7 +4039,6 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ) ); } @@ -4139,7 +4066,6 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ), hash_key( &f, @@ -4161,7 +4087,6 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ) ); } @@ -4191,7 +4116,6 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ), hash_key( &f, @@ -4213,54 +4137,22 @@ proc_macro false &[], nothing, preprocessor_cache_mode, - vec![], ) ); } - #[test] - fn test_basedirs_strips_cwd_and_cargo_manifest_dir() { - let f = TestFixture::new(); - let cwd = f.tempdir.path().to_string_lossy().into_owned(); - - let args = &[ - "--emit", - "link", - "foo.rs", - "--out-dir", - "out", - "--crate-name", - "foo", - "--crate-type", - "lib", - ]; - - let manifest_dir = format!("{}/some/pkg", cwd); - let env_vars = vec![ - ( - OsString::from("CARGO_MANIFEST_DIR"), - OsString::from(&manifest_dir), - ), - (OsString::from("CARGO_PKG_NAME"), OsString::from("foo")), - ]; - - let key_without = hash_key(&f, args, &env_vars, nothing, false, vec![]); + const BASEDIR_ARGS: &[&str] = &[ + "--emit", + "link", + "foo.rs", + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + ]; - // Basedirs are normalized at config time (forward slashes, lowercase - // on Windows, trailing slash); replicate that here. - let basedir = cwd.into_bytes(); - #[cfg(windows)] - let basedir = crate::util::normalize_win_path(&basedir); - let mut basedir = basedir; - basedir.push(b'/'); - let key_with = hash_key(&f, args, &env_vars, nothing, false, vec![basedir]); - - assert_ne!(key_without, key_with, "basedirs should change the hash key"); - } - - /// Build the canonical basedir byte-string for a given tempdir path: - /// normalize on Windows, append a trailing `/`. Matches how - /// `Config` stores basedirs at runtime. fn basedir_for(path: &Path) -> Vec { let bytes = path.to_string_lossy().into_owned().into_bytes(); #[cfg(windows)] @@ -4272,101 +4164,51 @@ proc_macro false #[test] fn test_basedirs_stable_across_absolute_paths() { - // The central guarantee of this feature: when two machines build the - // same crate from different absolute checkout paths, supplying each - // side's checkout root as a basedir produces identical hash keys, so - // one machine's cache entry is a hit on the other. let f1 = TestFixture::new(); let f2 = TestFixture::new(); - assert_ne!( - f1.tempdir.path(), - f2.tempdir.path(), - "fixtures must be at different absolute paths" - ); - - let args = &[ - "--emit", - "link", - "foo.rs", - "--out-dir", - "out", - "--crate-name", - "foo", - "--crate-type", - "lib", - ]; - - // Matching CARGO_MANIFEST_DIR under each fixture exercises the env-var - // basedir-stripping path in addition to the cwd-stripping path. - let manifest1 = format!("{}/some/pkg", f1.tempdir.path().display()); - let manifest2 = format!("{}/some/pkg", f2.tempdir.path().display()); - let env1 = vec![ - ( - OsString::from("CARGO_MANIFEST_DIR"), - OsString::from(&manifest1), - ), - (OsString::from("CARGO_PKG_NAME"), OsString::from("foo")), - ]; - let env2 = vec![ - ( - OsString::from("CARGO_MANIFEST_DIR"), - OsString::from(&manifest2), - ), - (OsString::from("CARGO_PKG_NAME"), OsString::from("foo")), - ]; - - let k1 = hash_key( - &f1, - args, - &env1, - nothing, - false, - vec![basedir_for(f1.tempdir.path())], - ); - let k2 = hash_key( - &f2, - args, - &env2, - nothing, - false, - vec![basedir_for(f2.tempdir.path())], - ); - assert_eq!( - k1, k2, - "basedir stripping must produce identical hashes across different checkout paths" - ); + let unremapped_key = |f: &TestFixture| { + hash_key_with_basedirs( + f, + BASEDIR_ARGS, + &[], + nothing, + vec![basedir_for(f.tempdir.path())], + ) + }; + assert_ne!(unremapped_key(&f1), unremapped_key(&f2)); + + let key = |f: &TestFixture| { + let manifest = f.tempdir.path().join("package"); + let remap = format!("{}=/workspace", f.tempdir.path().display()); + let mut args = BASEDIR_ARGS.to_vec(); + args.extend(["--remap-path-prefix", &remap]); + hash_key_with_basedirs( + f, + &args, + &[("CARGO_MANIFEST_DIR".into(), manifest.into_os_string())], + nothing, + vec![basedir_for(f.tempdir.path())], + ) + }; + assert_eq!(key(&f1), key(&f2)); } #[test] fn test_basedirs_preserve_path_sensitive_env_dependencies() { let f1 = TestFixture::new(); let f2 = TestFixture::new(); - let args = &[ - "--emit", - "link", - "foo.rs", - "--out-dir", - "out", - "--crate-name", - "foo", - "--crate-type", - "lib", - ]; let manifest1 = f1.tempdir.path().to_string_lossy().into_owned(); let manifest2 = f2.tempdir.path().to_string_lossy().into_owned(); - let env1 = vec![( - OsString::from("CARGO_MANIFEST_DIR"), - OsString::from(&manifest1), - )]; - let env2 = vec![( - OsString::from("CARGO_MANIFEST_DIR"), - OsString::from(&manifest2), - )]; - + let remap1 = format!("{}=/workspace", f1.tempdir.path().display()); + let remap2 = format!("{}=/workspace", f2.tempdir.path().display()); + let mut args1 = BASEDIR_ARGS.to_vec(); + let mut args2 = BASEDIR_ARGS.to_vec(); + args1.extend(["--remap-path-prefix", &remap1]); + args2.extend(["--remap-path-prefix", &remap2]); let k1 = hash_key_with_env_deps( &f1, - args, - &env1, + &args1, + &[("CARGO_MANIFEST_DIR".into(), manifest1.clone().into())], nothing, false, vec![basedir_for(f1.tempdir.path())], @@ -4374,303 +4216,94 @@ proc_macro false ); let k2 = hash_key_with_env_deps( &f2, - args, - &env2, + &args2, + &[("CARGO_MANIFEST_DIR".into(), manifest2.clone().into())], nothing, false, vec![basedir_for(f2.tempdir.path())], &[("CARGO_MANIFEST_DIR", &manifest2)], ); - assert_ne!( - k1, k2, - "env! values reported by rustc must remain location-sensitive" - ); - } - - #[test] - fn test_basedirs_hash_profile_use_contents() { - fn write_profile_a(path: &Path) -> Result<()> { - fs::write(path.join("profile.profdata"), b"profile-a")?; - Ok(()) - } - - fn write_profile_b(path: &Path) -> Result<()> { - fs::write(path.join("profile.profdata"), b"profile-b")?; - Ok(()) - } - - let f1 = TestFixture::new(); - let f2 = TestFixture::new(); - let args = &[ - "--emit", - "link", - "foo.rs", - "--out-dir", - "out", - "--crate-name", - "foo", - "--crate-type", - "lib", - "-C", - "profile-use=profile.profdata", - ]; - let k1 = hash_key( - &f1, - args, - &[], - write_profile_a, - false, - vec![basedir_for(f1.tempdir.path())], - ); - let k2 = hash_key( - &f2, - args, - &[], - write_profile_b, - false, - vec![basedir_for(f2.tempdir.path())], - ); - - assert_ne!(k1, k2, "different profile data must produce different keys"); - } - - #[test] - fn test_basedirs_hash_sample_profile_contents() { - fn write_profile_a(path: &Path) -> Result<()> { - fs::write(path.join("sample.prof"), b"sample-a")?; - Ok(()) - } - - fn write_profile_b(path: &Path) -> Result<()> { - fs::write(path.join("sample.prof"), b"sample-b")?; - Ok(()) - } - - let f1 = TestFixture::new(); - let f2 = TestFixture::new(); - let args = &[ - "--emit", - "link", - "foo.rs", - "--out-dir", - "out", - "--crate-name", - "foo", - "--crate-type", - "lib", - "-Z", - "profile-sample-use=sample.prof", - ]; - let k1 = hash_key( - &f1, - args, - &[], - write_profile_a, - false, - vec![basedir_for(f1.tempdir.path())], - ); - let k2 = hash_key( - &f2, - args, - &[], - write_profile_b, - false, - vec![basedir_for(f2.tempdir.path())], - ); - - assert_ne!( - k1, k2, - "different sample profiles must produce different keys" - ); + assert_ne!(k1, k2); } #[test] - fn test_basedirs_hash_all_llvm_plugins() { - fn write_plugins_a(path: &Path) -> Result<()> { - fs::write(path.join("first.so"), b"first")?; - fs::write(path.join("second.so"), b"second-a")?; - Ok(()) - } - - fn write_plugins_b(path: &Path) -> Result<()> { - fs::write(path.join("first.so"), b"first")?; - fs::write(path.join("second.so"), b"second-b")?; - Ok(()) + fn test_normalize_path_arguments() { + let basedirs = [b"/home/user/".to_vec()]; + for (flag, value, expected) in [ + ("--remap-path-prefix", "/home/user=/new", "=/new"), + ("--remap-path-prefix", "/home/user/src=/new", "src=/new"), + ( + "-C", + "profile-use=/home/user/profile", + "profile-use=/home/user/profile", + ), + ("-C", "metadata=/home/user/id", "metadata=/home/user/id"), + ( + "-C", + "link-arg=-Wl,-rpath,/home/user/lib", + "link-arg=-Wl,-rpath,/home/user/lib", + ), + ] { + let flag = flag.into(); + let value = value.into(); + assert_eq!( + &*super::normalize_arg_value(&flag, &value, &basedirs), + expected.as_bytes() + ); } - let f1 = TestFixture::new(); - let f2 = TestFixture::new(); - let args = &[ - "--emit", - "link", - "foo.rs", - "--out-dir", - "out", - "--crate-name", - "foo", - "--crate-type", - "lib", - "-Z", - "llvm-plugins=first.so second.so", - ]; - let k1 = hash_key( - &f1, - args, - &[], - write_plugins_a, - false, - vec![basedir_for(f1.tempdir.path())], - ); - let k2 = hash_key( - &f2, - args, - &[], - write_plugins_b, - false, - vec![basedir_for(f2.tempdir.path())], - ); - - assert_ne!(k1, k2, "all LLVM plugin contents must affect the key"); - } - - #[test] - - fn test_strip_basedir_prefix_no_match() { - let out = super::strip_basedir_prefix(b"/other/path", &[b"/home/runner/".to_vec()]); assert_eq!( - &*out, b"/other/path", - "no match should return value unchanged" + &*super::strip_basedir_prefix( + b"/home/user/src/lib.rs", + &[b"/home/".to_vec(), b"/home/user/".to_vec()] + ), + b"src/lib.rs" ); - } - - #[test] - - fn test_strip_basedir_prefix_longest_match_wins() { - let basedirs = vec![b"/home/".to_vec(), b"/home/runner/".to_vec()]; - let out = super::strip_basedir_prefix(b"/home/runner/src/foo.rs", &basedirs); - assert_eq!(&*out, b"src/foo.rs"); - } - - // strip_basedirs_in_arg covers embedded-path arg patterns (mozilla/sccache#2652). - - #[test] - - fn test_strip_basedirs_in_arg_prefix() { - let out = - super::strip_basedirs_in_arg(b"/home/user/src/lib.rs", &[b"/home/user/".to_vec()]); - assert_eq!(&*out, b"src/lib.rs"); - } - - #[test] - - fn test_strip_basedirs_in_arg_after_equals() { - // `--remap-path-prefix=/abs/path=/new` -- basedir appears after `=`. - let out = super::strip_basedirs_in_arg( - b"--remap-path-prefix=/home/user/a=/new", - &[b"/home/user/".to_vec()], + assert_eq!( + &*super::strip_basedir_prefix(b"/other/path", &basedirs), + b"/other/path" ); - assert_eq!(&*out, b"--remap-path-prefix=a=/new"); - } - - #[test] - fn test_strip_basedirs_in_arg_exact_basedir() { - let out = super::strip_basedirs_in_arg( - b"--remap-path-prefix=/home/user=/new", - &[b"/home/user/".to_vec()], + assert!(super::is_path_cargo_env(&"CARGO_MANIFEST_DIR".into())); + assert!(!super::is_path_cargo_env(&"CARGO_PKG_DESCRIPTION".into())); + + let remap = |prefix: &str| { + vec![( + "--remap-path-prefix".into(), + Some(format!("{prefix}=/workspace").into()), + )] + }; + assert!(super::remap_path(Path::new("/home/user/project"), &remap("/home/user")).is_some()); + assert!(super::remap_path(Path::new("/home/user"), &remap("/home/user=part")).is_none()); + assert_eq!( + super::remap_path(Path::new("/home/user"), &remap("/")), + Some("/workspace/home/user".into()) ); - assert_eq!(&*out, b"--remap-path-prefix==/new"); - } - - #[test] - fn test_windows_arg_matching_preserves_non_path_case() { - let arg = b"--remap-path-prefix=C:\\Work\\Repo=VIRTUAL"; - let out = super::strip_windows_basedirs_in_arg(arg, &[b"c:/work/repo/".to_vec()]); - assert_eq!(&*out, b"--remap-path-prefix==VIRTUAL"); - } - #[test] - fn test_windows_non_ascii_arg_remains_location_sensitive() { - let arg = b"--remap-path-prefix=C:\\Work\\Repo=\xc4\xb0"; - let out = super::strip_windows_basedirs_in_arg(arg, &[b"c:/work/repo/".to_vec()]); - assert_eq!(&*out, arg); + let mut diagnostics = remap("/home/user"); + diagnostics.push(("--remap-path-scope=diagnostics".into(), None)); + assert!(super::remap_path(Path::new("/home/user/project"), &diagnostics).is_none()); + diagnostics.push(("--remap-path-scope=all".into(), None)); + assert!(super::remap_path(Path::new("/home/user/project"), &diagnostics).is_some()); } + #[cfg(windows)] #[test] - fn test_strip_basedirs_in_arg_after_comma() { - let out = super::strip_basedirs_in_arg( - b"-Clink-arg=-Wl,-rpath,/home/user/lib", - &[b"/home/user/".to_vec()], + fn test_windows_basedir_matching_preserves_suffix() { + let basedirs = [b"c:/work/repo/".to_vec()]; + assert_eq!( + &*super::strip_basedir_prefix(b"C:\\Work\\Repo\\MixedCase", &basedirs), + b"MixedCase" ); - assert_eq!(&*out, b"-Clink-arg=-Wl,-rpath,lib"); - } - - #[test] - fn test_strip_basedirs_in_arg_after_comma_lowercase() { - let out = super::strip_basedirs_in_arg( - b"-clink-arg=-wl,-rpath,/home/user/lib", - &[b"/home/user/".to_vec()], + assert_eq!( + &*super::strip_basedir_prefix(b"C:\\Work\\Repo\\\xc4\xb0", &basedirs), + b"C:\\Work\\Repo\\\xc4\xb0" ); - assert_eq!(&*out, b"-clink-arg=-wl,-rpath,lib"); - } - - #[test] - - fn test_strip_basedirs_in_arg_multiple_in_one() { - let out = - super::strip_basedirs_in_arg(b"/home/user/a,/home/user/b", &[b"/home/user/".to_vec()]); - assert_eq!(&*out, b"a,b"); - } - - #[test] - - fn test_strip_basedirs_in_arg_no_match_inside() { - // Basedir preceded by a non-boundary byte: no strip. - let out = - super::strip_basedirs_in_arg(b"prefix/home/user/suffix", &[b"/home/user/".to_vec()]); - assert_eq!(&*out, b"prefix/home/user/suffix"); - } - - #[test] - - fn test_strip_basedirs_in_arg_longest_at_same_position() { - // When `/a/b/` and `/a/` both match at the same position, the longer - // one wins (sort is by start asc, length desc). - let basedirs = vec![b"/a/".to_vec(), b"/a/b/".to_vec()]; - let out = super::strip_basedirs_in_arg(b"/a/b/x", &basedirs); - assert_eq!(&*out, b"x"); - } - - #[test] - fn test_basedirs_deterministic() { - // Running the same compilation with the same basedirs twice should - // produce the same hash, and it should differ from no-basedirs. - let f = TestFixture::new(); - let cwd = f.tempdir.path().to_string_lossy().into_owned(); - - let args = &[ - "--emit", - "link", - "foo.rs", - "--out-dir", - "out", - "--crate-name", - "foo", - "--crate-type", - "lib", - ]; - let env_vars = vec![(OsString::from("CARGO_PKG_NAME"), OsString::from("foo"))]; - - let basedir = cwd.into_bytes(); - #[cfg(windows)] - let basedir = crate::util::normalize_win_path(&basedir); - let mut basedir = basedir; - basedir.push(b'/'); - - let key1 = hash_key(&f, args, &env_vars, nothing, false, vec![basedir.clone()]); - let key2 = hash_key(&f, args, &env_vars, nothing, false, vec![basedir]); - - assert_eq!(key1, key2, "Same basedir should produce deterministic hash"); + let remap = vec![( + "--remap-path-prefix".into(), + Some("C:\\WORK=/workspace".into()), + )]; + assert!(super::remap_path(Path::new("C:\\work\\project"), &remap).is_none()); } #[test] diff --git a/src/compiler/tasking_vx.rs b/src/compiler/tasking_vx.rs index 26ac7676f4..b3fff8238a 100644 --- a/src/compiler/tasking_vx.rs +++ b/src/compiler/tasking_vx.rs @@ -730,7 +730,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; @@ -784,7 +784,7 @@ mod test { too_hard_for_preprocessor_cache_mode: None, }; let runtime = single_threaded_runtime(); - let storage = MockStorage::default(); + let storage = MockStorage::new(None, false); let storage: std::sync::Arc = std::sync::Arc::new(storage); let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); let compiler = &f.bins[0]; diff --git a/src/test/mock_storage.rs b/src/test/mock_storage.rs index 122993d2e4..ab939e0262 100644 --- a/src/test/mock_storage.rs +++ b/src/test/mock_storage.rs @@ -32,47 +32,29 @@ pub struct MockStorage { } impl MockStorage { - /// Construct a `MockStorage`. - /// - /// # Arguments - /// - /// * `delay` - if `Some`, every `get`/`put` sleeps this long before - /// returning, to simulate slow storage. - /// * `preprocessor_cache_mode` - value returned from - /// [`Storage::preprocessor_cache_mode_config`]. - /// * `basedirs` - the list reported by [`Storage::basedirs`], used when - /// tests need to exercise basedir-prefix stripping in - /// `generate_hash_key`. - pub(crate) fn new( - delay: Option, - preprocessor_cache_mode: bool, - basedirs: Vec>, - ) -> MockStorage { + /// Create a new `MockStorage`. if `delay` is `Some`, wait for that amount of time before returning from operations. + pub(crate) fn new(delay: Option, preprocessor_cache_mode: bool) -> MockStorage { let (tx, rx) = mpsc::unbounded(); Self { tx, rx: Arc::new(Mutex::new(rx)), delay, preprocessor_cache_mode, - basedirs, + basedirs: Vec::new(), } } + pub(crate) fn with_basedirs(mut self, basedirs: Vec>) -> Self { + self.basedirs = basedirs; + self + } + /// Queue up `res` to be returned as the next result from `Storage::get`. pub(crate) fn next_get(&self, res: Result) { self.tx.unbounded_send(res).unwrap(); } } -impl Default for MockStorage { - /// Zero-delay mock with preprocessor cache mode disabled and no basedirs - /// configured -- the usual choice for tests that don't care about those - /// knobs. - fn default() -> Self { - Self::new(None, false, vec![]) - } -} - #[async_trait] impl Storage for MockStorage { async fn get(&self, _key: &str) -> Result { diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index e954ff629e..db0e978a5a 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -64,17 +64,9 @@ impl SccacheTest<'_> { trace!("sccache --start-server"); - let mut server_cmd = Command::new(SCCACHE_BIN.as_os_str()); - server_cmd + Command::new(SCCACHE_BIN.as_os_str()) .arg("--start-server") - .env("SCCACHE_DIR", &cache_dir); - // Forward `additional_envs` to the server too: some config (e.g. - // `SCCACHE_BASEDIRS`) is only read at server startup, so passing it - // here avoids a `restart_sccache` dance in each test. - if let Some(vec) = additional_envs { - server_cmd.envs(vec.iter().cloned()); - } - server_cmd + .env("SCCACHE_DIR", &cache_dir) .assert() .try_success() .context("Failed to start sccache server")?; diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index 8918984211..02bb1a4845 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -40,11 +40,6 @@ fn test_rust_cargo_build() -> Result<()> { #[test] #[serial] fn test_rust_cargo_basedirs_cross_dir_cache_hit() -> Result<()> { - // Copy tests/test-crate to two different absolute paths, then build in - // each with SCCACHE_BASEDIRS covering both roots. Without basedir - // stripping the two builds would compute different cache keys (cwd + - // CARGO_MANIFEST_DIR differ); with it, keys converge and the second - // build hits the first build's cache entries. let work = tempfile::Builder::new() .prefix("sccache_basedirs_xdir") .tempdir() @@ -58,70 +53,38 @@ fn test_rust_cargo_basedirs_cross_dir_cache_hit() -> Result<()> { let work_root = fs::canonicalize(work.path())?; #[cfg(not(unix))] let work_root = work.path().to_path_buf(); - let root_a = work_root.join("machine_a"); - let root_b = work_root.join("machine_b"); - let crate_a = root_a.join("project"); - let crate_b = root_b.join("project"); - copy_crate(&CRATE_DIR, &crate_a)?; - copy_crate(&CRATE_DIR, &crate_b)?; - - // Basedir separator: `:` on Unix, `;` on Windows (matches config.rs). - let sep = if cfg!(windows) { ';' } else { ':' }; - let basedirs = format!("{}{sep}{}", root_a.display(), root_b.display()); - let test = SccacheTest::new(Some(&[( - "SCCACHE_BASEDIRS", - std::ffi::OsString::from(basedirs), - )]))?; - - run_cargo_build(&test, &crate_a, &crate_a.join("target"))?; - run_cargo_build(&test, &crate_b, &crate_b.join("target"))?; - - // After the second build, sccache must report Rust cache hits. The exact - // count matches the existing `test_rust_cargo_cmd` baseline (2), which - // exercises the same crate in a single directory with `cargo clean` - // between runs; if basedirs works, a cross-directory run should behave - // identically. - test.show_stats()? - .try_stdout(predicates::str::contains(r#""cache_hits":{"counts":{"Rust":2}"#).from_utf8())? - .try_success()?; - Ok(()) -} + let crate_a = work_root.join("a"); + let crate_b = work_root.join("b"); + for crate_dir in [&crate_a, &crate_b] { + fs::create_dir_all(crate_dir.join("src"))?; + fs::write( + crate_dir.join("Cargo.toml"), + "[package]\nname = \"basedirs-test\"\nversion = \"0.0.0\"\nedition = \"2021\"\n", + )?; + fs::write(crate_dir.join("src/lib.rs"), "pub fn value() -> u8 { 1 }\n")?; + } -fn copy_crate(src: &Path, dst: &Path) -> Result<()> { - use walkdir::WalkDir; - fs::create_dir_all(dst)?; - for entry in WalkDir::new(src) { - let entry = entry.context("walkdir")?; - let rel = entry.path().strip_prefix(src).unwrap(); - let target = dst.join(rel); - if entry.file_type().is_dir() { - fs::create_dir_all(&target)?; - } else if entry.file_type().is_file() { - fs::copy(entry.path(), &target)?; - } + let sep = if cfg!(windows) { ';' } else { ':' }; + let basedirs = format!("{}{sep}{}", crate_a.display(), crate_b.display()); + let test = SccacheTest::new(None)?; + restart_sccache(&test, Some(vec![("SCCACHE_BASEDIRS".into(), basedirs)]))?; + + for crate_dir in [&crate_a, &crate_b] { + Command::new(CARGO.as_os_str()) + .args(["build", "--lib"]) + .envs(test.env.iter().cloned()) + .env("CARGO_TARGET_DIR", crate_dir.join("target")) + .env( + "RUSTFLAGS", + format!("--remap-path-prefix={}=/workspace", crate_dir.display()), + ) + .current_dir(crate_dir) + .assert() + .try_success()?; } - Ok(()) -} -fn run_cargo_build(test: &SccacheTest, cwd: &Path, target_dir: &Path) -> Result<()> { - // The harness's default CARGO_TARGET_DIR is shared across invocations, - // which would let cargo short-circuit recompiles. Override per-build so - // each `cargo build` actually invokes rustc for every crate. - let env: Vec<_> = test - .env - .iter() - .filter(|(k, _)| *k != "CARGO_TARGET_DIR") - .cloned() - .chain(std::iter::once(( - "CARGO_TARGET_DIR", - target_dir.as_os_str().to_owned(), - ))) - .collect(); - Command::new(CARGO.as_os_str()) - .arg("build") - .envs(env) - .current_dir(cwd) - .assert() + test.show_stats()? + .try_stdout(predicates::str::contains(r#""cache_hits":{"counts":{"Rust":1}"#).from_utf8())? .try_success()?; Ok(()) } From 72d9bd864dbfb1fc207667b0953e60e620c4cb95 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Wed, 5 Aug 2026 18:17:38 +0200 Subject: [PATCH 04/11] rust: close remaining basedir key soundness gaps Hash effective source paths without conflating duplicate remaps, and preserve Cargo path values whenever procedural macros can run. Honor rustc remap-scope precedence, keep non-identity distributed path transforms local, add CARGO_INSTALL_ROOT coverage, and bump the key version. --- docs/Rust.md | 3 + src/compiler/rust.rs | 431 +++++++++++++++++++++++++++++++++++++---- src/dist/mod.rs | 12 ++ tests/sccache_cargo.rs | 1 + 4 files changed, 412 insertions(+), 35 deletions(-) diff --git a/docs/Rust.md b/docs/Rust.md index 1f2c6c8ad1..0b80481933 100644 --- a/docs/Rust.md +++ b/docs/Rust.md @@ -10,5 +10,8 @@ sccache includes support for caching Rust compilation. This includes many caveat * `rustc`'s incremental compilation needs to be disabled. See [The Cargo Book](https://doc.rust-lang.org/cargo/reference/profiles.html#incremental) * Crates that invoke the system linker cannot be cached. Examples are `bin`, `dylib`, `cdylib`, and `proc-macro` crates. * `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys when rustc's `--remap-path-prefix` covers the working directory with the default or `all` remap scope. It does not rewrite compiler outputs itself. +* Cargo path variables remain location-sensitive when external crates are present because transitive procedural macros can read them without reporting an environment dependency to rustc. +* Rust path normalization is limited to ASCII paths on Windows. +* Distributed Rust compilation falls back to local compilation when `--remap-path-prefix` is used with a non-identity path transformer. If you are using Rust 1.18 or later, you can ask cargo to wrap all compilation with sccache by setting `RUSTC_WRAPPER=sccache` in your build environment. diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 851f05164e..6dfff86ea4 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -125,20 +125,19 @@ fn normalize_arg_value<'a>( } fn remap_scope_is_all(arguments: &[(OsString, Option)]) -> bool { - let mut scopes = arguments.iter().filter_map(|(flag, value)| { - if flag == "--remap-path-scope" { - return value.as_ref()?.to_str(); - } - if flag == "-Z" { - return value.as_ref()?.to_str()?.strip_prefix("remap-path-scope="); - } - flag.to_str()?.strip_prefix("--remap-path-scope=") - }); - scopes.next().is_none_or(|first| { - iter::once(first) - .chain(scopes) - .any(|scope| scope.split(',').any(|scope| scope == "all")) - }) + arguments + .iter() + .filter_map(|(flag, value)| { + if flag == "--remap-path-scope" { + return value.as_ref()?.to_str(); + } + if flag == "-Z" { + return value.as_ref()?.to_str()?.strip_prefix("remap-path-scope="); + } + flag.to_str()?.strip_prefix("--remap-path-scope=") + }) + .last() + .is_none_or(|scope| scope.split(',').any(|scope| scope == "all")) } fn remap_path(path: &Path, arguments: &[(OsString, Option)]) -> Option { @@ -167,6 +166,7 @@ fn is_path_cargo_env(var: &OsString) -> bool { var.to_str(), Some( "CARGO_HOME" + | "CARGO_INSTALL_ROOT" | "CARGO_MANIFEST_DIR" | "CARGO_MANIFEST_PATH" | "CARGO_TARGET_DIR" @@ -349,7 +349,7 @@ static ALLOWED_EMIT: LazyLock> = LazyLock::new(|| ["link", "metadata", "dep-info"].iter().copied().collect()); /// Version number for cache key. -const CACHE_VERSION: &[u8] = b"7"; +const CACHE_VERSION: &[u8] = b"8"; /// Get absolute paths for all source files and env-deps listed in rustc's dep-info output. async fn get_source_files_and_env_deps( @@ -1162,6 +1162,7 @@ counted_array!(static ARGS: [ArgInfo; _] = [ take_arg!("--pretty", OsString, CanBeSeparated(b'='), NotCompilation), take_arg!("--print", OsString, CanBeSeparated(b'='), NotCompilation), take_arg!("--remap-path-prefix", OsString, CanBeSeparated(b'='), PassThrough), + take_arg!("--remap-path-scope", OsString, CanBeSeparated(b'='), PassThrough), take_arg!("--sysroot", PathBuf, CanBeSeparated(b'='), TooHardPath), take_arg!("--target", ArgTarget, CanBeSeparated(b'='), Target), take_arg!("--unpretty", OsString, CanBeSeparated(b'='), NotCompilation), @@ -1652,10 +1653,12 @@ where }; for (arg, value) in rest.into_iter().chain(sortables) { let arg_bytes = arg.as_encoded_bytes(); - if value.is_none() - && let Some(remapped) = remap_path(Path::new(arg), &os_string_arguments) - { - hash_arg(true, remapped.as_encoded_bytes()); + if value.is_none() { + if let Some(remapped) = remap_path(Path::new(arg), &os_string_arguments) { + hash_arg(true, remapped.as_encoded_bytes()); + } else { + hash_arg(false, arg_bytes); + } } else { hash_arg(false, arg_bytes); } @@ -1667,13 +1670,49 @@ where } } } - // 4. The digest of all source files (this includes src file from cmdline). + // 4. The effective path and digest of all source files (this includes src file from cmdline). // 5. The digest of all files listed on the commandline (self.externs). // 6. The digest of all static libraries listed on the commandline (self.staticlibs). // 7. The digest of the content of the target json file specified via `--target` (if any). - for h in source_hashes + let mut source_inputs = source_files + .iter() + .zip(source_hashes) + .map(|(path, hash)| { + if let Some(remapped) = remap_path(path, &os_string_arguments) { + (true, remapped, path.as_os_str().to_owned(), hash) + } else { + ( + false, + path.as_os_str().to_owned(), + path.as_os_str().to_owned(), + hash, + ) + } + }) + .collect::>(); + source_inputs.sort(); + for index in 0..source_inputs.len() { + let (normalized, path, original_path, hash) = &source_inputs[index]; + let previous_is_duplicate = index > 0 + && normalized == &source_inputs[index - 1].0 + && path == &source_inputs[index - 1].1; + let next_is_duplicate = + source_inputs + .get(index + 1) + .is_some_and(|(other_normalized, other_path, _, _)| { + normalized == other_normalized && path == other_path + }); + let has_duplicate_path = previous_is_duplicate || next_is_duplicate; + normalized.hash(&mut HashToDigest { digest: &mut m }); + path.hash(&mut HashToDigest { digest: &mut m }); + has_duplicate_path.hash(&mut HashToDigest { digest: &mut m }); + if has_duplicate_path { + original_path.hash(&mut HashToDigest { digest: &mut m }); + } + m.update(hash.as_bytes()); + } + for h in extern_hashes .into_iter() - .chain(extern_hashes) .chain(staticlib_hashes) .chain(target_json_hash) { @@ -1698,6 +1737,26 @@ where .cloned() .collect(); env_vars.sort(); + // Procedural macros can read inherited environment variables without reporting them in + // rustc dep-info. They can be explicit externs or resolved from a crate search path. + let normalize_cargo_paths = !basedirs.is_empty() + && remap_path(&cwd, &os_string_arguments).is_some() + && env_vars.iter().any(|(var, _)| is_path_cargo_env(var)) + && self.parsed_args.externs.is_empty() + && !self.parsed_args.crate_link_paths.iter().any(|path| { + fs::read_dir(path) + .map(|mut entries| { + entries.any(|entry| { + entry.map_or(true, |entry| { + entry + .path() + .extension() + .is_some_and(|ext| ext == DLL_EXTENSION) + }) + }) + }) + .unwrap_or(true) + }); for (var, val) in env_vars.iter() { if !var.starts_with("CARGO_") { continue; @@ -1720,7 +1779,7 @@ where var.hash(&mut HashToDigest { digest: &mut m }); m.update(b"="); let val_bytes = val.as_encoded_bytes(); - let normalized = if is_path_cargo_env(var) { + let normalized = if normalize_cargo_paths && is_path_cargo_env(var) { strip_basedir_prefix(val_bytes, basedirs) } else { Cow::Borrowed(val_bytes) @@ -1963,6 +2022,17 @@ impl Compilation for RustCompilation { }; } + if !path_transformer.is_identity() + && arguments + .iter() + .any(|argument| argument.flag_str() == Some("--remap-path-prefix")) + { + debug!( + "Distributed Rust compilation does not support path remaps with a non-identity path transformer" + ); + return None; + } + let mut dist_arguments = vec![]; let mut saw_target = false; @@ -2908,6 +2978,69 @@ release: 1.66.1 LLVM version: 15.0.2 "#; + #[cfg(feature = "dist-client")] + fn remapped_compilation(root: &Path) -> RustCompilation { + RustCompilation { + executable: root.join("rustc"), + host: "x86_64-unknown-linux-gnu".to_owned(), + sysroot: root.join("sysroot"), + rlib_dep_reader: None, + arguments: vec![ + Argument::Raw(root.join("src/lib.rs").into_os_string()), + Argument::WithValue( + "--remap-path-prefix", + ArgData::PassThrough(format!("{}=/workspace", root.display()).into()), + ArgDisposition::Separated, + ), + ], + inputs: vec![root.join("src/lib.rs")], + outputs: HashMap::new(), + crate_link_paths: vec![], + crate_name: "test".to_owned(), + crate_types: CrateTypes { + rlib: true, + staticlib: false, + }, + dep_info: None, + cwd: root.to_owned(), + env_vars: vec![], + } + } + + #[cfg(all(feature = "dist-client", unix))] + #[test] + fn test_distribute_remap_with_identity_path_transformer() { + let compilation = remapped_compilation(Path::new("/work")); + let mut path_transformer = dist::PathTransformer::new(); + let (_, dist_command, _) = >, + >>::generate_compile_commands( + &compilation, &mut path_transformer, false + ) + .unwrap(); + let dist_command = dist_command.unwrap(); + assert!( + dist_command + .arguments + .windows(2) + .any(|args| args == ["--remap-path-prefix", "/work=/workspace"]) + ); + } + + #[cfg(all(feature = "dist-client", windows))] + #[test] + fn test_remap_falls_back_with_non_identity_path_transformer() { + let compilation = remapped_compilation(Path::new("C:\\work")); + let mut path_transformer = dist::PathTransformer::new(); + let (_, dist_command, _) = >, + >>::generate_compile_commands( + &compilation, &mut path_transformer, false + ) + .unwrap(); + assert!(dist_command.is_none()); + } + #[test] #[allow(clippy::cognitive_complexity)] fn test_parse_arguments_simple() { @@ -3747,10 +3880,17 @@ proc_macro false false.hash(&mut HashToDigest { digest: &mut m }); arg.as_bytes().hash(&mut HashToDigest { digest: &mut m }); } - // bar.rs (source file, from dep-info) - m.update(empty_digest.as_bytes()); - // foo.rs (source file, from dep-info) - m.update(empty_digest.as_bytes()); + // Source files, sorted by effective path. + for source in ["bar.rs", "foo.rs"] { + false.hash(&mut HashToDigest { digest: &mut m }); + f.tempdir + .path() + .join(source) + .into_os_string() + .hash(&mut HashToDigest { digest: &mut m }); + false.hash(&mut HashToDigest { digest: &mut m }); + m.update(empty_digest.as_bytes()); + } // bar.rlib (extern crate, from externs) m.update(empty_digest.as_bytes()); // libbaz.a (static library, from staticlibs), containing a single @@ -3797,7 +3937,7 @@ proc_macro false pre_func, preprocessor_cache_mode, vec![], - &[], + (&[], &["foo.rs"]), ) } @@ -3811,7 +3951,15 @@ proc_macro false where F: Fn(&Path) -> Result<()>, { - hash_key_with_env_deps(f, args, env_vars, pre_func, false, basedirs, &[]) + hash_key_with_env_deps( + f, + args, + env_vars, + pre_func, + false, + basedirs, + (&[], &["foo.rs"]), + ) } fn hash_key_with_env_deps( @@ -3821,7 +3969,7 @@ proc_macro false pre_func: F, preprocessor_cache_mode: bool, basedirs: Vec>, - env_deps: &[(&str, &str)], + dep_info: (&[(&str, &str)], &[&str]), ) -> String where F: Fn(&Path) -> Result<()>, @@ -3858,7 +4006,7 @@ proc_macro false let runtime = single_threaded_runtime(); let pool = runtime.handle().clone(); - mock_dep_info(&creator, &["foo.rs"], env_deps); + mock_dep_info(&creator, dep_info.1, dep_info.0); mock_file_names(&creator, &["foo.rlib"]); hasher .generate_hash_key( @@ -4193,6 +4341,155 @@ proc_macro false assert_eq!(key(&f1), key(&f2)); } + #[test] + fn test_basedirs_keep_unremapped_source_paths() { + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + f1.touch("external.rs").unwrap(); + let external = f1.tempdir.path().join("external.rs"); + let external = external.to_str().unwrap(); + + let key = |f: &TestFixture| { + let remap = format!("{}=/workspace", f.tempdir.path().display()); + let mut args = BASEDIR_ARGS.to_vec(); + args.extend(["--remap-path-prefix", &remap]); + hash_key_with_env_deps( + f, + &args, + &[], + nothing, + false, + vec![basedir_for(f.tempdir.path())], + (&[], &["foo.rs", external]), + ) + }; + + assert_ne!(key(&f1), key(&f2)); + } + + #[test] + fn test_basedirs_preserve_duplicate_remap_source_identity() { + let f = TestFixture::new(); + let first_dir = f.tempdir.path().join("first"); + let second_dir = f.tempdir.path().join("second"); + fs::create_dir_all(&first_dir).unwrap(); + fs::create_dir_all(&second_dir).unwrap(); + let first_source = first_dir.join("source.rs"); + let second_source = second_dir.join("source.rs"); + let first_source_str = first_source.to_str().unwrap(); + let second_source_str = second_source.to_str().unwrap(); + let first_remap = format!("{}=/workspace", first_dir.display()); + let second_remap = format!("{}=/workspace", second_dir.display()); + let mut args = BASEDIR_ARGS.to_vec(); + args.extend([ + "--remap-path-prefix", + &first_remap, + "--remap-path-prefix", + &second_remap, + ]); + + let key = |first_contents: &str, second_contents: &str| { + hash_key_with_env_deps( + &f, + &args, + &[], + |_| { + fs::write(&first_source, first_contents)?; + fs::write(&second_source, second_contents)?; + Ok(()) + }, + false, + vec![basedir_for(f.tempdir.path())], + (&[], &["foo.rs", first_source_str, second_source_str]), + ) + }; + + assert_ne!(key("first", "second"), key("second", "first")); + } + + #[test] + fn test_basedirs_preserve_cargo_paths_with_external_crates() { + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + let dependency_name = "dependency.rlib"; + f1.touch(dependency_name).unwrap(); + f2.touch(dependency_name).unwrap(); + + let key = |f: &TestFixture| { + let remap = format!("{}=/workspace", f.tempdir.path().display()); + let dependency = format!( + "dependency={}", + f.tempdir.path().join(dependency_name).display() + ); + let mut args = BASEDIR_ARGS + .iter() + .map(|arg| (*arg).to_owned()) + .collect::>(); + args.extend([ + "--remap-path-prefix".to_owned(), + remap, + "--extern".to_owned(), + dependency, + ]); + let args = args.iter().map(String::as_str).collect::>(); + hash_key_with_env_deps( + f, + &args, + &[( + "CARGO_MANIFEST_DIR".into(), + f.tempdir.path().as_os_str().to_owned(), + )], + nothing, + false, + vec![basedir_for(f.tempdir.path())], + (&[], &["foo.rs"]), + ) + }; + + assert_ne!(key(&f1), key(&f2)); + } + + #[test] + fn test_basedirs_preserve_cargo_paths_with_dynamic_crate_search() { + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + for f in [&f1, &f2] { + let deps = f.tempdir.path().join("deps"); + fs::create_dir(&deps).unwrap(); + File::create(deps.join(format!("proc_macro.{DLL_EXTENSION}"))).unwrap(); + } + + let key = |f: &TestFixture| { + let remap = format!("{}=/workspace", f.tempdir.path().display()); + let dependency_path = format!("dependency={}", f.tempdir.path().join("deps").display()); + let mut args = BASEDIR_ARGS + .iter() + .map(|arg| (*arg).to_owned()) + .collect::>(); + args.extend([ + "--remap-path-prefix".to_owned(), + remap, + "-L".to_owned(), + dependency_path, + ]); + let args = args.iter().map(String::as_str).collect::>(); + hash_key_with_env_deps( + f, + &args, + &[( + "CARGO_MANIFEST_DIR".into(), + f.tempdir.path().as_os_str().to_owned(), + )], + nothing, + false, + vec![basedir_for(f.tempdir.path())], + (&[], &["foo.rs"]), + ) + }; + + assert_ne!(key(&f1), key(&f2)); + } + #[test] fn test_basedirs_preserve_path_sensitive_env_dependencies() { let f1 = TestFixture::new(); @@ -4212,7 +4509,7 @@ proc_macro false nothing, false, vec![basedir_for(f1.tempdir.path())], - &[("CARGO_MANIFEST_DIR", &manifest1)], + (&[("CARGO_MANIFEST_DIR", &manifest1)], &["foo.rs"]), ); let k2 = hash_key_with_env_deps( &f2, @@ -4221,7 +4518,7 @@ proc_macro false nothing, false, vec![basedir_for(f2.tempdir.path())], - &[("CARGO_MANIFEST_DIR", &manifest2)], + (&[("CARGO_MANIFEST_DIR", &manifest2)], &["foo.rs"]), ); assert_ne!(k1, k2); @@ -4265,6 +4562,7 @@ proc_macro false b"/other/path" ); assert!(super::is_path_cargo_env(&"CARGO_MANIFEST_DIR".into())); + assert!(super::is_path_cargo_env(&"CARGO_INSTALL_ROOT".into())); assert!(!super::is_path_cargo_env(&"CARGO_PKG_DESCRIPTION".into())); let remap = |prefix: &str| { @@ -4279,12 +4577,40 @@ proc_macro false super::remap_path(Path::new("/home/user"), &remap("/")), Some("/workspace/home/user".into()) ); + assert_eq!( + super::remap_path(Path::new("/home/user/project"), &remap("/home/user/")), + Some("/workspace/project".into()) + ); + assert!( + super::remap_path(Path::new("/home/username/project"), &remap("/home/user")).is_none() + ); + assert_eq!( + super::remap_path(Path::new("/home/a=b/project"), &remap("/home/a=b")), + Some("/workspace/project".into()) + ); + + let overlapping = vec![ + ("--remap-path-prefix".into(), Some("/home=/first".into())), + ( + "--remap-path-prefix".into(), + Some("/home/user=/last".into()), + ), + ]; + assert_eq!( + super::remap_path(Path::new("/home/user/project"), &overlapping), + Some("/last/project".into()) + ); let mut diagnostics = remap("/home/user"); diagnostics.push(("--remap-path-scope=diagnostics".into(), None)); assert!(super::remap_path(Path::new("/home/user/project"), &diagnostics).is_none()); - diagnostics.push(("--remap-path-scope=all".into(), None)); - assert!(super::remap_path(Path::new("/home/user/project"), &diagnostics).is_some()); + let mut all = remap("/home/user"); + all.push(("--remap-path-scope=all".into(), None)); + assert!(super::remap_path(Path::new("/home/user/project"), &all).is_some()); + all.push(("-Z".into(), Some("remap-path-scope=diagnostics".into()))); + assert!(super::remap_path(Path::new("/home/user/project"), &all).is_none()); + all.push(("--remap-path-scope=all".into(), None)); + assert!(super::remap_path(Path::new("/home/user/project"), &all).is_some()); } #[cfg(windows)] @@ -4367,6 +4693,41 @@ proc_macro false ))); } + #[test] + fn test_parse_remap_path_scope() { + for h in [ + parses!( + "--crate-name", + "foo", + "--crate-type", + "lib", + "./src/lib.rs", + "--emit=dep-info,link", + "--out-dir", + "/out", + "--remap-path-scope", + "all" + ), + parses!( + "--crate-name", + "foo", + "--crate-type", + "lib", + "./src/lib.rs", + "--emit=dep-info,link", + "--out-dir", + "/out", + "--remap-path-scope=all" + ), + ] { + assert!(h.arguments.contains(&Argument::WithValue( + "--remap-path-scope", + ArgData::PassThrough(OsString::from("all")), + ArgDisposition::Separated + ))); + } + } + #[test] fn test_parse_target() { // Parse a --target argument that is a string (not a path to a .json file). diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 6bc1024aa8..3eb2d6ccb8 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -103,6 +103,9 @@ mod path_transform { dist_to_local_path: HashMap::new(), } } + pub fn is_identity(&self) -> bool { + false + } pub fn as_dist_abs(&mut self, p: &Path) -> Option { if !p.is_absolute() { return None; @@ -190,6 +193,7 @@ mod path_transform { #[test] fn test_basic() { let mut pt = PathTransformer::new(); + assert!(!pt.is_identity()); assert_eq!(pt.as_dist(Path::new("C:/a")).unwrap(), "/prefix/disk-C/a"); assert_eq!( pt.as_dist(Path::new(r#"C:\a\b.c"#)).unwrap(), @@ -276,6 +280,9 @@ mod path_transform { pub fn new() -> Self { PathTransformer } + pub fn is_identity(&self) -> bool { + true + } pub fn as_dist_abs(&mut self, p: &Path) -> Option { if !p.is_absolute() { return None; @@ -292,6 +299,11 @@ mod path_transform { Some(PathBuf::from(p)) } } + + #[test] + fn test_identity() { + assert!(PathTransformer::new().is_identity()); + } } pub fn osstrings_to_strings(osstrings: &[OsString]) -> Option> { diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index 02bb1a4845..aae4be025e 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -73,6 +73,7 @@ fn test_rust_cargo_basedirs_cross_dir_cache_hit() -> Result<()> { Command::new(CARGO.as_os_str()) .args(["build", "--lib"]) .envs(test.env.iter().cloned()) + .env("CARGO_INSTALL_ROOT", crate_dir.join("install")) .env("CARGO_TARGET_DIR", crate_dir.join("target")) .env( "RUSTFLAGS", From 928dbf6e4ada549074f208f3ce1e2d1a2ad615ba Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Wed, 5 Aug 2026 18:42:22 +0200 Subject: [PATCH 05/11] rust: keep basedir cache key at version 7 Version 7 was introduced by this unmerged change series, so later refinements do not require another upstream cache-key version. --- src/compiler/rust.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 6dfff86ea4..54ad922002 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -349,7 +349,7 @@ static ALLOWED_EMIT: LazyLock> = LazyLock::new(|| ["link", "metadata", "dep-info"].iter().copied().collect()); /// Version number for cache key. -const CACHE_VERSION: &[u8] = b"8"; +const CACHE_VERSION: &[u8] = b"7"; /// Get absolute paths for all source files and env-deps listed in rustc's dep-info output. async fn get_source_files_and_env_deps( From ea22a11bf641bb96c5b4b9abef063a29b78ac9b9 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Wed, 5 Aug 2026 20:58:29 +0200 Subject: [PATCH 06/11] tests: use native separators for Rust remap paths Build expected remapped paths with Path::join because rustc uses platform-native separators. This fixes the Windows test matrix without changing remap behavior. --- src/compiler/rust.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 54ad922002..60c3326e97 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -4571,22 +4571,24 @@ proc_macro false Some(format!("{prefix}=/workspace").into()), )] }; + let joined = + |prefix: &str, suffix: &str| Some(Path::new(prefix).join(suffix).into_os_string()); assert!(super::remap_path(Path::new("/home/user/project"), &remap("/home/user")).is_some()); assert!(super::remap_path(Path::new("/home/user"), &remap("/home/user=part")).is_none()); assert_eq!( super::remap_path(Path::new("/home/user"), &remap("/")), - Some("/workspace/home/user".into()) + joined("/workspace", "home/user") ); assert_eq!( super::remap_path(Path::new("/home/user/project"), &remap("/home/user/")), - Some("/workspace/project".into()) + joined("/workspace", "project") ); assert!( super::remap_path(Path::new("/home/username/project"), &remap("/home/user")).is_none() ); assert_eq!( super::remap_path(Path::new("/home/a=b/project"), &remap("/home/a=b")), - Some("/workspace/project".into()) + joined("/workspace", "project") ); let overlapping = vec![ @@ -4598,7 +4600,7 @@ proc_macro false ]; assert_eq!( super::remap_path(Path::new("/home/user/project"), &overlapping), - Some("/last/project".into()) + joined("/last", "project") ); let mut diagnostics = remap("/home/user"); From cff1582170f899156fa6cb0f5bad0e9ae8b473e4 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Thu, 6 Aug 2026 15:24:59 +0200 Subject: [PATCH 07/11] tests: cover Rust basedir normalization branches Exercise malformed and nonmatching remap values, separated scope syntax, and absolute source arguments. Coverage report: https://app.codecov.io/gh/mozilla/sccache/pull/2794?src=pr&el=tree --- src/compiler/rust.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 60c3326e97..d9cc7cc128 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -4341,6 +4341,33 @@ proc_macro false assert_eq!(key(&f1), key(&f2)); } + #[test] + fn test_basedirs_stable_across_absolute_source_arguments() { + let f1 = TestFixture::new(); + let f2 = TestFixture::new(); + let key = |f: &TestFixture| { + let source = f.tempdir.path().join("foo.rs"); + let source = source.to_str().unwrap(); + let remap = format!("{}=/workspace", f.tempdir.path().display()); + let args = [ + "--emit", + "link", + source, + "--out-dir", + "out", + "--crate-name", + "foo", + "--crate-type", + "lib", + "--remap-path-prefix", + &remap, + ]; + hash_key_with_basedirs(f, &args, &[], nothing, vec![basedir_for(f.tempdir.path())]) + }; + + assert_eq!(key(&f1), key(&f2)); + } + #[test] fn test_basedirs_keep_unremapped_source_paths() { let f1 = TestFixture::new(); @@ -4528,6 +4555,8 @@ proc_macro false fn test_normalize_path_arguments() { let basedirs = [b"/home/user/".to_vec()]; for (flag, value, expected) in [ + ("--remap-path-prefix", "/home/user", "/home/user"), + ("--remap-path-prefix", "/other=/new", "/other=/new"), ("--remap-path-prefix", "/home/user=/new", "=/new"), ("--remap-path-prefix", "/home/user/src=/new", "src=/new"), ( @@ -4606,6 +4635,9 @@ proc_macro false let mut diagnostics = remap("/home/user"); diagnostics.push(("--remap-path-scope=diagnostics".into(), None)); assert!(super::remap_path(Path::new("/home/user/project"), &diagnostics).is_none()); + let mut diagnostics = remap("/home/user"); + diagnostics.push(("--remap-path-scope".into(), Some("diagnostics".into()))); + assert!(super::remap_path(Path::new("/home/user/project"), &diagnostics).is_none()); let mut all = remap("/home/user"); all.push(("--remap-path-scope=all".into(), None)); assert!(super::remap_path(Path::new("/home/user/project"), &all).is_some()); From 25b2f7fe06e517f982e758edb22c04f9436bf0d4 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Mon, 10 Aug 2026 13:34:33 +0200 Subject: [PATCH 08/11] rust: remove unsupported Cargo workspace variable Cargo does not provide CARGO_WORKSPACE_DIR as a built-in environment variable. Do not treat a user-defined variable with that name as a path eligible for basedir normalization. --- src/compiler/rust.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index d9cc7cc128..c8bd18f5ac 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -171,7 +171,6 @@ fn is_path_cargo_env(var: &OsString) -> bool { | "CARGO_MANIFEST_PATH" | "CARGO_TARGET_DIR" | "CARGO_TARGET_TMPDIR" - | "CARGO_WORKSPACE_DIR" ) ) || var.as_encoded_bytes().starts_with(b"CARGO_BIN_EXE_") } @@ -4592,6 +4591,7 @@ proc_macro false ); assert!(super::is_path_cargo_env(&"CARGO_MANIFEST_DIR".into())); assert!(super::is_path_cargo_env(&"CARGO_INSTALL_ROOT".into())); + assert!(!super::is_path_cargo_env(&"CARGO_WORKSPACE_DIR".into())); assert!(!super::is_path_cargo_env(&"CARGO_PKG_DESCRIPTION".into())); let remap = |prefix: &str| { From 92718e095204c6ffd37bdc6f6f178a01932263f7 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Mon, 10 Aug 2026 14:47:57 +0200 Subject: [PATCH 09/11] rust: keep basedir cache hits path-safe Retain physical cache-key paths whenever a procedural macro may observe them. Rewrite cached dep-info targets for the current output paths while preserving permissions and rejecting ambiguous records. --- docs/Rust.md | 4 +- src/compiler/compiler.rs | 11 +- src/compiler/rust.rs | 257 +++++++++++++++++++++++++++++++++------ tests/sccache_cargo.rs | 14 ++- 4 files changed, 246 insertions(+), 40 deletions(-) diff --git a/docs/Rust.md b/docs/Rust.md index 0b80481933..5885d2e296 100644 --- a/docs/Rust.md +++ b/docs/Rust.md @@ -9,8 +9,8 @@ sccache includes support for caching Rust compilation. This includes many caveat * Procedural macros that read files from the filesystem may not be cached properly. * `rustc`'s incremental compilation needs to be disabled. See [The Cargo Book](https://doc.rust-lang.org/cargo/reference/profiles.html#incremental) * Crates that invoke the system linker cannot be cached. Examples are `bin`, `dylib`, `cdylib`, and `proc-macro` crates. -* `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys when rustc's `--remap-path-prefix` covers the working directory with the default or `all` remap scope. It does not rewrite compiler outputs itself. -* Cargo path variables remain location-sensitive when external crates are present because transitive procedural macros can read them without reporting an environment dependency to rustc. +* `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys when rustc's `--remap-path-prefix` covers the working directory with the default or `all` remap scope. Cached dep-info output targets are rewritten for the current invocation. +* Path normalization remains location-sensitive when explicit external crates or dynamic libraries on crate search paths may invoke procedural macros, because they can observe physical paths without reporting a dependency to rustc. * Rust path normalization is limited to ASCII paths on Windows. * Distributed Rust compilation falls back to local compilation when `--remap-path-prefix` is used with a non-identity path transformer. diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 78bb5a4332..86be5dde73 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -638,7 +638,11 @@ where }; let hit = CompileResult::CacheHit(duration); - match entry.extract_objects(filtered_outputs, &pool).await { + let extraction = entry + .extract_objects(filtered_outputs, &pool) + .await + .and_then(|()| compilation.postprocess_cache_hit(&cwd)); + match extraction { Ok(()) => Ok(CacheLookupResult::Success(hit, output)), Err(e) => { if e.downcast_ref::().is_some() { @@ -1112,6 +1116,11 @@ where true } + /// Adjust extracted outputs for the current invocation after a cache hit. + fn postprocess_cache_hit(&self, _cwd: &Path) -> Result<()> { + Ok(()) + } + /// Returns an iterator over the results of this compilation. /// /// Each item is a descriptive (and unique) name of the output paired with diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index c8bd18f5ac..1b94dbdf38 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -50,7 +50,7 @@ use std::future::Future; use std::hash::Hash; #[cfg(feature = "dist-client")] use std::io; -use std::io::{BufReader, Read}; +use std::io::{BufReader, Read, Write}; use std::iter; use std::path::{Path, PathBuf}; use std::pin::Pin; @@ -136,7 +136,7 @@ fn remap_scope_is_all(arguments: &[(OsString, Option)]) -> bool { } flag.to_str()?.strip_prefix("--remap-path-scope=") }) - .last() + .next_back() .is_none_or(|scope| scope.split(',').any(|scope| scope == "all")) } @@ -175,6 +175,109 @@ fn is_path_cargo_env(var: &OsString) -> bool { ) || var.as_encoded_bytes().starts_with(b"CARGO_BIN_EXE_") } +fn rewrite_dep_info_targets( + dep_info: &Path, + outputs: &HashMap, + cwd: &Path, +) -> Result<()> { + let dep_info = cwd.join(dep_info); + let deps = fs::read_to_string(&dep_info).context("Failed to read cached Rust dep-info")?; + let mut rewritten = String::with_capacity(deps.len()); + let mut matched_output = false; + let mut scanning_output_targets = true; + + for line in deps.split_inclusive('\n') { + if !scanning_output_targets || line.trim().is_empty() { + rewritten.push_str(line); + continue; + } + + let mut matched = None; + for (name, output) in outputs { + let marker = format!("{name}: "); + for (marker_start, _) in line.match_indices(&marker) { + let is_boundary = if marker_start == 0 { + true + } else { + let previous = line.as_bytes()[marker_start - 1]; + #[cfg(windows)] + let is_separator = matches!(previous, b'/' | b'\\'); + #[cfg(not(windows))] + let is_separator = previous == b'/'; + #[cfg(windows)] + let is_drive_relative = marker_start == 2 + && previous == b':' + && line.as_bytes()[0].is_ascii_alphabetic(); + #[cfg(not(windows))] + let is_drive_relative = false; + is_separator || is_drive_relative + }; + if !is_boundary { + continue; + } + + let separator = marker_start + name.len(); + if let Some((best_separator, best_name_len, _)) = matched { + if separator != best_separator { + bail!( + "Ambiguous output target in cached Rust dep-info {}", + dep_info.display() + ); + } + if name.len() > best_name_len { + matched = Some((separator, name.len(), output)); + } + } else { + matched = Some((separator, name.len(), output)); + } + } + } + let Some((separator, _, output)) = matched else { + if !matched_output { + bail!( + "No output targets matched cached Rust dep-info {}", + dep_info.display() + ); + } + scanning_output_targets = false; + rewritten.push_str(line); + continue; + }; + + matched_output = true; + rewritten.push_str(&output.path.to_string_lossy()); + rewritten.push_str(&line[separator..]); + } + + if rewritten != deps { + let parent = dep_info + .parent() + .context("Cached Rust dep-info has no parent directory")?; + let permissions = fs::metadata(&dep_info)?.permissions(); + let mut temp = tempfile::NamedTempFile::new_in(parent) + .context("Failed to create temporary Rust dep-info")?; + temp.write_all(rewritten.as_bytes())?; + #[cfg(not(windows))] + temp.as_file().set_permissions(permissions.clone())?; + #[cfg(windows)] + if permissions.readonly() { + let mut writable = permissions.clone(); + writable.set_readonly(false); + fs::set_permissions(&dep_info, writable)?; + } + if let Err(error) = temp.persist(&dep_info) { + #[cfg(windows)] + if permissions.readonly() { + let _ = fs::set_permissions(&dep_info, permissions); + } + return Err(error.error).context("Failed to replace cached Rust dep-info"); + } + #[cfg(windows)] + fs::set_permissions(&dep_info, permissions)?; + } + Ok(()) +} + #[cfg(feature = "dist-client")] const RLIB_PREFIX: &str = "lib"; #[cfg(feature = "dist-client")] @@ -1511,6 +1614,26 @@ where ) -> Result> { trace!("[{}]: generate_hash_key", self.parsed_args.crate_name); let basedirs = storage.basedirs(); + // Procedural macros can observe the physical working directory and inherited environment + // variables without reporting them in rustc dep-info. Conservatively retain physical paths + // whenever an explicit extern or dynamic crate search entry may load one. + let can_normalize_paths = !basedirs.is_empty() + && self.parsed_args.externs.is_empty() + && !self.parsed_args.crate_link_paths.iter().any(|path| { + fs::read_dir(path) + .map(|mut entries| { + entries.any(|entry| { + entry.map_or(true, |entry| { + entry + .path() + .extension() + .is_some_and(|ext| ext == DLL_EXTENSION) + }) + }) + }) + .unwrap_or(true) + }); + let normalized_basedirs: &[Vec] = if can_normalize_paths { basedirs } else { &[] }; // TODO: this doesn't produce correct arguments if they should be concatenated - should use iter_os_strings let os_string_arguments: Vec<(OsString, Option)> = self .parsed_args @@ -1523,6 +1646,13 @@ where ) }) .collect(); + let remap_input_path = |path: &Path| { + if can_normalize_paths { + remap_path(path, &os_string_arguments) + } else { + None + } + }; // `filtered_arguments` omits --emit and --out-dir arguments. // It's used for invoking rustc with `--emit=dep-info` to get the list of // source files for this crate. @@ -1653,7 +1783,7 @@ where for (arg, value) in rest.into_iter().chain(sortables) { let arg_bytes = arg.as_encoded_bytes(); if value.is_none() { - if let Some(remapped) = remap_path(Path::new(arg), &os_string_arguments) { + if let Some(remapped) = remap_input_path(Path::new(arg)) { hash_arg(true, remapped.as_encoded_bytes()); } else { hash_arg(false, arg_bytes); @@ -1663,7 +1793,7 @@ where } if let Some(value) = value { let value_bytes = value.as_encoded_bytes(); - let normalized = normalize_arg_value(arg, value, basedirs); + let normalized = normalize_arg_value(arg, value, normalized_basedirs); let normalized_bytes: &[u8] = &normalized; hash_arg(normalized_bytes != value_bytes, normalized_bytes); } @@ -1677,7 +1807,7 @@ where .iter() .zip(source_hashes) .map(|(path, hash)| { - if let Some(remapped) = remap_path(path, &os_string_arguments) { + if let Some(remapped) = remap_input_path(path) { (true, remapped, path.as_os_str().to_owned(), hash) } else { ( @@ -1736,26 +1866,9 @@ where .cloned() .collect(); env_vars.sort(); - // Procedural macros can read inherited environment variables without reporting them in - // rustc dep-info. They can be explicit externs or resolved from a crate search path. - let normalize_cargo_paths = !basedirs.is_empty() - && remap_path(&cwd, &os_string_arguments).is_some() - && env_vars.iter().any(|(var, _)| is_path_cargo_env(var)) - && self.parsed_args.externs.is_empty() - && !self.parsed_args.crate_link_paths.iter().any(|path| { - fs::read_dir(path) - .map(|mut entries| { - entries.any(|entry| { - entry.map_or(true, |entry| { - entry - .path() - .extension() - .is_some_and(|ext| ext == DLL_EXTENSION) - }) - }) - }) - .unwrap_or(true) - }); + let normalize_cargo_paths = can_normalize_paths + && remap_input_path(&cwd).is_some() + && env_vars.iter().any(|(var, _)| is_path_cargo_env(var)); for (var, val) in env_vars.iter() { if !var.starts_with("CARGO_") { continue; @@ -1789,7 +1902,7 @@ where } // 9. The cwd of the compile. This will wind up in the rlib. let cwd_bytes = cwd.as_os_str().as_encoded_bytes(); - if let Some(remapped) = remap_path(&cwd, &os_string_arguments) { + if let Some(remapped) = remap_input_path(&cwd) { true.hash(&mut HashToDigest { digest: &mut m }); remapped .as_encoded_bytes() @@ -2124,6 +2237,13 @@ impl Compilation for RustCompilation { Ok((CCompileCommand::new(command), dist_command, Cacheable::Yes)) } + fn postprocess_cache_hit(&self, cwd: &Path) -> Result<()> { + if let Some(dep_info) = &self.dep_info { + rewrite_dep_info_targets(dep_info, &self.outputs, cwd)?; + } + Ok(()) + } + #[cfg(feature = "dist-client")] fn into_dist_packagers( self: Box, @@ -4300,6 +4420,77 @@ proc_macro false "lib", ]; + #[test] + fn test_rewrite_cached_dep_info_targets() { + let tempdir = tempfile::tempdir().unwrap(); + let dep_info = PathBuf::from("new/out/foo.d"); + fs::create_dir_all(tempdir.path().join("new/out")).unwrap(); + #[cfg(windows)] + let cached_targets = "C:foo.d: src/lib.rs\n\nC:libfoo.rlib: src/lib.rs\n\nsrc/lib.rs:\n\n"; + #[cfg(not(windows))] + let cached_targets = "/old/a: b/out/foo.d: src/lib.rs\n\n/old/a: b/out/libfoo.rlib: src/lib.rs\n\nsrc/lib.rs:\n\n"; + fs::write( + tempdir.path().join(&dep_info), + format!("{cached_targets}# env-dep:OUT_DIR=/tmp/foo.d: value\n"), + ) + .unwrap(); + let mut permissions = fs::metadata(tempdir.path().join(&dep_info)) + .unwrap() + .permissions(); + permissions.set_readonly(true); + fs::set_permissions(tempdir.path().join(&dep_info), permissions).unwrap(); + let outputs = HashMap::from([ + ( + "foo.d".to_owned(), + ArtifactDescriptor { + path: "new/a: b/out/foo.d".into(), + optional: false, + }, + ), + ( + "libfoo.rlib".to_owned(), + ArtifactDescriptor { + path: "new/a: b/out/libfoo.rlib".into(), + optional: false, + }, + ), + ]); + + rewrite_dep_info_targets(&dep_info, &outputs, tempdir.path()).unwrap(); + + assert_eq!( + fs::read_to_string(tempdir.path().join(&dep_info)).unwrap(), + "new/a: b/out/foo.d: src/lib.rs\n\nnew/a: b/out/libfoo.rlib: src/lib.rs\n\nsrc/lib.rs:\n\n# env-dep:OUT_DIR=/tmp/foo.d: value\n" + ); + assert!( + fs::metadata(tempdir.path().join(dep_info)) + .unwrap() + .permissions() + .readonly() + ); + } + + #[test] + fn test_rewrite_cached_dep_info_rejects_ambiguous_target() { + let tempdir = tempfile::tempdir().unwrap(); + let dep_info = PathBuf::from("out/foo.d"); + fs::create_dir_all(tempdir.path().join("out")).unwrap(); + fs::write( + tempdir.path().join(&dep_info), + "/old/foo.d: dir/out/foo.d: src/lib.rs\n", + ) + .unwrap(); + let outputs = HashMap::from([( + "foo.d".to_owned(), + ArtifactDescriptor { + path: dep_info.clone(), + optional: false, + }, + )]); + + assert!(rewrite_dep_info_targets(&dep_info, &outputs, tempdir.path()).is_err()); + } + fn basedir_for(path: &Path) -> Vec { let bytes = path.to_string_lossy().into_owned().into_bytes(); #[cfg(windows)] @@ -4434,7 +4625,7 @@ proc_macro false } #[test] - fn test_basedirs_preserve_cargo_paths_with_external_crates() { + fn test_basedirs_preserve_paths_with_external_crates() { let f1 = TestFixture::new(); let f2 = TestFixture::new(); let dependency_name = "dependency.rlib"; @@ -4461,10 +4652,7 @@ proc_macro false hash_key_with_env_deps( f, &args, - &[( - "CARGO_MANIFEST_DIR".into(), - f.tempdir.path().as_os_str().to_owned(), - )], + &[], nothing, false, vec![basedir_for(f.tempdir.path())], @@ -4476,7 +4664,7 @@ proc_macro false } #[test] - fn test_basedirs_preserve_cargo_paths_with_dynamic_crate_search() { + fn test_basedirs_preserve_paths_with_dynamic_crate_search() { let f1 = TestFixture::new(); let f2 = TestFixture::new(); for f in [&f1, &f2] { @@ -4502,10 +4690,7 @@ proc_macro false hash_key_with_env_deps( f, &args, - &[( - "CARGO_MANIFEST_DIR".into(), - f.tempdir.path().as_os_str().to_owned(), - )], + &[], nothing, false, vec![basedir_for(f.tempdir.path())], diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index aae4be025e..4076c1c3b2 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -76,7 +76,7 @@ fn test_rust_cargo_basedirs_cross_dir_cache_hit() -> Result<()> { .env("CARGO_INSTALL_ROOT", crate_dir.join("install")) .env("CARGO_TARGET_DIR", crate_dir.join("target")) .env( - "RUSTFLAGS", + "CARGO_ENCODED_RUSTFLAGS", format!("--remap-path-prefix={}=/workspace", crate_dir.display()), ) .current_dir(crate_dir) @@ -84,6 +84,18 @@ fn test_rust_cargo_basedirs_cross_dir_cache_hit() -> Result<()> { .try_success()?; } + let dep_info = fs::read_dir(crate_b.join("target/debug/deps"))? + .find_map(|entry| { + let path = entry.ok()?.path(); + path.extension() + .is_some_and(|extension| extension == "d") + .then_some(path) + }) + .context("missing dep-info for second checkout")?; + let dep_info = fs::read_to_string(dep_info)?; + assert!(!dep_info.contains(crate_a.to_string_lossy().as_ref())); + assert!(dep_info.contains(crate_b.to_string_lossy().as_ref())); + test.show_stats()? .try_stdout(predicates::str::contains(r#""cache_hits":{"counts":{"Rust":1}"#).from_utf8())? .try_success()?; From 2729861941355be501cbee20d80d3d5c6bf74920 Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Mon, 10 Aug 2026 15:51:49 +0200 Subject: [PATCH 10/11] rust: rebuild dep-info from current paths Keep the locally generated dependency records from the hash probe and use them to rebuild cached dep-info after extraction. This prevents absolute source paths, environment records, and checksums from referring to the producer checkout. --- docs/Rust.md | 2 +- src/compiler/rust.rs | 148 +++++++++++++++++++++++++++++++++-------- tests/sccache_cargo.rs | 8 ++- tests/sccache_rustc.rs | 2 +- 4 files changed, 129 insertions(+), 31 deletions(-) diff --git a/docs/Rust.md b/docs/Rust.md index 5885d2e296..f36b1a2704 100644 --- a/docs/Rust.md +++ b/docs/Rust.md @@ -9,7 +9,7 @@ sccache includes support for caching Rust compilation. This includes many caveat * Procedural macros that read files from the filesystem may not be cached properly. * `rustc`'s incremental compilation needs to be disabled. See [The Cargo Book](https://doc.rust-lang.org/cargo/reference/profiles.html#incremental) * Crates that invoke the system linker cannot be cached. Examples are `bin`, `dylib`, `cdylib`, and `proc-macro` crates. -* `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys when rustc's `--remap-path-prefix` covers the working directory with the default or `all` remap scope. Cached dep-info output targets are rewritten for the current invocation. +* `SCCACHE_BASEDIRS` normalizes paths in Rust cache keys when rustc's `--remap-path-prefix` covers the working directory with the default or `all` remap scope. Cached dep-info is rebuilt with output targets and dependency records from the current invocation. * Path normalization remains location-sensitive when explicit external crates or dynamic libraries on crate search paths may invoke procedural macros, because they can observe physical paths without reporting a dependency to rustc. * Rust path normalization is limited to ASCII paths on Windows. * Distributed Rust compilation falls back to local compilation when `--remap-path-prefix` is used with a non-identity path transformer. diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index fd4b5679e0..b45b264adb 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -175,20 +175,31 @@ fn is_path_cargo_env(var: &OsString) -> bool { ) || var.as_encoded_bytes().starts_with(b"CARGO_BIN_EXE_") } +fn omits_dep_info_target(flag: &OsString, value: Option<&OsString>) -> bool { + flag == "-Z" + && value.and_then(|value| value.to_str()).is_some_and(|value| { + value.split_once('=').map_or(value, |(name, _)| name) == "dep-info-omit-d-target" + }) +} + +#[derive(Clone, Debug)] +struct DepInfoTemplate { + target_dependencies: String, + tail: String, +} + fn rewrite_dep_info_targets( dep_info: &Path, outputs: &HashMap, cwd: &Path, + template: &DepInfoTemplate, ) -> Result<()> { let dep_info = cwd.join(dep_info); let deps = fs::read_to_string(&dep_info).context("Failed to read cached Rust dep-info")?; - let mut rewritten = String::with_capacity(deps.len()); - let mut matched_output = false; - let mut scanning_output_targets = true; + let mut output_targets = Vec::new(); for line in deps.split_inclusive('\n') { - if !scanning_output_targets || line.trim().is_empty() { - rewritten.push_str(line); + if line.trim().is_empty() { continue; } @@ -232,22 +243,28 @@ fn rewrite_dep_info_targets( } } } - let Some((separator, _, output)) = matched else { - if !matched_output { + let Some((_, _, output)) = matched else { + if output_targets.is_empty() { bail!( "No output targets matched cached Rust dep-info {}", dep_info.display() ); } - scanning_output_targets = false; - rewritten.push_str(line); - continue; + break; }; - matched_output = true; + output_targets.push(output); + } + + let mut rewritten = String::with_capacity(deps.len()); + for (index, output) in output_targets.iter().enumerate() { rewritten.push_str(&output.path.to_string_lossy()); - rewritten.push_str(&line[separator..]); + rewritten.push_str(&template.target_dependencies); + if index + 1 < output_targets.len() { + rewritten.push('\n'); + } } + rewritten.push_str(&template.tail); if rewritten != deps { let parent = dep_info @@ -433,6 +450,8 @@ pub struct RustCompilation { crate_types: CrateTypes, /// If dependency info is being emitted, the name of the dep info file. dep_info: Option, + /// Dependency records generated locally for the current invocation. + dep_info_template: Option, /// The current working directory cwd: PathBuf, /// The environment variables @@ -462,7 +481,7 @@ async fn get_source_files_and_env_deps( cwd: &Path, env_vars: &[(OsString, OsString)], pool: &tokio::runtime::Handle, -) -> Result<(Vec, Vec<(OsString, OsString)>)> +) -> Result<(Vec, Vec<(OsString, OsString)>, DepInfoTemplate)> where T: CommandCreatorSync, { @@ -494,7 +513,7 @@ where }) .await?; - parsed.map(move |(files, env_deps)| { + parsed.map(move |(files, env_deps, template)| { trace!( "[{}]: got {} source files and {} env-deps from dep-info in {}", crate_name, @@ -504,13 +523,16 @@ where ); // Just to make sure we capture temp_dir. drop(temp_dir); - (files, env_deps) + (files, env_deps, template) }) } /// Parse dependency info from `file` and return a Vec of files mentioned. /// Treat paths as relative to `cwd`. -fn parse_dep_file(file: T, cwd: U) -> Result<(Vec, Vec<(OsString, OsString)>)> +fn parse_dep_file( + file: T, + cwd: U, +) -> Result<(Vec, Vec<(OsString, OsString)>, DepInfoTemplate)> where T: AsRef, U: AsRef, @@ -518,7 +540,25 @@ where let mut f = fs::File::open(file.as_ref())?; let mut deps = String::new(); f.read_to_string(&mut deps)?; - Ok((parse_dep_info(&deps, cwd), parse_env_dep_info(&deps))) + let (target_line, tail) = deps + .split_once('\n') + .context("Rust dep-info has no target line")?; + let target = file.as_ref().to_string_lossy(); + let mut target_dependencies = target_line + .strip_prefix(&*target) + .filter(|suffix| suffix.starts_with(": ")) + .context("Rust dep-info target does not match requested path")? + .to_owned(); + target_dependencies.push('\n'); + let source_files = parse_dep_info(&format!("target{target_dependencies}"), cwd); + Ok(( + source_files, + parse_env_dep_info(&deps), + DepInfoTemplate { + target_dependencies, + tail: tail.to_owned(), + }, + )) } fn parse_dep_info(dep_info: &str, cwd: T) -> Vec @@ -1655,7 +1695,8 @@ where let filtered_arguments = os_string_arguments .iter() .filter_map(|(arg, val)| { - if arg == "--emit" || arg == "--out-dir" { + if arg == "--emit" || arg == "--out-dir" || omits_dep_info_target(arg, val.as_ref()) + { None } else { Some((arg, val)) @@ -1667,7 +1708,7 @@ where // Find all the source files and hash them let source_hashes_pool = pool.clone(); let source_files_and_hashes_and_env_deps = async { - let (source_files, env_deps) = get_source_files_and_env_deps( + let (source_files, env_deps, dep_info_template) = get_source_files_and_env_deps( creator, &self.parsed_args.crate_name, &self.executable, @@ -1678,7 +1719,7 @@ where ) .await?; let source_hashes = hash_all(&source_files, &source_hashes_pool).await?; - Ok((source_files, source_hashes, env_deps)) + Ok((source_files, source_hashes, env_deps, dep_info_template)) }; // Hash the contents of the externs listed on the commandline. @@ -1723,7 +1764,7 @@ where // Perform all hashing operations on the files. let ( - (source_files, source_hashes, mut env_deps), + (source_files, source_hashes, mut env_deps, dep_info_template), extern_hashes, staticlib_hashes, target_json_hash, @@ -2033,6 +2074,7 @@ where .chain(abs_externs) .chain(abs_staticlibs) .collect(); + let dep_info_template = dep_info.as_ref().map(|_| dep_info_template); Ok(HashResult { key: m.finish(), @@ -2047,6 +2089,7 @@ where crate_name: self.parsed_args.crate_name.clone(), crate_types: self.parsed_args.crate_types.clone(), dep_info, + dep_info_template, cwd, env_vars, #[cfg(feature = "dist-client")] @@ -2235,7 +2278,11 @@ impl Compilation for RustCompilation { fn postprocess_cache_hit(&self, cwd: &Path) -> Result<()> { if let Some(dep_info) = &self.dep_info { - rewrite_dep_info_targets(dep_info, &self.outputs, cwd)?; + let template = self + .dep_info_template + .as_ref() + .context("Missing local Rust dep-info template")?; + rewrite_dep_info_targets(dep_info, &self.outputs, cwd, template)?; } Ok(()) } @@ -3116,6 +3163,7 @@ LLVM version: 15.0.2 staticlib: false, }, dep_info: None, + dep_info_template: None, cwd: root.to_owned(), env_vars: vec![], } @@ -3630,6 +3678,36 @@ abc def.rs: assert_eq!(pathvec!["abc def.rs", "baz.rs"], parse_dep_info(deps, "")); } + #[cfg(not(windows))] + #[test] + fn test_parse_dep_file_with_colon_space_target() { + let tempdir = tempfile::tempdir().unwrap(); + let dep_dir = tempdir.path().join("a: b"); + fs::create_dir(&dep_dir).unwrap(); + let dep_file = dep_dir.join("deps.d"); + fs::write( + &dep_file, + format!("{}: source.rs\n\nsource.rs:\n", dep_file.display()), + ) + .unwrap(); + + let (files, _, template) = parse_dep_file(&dep_file, tempdir.path()).unwrap(); + + assert_eq!(vec![tempdir.path().join("source.rs")], files); + assert_eq!(": source.rs\n", template.target_dependencies); + assert_eq!("\nsource.rs:\n", template.tail); + } + + #[test] + fn test_omits_dep_info_target() { + let z_flag = OsString::from("-Z"); + let omit_target = OsString::from("dep-info-omit-d-target"); + let omit_target_value = OsString::from("dep-info-omit-d-target=yes"); + assert!(omits_dep_info_target(&z_flag, Some(&omit_target))); + assert!(omits_dep_info_target(&z_flag, Some(&omit_target_value))); + assert!(!omits_dep_info_target(&z_flag, None)); + } + #[cfg(not(windows))] #[test] fn test_parse_dep_info_cwd() { @@ -3858,7 +3936,12 @@ proc_macro false } let dep_info_path = dep_info_path.unwrap(); let mut f = File::create(dep_info_path)?; - writeln!(f, "blah: {}", sorted_deps.iter().join(" "))?; + writeln!( + f, + "{}: {}", + Path::new(dep_info_path).display(), + sorted_deps.iter().join(" ") + )?; for d in sorted_deps.iter() { writeln!(f, "{}:", d)?; } @@ -4421,9 +4504,9 @@ proc_macro false let dep_info = PathBuf::from("new/out/foo.d"); fs::create_dir_all(tempdir.path().join("new/out")).unwrap(); #[cfg(windows)] - let cached_targets = "C:foo.d: src/lib.rs\n\nC:libfoo.rlib: src/lib.rs\n\nsrc/lib.rs:\n\n"; + let cached_targets = "C:foo.d: /old/source/lib.rs\n\nC:libfoo.rlib: /old/source/lib.rs\n\n/old/source/lib.rs:\n\n"; #[cfg(not(windows))] - let cached_targets = "/old/a: b/out/foo.d: src/lib.rs\n\n/old/a: b/out/libfoo.rlib: src/lib.rs\n\nsrc/lib.rs:\n\n"; + let cached_targets = "/old/a: b/out/foo.d: /old/source/lib.rs\n\n/old/a: b/out/libfoo.rlib: /old/source/lib.rs\n\n/old/source/lib.rs:\n\n"; fs::write( tempdir.path().join(&dep_info), format!("{cached_targets}# env-dep:OUT_DIR=/tmp/foo.d: value\n"), @@ -4450,12 +4533,17 @@ proc_macro false }, ), ]); + let template = DepInfoTemplate { + target_dependencies: ": /new/source/lib.rs\n".to_owned(), + tail: "\n/new/source/lib.rs:\n\n# env-dep:OUT_DIR=/new/out\n# checksum:123 file_len:1 /new/source/lib.rs\n" + .to_owned(), + }; - rewrite_dep_info_targets(&dep_info, &outputs, tempdir.path()).unwrap(); + rewrite_dep_info_targets(&dep_info, &outputs, tempdir.path(), &template).unwrap(); assert_eq!( fs::read_to_string(tempdir.path().join(&dep_info)).unwrap(), - "new/a: b/out/foo.d: src/lib.rs\n\nnew/a: b/out/libfoo.rlib: src/lib.rs\n\nsrc/lib.rs:\n\n# env-dep:OUT_DIR=/tmp/foo.d: value\n" + "new/a: b/out/foo.d: /new/source/lib.rs\n\nnew/a: b/out/libfoo.rlib: /new/source/lib.rs\n\n/new/source/lib.rs:\n\n# env-dep:OUT_DIR=/new/out\n# checksum:123 file_len:1 /new/source/lib.rs\n" ); assert!( fs::metadata(tempdir.path().join(dep_info)) @@ -4482,8 +4570,12 @@ proc_macro false optional: false, }, )]); + let template = DepInfoTemplate { + target_dependencies: ": src/lib.rs\n".to_owned(), + tail: "\nsrc/lib.rs:\n".to_owned(), + }; - assert!(rewrite_dep_info_targets(&dep_info, &outputs, tempdir.path()).is_err()); + assert!(rewrite_dep_info_targets(&dep_info, &outputs, tempdir.path(), &template).is_err()); } fn basedir_for(path: &Path) -> Vec { diff --git a/tests/sccache_cargo.rs b/tests/sccache_cargo.rs index 4076c1c3b2..55ae0db0f1 100644 --- a/tests/sccache_cargo.rs +++ b/tests/sccache_cargo.rs @@ -57,9 +57,15 @@ fn test_rust_cargo_basedirs_cross_dir_cache_hit() -> Result<()> { let crate_b = work_root.join("b"); for crate_dir in [&crate_a, &crate_b] { fs::create_dir_all(crate_dir.join("src"))?; + let lib_path = crate_dir + .join("src/lib.rs") + .to_string_lossy() + .replace('\\', "/"); fs::write( crate_dir.join("Cargo.toml"), - "[package]\nname = \"basedirs-test\"\nversion = \"0.0.0\"\nedition = \"2021\"\n", + format!( + "[package]\nname = \"basedirs-test\"\nversion = \"0.0.0\"\nedition = \"2021\"\n\n[lib]\npath = {lib_path:?}\n" + ), )?; fs::write(crate_dir.join("src/lib.rs"), "pub fn value() -> u8 { 1 }\n")?; } diff --git a/tests/sccache_rustc.rs b/tests/sccache_rustc.rs index 8fd4e22649..49d35b4cd3 100644 --- a/tests/sccache_rustc.rs +++ b/tests/sccache_rustc.rs @@ -112,7 +112,7 @@ while [ "$#" -gt 0 ]; do --emit) shift if [ "$1" = dep-info ]; then - echo "deps.d: RUST_FILE.rs" > "$3" + echo "$3: RUST_FILE.rs" > "$3" exec echo "RUST_FILE.rs:" "$3" fi ;; From 8fca89f22946ea4c8fe0053397a29ba33d4475ef Mon Sep 17 00:00:00 2001 From: Pablo Marcos Date: Mon, 10 Aug 2026 16:23:12 +0200 Subject: [PATCH 11/11] tests: drop nonexistent Cargo variable assertion CARGO_WORKSPACE_DIR is not provided by Cargo, so asserting its treatment as an arbitrary user-defined variable does not document useful built-in behavior. --- src/compiler/rust.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index b45b264adb..7502e9ab0a 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -4863,7 +4863,6 @@ proc_macro false ); assert!(super::is_path_cargo_env(&"CARGO_MANIFEST_DIR".into())); assert!(super::is_path_cargo_env(&"CARGO_INSTALL_ROOT".into())); - assert!(!super::is_path_cargo_env(&"CARGO_WORKSPACE_DIR".into())); assert!(!super::is_path_cargo_env(&"CARGO_PKG_DESCRIPTION".into())); let remap = |prefix: &str| {