From 0b530c30867da26a4b59146f490c9f1d5377c20a Mon Sep 17 00:00:00 2001 From: Simon Smith Date: Tue, 24 Apr 2018 02:15:55 -0400 Subject: [PATCH] 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 60fb4b5f9..72985eef3 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 fde665eab..a793826cb 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 30c63e279..a33c34db0 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 574d84bff..d1ace1433 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 acc315cea..12cdd8ae7 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), );