Auto merge of #4852 - jtgeibel:cargo-install-target-dir, r=alexcrichton

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.

/cc @alexcrichton, @ishanjain28

Author: bors

src/bin/install.rs

@@ -100,6 +100,12 @@ As a special convenience, omitting the <crate> 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).
 ";
 

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

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

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