From 1d0fe8a19c8bb50f20556e63b99d2e5290c2e4e9 Mon Sep 17 00:00:00 2001 From: David Havas Date: Fri, 3 Jan 2025 22:40:23 +0100 Subject: [PATCH] Replace symlinks in the output of cargo build scripts (#3067) #2948 breaks building of rdkafka with `cmake` because of dangling symlinks. When building with latest version we get the following error: ``` ERROR: /home/wincent/.cache/bazel/_bazel_wincent/394c4c1d21c5490d4a70260a2cfccaf5/external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/BUILD.bazel:68:19: error while validating output tree artifact external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/_bs.out_dir: child lib/cmake/RdKafka/FindLZ4.cmake is a dangling symbolic link ERROR: /home/wincent/.cache/bazel/_bazel_wincent/394c4c1d21c5490d4a70260a2cfccaf5/external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/BUILD.bazel:68:19: Running Cargo build script rdkafka-sys failed: not all outputs were created or valid Target @@rules_rust~~crate~crates__rdkafka-0.37.0//:rdkafka failed to build Use --verbose_failures to see the command lines of failed build steps. ERROR: /home/wincent/.cache/bazel/_bazel_wincent/394c4c1d21c5490d4a70260a2cfccaf5/external/rules_rust~~crate~crates__rdkafka-sys-4.8.0-2.3.0/BUILD.bazel:18:13 Compiling Rust rlib rdkafka_sys v4.8.0+2.3.0 (7 files) failed: not all outputs were created or valid ``` --- 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); +}