From 695bc5e488fb20fa6053c0ef00602f7701988323 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Thu, 3 May 2018 20:23:49 +1200 Subject: [PATCH] Revert part to #5393 Commit 0b530c30867da26a4b59146f490c9f1d5377c20a (which this commit mostly reverts) did some refactoring around the `target_dir` function. However, it introduced a bug because it meant that where `CARGO_TARGET_DIR` was specified but `--target-dir` was not, the value from `CARGO_TARGET_DIR` was ignored. --- src/cargo/core/workspace.rs | 4 ++-- src/cargo/ops/cargo_install.rs | 2 +- src/cargo/util/config.rs | 27 +++++++++++++++------------ 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 679daaddd97..5e15b2e2059 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 87fd1470357..d5f09ff9b6d 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 a33c34db0e2..263de41beeb 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -242,8 +242,17 @@ impl Config { &self.cwd } - pub fn target_dir(&self) -> Option { - self.target_dir.clone() + pub fn target_dir(&self) -> CargoResult> { + if let Some(ref dir) = self.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) + } } fn get(&self, key: &str) -> CargoResult> { @@ -491,15 +500,9 @@ 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 + let cli_target_dir = match target_dir.as_ref() { + Some(dir) => Some(Filesystem::new(dir.clone())), + None => None, }; self.shell().set_verbosity(verbosity); @@ -507,7 +510,7 @@ impl Config { self.extra_verbose = extra_verbose; self.frozen = frozen; self.locked = locked; - self.target_dir = target_dir; + self.target_dir = cli_target_dir; self.cli_flags.parse(unstable_flags)?; Ok(())