diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 074c137473f..d4c32e57247 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -6,7 +6,7 @@ use std::slice; use core::{Package, VirtualManifest, EitherManifest, SourceId}; use core::{PackageIdSpec, Dependency}; use ops; -use util::{Config, CargoResult}; +use util::{Config, CargoResult, Filesystem}; use util::paths; /// The core abstraction in Cargo for working with a workspace of crates. @@ -32,6 +32,10 @@ pub struct Workspace<'cfg> { // `current_manifest` was found on the filesystem with `[workspace]`. root_manifest: Option, + // Shared target directory for all the packages of this workspace. + // `None` if the default path of `root/target` should be used. + target_dir: Option, + // List of members in this workspace with a listing of all their manifest // paths. The packages themselves can be looked up through the `packages` // set above. @@ -78,6 +82,8 @@ impl<'cfg> Workspace<'cfg> { /// before returning it, so `Ok` is only returned for valid workspaces. pub fn new(manifest_path: &Path, config: &'cfg Config) -> CargoResult> { + let target_dir = try!(config.target_dir()); + let mut ws = Workspace { config: config, current_manifest: manifest_path.to_path_buf(), @@ -86,6 +92,7 @@ impl<'cfg> Workspace<'cfg> { packages: HashMap::new(), }, root_manifest: None, + target_dir: target_dir, members: Vec::new(), }; ws.root_manifest = try!(ws.find_root(manifest_path)); @@ -103,7 +110,8 @@ impl<'cfg> Workspace<'cfg> { /// /// This is currently only used in niche situations like `cargo install` or /// `cargo package`. - pub fn one(package: Package, config: &'cfg Config) -> Workspace<'cfg> { + pub fn one(package: Package, config: &'cfg Config, target_dir: Option) + -> CargoResult> { let mut ws = Workspace { config: config, current_manifest: package.manifest_path().to_path_buf(), @@ -112,15 +120,21 @@ impl<'cfg> Workspace<'cfg> { packages: HashMap::new(), }, root_manifest: None, + target_dir: None, members: Vec::new(), }; { let key = ws.current_manifest.parent().unwrap(); let package = MaybePackage::Package(package); ws.packages.packages.insert(key.to_path_buf(), package); + ws.target_dir = if let Some(dir) = target_dir { + Some(dir) + } else { + try!(ws.config.target_dir()) + }; ws.members.push(ws.current_manifest.clone()); } - return ws + return Ok(ws) } /// Returns the current package of this workspace. @@ -155,6 +169,12 @@ impl<'cfg> Workspace<'cfg> { }.parent().unwrap() } + pub fn target_dir(&self) -> Filesystem { + self.target_dir.clone().unwrap_or_else(|| { + Filesystem::new(self.root().join("target")) + }) + } + /// Returns the root [replace] section of this workspace. /// /// This may be from a virtual crate or an actual crate. diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index a5eba05cb7c..88c8c933ad9 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -16,7 +16,7 @@ pub struct CleanOptions<'a> { /// Cleans the project from build artifacts. pub fn clean(ws: &Workspace, opts: &CleanOptions) -> CargoResult<()> { - let target_dir = opts.config.target_dir(&ws); + let target_dir = ws.target_dir(); // If we have a spec, then we need to delete some packages, otherwise, just // remove the whole target directory and be done with it! diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index c8a8b4719a2..b764be1a2e9 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -53,7 +53,7 @@ pub fn doc(ws: &Workspace, // Don't bother locking here as if this is getting deleted there's // nothing we can do about it and otherwise if it's getting overwritten // then that's also ok! - let target_dir = options.compile_opts.config.target_dir(ws); + let target_dir = ws.target_dir(); let path = target_dir.join("doc").join(&name).join("index.html"); let path = path.into_path_unlocked(); if fs::metadata(&path).is_ok() { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 071bb58c8c8..b120ace95e1 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -76,7 +76,20 @@ pub fn install(root: Option<&str>, crates.io, or use --path or --git to \ specify alternate source")))) }; - let ws = Workspace::one(pkg, config); + + + let mut td_opt = None; + let overidden_target_dir = if source_id.is_path() { + None + } else if let Ok(td) = TempDir::new("cargo-install") { + let p = td.path().to_owned(); + td_opt = Some(td); + Some(Filesystem::new(p)) + } else { + Some(Filesystem::new(config.cwd().join("target-install"))) + }; + + let ws = try!(Workspace::one(pkg, config, overidden_target_dir)); let pkg = try!(ws.current()); // Preflight checks to check up front whether we'll overwrite something. @@ -89,19 +102,6 @@ pub fn install(root: Option<&str>, try!(check_overwrites(&dst, pkg, &opts.filter, &list, force)); } - let mut td_opt = None; - let target_dir = if source_id.is_path() { - config.target_dir(&ws) - } else { - if let Ok(td) = TempDir::new("cargo-install") { - let p = td.path().to_owned(); - td_opt = Some(td); - Filesystem::new(p) - } else { - Filesystem::new(config.cwd().join("target-install")) - } - }; - config.set_target_dir(target_dir.clone()); let compile = try!(ops::compile_ws(&ws, Some(source), opts).chain_error(|| { if let Some(td) = td_opt.take() { // preserve the temporary directory, so the user can inspect it @@ -109,7 +109,7 @@ pub fn install(root: Option<&str>, } human(format!("failed to compile `{}`, intermediate artifacts can be \ - found at `{}`", pkg, target_dir.display())) + found at `{}`", pkg, ws.target_dir().display())) })); let binaries: Vec<(&str, &Path)> = try!(compile.binaries.iter().map(|bin| { let name = bin.file_name().unwrap(); @@ -224,7 +224,7 @@ pub fn install(root: Option<&str>, if !source_id.is_path() { // Don't bother grabbing a lock as we're going to blow it all away // anyway. - let target_dir = target_dir.into_path_unlocked(); + let target_dir = ws.target_dir().into_path_unlocked(); try!(fs::remove_dir_all(&target_dir)); } diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index c22f44cd6ee..8201a5fa867 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -52,7 +52,7 @@ pub fn package(ws: &Workspace, } let filename = format!("{}-{}.crate", pkg.name(), pkg.version()); - let dir = config.target_dir(ws).join("package"); + let dir = ws.target_dir().join("package"); let mut dst = { let tmp = format!(".{}", filename); try!(dir.open_rw(&tmp, config, "package scratch space")) @@ -266,7 +266,7 @@ fn run_verify(ws: &Workspace, tar: &File, opts: &PackageOpts) -> CargoResult<()> let new_pkg = Package::new(new_manifest, &manifest_path); // Now that we've rewritten all our path dependencies, compile it! - let ws = Workspace::one(new_pkg, config); + let ws = try!(Workspace::one(new_pkg, config, None)); try!(ops::compile_ws(&ws, None, &ops::CompileOptions { config: config, jobs: opts.jobs, diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs index a7575e7fdce..67c2341fa79 100644 --- a/src/cargo/ops/cargo_rustc/layout.rs +++ b/src/cargo/ops/cargo_rustc/layout.rs @@ -72,7 +72,7 @@ impl Layout { pub fn new(ws: &Workspace, triple: Option<&str>, dest: &str) -> CargoResult { - let mut path = ws.config().target_dir(ws); + let mut path = ws.target_dir(); // Flexible target specifications often point at filenames, so interpret // the target triple as a Path and then just use the file stem as the // component for the directory name. diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 16298a43ac5..28d890d7604 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -13,7 +13,7 @@ use std::str::FromStr; use rustc_serialize::{Encodable,Encoder}; use toml; use core::shell::{Verbosity, ColorConfig}; -use core::{MultiShell, Workspace}; +use core::MultiShell; use util::{CargoResult, CargoError, ChainError, Rustc, internal, human}; use util::{Filesystem, LazyCell}; @@ -28,7 +28,6 @@ pub struct Config { values: LazyCell>, cwd: PathBuf, rustdoc: LazyCell, - target_dir: RefCell>, extra_verbose: Cell, frozen: Cell, locked: Cell, @@ -37,23 +36,18 @@ pub struct Config { impl Config { pub fn new(shell: MultiShell, cwd: PathBuf, - homedir: PathBuf) -> CargoResult { - let mut cfg = Config { + homedir: PathBuf) -> Config { + Config { home_path: Filesystem::new(homedir), shell: RefCell::new(shell), rustc: LazyCell::new(), cwd: cwd, values: LazyCell::new(), rustdoc: LazyCell::new(), - target_dir: RefCell::new(None), extra_verbose: Cell::new(false), frozen: Cell::new(false), locked: Cell::new(false), - }; - - try!(cfg.scrape_target_dir_config()); - - Ok(cfg) + } } pub fn default() -> CargoResult { @@ -65,7 +59,7 @@ impl Config { human("Cargo couldn't find your home directory. \ This probably means that $HOME was not set.") })); - Config::new(shell, cwd, homedir) + Ok(Config::new(shell, cwd, homedir)) } pub fn home(&self) -> &Filesystem { &self.home_path } @@ -108,14 +102,15 @@ impl Config { pub fn cwd(&self) -> &Path { &self.cwd } - pub fn target_dir(&self, ws: &Workspace) -> Filesystem { - self.target_dir.borrow().clone().unwrap_or_else(|| { - Filesystem::new(ws.root().join("target")) - }) - } - - pub fn set_target_dir(&self, path: Filesystem) { - *self.target_dir.borrow_mut() = Some(path); + pub fn target_dir(&self) -> CargoResult> { + if let Some(dir) = env::var_os("CARGO_TARGET_DIR") { + Ok(Some(Filesystem::new(self.cwd.join(dir)))) + } else if let Some(val) = try!(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> { @@ -292,8 +287,11 @@ impl Config { locked: bool) -> CargoResult<()> { let extra_verbose = verbose >= 2; let verbose = if verbose == 0 {None} else {Some(true)}; - let cfg_verbose = try!(self.get_bool("term.verbose")).map(|v| v.val); - let cfg_color = try!(self.get_string("term.color")).map(|v| v.val); + + // Ignore errors in the configuration files. + let cfg_verbose = self.get_bool("term.verbose").unwrap_or(None).map(|v| v.val); + let cfg_color = self.get_string("term.color").unwrap_or(None).map(|v| v.val); + let color = color.as_ref().or(cfg_color.as_ref()); let verbosity = match (verbose, cfg_verbose, quiet) { @@ -369,16 +367,6 @@ impl Config { } } - fn scrape_target_dir_config(&mut self) -> CargoResult<()> { - if let Some(dir) = env::var_os("CARGO_TARGET_DIR") { - *self.target_dir.borrow_mut() = Some(Filesystem::new(self.cwd.join(dir))); - } else if let Some(val) = try!(self.get_path("build.target-dir")) { - let val = self.cwd.join(val.val); - *self.target_dir.borrow_mut() = Some(Filesystem::new(val)); - } - Ok(()) - } - fn get_tool(&self, tool: &str) -> CargoResult { let var = tool.chars().flat_map(|c| c.to_uppercase()).collect::(); if let Some(tool_path) = env::var_os(&var) { diff --git a/tests/bad-config.rs b/tests/bad-config.rs index 3b806ef463a..710416b8e2e 100644 --- a/tests/bad-config.rs +++ b/tests/bad-config.rs @@ -112,7 +112,10 @@ fn bad5() { assert_that(foo.cargo("new") .arg("-v").arg("foo").cwd(&foo.root().join("foo")), execs().with_status(101).with_stderr("\ -[ERROR] Couldn't load Cargo configuration +[ERROR] Failed to create project `foo` at `[..]` + +Caused by: + Couldn't load Cargo configuration Caused by: failed to merge key `foo` between files: diff --git a/tests/version.rs b/tests/version.rs index bc1040ed185..b017830b272 100644 --- a/tests/version.rs +++ b/tests/version.rs @@ -56,3 +56,22 @@ fn version_works_without_rustc() { assert_that(p.cargo_process("version").env("PATH", ""), execs().with_status(0)); } + +#[test] +fn version_works_with_bad_config() { + let p = project("foo") + .file(".cargo/config", "this is not toml"); + assert_that(p.cargo_process("version"), + execs().with_status(0)); +} + +#[test] +fn version_works_with_bad_target_dir() { + let p = project("foo") + .file(".cargo/config", r#" + [build] + target-dir = 4 + "#); + assert_that(p.cargo_process("version"), + execs().with_status(0)); +}