From a81b1437de95920618119272c7a92bab9974ee30 Mon Sep 17 00:00:00 2001 From: Justin Geibel Date: Thu, 21 Dec 2017 20:14:12 -0500 Subject: [PATCH] Always respect `CARGO_TARGET_DIR` during `cargo install` This aligns the behavior of crates.io and `--git` sources with that of `--path` regarding the `CARGO_TARGET_DIR` and `CARGO_BUILD_TARGET_DIR` environment variables. If neither environment variable is set, then a temporary directory is still used when installing from crates.io or `--git`. As discussed in #4725, this can be used to enable caching of artifacts between continuous integration builds. --- src/bin/install.rs | 6 ++++++ src/cargo/ops/cargo_install.rs | 6 +++++- src/etc/man/cargo-install.1 | 4 ++++ tests/install.rs | 28 +++++++++++++++++++++++++++- 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/bin/install.rs b/src/bin/install.rs index 2366f788a..a0c000202 100644 --- a/src/bin/install.rs +++ b/src/bin/install.rs @@ -100,6 +100,12 @@ As a special convenience, omitting the specification entirely will install the crate in the current directory. That is, `install` is equivalent to the more explicit `install --path .`. +If the source is crates.io or `--git` then by default the crate will be built +in a temporary target directory. To avoid this, the target directory can be +specified by setting the `CARGO_TARGET_DIR` environment variable to a relative +path. In particular, this can be useful for caching build artifacts on +continuous integration systems. + The `--list` option will list all installed packages (and their versions). "; diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 2c8c405fe..c85923e45 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -159,13 +159,17 @@ fn install_one(root: &Filesystem, }; let mut td_opt = None; + let mut needs_cleanup = false; let overidden_target_dir = if source_id.is_path() { None + } else if let Some(dir) = config.target_dir()? { + Some(dir) } else if let Ok(td) = TempDir::new("cargo-install") { let p = td.path().to_owned(); td_opt = Some(td); Some(Filesystem::new(p)) } else { + needs_cleanup = true; Some(Filesystem::new(config.cwd().join("target-install"))) }; @@ -311,7 +315,7 @@ fn install_one(root: &Filesystem, // Reaching here means all actions have succeeded. Clean up. installed.success(); - if !source_id.is_path() { + if needs_cleanup { // Don't bother grabbing a lock as we're going to blow it all away // anyway. let target_dir = ws.target_dir().into_path_unlocked(); diff --git a/src/etc/man/cargo-install.1 b/src/etc/man/cargo-install.1 index b85e68a49..f90ad08aa 100644 --- a/src/etc/man/cargo-install.1 +++ b/src/etc/man/cargo-install.1 @@ -42,6 +42,10 @@ will install the crate in the current directory. That is, \f[I]install\f[] is equivalent to the more explicit "install \-\-path .". .PP +If the source is crates.io or \f[B]\-\-git\f[] then by default the crate will be built in a temporary target directory. +To avoid this, the target directory can be specified by setting the \f[B]CARGO_TARGET_DIR\f[] environment variable to a relative path. +In particular, this can be useful for caching build artifacts on continuous integration systems. +.PP The \f[B]\-\-list\f[] option will list all installed packages (and their versions). .SH OPTIONS diff --git a/tests/install.rs b/tests/install.rs index 0da65d515..1ae6806c4 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -11,7 +11,7 @@ use cargotest::support::git; use cargotest::support::paths; use cargotest::support::registry::Package; use cargotest::support::{project, execs}; -use hamcrest::{assert_that, is_not}; +use hamcrest::{assert_that, existing_dir, is_not}; fn cargo_process(s: &str) -> ProcessBuilder { let mut p = cargotest::cargo_process(); @@ -988,3 +988,29 @@ error: some packages failed to uninstall assert_that(cargo_home(), is_not(has_installed_exe("foo"))); assert_that(cargo_home(), is_not(has_installed_exe("bar"))); } + +#[test] +fn custom_target_dir_for_git_source() { + let p = git::repo(&paths::root().join("foo")) + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + "#) + .file("src/main.rs", "fn main() {}") + .build(); + + assert_that(cargo_process("install") + .arg("--git").arg(p.url().to_string()), + execs().with_status(0)); + assert_that(&paths::root().join("target/release"), + is_not(existing_dir())); + + assert_that(cargo_process("install").arg("--force") + .arg("--git").arg(p.url().to_string()) + .env("CARGO_TARGET_DIR", "target"), + execs().with_status(0)); + assert_that(&paths::root().join("target/release"), + existing_dir()); +} -- 2.30.2