diff --git a/cargo/private/cargo_build_script_runner/bin.rs b/cargo/private/cargo_build_script_runner/bin.rs index 27f5848edc..d546c29ccc 100644 --- a/cargo/private/cargo_build_script_runner/bin.rs +++ b/cargo/private/cargo_build_script_runner/bin.rs @@ -77,10 +77,12 @@ fn run_buildrs() -> Result<(), String> { .ok_or_else(|| "Failed while getting file name".to_string())?; 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 +221,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 + )); + } } } @@ -247,10 +259,13 @@ fn should_symlink_exec_root() -> bool { } /// 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 +285,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, 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..895ef6155d 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,84 @@ 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; + } + } + 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 +339,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 +442,160 @@ 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 replace_symlinks_in_out_dir() {