From dd0b7a2cdab0af200efccb30b117b1b72d8d06f5 Mon Sep 17 00:00:00 2001 From: Simon Smith Date: Mon, 16 Apr 2018 19:50:30 -0400 Subject: [PATCH 1/2] Add target directory parameter --target-dir --- src/bin/cli.rs | 33 +++++----- src/bin/command_prelude.rs | 4 ++ src/bin/commands/bench.rs | 1 + src/bin/commands/build.rs | 1 + src/bin/commands/check.rs | 1 + src/bin/commands/clean.rs | 1 + src/bin/commands/doc.rs | 1 + src/bin/commands/package.rs | 1 + src/bin/commands/publish.rs | 1 + src/bin/commands/run.rs | 1 + src/bin/commands/rustc.rs | 1 + src/bin/commands/rustdoc.rs | 1 + src/bin/commands/test.rs | 1 + src/cargo/util/config.rs | 14 ++++- tests/testsuite/build.rs | 119 +++++++++++++++++++++++++++++++++++- tests/testsuite/resolve.rs | 1 + 16 files changed, 165 insertions(+), 17 deletions(-) diff --git a/src/bin/cli.rs b/src/bin/cli.rs index 0161a0f6bf9..dd33ce5387e 100644 --- a/src/bin/cli.rs +++ b/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) } diff --git a/src/bin/command_prelude.rs b/src/bin/command_prelude.rs index 79572d8d34b..9875b6b7c22 100644 --- a/src/bin/command_prelude.rs +++ b/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")) } diff --git a/src/bin/commands/bench.rs b/src/bin/commands/bench.rs index af70656980d..83e3ab5af53 100644 --- a/src/bin/commands/bench.rs +++ b/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( diff --git a/src/bin/commands/build.rs b/src/bin/commands/build.rs index 70e9d322528..0ab86aa3a3a 100644 --- a/src/bin/commands/build.rs +++ b/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() diff --git a/src/bin/commands/check.rs b/src/bin/commands/check.rs index 45361740d61..e4295c5c32b 100644 --- a/src/bin/commands/check.rs +++ b/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( diff --git a/src/bin/commands/clean.rs b/src/bin/commands/clean.rs index 8d0928cc2b2..1661091d8c7 100644 --- a/src/bin/commands/clean.rs +++ b/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( "\ diff --git a/src/bin/commands/doc.rs b/src/bin/commands/doc.rs index a6a2546750f..54ba9679794 100644 --- a/src/bin/commands/doc.rs +++ b/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( diff --git a/src/bin/commands/package.rs b/src/bin/commands/package.rs index bc02332bc54..f5e9d918462 100644 --- a/src/bin/commands/package.rs +++ b/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() } diff --git a/src/bin/commands/publish.rs b/src/bin/commands/publish.rs index fe7ea2a551d..b50d3619cf4 100644 --- a/src/bin/commands/publish.rs +++ b/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")) diff --git a/src/bin/commands/run.rs b/src/bin/commands/run.rs index 0858e96a591..763263d5a4b 100644 --- a/src/bin/commands/run.rs +++ b/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( diff --git a/src/bin/commands/rustc.rs b/src/bin/commands/rustc.rs index fb998c911a5..35fb59e8e12 100644 --- a/src/bin/commands/rustc.rs +++ b/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( diff --git a/src/bin/commands/rustdoc.rs b/src/bin/commands/rustdoc.rs index abd2c7405a3..301e65449b8 100644 --- a/src/bin/commands/rustdoc.rs +++ b/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( diff --git a/src/bin/commands/test.rs b/src/bin/commands/test.rs index 35cab8762d2..a25f62f8ec3 100644 --- a/src/bin/commands/test.rs +++ b/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( diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index ac8bf21f6a4..30c63e27933 100644 --- a/src/cargo/util/config.rs +++ b/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 + cli_target_dir: Option, } impl Config { @@ -113,6 +115,7 @@ impl Config { crates_io_source_id: LazyCell::new(), cache_rustc_info, creation_time: Instant::now(), + cli_target_dir: None, } } @@ -240,7 +243,9 @@ impl Config { } pub fn target_dir(&self) -> CargoResult> { - if let Some(dir) = env::var_os("CARGO_TARGET_DIR") { + if let Some(ref dir) = self.cli_target_dir { + Ok(Some(dir.clone())) + } else 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); @@ -461,6 +466,7 @@ impl Config { color: &Option, frozen: bool, locked: bool, + target_dir: &Option, unstable_flags: &[String], ) -> CargoResult<()> { let extra_verbose = verbose >= 2; @@ -494,11 +500,17 @@ impl Config { | (None, None, None) => Verbosity::Normal, }; + let cli_target_dir = match target_dir.as_ref() { + Some(dir) => Some(Filesystem::new(dir.clone())), + None => 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.cli_target_dir = cli_target_dir; self.cli_flags.parse(unstable_flags)?; Ok(()) diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index 403ef8a3a2d..acc315ceaa6 100644 --- a/tests/testsuite/build.rs +++ b/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,123 @@ 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").arg("--target-dir").arg("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").arg("--target-dir").arg("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") + .arg("--target-dir") + .arg("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") diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs index 427f06938ea..a8bf3545b25 100644 --- a/tests/testsuite/resolve.rs +++ b/tests/testsuite/resolve.rs @@ -362,6 +362,7 @@ fn test_resolving_minimum_version_with_transitive_deps() { &None, false, false, + &None, &["minimal-versions".to_string()], ) .unwrap(); From 0b530c30867da26a4b59146f490c9f1d5377c20a Mon Sep 17 00:00:00 2001 From: Simon Smith Date: Tue, 24 Apr 2018 02:15:55 -0400 Subject: [PATCH 2/2] Add target directory parameter: address suggestions --- src/cargo/core/workspace.rs | 4 ++-- src/cargo/ops/cargo_install.rs | 2 +- src/cargo/util/config.rs | 33 +++++++++++++++------------------ tests/testsuite/bad_config.rs | 5 ++++- tests/testsuite/build.rs | 8 +++----- 5 files changed, 25 insertions(+), 27 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 60fb4b5f9bd..72985eef3d5 100644 --- a/src/cargo/core/workspace.rs +++ b/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> { - 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()); diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index fde665eabf8..a793826cb6e 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/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(); diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 30c63e27933..a33c34db0e2 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -70,8 +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 - cli_target_dir: Option, + /// Target Directory via resolved Cli parameter + target_dir: Option, } impl Config { @@ -115,7 +115,7 @@ impl Config { crates_io_source_id: LazyCell::new(), cache_rustc_info, creation_time: Instant::now(), - cli_target_dir: None, + target_dir: None, } } @@ -242,17 +242,8 @@ impl Config { &self.cwd } - pub fn target_dir(&self) -> CargoResult> { - if let Some(ref dir) = self.cli_target_dir { - Ok(Some(dir.clone())) - } else 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 { + self.target_dir.clone() } fn get(&self, key: &str) -> CargoResult> { @@ -500,9 +491,15 @@ impl Config { | (None, None, None) => Verbosity::Normal, }; - let cli_target_dir = match target_dir.as_ref() { - Some(dir) => Some(Filesystem::new(dir.clone())), - None => None, + 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); @@ -510,7 +507,7 @@ impl Config { self.extra_verbose = extra_verbose; self.frozen = frozen; self.locked = locked; - self.cli_target_dir = cli_target_dir; + self.target_dir = target_dir; self.cli_flags.parse(unstable_flags)?; Ok(()) diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs index 574d84bff1b..d1ace1433cd 100644 --- a/tests/testsuite/bad_config.rs +++ b/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 `[..]` diff --git a/tests/testsuite/build.rs b/tests/testsuite/build.rs index acc315ceaa6..12cdd8ae766 100644 --- a/tests/testsuite/build.rs +++ b/tests/testsuite/build.rs @@ -3688,7 +3688,7 @@ fn custom_target_dir_line_parameter() { let exe_name = format!("foo{}", env::consts::EXE_SUFFIX); assert_that( - p.cargo("build").arg("--target-dir").arg("foo/target"), + p.cargo("build --target-dir foo/target"), execs().with_status(0), ); assert_that( @@ -3721,7 +3721,7 @@ fn custom_target_dir_line_parameter() { ) .unwrap(); assert_that( - p.cargo("build").arg("--target-dir").arg("bar/target"), + p.cargo("build --target-dir bar/target"), execs().with_status(0), ); assert_that( @@ -3738,9 +3738,7 @@ fn custom_target_dir_line_parameter() { ); assert_that( - p.cargo("build") - .arg("--target-dir") - .arg("foobar/target") + p.cargo("build --target-dir foobar/target") .env("CARGO_TARGET_DIR", "bar/target"), execs().with_status(0), );