From a1ec6a8d923d861ebc6ce58e1436b84cbba39ddd Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 20 Mar 2026 15:40:07 -0700 Subject: [PATCH 1/3] Add configurable skip patterns for cargo build script execroot symlinking Adds symlink_exec_root_skip_patterns setting to cargo/settings, allowing users to control which execroot entries are skipped during CARGO_MANIFEST_DIR symlinking. Defaults skip worker temp dirs and VCS directories. Also improves Windows symlink handling: - Correctly handle file symlinks, directory symlinks, and junctions - Tolerate missing runfile entries - Downgrade symlink removal failures to warnings on Windows --- cargo/private/BUILD.bazel | 4 +- cargo/private/cargo_build_script.bzl | 6 + .../private/cargo_build_script_runner/bin.rs | 146 ++++++-- .../cargo_manifest_dir.rs | 352 ++++++++++++++---- cargo/settings/BUILD.bazel | 3 + cargo/settings/settings.bzl | 23 ++ 6 files changed, 443 insertions(+), 91 deletions(-) diff --git a/cargo/private/BUILD.bazel b/cargo/private/BUILD.bazel index db04afa7b1..6872881cbc 100644 --- a/cargo/private/BUILD.bazel +++ b/cargo/private/BUILD.bazel @@ -39,9 +39,9 @@ copy_file( bzl_library( name = "bzl_lib", + srcs = glob(["**/*.bzl"]), + visibility = ["//visibility:public"], deps = [ "//rust:bzl_lib", ], - srcs = glob(["**/*.bzl"]), - visibility = ["//visibility:public"], ) diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_script.bzl index c23108851a..b68010746d 100644 --- a/cargo/private/cargo_build_script.bzl +++ b/cargo/private/cargo_build_script.bzl @@ -632,6 +632,9 @@ def _cargo_build_script_impl(ctx): if experimental_symlink_execroot: env["RULES_RUST_SYMLINK_EXEC_ROOT"] = "1" + skip_patterns = ctx.attr._symlink_exec_root_skip_patterns[BuildSettingInfo].value + if skip_patterns: + env["RULES_RUST_SYMLINK_EXEC_ROOT_SKIP_PATTERNS"] = ",".join(skip_patterns) ctx.actions.run( executable = ctx.executable._cargo_build_script_runner, @@ -787,6 +790,9 @@ cargo_build_script = rule( "_experimental_symlink_execroot": attr.label( default = Label("//cargo/settings:experimental_symlink_execroot"), ), + "_symlink_exec_root_skip_patterns": attr.label( + default = Label("//cargo/settings:symlink_exec_root_skip_patterns"), + ), "_fallback_ar": attr.label( cfg = "exec", executable = True, diff --git a/cargo/private/cargo_build_script_runner/bin.rs b/cargo/private/cargo_build_script_runner/bin.rs index 27f5848edc..1370b8881d 100644 --- a/cargo/private/cargo_build_script_runner/bin.rs +++ b/cargo/private/cargo_build_script_runner/bin.rs @@ -62,6 +62,7 @@ fn run_buildrs() -> Result<(), String> { let mut exec_root_links = Vec::new(); if should_symlink_exec_root() { + let exec_root_skip_patterns = symlink_exec_root_skip_patterns(); // Symlink the execroot to the manifest_dir so that we can use relative paths in the arguments. let exec_root_paths = std::fs::read_dir(&exec_root) .map_err(|err| format!("Failed while listing exec root: {err:?}"))?; @@ -75,12 +76,26 @@ fn run_buildrs() -> Result<(), String> { let file_name = path .file_name() .ok_or_else(|| "Failed while getting file name".to_string())?; + + // Skip entries matching user-configurable patterns from + // RULES_RUST_SYMLINK_EXEC_ROOT_SKIP_PATTERNS (comma-separated). + // Patterns support trailing '*' as a prefix glob. + let name = file_name.to_string_lossy(); + if exec_root_skip_patterns + .iter() + .any(|p| match_skip_pattern(p, &name)) + { + continue; + } + let link = manifest_dir.join(file_name); - symlink_if_not_exists(&path, &link) + let created = symlink_if_not_exists(&path, &link) .map_err(|err| format!("Failed to symlink {path:?} to {link:?}: {err}"))?; - exec_root_links.push(link) + if created { + exec_root_links.push(link); + } } } @@ -219,15 +234,25 @@ fn run_buildrs() -> Result<(), String> { ) }); - if !exec_root_links.is_empty() { - for link in exec_root_links { - remove_symlink(&link).map_err(|e| { - format!( - "Failed to remove exec_root link '{}' with {:?}", + for link in exec_root_links { + if let Err(e) = remove_symlink(&link) { + if cfg!(target_family = "windows") { + // On Windows, symlink removal can fail with PermissionDenied if + // another process still holds a handle to the target directory. + // These are temporary symlinks in the build sandbox that Bazel + // will clean up, so we log and continue rather than failing. + eprintln!( + "Warning: could not remove exec_root link '{}': {:?}", + link.display(), + e + ); + } else { + return Err(format!( + "Failed to remove exec_root link '{}': {:?}", link.display(), e - ) - })?; + )); + } } } @@ -246,11 +271,39 @@ fn should_symlink_exec_root() -> bool { .unwrap_or(false) } +/// Parse skip patterns from `RULES_RUST_SYMLINK_EXEC_ROOT_SKIP_PATTERNS`. +/// Returns a comma-separated list of patterns. Each pattern is either an exact +/// match or a prefix glob (trailing `*`). +fn symlink_exec_root_skip_patterns() -> Vec { + env::var("RULES_RUST_SYMLINK_EXEC_ROOT_SKIP_PATTERNS") + .map(|s| { + s.split(',') + .filter(|p| !p.is_empty()) + .map(|p| p.to_owned()) + .collect() + }) + .unwrap_or_default() +} + +/// Match a skip pattern against a file name. Supports exact match and +/// trailing `*` as a prefix glob (e.g. `local-spawn-runner.*` matches +/// `local-spawn-runner.12345`). +fn match_skip_pattern(pattern: &str, name: &str) -> bool { + if let Some(prefix) = pattern.strip_suffix('*') { + name.starts_with(prefix) + } else { + name == pattern + } +} + /// Create a symlink from `link` to `original` if `link` doesn't already exist. -fn symlink_if_not_exists(original: &Path, link: &Path) -> Result<(), String> { - symlink(original, link) - .or_else(swallow_already_exists) - .map_err(|err| format!("Failed to create symlink: {err}")) +/// Returns `true` if a new symlink was created, `false` if the path already existed. +fn symlink_if_not_exists(original: &Path, link: &Path) -> Result { + match symlink(original, link) { + Ok(()) => Ok(true), + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => Ok(false), + Err(err) => Err(format!("Failed to create symlink: {err}")), + } } fn resolve_rundir(rundir: &str, exec_root: &Path, manifest_dir: &Path) -> Result { @@ -270,14 +323,6 @@ fn resolve_rundir(rundir: &str, exec_root: &Path, manifest_dir: &Path) -> Result Ok(exec_root.join(rundir_path)) } -fn swallow_already_exists(err: std::io::Error) -> std::io::Result<()> { - if err.kind() == std::io::ErrorKind::AlreadyExists { - Ok(()) - } else { - Err(err) - } -} - /// A representation of expected command line arguments. struct Args { progname: String, @@ -470,4 +515,63 @@ windows assert_eq!(tree["CARGO_CFG_WINDOWS"], ""); assert_eq!(tree["CARGO_CFG_TARGET_FAMILY"], "windows"); } + + #[test] + fn skip_pattern_exact_match() { + assert!(match_skip_pattern(".git", ".git")); + assert!(!match_skip_pattern(".git", ".github")); + assert!(!match_skip_pattern(".git", ".gi")); + assert!(!match_skip_pattern(".git", "")); + } + + #[test] + fn skip_pattern_prefix_glob() { + assert!(match_skip_pattern("local-spawn-runner.*", "local-spawn-runner.12345")); + assert!(match_skip_pattern("local-spawn-runner.*", "local-spawn-runner.")); + assert!(!match_skip_pattern("local-spawn-runner.*", "local-spawn-runner")); + assert!(!match_skip_pattern("local-spawn-runner.*", "other-thing")); + } + + #[test] + fn skip_pattern_star_alone() { + // A bare "*" pattern matches everything. + assert!(match_skip_pattern("*", "anything")); + assert!(match_skip_pattern("*", "")); + } + + #[test] + fn skip_pattern_empty_never_matches() { + assert!(!match_skip_pattern("", "anything")); + assert!(match_skip_pattern("", "")); + } + + #[test] + fn symlink_if_not_exists_returns_true_on_creation() { + let dir = std::env::temp_dir().join("symlink_if_not_exists_test_create"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let original = dir.join("original"); + std::fs::write(&original, "data").unwrap(); + let link = dir.join("link"); + + assert_eq!(symlink_if_not_exists(&original, &link).unwrap(), true); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn symlink_if_not_exists_returns_false_when_exists() { + let dir = std::env::temp_dir().join("symlink_if_not_exists_test_exists"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let original = dir.join("original"); + std::fs::write(&original, "data").unwrap(); + let link = dir.join("link"); + // Create it first + symlink_if_not_exists(&original, &link).unwrap(); + // Second call should return false + assert_eq!(symlink_if_not_exists(&original, &link).unwrap(), false); + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs b/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs index def343a34e..0c906e483a 100644 --- a/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs +++ b/cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs @@ -27,14 +27,41 @@ pub fn remove_symlink(path: &Path) -> Result<(), std::io::Error> { std::fs::remove_file(path) } -/// Create a symlink file on windows systems +/// Remove a symlink or junction on Windows. +/// +/// Windows has three kinds of reparse points we may encounter: +/// 1. File symlinks — `remove_file` works. +/// 2. Directory symlinks — `remove_dir` removes the link itself (not the +/// target contents), but `remove_file` also works on some Windows versions. +/// 3. Junctions — similar to directory symlinks; `remove_dir` removes the +/// junction entry. +/// +/// We use `symlink_metadata` + `FileTypeExt` to classify the entry and try +/// the most appropriate removal call first, with a fallback for edge cases. #[cfg(target_family = "windows")] pub fn remove_symlink(path: &Path) -> Result<(), std::io::Error> { - if path.is_dir() { - std::fs::remove_dir(path) - } else { - std::fs::remove_file(path) + use std::os::windows::fs::FileTypeExt; + + let metadata = std::fs::symlink_metadata(path)?; + let ft = metadata.file_type(); + + if ft.is_symlink_file() { + return std::fs::remove_file(path); } + + if ft.is_symlink_dir() { + // remove_dir removes the symlink entry itself, not the target contents. + // Fall back to remove_file if remove_dir fails (some Windows versions). + return std::fs::remove_dir(path).or_else(|_| std::fs::remove_file(path)); + } + + // Junctions appear as directories but are not symlinks per FileTypeExt. + // remove_dir removes the junction entry itself. + if ft.is_dir() { + return std::fs::remove_dir(path).or_else(|_| std::fs::remove_file(path)); + } + + std::fs::remove_file(path) } /// Check if the system supports symlinks by attempting to create one. @@ -227,73 +254,85 @@ impl RunfilesMaker { Ok(()) } - /// Delete runfiles from the runfiles directory that do not match user defined suffixes + /// Strip runfiles that do not match a retained suffix. /// - /// The Unix implementation assumes symlinks are supported and that the runfiles directory - /// was created using symlinks. - fn drain_runfiles_dir_unix(&self) -> Result<(), String> { + /// When `symlinks_used` is true the runfiles directory was populated with + /// symlinks: every entry is removed and only retained entries are copied + /// back as real files. When false, real file copies were used (Windows + /// without symlink support) and only retained entries are deleted so that + /// downstream steps can recreate them. + /// + /// Missing entries are tolerated in either mode — on Windows the runfiles + /// directory may be incomplete (e.g. a Cargo.lock that was never created). + fn drain_runfiles_dir_impl(&self, symlinks_used: bool) -> Result<(), String> { for (src, dest) in &self.runfiles { let abs_dest = self.output_dir.join(dest); - - remove_symlink(&abs_dest).map_err(|e| { - format!( - "Failed to delete symlink '{}' with {:?}", - abs_dest.display(), - e - ) - })?; - - if !self + let should_retain = self .filename_suffixes_to_retain .iter() - .any(|suffix| dest.ends_with(suffix)) - { - if let Some(parent) = abs_dest.parent() { - if is_dir_empty(parent).map_err(|e| { - format!("Failed to determine if directory was empty with: {:?}", e) - })? { - std::fs::remove_dir(parent).map_err(|e| { - format!( - "Failed to delete directory {} with {:?}", - parent.display(), - e - ) - })?; + .any(|suffix| dest.ends_with(suffix)); + + if symlinks_used { + match remove_symlink(&abs_dest) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + if !should_retain { + continue; + } + // Retained entry was missing — fall through to copy from src. + } + Err(e) => { + return Err(format!( + "Failed to delete symlink '{}' with {:?}", + abs_dest.display(), + e + )); } } - continue; - } - std::fs::copy(src, &abs_dest).map_err(|e| { - format!( - "Failed to copy `{} -> {}` with {:?}", - src.display(), - abs_dest.display(), - e - ) - })?; - } - Ok(()) - } + if !should_retain { + if let Some(parent) = abs_dest.parent() { + if is_dir_empty(parent).map_err(|e| { + format!("Failed to determine if directory was empty with: {:?}", e) + })? { + std::fs::remove_dir(parent).map_err(|e| { + format!( + "Failed to delete directory {} with {:?}", + parent.display(), + e + ) + })?; + } + } + continue; + } - /// Delete runfiles from the runfiles directory that do not match user defined suffixes - /// - /// The Windows implementation assumes symlinks are not supported and real files will have - /// been copied into the runfiles directory. - fn drain_runfiles_dir_windows(&self) -> Result<(), String> { - for dest in self.runfiles.values() { - if !self - .filename_suffixes_to_retain - .iter() - .any(|suffix| dest.ends_with(suffix)) - { + std::fs::copy(src, &abs_dest).map_err(|e| { + format!( + "Failed to copy `{} -> {}` with {:?}", + src.display(), + abs_dest.display(), + e + ) + })?; + } else if !should_retain { + // Non-symlink mode: non-retained files are left as-is (no + // empty-directory cleanup needed since the files were never + // removed in the first place). continue; + } else { + match std::fs::remove_file(&abs_dest) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => { + return Err(format!( + "Failed to remove file {} with {:?}", + abs_dest.display(), + e + )); + } + } } - - let abs_dest = self.output_dir.join(dest); - std::fs::remove_file(&abs_dest).map_err(|e| { - format!("Failed to remove file {} with {:?}", abs_dest.display(), e) - })?; } Ok(()) } @@ -301,15 +340,10 @@ impl RunfilesMaker { /// Delete runfiles from the runfiles directory that do not match user defined suffixes pub fn drain_runfiles_dir(&self, out_dir: &Path) -> Result<(), String> { if cfg!(target_family = "windows") { - // If symlinks are supported then symlinks will have been used. let supports_symlinks = system_supports_symlinks(&self.output_dir)?; - if supports_symlinks { - self.drain_runfiles_dir_unix()?; - } else { - self.drain_runfiles_dir_windows()?; - } + self.drain_runfiles_dir_impl(supports_symlinks)?; } else { - self.drain_runfiles_dir_unix()?; + self.drain_runfiles_dir_impl(true)?; } // Due to the symlinks in `CARGO_MANIFEST_DIR`, some build scripts @@ -409,6 +443,188 @@ mod tests { out_dir } + /// Create a `RunfilesMaker` for testing without needing a param file. + fn make_runfiles_maker( + output_dir: PathBuf, + suffixes: &[&str], + runfiles: Vec<(PathBuf, RlocationPath)>, + ) -> RunfilesMaker { + RunfilesMaker { + output_dir, + filename_suffixes_to_retain: suffixes.iter().map(|s| s.to_string()).collect(), + runfiles: runfiles.into_iter().collect(), + } + } + + /// Helper to create a unique test directory under TEST_TMPDIR. + fn test_dir(name: &str) -> PathBuf { + let test_tmp = PathBuf::from(std::env::var("TEST_TMPDIR").unwrap()); + let dir = test_tmp.join(name); + if dir.exists() { + fs::remove_dir_all(&dir).unwrap(); + } + fs::create_dir_all(&dir).unwrap(); + dir + } + + #[cfg(any(target_family = "windows", target_family = "unix"))] + #[test] + fn drain_symlinks_tolerates_missing_symlinks() { + let base = test_dir("drain_sym_missing"); + let output_dir = base.join("runfiles"); + fs::create_dir_all(&output_dir).unwrap(); + + // Two distinct source files so BTreeMap keeps both entries. + let src_real = base.join("real.txt"); + fs::write(&src_real, "content").unwrap(); + let src_lock = base.join("Cargo.lock"); + fs::write(&src_lock, "lock data").unwrap(); + + // Two runfile entries: one exists as a symlink, one does not. + let existing_dest = "pkg/real.txt"; + let missing_dest = "pkg/Cargo.lock"; + let abs_existing = output_dir.join(existing_dest); + fs::create_dir_all(abs_existing.parent().unwrap()).unwrap(); + symlink(&src_real, &abs_existing).unwrap(); + // Intentionally do NOT create a symlink for missing_dest. + + let maker = make_runfiles_maker( + output_dir.clone(), + &[], // retain nothing + vec![ + (src_real.clone(), existing_dest.to_string()), + (src_lock.clone(), missing_dest.to_string()), + ], + ); + + // Should succeed despite the missing symlink. + maker.drain_runfiles_dir_impl(true).unwrap(); + + // The existing symlink should have been removed. + assert!(!abs_existing.exists()); + } + + #[cfg(any(target_family = "windows", target_family = "unix"))] + #[test] + fn drain_symlinks_retains_matching_suffixes() { + let base = test_dir("drain_sym_retain"); + let output_dir = base.join("runfiles"); + fs::create_dir_all(&output_dir).unwrap(); + + let src_file = base.join("lib.rs"); + fs::write(&src_file, "fn main() {}").unwrap(); + + let src_lock = base.join("Cargo.lock"); + fs::write(&src_lock, "lock contents").unwrap(); + + let rs_dest = "pkg/lib.rs"; + let lock_dest = "pkg/Cargo.lock"; + + // Create symlinks for both entries. + let abs_rs = output_dir.join(rs_dest); + let abs_lock = output_dir.join(lock_dest); + fs::create_dir_all(abs_rs.parent().unwrap()).unwrap(); + symlink(&src_file, &abs_rs).unwrap(); + symlink(&src_lock, &abs_lock).unwrap(); + + let maker = make_runfiles_maker( + output_dir.clone(), + &[".rs"], // only retain .rs files + vec![ + (src_file.clone(), rs_dest.to_string()), + (src_lock.clone(), lock_dest.to_string()), + ], + ); + + maker.drain_runfiles_dir_impl(true).unwrap(); + + // .rs file should be retained (copied back as a real file, not a symlink). + assert!(abs_rs.exists()); + assert!(!abs_rs.is_symlink()); + assert_eq!(fs::read_to_string(&abs_rs).unwrap(), "fn main() {}"); + + // .lock file should have been removed. + assert!(!abs_lock.exists()); + } + + #[cfg(any(target_family = "windows", target_family = "unix"))] + #[test] + fn drain_symlinks_missing_with_retained_suffix_still_copies() { + let base = test_dir("drain_sym_missing_retain"); + let output_dir = base.join("runfiles"); + fs::create_dir_all(&output_dir).unwrap(); + + let src_file = base.join("lib.rs"); + fs::write(&src_file, "fn main() {}").unwrap(); + + let dest = "pkg/lib.rs"; + // Create the parent dir but NOT the symlink. + fs::create_dir_all(output_dir.join("pkg")).unwrap(); + + let maker = make_runfiles_maker( + output_dir.clone(), + &[".rs"], // retain .rs files + vec![(src_file.clone(), dest.to_string())], + ); + + // Should succeed — missing symlink is tolerated, file is still copied. + maker.drain_runfiles_dir_impl(true).unwrap(); + + let abs_dest = output_dir.join(dest); + assert!(abs_dest.exists()); + assert!(!abs_dest.is_symlink()); + assert_eq!(fs::read_to_string(&abs_dest).unwrap(), "fn main() {}"); + } + + #[cfg(any(target_family = "windows", target_family = "unix"))] + #[test] + fn drain_no_symlinks_tolerates_missing_files() { + let base = test_dir("drain_nosym_missing"); + let output_dir = base.join("runfiles"); + fs::create_dir_all(&output_dir).unwrap(); + + let src_file = base.join("real.txt"); + fs::write(&src_file, "content").unwrap(); + + // Retain .txt but the file doesn't exist in the runfiles dir. + let maker = make_runfiles_maker( + output_dir.clone(), + &[".txt"], + vec![(src_file.clone(), "pkg/real.txt".to_string())], + ); + + // Should succeed despite the missing file. + maker.drain_runfiles_dir_impl(false).unwrap(); + } + + #[cfg(any(target_family = "windows", target_family = "unix"))] + #[test] + fn drain_symlinks_cleans_empty_parent_dir() { + let base = test_dir("drain_sym_empty_parent"); + let output_dir = base.join("runfiles"); + fs::create_dir_all(&output_dir).unwrap(); + + let src_file = base.join("throwaway.txt"); + fs::write(&src_file, "data").unwrap(); + + let dest = "subdir/throwaway.txt"; + let abs_dest = output_dir.join(dest); + fs::create_dir_all(abs_dest.parent().unwrap()).unwrap(); + symlink(&src_file, &abs_dest).unwrap(); + + let maker = make_runfiles_maker( + output_dir.clone(), + &[], // retain nothing + vec![(src_file.clone(), dest.to_string())], + ); + + maker.drain_runfiles_dir_impl(true).unwrap(); + + // Both the symlink and its now-empty parent should be removed. + assert!(!abs_dest.exists()); + assert!(!output_dir.join("subdir").exists()); + } + #[cfg(any(target_family = "windows", target_family = "unix"))] #[test] fn replace_symlinks_in_out_dir() { diff --git a/cargo/settings/BUILD.bazel b/cargo/settings/BUILD.bazel index 21d6cb7ef4..1bd5427101 100644 --- a/cargo/settings/BUILD.bazel +++ b/cargo/settings/BUILD.bazel @@ -5,6 +5,7 @@ load( "debug_std_streams_output_group", "experimental_symlink_execroot", "incompatible_runfiles_cargo_manifest_dir", + "symlink_exec_root_skip_patterns", "use_default_shell_env", ) @@ -29,4 +30,6 @@ experimental_symlink_execroot() incompatible_runfiles_cargo_manifest_dir() +symlink_exec_root_skip_patterns() + use_default_shell_env() diff --git a/cargo/settings/settings.bzl b/cargo/settings/settings.bzl index adfcf6bf21..168ac8cea4 100644 --- a/cargo/settings/settings.bzl +++ b/cargo/settings/settings.bzl @@ -38,6 +38,29 @@ def cargo_manifest_dir_filename_suffixes_to_retain(): ], ) +def symlink_exec_root_skip_patterns(): + """A flag which specifies glob-like patterns for exec root entries to skip when symlinking + the exec root into `CARGO_MANIFEST_DIR`. + + Each pattern is matched against exec root entry names. A pattern matches if it equals the + name exactly, or if it ends with `*` and the name starts with the prefix before the `*`. + + Defaults cover Bazel worker directories (which hold locked handles on Windows and are + unnecessary on all platforms) and VCS directories (which can trick build scripts into + thinking they're in a checkout). + """ + string_list_flag( + name = "symlink_exec_root_skip_patterns", + build_setting_default = [ + # Worker temp dirs — hold locked handles on Windows; harmless to skip everywhere. + "local-spawn-runner.*", + "rules_rust_process_wrapper_deps_*", + # VCS directories — prevent build scripts from detecting a checkout. + ".git", + ".github", + ], + ) + def debug_std_streams_output_group(): """A flag which adds a `streams` output group to `cargo_build_script` targets that contain \ the raw `stderr` and `stdout` streams from the build script. From d88317bdb695df2785017e971e6971feec54ef0b Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 20 Mar 2026 15:40:07 -0700 Subject: [PATCH 2/3] Add configurable skip patterns for cargo build script execroot symlinking Adds symlink_exec_root_skip_patterns setting to cargo/settings, allowing users to control which execroot entries are skipped during CARGO_MANIFEST_DIR symlinking. Defaults skip worker temp dirs and VCS directories. Also improves Windows symlink handling: - Correctly handle file symlinks, directory symlinks, and junctions - Tolerate missing runfile entries - Downgrade symlink removal failures to warnings on Windows --- cargo/private/cargo_build_script_runner/bin.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/cargo/private/cargo_build_script_runner/bin.rs b/cargo/private/cargo_build_script_runner/bin.rs index 1370b8881d..15828dada0 100644 --- a/cargo/private/cargo_build_script_runner/bin.rs +++ b/cargo/private/cargo_build_script_runner/bin.rs @@ -526,9 +526,18 @@ windows #[test] fn skip_pattern_prefix_glob() { - assert!(match_skip_pattern("local-spawn-runner.*", "local-spawn-runner.12345")); - assert!(match_skip_pattern("local-spawn-runner.*", "local-spawn-runner.")); - assert!(!match_skip_pattern("local-spawn-runner.*", "local-spawn-runner")); + assert!(match_skip_pattern( + "local-spawn-runner.*", + "local-spawn-runner.12345" + )); + assert!(match_skip_pattern( + "local-spawn-runner.*", + "local-spawn-runner." + )); + assert!(!match_skip_pattern( + "local-spawn-runner.*", + "local-spawn-runner" + )); assert!(!match_skip_pattern("local-spawn-runner.*", "other-thing")); } From 1eba4d9be44fc6bcbd6e92521e24f9d4e294fcbd Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 20 Mar 2026 17:29:55 -0700 Subject: [PATCH 3/3] Fix issues with windows path resolution --- util/process_wrapper/main.rs | 64 +++++++++++++++++++++++++++++++++ util/process_wrapper/options.rs | 59 ++++++++++++++++++++++++++++-- 2 files changed, 121 insertions(+), 2 deletions(-) diff --git a/util/process_wrapper/main.rs b/util/process_wrapper/main.rs index 2a7cbd8565..f28ad398af 100644 --- a/util/process_wrapper/main.rs +++ b/util/process_wrapper/main.rs @@ -254,6 +254,68 @@ fn consolidate_dependency_search_paths( Ok((args.to_vec(), None)) } +/// On Windows, convert the path to its 8.3 short form using `GetShortPathNameW`. +/// Returns the original string unchanged if the conversion fails (e.g. 8.3 names +/// are disabled on the volume, or the path does not yet exist). +#[cfg(windows)] +fn to_short_path(path_str: &str) -> String { + use std::ffi::OsStr; + use std::os::windows::ffi::{OsStrExt, OsStringExt}; + + extern "system" { + fn GetShortPathNameW( + lpszLongPath: *const u16, + lpszShortPath: *mut u16, + cchBuffer: u32, + ) -> u32; + } + + let wide: Vec = OsStr::new(path_str) + .encode_wide() + .chain(std::iter::once(0)) + .collect(); + + let needed = unsafe { GetShortPathNameW(wide.as_ptr(), std::ptr::null_mut(), 0) }; + if needed == 0 { + return path_str.to_owned(); + } + + let mut buf = vec![0u16; needed as usize]; + let len = unsafe { GetShortPathNameW(wide.as_ptr(), buf.as_mut_ptr(), needed) }; + if len == 0 { + return path_str.to_owned(); + } + + std::ffi::OsString::from_wide(&buf[..len as usize]) + .to_string_lossy() + .into_owned() +} + +/// On Windows, convert any `-L=` argument whose total length +/// exceeds MAX_PATH (260) to use the 8.3 short form of the path. This +/// prevents LINK.EXE from failing with LNK1104 when the linker search path +/// is too long. +#[cfg(windows)] +fn shorten_long_link_search_paths(args: Vec) -> Vec { + const MAX_PATH: usize = 260; + args.into_iter() + .map(|arg| { + if arg.len() <= MAX_PATH { + return arg; + } + if let Some(rest) = arg.strip_prefix("-L") { + if let Some(eq_pos) = rest.find('=') { + let variant = &rest[..eq_pos]; + let path = &rest[eq_pos + 1..]; + let short = to_short_path(path); + return format!("-L{}={}", variant, short); + } + } + arg + }) + .collect() +} + fn json_warning(line: &str) -> JsonValue { JsonValue::Object(HashMap::from([ ( @@ -297,6 +359,8 @@ fn main() -> Result<(), ProcessWrapperError> { let (child_arguments, dep_dir_cleanup) = consolidate_dependency_search_paths(&opts.child_arguments)?; + #[cfg(windows)] + let child_arguments = shorten_long_link_search_paths(child_arguments); let mut temp_dir_guard = TemporaryDirectoryGuard::new(dep_dir_cleanup); let mut command = Command::new(opts.executable); diff --git a/util/process_wrapper/options.rs b/util/process_wrapper/options.rs index 6dbc898a11..aed91c9878 100644 --- a/util/process_wrapper/options.rs +++ b/util/process_wrapper/options.rs @@ -255,9 +255,59 @@ fn prepare_arg(mut arg: String, subst_mappings: &[(String, String)]) -> String { let from = format!("${{{f}}}"); arg = arg.replace(&from, replace_with); } + // On Windows, canonicalize() produces \\?\ verbatim paths where forward + // slashes are literal, not separators. After substituting ${pwd} (which + // comes from current_dir and may be verbatim), any remaining `/` would + // break path resolution. + #[cfg(windows)] + if arg.contains(r"\\?\") { + arg = arg.replace('/', r"\"); + } arg } +/// On Windows, resolve `.rs` source file paths that pass through junctions +/// containing relative symlinks. Windows cannot resolve chained reparse +/// points (junction -> relative symlink -> target) in a single traversal, +/// causing rustc to fail with ERROR_PATH_NOT_FOUND. +/// +/// Only resolves paths ending in `.rs` to avoid changing crate identity +/// for `--extern` and `-L` paths (which would cause SVH mismatches). +#[cfg(windows)] +fn resolve_external_path(arg: &str) -> std::borrow::Cow<'_, str> { + use std::borrow::Cow; + use std::path::Path; + if !arg.ends_with(".rs") { + return Cow::Borrowed(arg); + } + if !arg.starts_with("external/") && !arg.starts_with("external\\") { + return Cow::Borrowed(arg); + } + let path = Path::new(arg); + let mut components = path.components(); + let Some(_external) = components.next() else { + return Cow::Borrowed(arg); + }; + let Some(repo_name) = components.next() else { + return Cow::Borrowed(arg); + }; + let junction = Path::new("external").join(repo_name); + let Ok(resolved) = std::fs::read_link(&junction) else { + return Cow::Borrowed(arg); + }; + let remainder: std::path::PathBuf = components.collect(); + if remainder.as_os_str().is_empty() { + return Cow::Borrowed(arg); + } + Cow::Owned(resolved.join(remainder).to_string_lossy().into_owned()) +} + +#[cfg(not(windows))] +#[inline] +fn resolve_external_path(arg: &str) -> std::borrow::Cow<'_, str> { + std::borrow::Cow::Borrowed(arg) +} + /// Apply substitutions to the given param file. Returns true iff any allow-features flags were found. fn prepare_param_file( filename: &str, @@ -327,8 +377,9 @@ fn prepare_args( )), }; let mut write_to_file = |s: &str| -> Result<(), OptionError> { + let s = resolve_external_path(s); match out { - Writer::Function(ref mut f) => f(&expanded_file, s), + Writer::Function(ref mut f) => f(&expanded_file, &s), Writer::BufWriter(ref mut bw) => writeln!(bw, "{s}").map_err(format_err), } }; @@ -345,7 +396,11 @@ fn prepare_args( processed_args.push(file); } else { allowed_features |= is_allow_features_flag(&arg); - processed_args.push(arg); + let resolved = resolve_external_path(&arg); + processed_args.push(match resolved { + std::borrow::Cow::Borrowed(_) => arg, + std::borrow::Cow::Owned(s) => s, + }); } } if !allowed_features && require_explicit_unstable_features {