From c3ccf850b31e32b6ec2406403dd9977b1cc42d73 Mon Sep 17 00:00:00 2001 From: David Havas Date: Mon, 9 Dec 2024 10:46:44 +0100 Subject: [PATCH] Replace symlinks in the output of cargo build scripts After #2948 we symlink the source files in sandbox for most external repositories. When a native build system is involved (e.x.: cmake) it is possible that the installed files will be symlinks especially when header files are installed. This will be placed in the output dir of cargo build scripts and when copied back to the repository cache they become dangling symlinks with references to the sandbox. As a fix we replace symlinks with a copy of them in the output directory. --- cargo/cargo_build_script_runner/bin.rs | 132 +++++++++++++++++- .../resolve_abs_symlink_out_dir/BUILD.bazel | 20 +++ .../resolve_abs_symlink_out_dir/build.rs | 28 ++++ .../resolve_abs_symlink_out_dir/data.txt | 1 + .../resolve_abs_symlink_out_dir/test.rs | 17 +++ 5 files changed, 191 insertions(+), 7 deletions(-) create mode 100644 test/cargo_build_script/resolve_abs_symlink_out_dir/BUILD.bazel create mode 100644 test/cargo_build_script/resolve_abs_symlink_out_dir/build.rs create mode 100644 test/cargo_build_script/resolve_abs_symlink_out_dir/data.txt create mode 100644 test/cargo_build_script/resolve_abs_symlink_out_dir/test.rs diff --git a/cargo/cargo_build_script_runner/bin.rs b/cargo/cargo_build_script_runner/bin.rs index 6ae1741e1a..6cc0ed6568 100644 --- a/cargo/cargo_build_script_runner/bin.rs +++ b/cargo/cargo_build_script_runner/bin.rs @@ -93,7 +93,7 @@ fn run_buildrs() -> Result<(), String> { command .current_dir(&working_directory) .envs(target_env_vars) - .env("OUT_DIR", out_dir_abs) + .env("OUT_DIR", &out_dir_abs) .env("CARGO_MANIFEST_DIR", manifest_dir) .env("RUSTC", rustc) .env("RUST_BACKTRACE", "full"); @@ -228,9 +228,10 @@ fn run_buildrs() -> Result<(), String> { // Delete any runfiles that do not need to be propagated to down stream dependents. if let Some(cargo_manifest_maker) = cargo_manifest_maker { - cargo_manifest_maker.drain_runfiles_dir().unwrap(); + cargo_manifest_maker + .drain_runfiles_dir(&out_dir_abs) + .unwrap(); } - Ok(()) } @@ -568,18 +569,19 @@ impl RunfilesMaker { } /// Delete runfiles from the runfiles directory that do not match user defined suffixes - fn drain_runfiles_dir(&self) -> Result<(), String> { + 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() + self.drain_runfiles_dir_unix()?; } else { - self.drain_runfiles_dir_windows() + self.drain_runfiles_dir_windows()?; } } else { - self.drain_runfiles_dir_unix() + self.drain_runfiles_dir_unix()?; } + replace_symlinks_in_out_dir(out_dir) } } @@ -720,6 +722,56 @@ fn parse_rustc_cfg_output(stdout: &str) -> BTreeMap { .collect() } +/// Iterates over the given directory recursively and resolves any symlinks +/// +/// Symlinks shouldn't present in `out_dir` as those amy contain paths to sandboxes which doesn't exists anymore. +/// Therefore, bazel will fail because of dangling symlinks. +fn replace_symlinks_in_out_dir(out_dir: &Path) -> Result<(), String> { + if out_dir.is_dir() { + let out_dir_paths = std::fs::read_dir(out_dir).map_err(|e| { + format!( + "Failed to read directory `{}` with {:?}", + out_dir.display(), + e + ) + })?; + for entry in out_dir_paths { + let entry = + entry.map_err(|e| format!("Failed to read directory entry with {:?}", e,))?; + let path = entry.path(); + + if path.is_symlink() { + let target_path = std::fs::read_link(&path).map_err(|e| { + format!("Failed to read symlink `{}` with {:?}", path.display(), e,) + })?; + // we don't want to replace relative symlinks + if target_path.is_relative() { + continue; + } + std::fs::remove_file(&path) + .map_err(|e| format!("Failed remove file `{}` with {:?}", path.display(), e))?; + std::fs::copy(&target_path, &path).map_err(|e| { + format!( + "Failed to copy `{} -> {}` with {:?}", + target_path.display(), + path.display(), + e + ) + })?; + } else if path.is_dir() { + replace_symlinks_in_out_dir(&path).map_err(|e| { + format!( + "Failed to normalize nested directory `{}` with {}", + path.display(), + e, + ) + })?; + } + } + } + Ok(()) +} + fn main() { std::process::exit(match run_buildrs() { Ok(_) => 0, @@ -735,6 +787,9 @@ fn main() { mod test { use super::*; + use std::fs; + use std::io::Write; + #[test] fn rustc_cfg_parsing() { let macos_output = r#"\ @@ -775,4 +830,67 @@ windows assert_eq!(tree["CARGO_CFG_WINDOWS"], ""); assert_eq!(tree["CARGO_CFG_TARGET_FAMILY"], "windows"); } + + fn prepare_output_dir_with_symlinks() -> PathBuf { + let test_tmp = PathBuf::from(std::env::var("TEST_TMPDIR").unwrap()); + let out_dir = test_tmp.join("out_dir"); + fs::create_dir(&out_dir).unwrap(); + let nested_dir = out_dir.join("nested"); + fs::create_dir(&nested_dir).unwrap(); + + let temp_dir_file = test_tmp.join("outside.txt"); + let mut file = fs::File::create(&temp_dir_file).unwrap(); + file.write_all(b"outside world").unwrap(); + // symlink abs path outside of the out_dir + symlink(&temp_dir_file, &out_dir.join("outside.txt")).unwrap(); + + let inside_dir_file = out_dir.join("inside.txt"); + let mut file = fs::File::create(inside_dir_file).unwrap(); + file.write_all(b"inside world").unwrap(); + // symlink relative next to the file in the out_dir + symlink( + &PathBuf::from("inside.txt"), + &out_dir.join("inside_link.txt"), + ) + .unwrap(); + // symlink relative within a subdir in the out_dir + symlink( + &PathBuf::from("..").join("inside.txt"), + &out_dir.join("nested").join("inside_link.txt"), + ) + .unwrap(); + + out_dir + } + + #[cfg(any(target_family = "windows", target_family = "unix"))] + #[test] + fn replace_symlinks_in_out_dir() { + let out_dir = prepare_output_dir_with_symlinks(); + super::replace_symlinks_in_out_dir(&out_dir).unwrap(); + + // this should be replaced because it is an absolute symlink + let file_path = out_dir.join("outside.txt"); + assert!(!file_path.is_symlink()); + let contents = fs::read_to_string(file_path).unwrap(); + assert_eq!(contents, "outside world"); + + // this is the file created inside the out_dir + let file_path = out_dir.join("inside.txt"); + assert!(!file_path.is_symlink()); + let contents = fs::read_to_string(file_path).unwrap(); + assert_eq!(contents, "inside world"); + + // this is the symlink in the out_dir + let file_path = out_dir.join("inside_link.txt"); + assert!(file_path.is_symlink()); + let contents = fs::read_to_string(file_path).unwrap(); + assert_eq!(contents, "inside world"); + + // this is the symlink in the out_dir under another directory which refers to ../inside.txt + let file_path = out_dir.join("nested").join("inside_link.txt"); + assert!(file_path.is_symlink()); + let contents = fs::read_to_string(file_path).unwrap(); + assert_eq!(contents, "inside world"); + } } diff --git a/test/cargo_build_script/resolve_abs_symlink_out_dir/BUILD.bazel b/test/cargo_build_script/resolve_abs_symlink_out_dir/BUILD.bazel new file mode 100644 index 0000000000..2f3dd9ce8e --- /dev/null +++ b/test/cargo_build_script/resolve_abs_symlink_out_dir/BUILD.bazel @@ -0,0 +1,20 @@ +load("//cargo:defs.bzl", "cargo_build_script") +load("//rust:defs.bzl", "rust_test") + +# We are testing the cargo build script behavior that it correctly resolves absolute path symlinks in the out_dir. +# Additionally, it keeps out_dir relative symlinks intact. + +cargo_build_script( + name = "symlink_build_rs", + srcs = ["build.rs"], + data = ["data.txt"], + edition = "2018", +) + +rust_test( + name = "test", + srcs = ["test.rs"], + data = [":symlink_build_rs"], + edition = "2018", + deps = [":symlink_build_rs"], +) diff --git a/test/cargo_build_script/resolve_abs_symlink_out_dir/build.rs b/test/cargo_build_script/resolve_abs_symlink_out_dir/build.rs new file mode 100644 index 0000000000..db119d41cb --- /dev/null +++ b/test/cargo_build_script/resolve_abs_symlink_out_dir/build.rs @@ -0,0 +1,28 @@ +use std::path::{Path, PathBuf}; + +#[cfg(target_family = "unix")] +fn symlink(original: impl AsRef, link: impl AsRef) { + std::os::unix::fs::symlink(original, link).unwrap(); +} + +#[cfg(target_family = "windows")] +fn symlink(original: impl AsRef, link: impl AsRef) { + std::os::windows::fs::symlink_file(original, link).unwrap(); +} + +fn main() { + let path = "data.txt"; + if !PathBuf::from(path).exists() { + panic!("File does not exist in path."); + } + let out_dir = std::env::var("OUT_DIR").unwrap(); + let out_dir = PathBuf::from(&out_dir); + let original_cwd = std::env::current_dir().unwrap(); + std::fs::copy(&path, &out_dir.join("data.txt")).unwrap(); + std::env::set_current_dir(&out_dir).unwrap(); + std::fs::create_dir("nested").unwrap(); + symlink("data.txt", "relative_symlink.txt"); + symlink("../data.txt", "nested/relative_symlink.txt"); + std::env::set_current_dir(&original_cwd).unwrap(); + println!("{}", out_dir.display()); +} diff --git a/test/cargo_build_script/resolve_abs_symlink_out_dir/data.txt b/test/cargo_build_script/resolve_abs_symlink_out_dir/data.txt new file mode 100644 index 0000000000..35cde921d2 --- /dev/null +++ b/test/cargo_build_script/resolve_abs_symlink_out_dir/data.txt @@ -0,0 +1 @@ +Resolved symlink file or relative symlink diff --git a/test/cargo_build_script/resolve_abs_symlink_out_dir/test.rs b/test/cargo_build_script/resolve_abs_symlink_out_dir/test.rs new file mode 100644 index 0000000000..0c0f7c733e --- /dev/null +++ b/test/cargo_build_script/resolve_abs_symlink_out_dir/test.rs @@ -0,0 +1,17 @@ +#[test] +pub fn test_compile_data_resolved_symlink() { + let data = include_str!(concat!(env!("OUT_DIR"), "/data.txt")); + assert_eq!("Resolved symlink file or relative symlink\n", data); +} + +#[test] +pub fn test_compile_data_relative_symlink() { + let data = include_str!(concat!(env!("OUT_DIR"), "/relative_symlink.txt")); + assert_eq!("Resolved symlink file or relative symlink\n", data); +} + +#[test] +pub fn test_compile_data_relative_nested_symlink() { + let data = include_str!(concat!(env!("OUT_DIR"), "/nested/relative_symlink.txt")); + assert_eq!("Resolved symlink file or relative symlink\n", data); +}