Auto merge of #9375 - vojtechkral:tests-target-dir, r=ehuss

Add CARGO_TARGET_TMPDIR env var for integration tests & benches

Hi.
Recently I [ran into](https://github.com/vojtechkral/bard/blob/main/tests/util/mod.rs#L32) the problem that integration tests don't have a good way to figure out where `target` dir is.

Knowing where `target` is is useful for integration tests that need to setup some filesystem structure for test cases. In fact `cargo` itself does this too (and figures out the path rather clumsily).

Another popular way of doing this is to create a directory in `/tmp` (or quivalent on other systems), however, I believe using subdirectory in `target` is better as testcases are easier to debug that way and temporary  locations aren't polluted.

I think this would also address some concerns in #2841

Another solution might be to provide a dedicated subdirectory in `target` for this, something like `target/scratchpad`, but I'm not convinced this is warranted... Edit: That's what was decided to do, see below...

Let me know what you think 🙂

Author: bors

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

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));

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<Option<PathBuf>> = Mutex::new(None);
 
     static ref TEST_ROOTS: Mutex<HashMap<String, PathBuf>> = 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 {

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)
+    }
 }

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);

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

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]