Add target directory parameter: address suggestions

diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs
index 60fb4b5f9bd9d95488c22e896dd2b675abffa2cf..72985eef3d52c80ac176e9cdd324fe90fbdd9047 100644 (file)
--- 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<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());

diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs
index fde665eabf84c61499ee991414787053430871f6..a793826cb6edb04998e13d36f61f846ad80eccf7 100644 (file)
--- 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 30c63e279339f5c7b749b3a3afa4720eaa454fe2..a33c34db0e2f5f63a9b046f270e88f1c19a9b3b2 100644 (file)
--- 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<Filesystem>,
+    /// Target Directory via resolved Cli parameter
+    target_dir: Option<Filesystem>,
 }
 
 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<Option<Filesystem>> {
-        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<Filesystem> {
+        self.target_dir.clone()
     }
 
     fn get(&self, key: &str) -> CargoResult<Option<ConfigValue>> {
@@ -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 574d84bff1be97b66d17a7cde8269e13c644a05a..d1ace1433cd8b48aab06c8e353610a4cafd62f9c 100644 (file)
--- 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 acc315ceaa6d5c343fe35452e38f7446bb811aa2..12cdd8ae76613da9ef7b5007ae8475cd0598f8c2 100644 (file)
--- 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),
     );