From 64bfe7f1de9646b20bbd9e3d14aa30e55339f028 Mon Sep 17 00:00:00 2001 From: Vojtech Kral Date: Thu, 6 May 2021 23:04:52 +0200 Subject: [PATCH 1/2] Add CARGO_TARGET_TMPDIR env var for integration tests & benches The variable is set to $target_dir/$config/tmp This is a directory where tests & benches can place testcasei-related data for use by the tests. cargo makes sure the directory exists when building tests/benches. --- src/cargo/core/compiler/layout.rs | 8 +++ src/cargo/core/compiler/mod.rs | 5 ++ .../src/reference/environment-variables.md | 6 ++ tests/testsuite/build.rs | 64 ++++++++++++++++++- 4 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/layout.rs b/src/cargo/core/compiler/layout.rs index 4f2aa5966c5..b5d7dea6e64 100644 --- a/src/cargo/core/compiler/layout.rs +++ b/src/cargo/core/compiler/layout.rs @@ -125,6 +125,8 @@ pub struct Layout { examples: PathBuf, /// The directory for rustdoc output: `$root/doc` doc: PathBuf, + /// The directory for temporary data of integration tests and benches: `$dest/tmp` + tmp: PathBuf, /// The lockfile for a build (`.cargo-lock`). Will be unlocked when this /// struct is `drop`ped. _lock: FileLock, @@ -170,6 +172,7 @@ impl Layout { fingerprint: dest.join(".fingerprint"), examples: dest.join("examples"), doc: root.join("doc"), + tmp: dest.join("tmp"), root, dest, _lock: lock, @@ -219,4 +222,9 @@ impl Layout { pub fn build(&self) -> &Path { &self.build } + /// Create and return the tmp path. + pub fn prepare_tmp(&self) -> CargoResult<&Path> { + paths::create_dir_all(&self.tmp)?; + Ok(&self.tmp) + } } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index b210ad0f407..969e5996180 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -581,6 +581,11 @@ fn prepare_rustc( base.env("CARGO_PRIMARY_PACKAGE", "1"); } + if unit.target.is_test() || unit.target.is_bench() { + let tmp = cx.files().layout(unit.kind).prepare_tmp()?; + base.env("CARGO_TARGET_TMPDIR", tmp.display().to_string()); + } + if cx.bcx.config.cli_unstable().jobserver_per_rustc { let client = cx.new_jobserver()?; base.inherit_jobserver(&client); diff --git a/src/doc/src/reference/environment-variables.md b/src/doc/src/reference/environment-variables.md index 8b66d5cc5d5..4fff3b0ec89 100644 --- a/src/doc/src/reference/environment-variables.md +++ b/src/doc/src/reference/environment-variables.md @@ -221,6 +221,12 @@ corresponding environment variable is set to the empty string, `""`. on the current directory and the default workspace members. This environment variable will not be set when building dependencies. This is only set when compiling the package (not when running binaries or tests). +* `CARGO_TARGET_TMPDIR` — Only set when building [integration test] or benchmark code. + This is a path to a directory inside the target directory + where integration tests or benchmarks are free to put any data needed by + the tests/benches. Cargo initially creates this directory but doesn't + manage its content in any way, this is the responsibility of the test code. + There are separate directories for `debug` and `release` profiles. [integration test]: cargo-targets.md#integration-tests [`env` macro]: ../../std/macro.env.html diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 65aa90862b7..b24ddc358b6 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -1334,12 +1334,18 @@ fn crate_env_vars() { let s = format!("{}.{}.{}-{}", VERSION_MAJOR, VERSION_MINOR, VERSION_PATCH, VERSION_PRE); assert_eq!(s, VERSION); + + // Verify CARGO_TARGET_TMPDIR isn't set for bins + assert!(option_env!("CARGO_TARGET_TMPDIR").is_none()); } "#, ) .file( "src/lib.rs", r#" + use std::env; + use std::path::PathBuf; + pub fn version() -> String { format!("{}-{}-{} @ {} in {}", env!("CARGO_PKG_VERSION_MAJOR"), @@ -1348,9 +1354,60 @@ fn crate_env_vars() { env!("CARGO_PKG_VERSION_PRE"), env!("CARGO_MANIFEST_DIR")) } + + pub fn check_no_int_test_env() { + env::var("CARGO_TARGET_DIR").unwrap_err(); + } + + pub fn check_tmpdir(tmp: Option<&'static str>) { + let tmpdir: PathBuf = tmp.unwrap().into(); + + let exe: PathBuf = env::current_exe().unwrap().into(); + let mut expected: PathBuf = exe.parent().unwrap().parent().unwrap().into(); + expected.push("tmp"); + assert_eq!(tmpdir, expected); + + // Check that CARGO_TARGET_TMPDIR isn't set for lib code + assert!(option_env!("CARGO_TARGET_TMPDIR").is_none()); + env::var("CARGO_TARGET_TMPDIR").unwrap_err(); + } + + #[test] + fn env() { + // Check that CARGO_TARGET_TMPDIR isn't set for unit tests + assert!(option_env!("CARGO_TARGET_TMPDIR").is_none()); + env::var("CARGO_TARGET_TMPDIR").unwrap_err(); + } "#, ) - .build(); + .file( + "tests/env.rs", + r#" + #[test] + fn env() { + foo::check_tmpdir(option_env!("CARGO_TARGET_TMPDIR")); + } + "#, + ); + + let p = if is_nightly() { + p.file( + "benches/env.rs", + r#" + #![feature(test)] + extern crate test; + use test::Bencher; + + #[bench] + fn env(_: &mut Bencher) { + foo::check_tmpdir(option_env!("CARGO_TARGET_TMPDIR")); + } + "#, + ) + .build() + } else { + p.build() + }; println!("build"); p.cargo("build -v").run(); @@ -1362,6 +1419,11 @@ fn crate_env_vars() { println!("test"); p.cargo("test -v").run(); + + if is_nightly() { + println!("bench"); + p.cargo("bench -v").run(); + } } #[cargo_test] From 53343bc39893ac50fc9fb51bbb86ddd28c3bac74 Mon Sep 17 00:00:00 2001 From: Vojtech Kral Date: Thu, 6 May 2021 23:12:28 +0200 Subject: [PATCH 2/2] Use CARGO_TARGET_TMPDIR in integration tests if available --- crates/cargo-test-macro/src/lib.rs | 8 +++- crates/cargo-test-support/src/git.rs | 5 ++- crates/cargo-test-support/src/paths.rs | 62 +++++++++++++++++--------- 3 files changed, 50 insertions(+), 25 deletions(-) diff --git a/crates/cargo-test-macro/src/lib.rs b/crates/cargo-test-macro/src/lib.rs index dae129ade66..11d417df8ac 100644 --- a/crates/cargo-test-macro/src/lib.rs +++ b/crates/cargo-test-macro/src/lib.rs @@ -31,8 +31,12 @@ pub fn cargo_test(attr: TokenStream, item: TokenStream) -> TokenStream { } }; - let mut new_body = - to_token_stream("let _test_guard = cargo_test_support::paths::init_root();"); + let mut new_body = to_token_stream( + r#"let _test_guard = { + let tmp_dir = option_env!("CARGO_TARGET_TMPDIR"); + cargo_test_support::paths::init_root(tmp_dir) + };"#, + ); // If this is a `build_std` test (aka `tests/build-std/*.rs`) then they // only run on nightly and they only run when specifically instructed to diff --git a/crates/cargo-test-support/src/git.rs b/crates/cargo-test-support/src/git.rs index cc6b0c7462a..18c4646b37d 100644 --- a/crates/cargo-test-support/src/git.rs +++ b/crates/cargo-test-support/src/git.rs @@ -132,11 +132,12 @@ pub fn init(path: &Path) -> git2::Repository { } fn default_search_path() { - use crate::paths::GLOBAL_ROOT; + use crate::paths::global_root; use git2::{opts::set_search_path, ConfigLevel}; + static INIT: Once = Once::new(); INIT.call_once(|| unsafe { - let path = GLOBAL_ROOT.join("blank_git_search_path"); + let path = global_root().join("blank_git_search_path"); t!(set_search_path(ConfigLevel::System, &path)); t!(set_search_path(ConfigLevel::Global, &path)); t!(set_search_path(ConfigLevel::XDG, &path)); diff --git a/crates/cargo-test-support/src/paths.rs b/crates/cargo-test-support/src/paths.rs index 6e0dbf9d68b..192657dae58 100644 --- a/crates/cargo-test-support/src/paths.rs +++ b/crates/cargo-test-support/src/paths.rs @@ -14,28 +14,44 @@ use std::sync::Mutex; static CARGO_INTEGRATION_TEST_DIR: &str = "cit"; lazy_static! { - pub static ref GLOBAL_ROOT: PathBuf = { - let mut path = t!(env::current_exe()); - path.pop(); // chop off exe name - path.pop(); // chop off 'debug' - - // If `cargo test` is run manually then our path looks like - // `target/debug/foo`, in which case our `path` is already pointing at - // `target`. If, however, `cargo test --target $target` is used then the - // output is `target/$target/debug/foo`, so our path is pointing at - // `target/$target`. Here we conditionally pop the `$target` name. - if path.file_name().and_then(|s| s.to_str()) != Some("target") { - path.pop(); - } - - path.push(CARGO_INTEGRATION_TEST_DIR); - path.mkdir_p(); - path - }; + // TODO: Use `SyncOnceCell` when stable + static ref GLOBAL_ROOT: Mutex> = Mutex::new(None); static ref TEST_ROOTS: Mutex> = Default::default(); } +/// This is used when running cargo is pre-CARGO_TARGET_TMPDIR +/// TODO: Remove when CARGO_TARGET_TMPDIR grows old enough. +fn global_root_legacy() -> PathBuf { + let mut path = t!(env::current_exe()); + path.pop(); // chop off exe name + path.pop(); // chop off "deps" + path.push("tmp"); + path.mkdir_p(); + path +} + +fn set_global_root(tmp_dir: Option<&'static str>) { + let mut lock = GLOBAL_ROOT.lock().unwrap(); + if lock.is_none() { + let mut root = match tmp_dir { + Some(tmp_dir) => PathBuf::from(tmp_dir), + None => global_root_legacy(), + }; + + root.push(CARGO_INTEGRATION_TEST_DIR); + *lock = Some(root); + } +} + +pub fn global_root() -> PathBuf { + let lock = GLOBAL_ROOT.lock().unwrap(); + match lock.as_ref() { + Some(p) => p.clone(), + None => unreachable!("GLOBAL_ROOT not set yet"), + } +} + // We need to give each test a unique id. The test name could serve this // purpose, but the `test` crate doesn't have a way to obtain the current test // name.[*] Instead, we used the `cargo-test-macro` crate to automatically @@ -52,14 +68,15 @@ pub struct TestIdGuard { _private: (), } -pub fn init_root() -> TestIdGuard { +pub fn init_root(tmp_dir: Option<&'static str>) -> TestIdGuard { static NEXT_ID: AtomicUsize = AtomicUsize::new(0); - let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); + let id = NEXT_ID.fetch_add(1, Ordering::SeqCst); TEST_ID.with(|n| *n.borrow_mut() = Some(id)); let guard = TestIdGuard { _private: () }; + set_global_root(tmp_dir); let r = root(); r.rm_rf(); r.mkdir_p(); @@ -80,7 +97,10 @@ pub fn root() -> PathBuf { order to be able to use the crate root.", ) }); - GLOBAL_ROOT.join(&format!("t{}", id)) + + let mut root = global_root(); + root.push(&format!("t{}", id)); + root } pub fn home() -> PathBuf {