From f8efe01d7f23c56b593ec686f0b430e6d47404e0 Mon Sep 17 00:00:00 2001 From: Bryan Lai Date: Fri, 11 Oct 2024 17:28:19 +0800 Subject: [PATCH 1/3] Fix: Ignore `CARGO_BUILD_TARGET` in tests When build target triples are explicitly specified, the layout of the target directory is changed from `target/...` to `target//...`. See: https://doc.rust-lang.org/cargo/guide/build-cache.html This caused bin tests failures in `tests/profile.rs`. This patch proposes that we simply ignore the `CARGO_BUILD_TARGET` env var in tests to avoid such problems. A new test is introduced to ensure the correct behavior. --- tests/profile.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/profile.rs b/tests/profile.rs index 69433a2be..35ce3498d 100644 --- a/tests/profile.rs +++ b/tests/profile.rs @@ -45,6 +45,9 @@ fn build_using_bin(extra_args: &[&str]) -> tempfile::TempDir { Command::new(cbindgen_path) .current_dir(expand_dep_test_dir) .env("CARGO_EXPAND_TARGET_DIR", tmp_dir.path()) + .env_remove("CARGO_BUILD_TARGET") + // ^ avoid unexpected change of layout of the target directory; + // ... see: https://doc.rust-lang.org/cargo/guide/build-cache.html .args(extra_args) .output() .expect("build should succeed"); @@ -87,6 +90,18 @@ fn bin_default_uses_debug_build() { assert_eq!(get_contents_of_dir(target_dir.path()), &["debug"]); } +#[test] +fn bin_ignore_cargo_build_target_in_tests() { + use std::env; + env::set_var("CARGO_BUILD_TARGET", "x86_64-unknown-linux-gnu"); + assert_eq!( + env::var("CARGO_BUILD_TARGET"), + Ok("x86_64-unknown-linux-gnu".into()) + ); + // ^ this env var should be ignored: + bin_default_uses_debug_build(); +} + #[test] fn bin_explicit_debug_build() { let target_dir = build_using_bin(&["--profile", "debug"]); From 734aa15084fc72cf7a5af37271733f9c63e7e5b2 Mon Sep 17 00:00:00 2001 From: Bryan Lai Date: Sat, 12 Oct 2024 11:19:50 +0800 Subject: [PATCH 2/3] Fix: Ignore `CARGO_BUILD_TARGET` in lib tests too This follows from the previous commit. It turns out that `std::env::set_var` is unsafe: the `set_var` in bin tests leaked through the threads and caused `lib` tests to fail. To mitigate this problem we simply remove the `CARGO_BUILD_TARGET` variable in lib tests as well. This also improves correctness. Meanwhile we put `std::env::set_var` and `remove_var` in unsafe blocks as it would be required by Rust 2024. --- tests/profile.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/profile.rs b/tests/profile.rs index 35ce3498d..596829d4a 100644 --- a/tests/profile.rs +++ b/tests/profile.rs @@ -1,6 +1,7 @@ use cbindgen::*; use serial_test::serial; +use std::env; use std::path::{Path, PathBuf}; use std::process::Command; @@ -17,7 +18,12 @@ fn build_using_lib(config: fn(Builder) -> Builder) -> tempfile::TempDir { .tempdir() .expect("Creating tmp dir failed"); - std::env::set_var("CARGO_EXPAND_TARGET_DIR", tmp_dir.path()); + unsafe { + env::set_var("CARGO_EXPAND_TARGET_DIR", tmp_dir.path()); + env::remove_var("CARGO_BUILD_TARGET"); + // ^ avoid unexpected change of layout of the target directory; + // ... see: https://doc.rust-lang.org/cargo/guide/build-cache.html + } let builder = Builder::new() .with_config(Config::from_file(expand_dep_test_dir.join("cbindgen.toml")).unwrap()) .with_crate(expand_dep_test_dir); @@ -92,8 +98,9 @@ fn bin_default_uses_debug_build() { #[test] fn bin_ignore_cargo_build_target_in_tests() { - use std::env; - env::set_var("CARGO_BUILD_TARGET", "x86_64-unknown-linux-gnu"); + unsafe { + env::set_var("CARGO_BUILD_TARGET", "x86_64-unknown-linux-gnu"); + } assert_eq!( env::var("CARGO_BUILD_TARGET"), Ok("x86_64-unknown-linux-gnu".into()) From 267b879ed408df46b7b5d78c8aeb010eb53f634f Mon Sep 17 00:00:00 2001 From: Bryan Lai Date: Mon, 28 Oct 2024 17:07:20 +0800 Subject: [PATCH 3/3] Fix: avoid potential race in tests/profile.rs ... also remove an unnecessary `assert`. --- tests/profile.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/profile.rs b/tests/profile.rs index 596829d4a..0e7eba7dc 100644 --- a/tests/profile.rs +++ b/tests/profile.rs @@ -97,14 +97,11 @@ fn bin_default_uses_debug_build() { } #[test] +#[serial] fn bin_ignore_cargo_build_target_in_tests() { unsafe { env::set_var("CARGO_BUILD_TARGET", "x86_64-unknown-linux-gnu"); } - assert_eq!( - env::var("CARGO_BUILD_TARGET"), - Ok("x86_64-unknown-linux-gnu".into()) - ); // ^ this env var should be ignored: bin_default_uses_debug_build(); }