Auto merge of #5393 - smithsps:master, r=matklad

Add target directory parameter --target-dir

Implements: #5308

Adds a target directory parameter, that acts in the same manner as the environment variable `CARGO_TARGET_DIR`, to the following subcommands:
- `bench`
- `build`
- `check`
- `clean`
- `doc`
- `package`
- `publish`
- `run`
- `rustc`
- `rustdoc`
- `test`

Author: bors

src/bin/cli.rs

@@ -72,6 +72,16 @@ Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"
 }
 
 fn execute_subcommand(config: &mut Config, args: ArgMatches) -> CliResult {
+    let (cmd, subcommand_args) = match args.subcommand() {
+        (cmd, Some(args)) => (cmd, args),
+        _ => {
+            cli().print_help()?;
+            return Ok(());
+        }
+    };
+    
+    let arg_target_dir = &subcommand_args.value_of_path("target-dir", config);
+
     config.configure(
         args.occurrences_of("verbose") as u32,
         if args.is_present("quiet") {
@@ -82,35 +92,28 @@ fn execute_subcommand(config: &mut Config, args: ArgMatches) -> CliResult {
         &args.value_of("color").map(|s| s.to_string()),
         args.is_present("frozen"),
         args.is_present("locked"),
+        arg_target_dir,
         &args.values_of_lossy("unstable-features")
             .unwrap_or_default(),
     )?;
 
-    let (cmd, args) = match args.subcommand() {
-        (cmd, Some(args)) => (cmd, args),
-        _ => {
-            cli().print_help()?;
-            return Ok(());
-        }
-    };
-
     if let Some(exec) = commands::builtin_exec(cmd) {
-        return exec(config, args);
+        return exec(config, subcommand_args);
     }
 
     if let Some(mut alias) = super::aliased_command(config, cmd)? {
         alias.extend(
-            args.values_of("")
-                .unwrap_or_default()
-                .map(|s| s.to_string()),
+            subcommand_args.values_of("")
+                           .unwrap_or_default()
+                           .map(|s| s.to_string()),
         );
-        let args = cli()
+        let subcommand_args = cli()
             .setting(AppSettings::NoBinaryName)
             .get_matches_from_safe(alias)?;
-        return execute_subcommand(config, args);
+        return execute_subcommand(config, subcommand_args);
     }
     let mut ext_args: Vec<&str> = vec![cmd];
-    ext_args.extend(args.values_of("").unwrap_or_default());
+    ext_args.extend(subcommand_args.values_of("").unwrap_or_default());
     super::execute_external_subcommand(config, cmd, &ext_args)
 }
 

src/bin/command_prelude.rs

@@ -109,6 +109,10 @@ pub trait AppExt: Sized {
         self._arg(opt("target", target).value_name("TRIPLE"))
     }
 
+    fn arg_target_dir(self) -> Self {
+        self._arg(opt("target-dir", "Directory for all generated artifacts").value_name("DIRECTORY"))
+    }
+
     fn arg_manifest_path(self) -> Self {
         self._arg(opt("manifest-path", "Path to Cargo.toml").value_name("PATH"))
     }

src/bin/commands/bench.rs

@@ -37,6 +37,7 @@ pub fn cli() -> App {
         .arg_jobs()
         .arg_features()
         .arg_target_triple("Build for the target triple")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_message_format()
         .arg(opt(

src/bin/commands/build.rs

@@ -27,6 +27,7 @@ pub fn cli() -> App {
         .arg_release("Build artifacts in release mode, with optimizations")
         .arg_features()
         .arg_target_triple("Build for the target triple")
+        .arg_target_dir()
         .arg(opt("out-dir", "Copy final artifacts to this directory").value_name("PATH"))
         .arg_manifest_path()
         .arg_message_format()

src/bin/commands/check.rs

@@ -27,6 +27,7 @@ pub fn cli() -> App {
         .arg(opt("profile", "Profile to build the selected target for").value_name("PROFILE"))
         .arg_features()
         .arg_target_triple("Check for the target triple")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_message_format()
         .after_help(

src/bin/commands/clean.rs

@@ -8,6 +8,7 @@ pub fn cli() -> App {
         .arg_package_spec_simple("Package to clean artifacts for")
         .arg_manifest_path()
         .arg_target_triple("Target triple to clean output for (default all)")
+        .arg_target_dir()
         .arg_release("Whether or not to clean release artifacts")
         .after_help(
             "\

src/bin/commands/doc.rs

@@ -24,6 +24,7 @@ pub fn cli() -> App {
         .arg_release("Build artifacts in release mode, with optimizations")
         .arg_features()
         .arg_target_triple("Build for the target triple")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_message_format()
         .after_help(

src/bin/commands/package.rs

@@ -24,6 +24,7 @@ pub fn cli() -> App {
             "Allow dirty working directories to be packaged",
         ))
         .arg_target_triple("Build for the target triple")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_jobs()
 }

src/bin/commands/publish.rs

@@ -16,6 +16,7 @@ pub fn cli() -> App {
             "Allow dirty working directories to be packaged",
         ))
         .arg_target_triple("Build for the target triple")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_jobs()
         .arg(opt("dry-run", "Perform all checks without uploading"))

src/bin/commands/run.rs

@@ -18,6 +18,7 @@ pub fn cli() -> App {
         .arg_release("Build artifacts in release mode, with optimizations")
         .arg_features()
         .arg_target_triple("Build for the target triple")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_message_format()
         .after_help(

src/bin/commands/rustc.rs

@@ -25,6 +25,7 @@ pub fn cli() -> App {
         .arg(opt("profile", "Profile to build the selected target for").value_name("PROFILE"))
         .arg_features()
         .arg_target_triple("Target triple which compiles will be for")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_message_format()
         .after_help(

src/bin/commands/rustdoc.rs

@@ -27,6 +27,7 @@ pub fn cli() -> App {
         )
         .arg_release("Build artifacts in release mode, with optimizations")
         .arg_features()
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_message_format()
         .after_help(

src/bin/commands/test.rs

@@ -41,6 +41,7 @@ pub fn cli() -> App {
         .arg_release("Build artifacts in release mode, with optimizations")
         .arg_features()
         .arg_target_triple("Build for the target triple")
+        .arg_target_dir()
         .arg_manifest_path()
         .arg_message_format()
         .after_help(

src/cargo/core/workspace.rs

@@ -131,7 +131,7 @@ impl<'cfg> Workspace<'cfg> {
     /// root and all member packages. It will then validate the workspace
     /// before returning it, so `Ok` is only returned for valid workspaces.
     pub fn new(manifest_path: &Path, config: &'cfg Config) -> CargoResult<Workspace<'cfg>> {
-        let target_dir = config.target_dir()?;
+        let target_dir = config.target_dir();
 
         let mut ws = Workspace {
             config,
@@ -191,7 +191,7 @@ impl<'cfg> Workspace<'cfg> {
             ws.target_dir = if let Some(dir) = target_dir {
                 Some(dir)
             } else {
-                ws.config.target_dir()?
+                ws.config.target_dir()
             };
             ws.members.push(ws.current_manifest.clone());
             ws.default_members.push(ws.current_manifest.clone());

src/cargo/ops/cargo_install.rs

@@ -213,7 +213,7 @@ fn install_one(
     let mut needs_cleanup = false;
     let overidden_target_dir = if source_id.is_path() {
         None
-    } else if let Some(dir) = config.target_dir()? {
+    } else if let Some(dir) = config.target_dir() {
         Some(dir)
     } else if let Ok(td) = TempFileBuilder::new().prefix("cargo-install").tempdir() {
         let p = td.path().to_owned();

src/cargo/util/config.rs

@@ -70,6 +70,8 @@ pub struct Config {
     cache_rustc_info: bool,
     /// Creation time of this config, used to output the total build time
     creation_time: Instant,
+    /// Target Directory via resolved Cli parameter
+    target_dir: Option<Filesystem>,
 }
 
 impl Config {
@@ -113,6 +115,7 @@ impl Config {
             crates_io_source_id: LazyCell::new(),
             cache_rustc_info,
             creation_time: Instant::now(),
+            target_dir: None,
         }
     }
 
@@ -239,15 +242,8 @@ impl Config {
         &self.cwd
     }
 
-    pub fn target_dir(&self) -> CargoResult<Option<Filesystem>> {
-        if let Some(dir) = env::var_os("CARGO_TARGET_DIR") {
-            Ok(Some(Filesystem::new(self.cwd.join(dir))))
-        } else if let Some(val) = self.get_path("build.target-dir")? {
-            let val = self.cwd.join(val.val);
-            Ok(Some(Filesystem::new(val)))
-        } else {
-            Ok(None)
-        }
+    pub fn target_dir(&self) -> Option<Filesystem> {
+        self.target_dir.clone()
     }
 
     fn get(&self, key: &str) -> CargoResult<Option<ConfigValue>> {
@@ -461,6 +457,7 @@ impl Config {
         color: &Option<String>,
         frozen: bool,
         locked: bool,
+        target_dir: &Option<PathBuf>,
         unstable_flags: &[String],
     ) -> CargoResult<()> {
         let extra_verbose = verbose >= 2;
@@ -494,11 +491,23 @@ impl Config {
             | (None, None, None) => Verbosity::Normal,
         };
 
+        let target_dir = if let Some(dir) = target_dir.as_ref() {
+            Some(Filesystem::new(self.cwd.join(dir)))
+        } else if let Some(dir) = env::var_os("CARGO_TARGET_DIR") {
+            Some(Filesystem::new(self.cwd.join(dir)))
+        } else if let Ok(Some(val)) = self.get_path("build.target-dir") {
+            let val = self.cwd.join(val.val);
+            Some(Filesystem::new(val))
+        } else {
+            None
+        };
+
         self.shell().set_verbosity(verbosity);
         self.shell().set_color_choice(color.map(|s| &s[..]))?;
         self.extra_verbose = extra_verbose;
         self.frozen = frozen;
         self.locked = locked;
+        self.target_dir = target_dir;
         self.cli_flags.parse(unstable_flags)?;
 
         Ok(())

tests/testsuite/bad_config.rs

@@ -284,7 +284,10 @@ fn invalid_global_config() {
         p.cargo("build").arg("-v"),
         execs().with_status(101).with_stderr(
             "\
-[ERROR] Couldn't load Cargo configuration
+error: failed to parse manifest at `[..]`
+
+Caused by:
+  Couldn't load Cargo configuration
 
 Caused by:
   could not parse TOML configuration in `[..]`

tests/testsuite/build.rs

@@ -3603,7 +3603,7 @@ fn dotdir_root() {
 }
 
 #[test]
-fn custom_target_dir() {
+fn custom_target_dir_env() {
     let p = project("foo")
         .file(
             "Cargo.toml",
@@ -3670,6 +3670,121 @@ fn custom_target_dir() {
     );
 }
 
+#[test]
+fn custom_target_dir_line_parameter() {
+    let p = project("foo")
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    let exe_name = format!("foo{}", env::consts::EXE_SUFFIX);
+
+    assert_that(
+        p.cargo("build --target-dir foo/target"),
+        execs().with_status(0),
+    );
+    assert_that(
+        &p.root().join("foo/target/debug").join(&exe_name),
+        existing_file(),
+    );
+    assert_that(
+        &p.root().join("target/debug").join(&exe_name),
+        is_not(existing_file()),
+    );
+
+    assert_that(p.cargo("build"), execs().with_status(0));
+    assert_that(
+        &p.root().join("foo/target/debug").join(&exe_name),
+        existing_file(),
+    );
+    assert_that(
+        &p.root().join("target/debug").join(&exe_name),
+        existing_file(),
+    );
+
+    fs::create_dir(p.root().join(".cargo")).unwrap();
+    File::create(p.root().join(".cargo/config"))
+        .unwrap()
+        .write_all(
+            br#"
+        [build]
+        target-dir = "foo/target"
+    "#,
+        )
+        .unwrap();
+    assert_that(
+        p.cargo("build --target-dir bar/target"),
+        execs().with_status(0),
+    );
+    assert_that(
+        &p.root().join("bar/target/debug").join(&exe_name),
+        existing_file(),
+    );
+    assert_that(
+        &p.root().join("foo/target/debug").join(&exe_name),
+        existing_file(),
+    );
+    assert_that(
+        &p.root().join("target/debug").join(&exe_name),
+        existing_file(),
+    );
+
+    assert_that(
+        p.cargo("build --target-dir foobar/target")
+            .env("CARGO_TARGET_DIR", "bar/target"),
+        execs().with_status(0),
+    );
+    assert_that(
+        &p.root().join("foobar/target/debug").join(&exe_name),
+        existing_file(),
+    );
+    assert_that(
+        &p.root().join("bar/target/debug").join(&exe_name),
+        existing_file(),
+    );
+    assert_that(
+        &p.root().join("foo/target/debug").join(&exe_name),
+        existing_file(),
+    );
+    assert_that(
+        &p.root().join("target/debug").join(&exe_name),
+        existing_file(),
+    );
+}
+
+#[test]
+fn rustc_no_trans() {
+    if !is_nightly() {
+        return;
+    }
+
+    let p = project("foo")
+        .file(
+            "Cargo.toml",
+            r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    assert_that(
+        p.cargo("rustc").arg("-v").arg("--").arg("-Zno-trans"),
+        execs().with_status(0),
+    );
+}
+
 #[test]
 fn build_multiple_packages() {
     let p = project("foo")