Change Config::target_dir to return Filesystem

This is a shared directory among multiple Cargo processes so access to it needs
to be properly synchronized. This commit changes the API of `Config::target_dir`
and then propagates the changes outward as necessary.

One fallout of this change is now we allow release/debug builds to proceed in
parallel.

Author: alexcrichton

src/cargo/ops/cargo_clean.rs

@@ -21,17 +21,24 @@ pub fn clean(manifest_path: &Path, opts: &CleanOptions) -> CargoResult<()> {
 
     // If we have a spec, then we need to delete some packages, otherwise, just
     // remove the whole target directory and be done with it!
+    //
+    // Note that we don't bother grabbing a lock here as we're just going to
+    // blow it all away anyway.
     if opts.spec.is_empty() {
+        let target_dir = target_dir.into_path_unlocked();
         return rm_rf(&target_dir);
     }
 
     let (resolve, packages) = try!(ops::fetch(manifest_path, opts.config));
 
     let dest = if opts.release {"release"} else {"debug"};
-    let host_layout = Layout::new(opts.config, &root, None, dest);
-    let target_layout = opts.target.map(|target| {
-        Layout::new(opts.config, &root, Some(target), dest)
-    });
+    let host_layout = try!(Layout::new(opts.config, &root, None, dest));
+    let target_layout = match opts.target {
+        Some(target) => {
+            Some(try!(Layout::new(opts.config, &root, Some(target), dest)))
+        }
+        None => None,
+    };
 
     let cx = try!(Context::new(&resolve, &packages, opts.config,
                                host_layout, target_layout,

src/cargo/ops/cargo_doc.rs

@@ -50,8 +50,12 @@ pub fn doc(manifest_path: &Path,
             }
         };
 
+        // 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(&package);
         let path = target_dir.join("doc").join(&name).join("index.html");
+        let path = path.into_path_unlocked();
         if fs::metadata(&path).is_ok() {
             let mut shell = options.compile_opts.config.shell();
             match open_docs(&path) {

src/cargo/ops/cargo_install.rs

@@ -82,9 +82,9 @@ pub fn install(root: Option<&str>,
     let target_dir = if source_id.is_path() {
         config.target_dir(&pkg)
     } else {
-        config.cwd().join("target-install")
+        Filesystem::new(config.cwd().join("target-install"))
     };
-    config.set_target_dir(&target_dir);
+    config.set_target_dir(target_dir.clone());
     let compile = try!(ops::compile_pkg(&pkg, Some(source), opts).chain_error(|| {
         human(format!("failed to compile `{}`, intermediate artifacts can be \
                        found at `{}`", pkg, target_dir.display()))
@@ -108,6 +108,9 @@ 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();
         try!(fs::remove_dir_all(&target_dir));
     }
 

src/cargo/ops/cargo_package.rs

@@ -1,21 +1,22 @@
-use std::io::prelude::*;
 use std::fs::{self, File};
-use std::path::{self, Path, PathBuf};
+use std::io::SeekFrom;
+use std::io::prelude::*;
+use std::path::{self, Path};
 
 use tar::{Archive, Builder, Header};
 use flate2::{GzBuilder, Compression};
 use flate2::read::GzDecoder;
 
 use core::{SourceId, Package, PackageId};
 use sources::PathSource;
-use util::{self, CargoResult, human, internal, ChainError, Config};
+use util::{self, CargoResult, human, internal, ChainError, Config, FileLock};
 use ops;
 
 pub fn package(manifest_path: &Path,
                config: &Config,
                verify: bool,
                list: bool,
-               metadata: bool) -> CargoResult<Option<PathBuf>> {
+               metadata: bool) -> CargoResult<Option<FileLock>> {
     let path = manifest_path.parent().unwrap();
     let id = try!(SourceId::for_path(path));
     let mut src = PathSource::new(path, &id, config);
@@ -39,29 +40,37 @@ pub fn package(manifest_path: &Path,
 
     let filename = format!("{}-{}.crate", pkg.name(), pkg.version());
     let dir = config.target_dir(&pkg).join("package");
-    let dst = dir.join(&filename);
-    if fs::metadata(&dst).is_ok() {
-        return Ok(Some(dst))
-    }
+    let mut dst = match dir.open_ro(&filename, config, "packaged crate") {
+        Ok(f) => return Ok(Some(f)),
+        Err(..) => {
+            let tmp = format!(".{}", filename);
+            try!(dir.open_rw(&tmp, config, "package scratch space"))
+        }
+    };
 
     // Package up and test a temporary tarball and only move it to the final
     // location if it actually passes all our tests. Any previously existing
     // tarball can be assumed as corrupt or invalid, so we just blow it away if
     // it exists.
     try!(config.shell().status("Packaging", pkg.package_id().to_string()));
-    let tmp_dst = dir.join(format!(".{}", filename));
-    let _ = fs::remove_file(&tmp_dst);
-    try!(tar(&pkg, &src, config, &tmp_dst, &filename).chain_error(|| {
+    try!(dst.file().set_len(0));
+    try!(tar(&pkg, &src, config, dst.file(), &filename).chain_error(|| {
         human("failed to prepare local package for uploading")
     }));
     if verify {
-        try!(run_verify(config, &pkg, &tmp_dst).chain_error(|| {
+        try!(dst.seek(SeekFrom::Start(0)));
+        try!(run_verify(config, &pkg, dst.file()).chain_error(|| {
             human("failed to verify package tarball")
         }))
     }
-    try!(fs::rename(&tmp_dst, &dst).chain_error(|| {
-        human("failed to move temporary tarball into final location")
-    }));
+    try!(dst.seek(SeekFrom::Start(0)));
+    {
+        let src_path = dst.path();
+        let dst_path = dst.parent().join(&filename);
+        try!(fs::rename(&src_path, &dst_path).chain_error(|| {
+            human("failed to move temporary tarball into final location")
+        }));
+    }
     Ok(Some(dst))
 }
 
@@ -103,26 +112,17 @@ fn check_metadata(pkg: &Package, config: &Config) -> CargoResult<()> {
 fn tar(pkg: &Package,
        src: &PathSource,
        config: &Config,
-       dst: &Path,
+       dst: &File,
        filename: &str) -> CargoResult<()> {
-    if fs::metadata(&dst).is_ok() {
-        bail!("destination already exists: {}", dst.display())
-    }
-
-    try!(fs::create_dir_all(dst.parent().unwrap()));
-
-    let tmpfile = try!(File::create(dst));
-
     // Prepare the encoder and its header
     let filename = Path::new(filename);
     let encoder = GzBuilder::new().filename(try!(util::path2bytes(filename)))
-                                  .write(tmpfile, Compression::Best);
+                                  .write(dst, Compression::Best);
 
     // Put all package files into a compressed archive
     let mut ar = Builder::new(encoder);
     let root = pkg.root();
     for file in try!(src.list_files(pkg)).iter() {
-        if &**file == dst { continue }
         let relative = util::without_prefix(&file, &root).unwrap();
         try!(check_filename(relative));
         let relative = try!(relative.to_str().chain_error(|| {
@@ -173,11 +173,13 @@ fn tar(pkg: &Package,
     Ok(())
 }
 
-fn run_verify(config: &Config, pkg: &Package, tar: &Path)
+fn run_verify(config: &Config,
+              pkg: &Package,
+              tar: &File)
               -> CargoResult<()> {
     try!(config.shell().status("Verifying", pkg));
 
-    let f = try!(GzDecoder::new(try!(File::open(tar))));
+    let f = try!(GzDecoder::new(tar));
     let dst = pkg.root().join(&format!("target/package/{}-{}",
                                        pkg.name(), pkg.version()));
     if fs::metadata(&dst).is_ok() {

src/cargo/ops/cargo_rustc/layout.rs

@@ -50,7 +50,7 @@ use std::io;
 use std::path::{PathBuf, Path};
 
 use core::{Package, Target};
-use util::Config;
+use util::{Config, FileLock, CargoResult, Filesystem};
 use util::hex::short_hash;
 
 pub struct Layout {
@@ -60,6 +60,7 @@ pub struct Layout {
     build: PathBuf,
     fingerprint: PathBuf,
     examples: PathBuf,
+    _lock: FileLock,
 }
 
 pub struct LayoutProxy<'a> {
@@ -68,8 +69,10 @@ pub struct LayoutProxy<'a> {
 }
 
 impl Layout {
-    pub fn new(config: &Config, pkg: &Package, triple: Option<&str>,
-               dest: &str) -> Layout {
+    pub fn new(config: &Config,
+               pkg: &Package,
+               triple: Option<&str>,
+               dest: &str) -> CargoResult<Layout> {
         let mut path = config.target_dir(pkg);
         // Flexible target specifications often point at filenames, so interpret
         // the target triple as a Path and then just use the file stem as the
@@ -78,18 +81,25 @@ impl Layout {
             path.push(Path::new(triple).file_stem().unwrap());
         }
         path.push(dest);
-        Layout::at(path)
+        Layout::at(config, path)
     }
 
-    pub fn at(root: PathBuf) -> Layout {
-        Layout {
+    pub fn at(config: &Config, root: Filesystem) -> CargoResult<Layout> {
+        // For now we don't do any more finer-grained locking on the artifact
+        // directory, so just lock the entire thing for the duration of this
+        // compile.
+        let lock = try!(root.open_rw(".cargo-lock", config, "build directory"));
+        let root = root.into_path_unlocked();
+
+        Ok(Layout {
             deps: root.join("deps"),
             native: root.join("native"),
             build: root.join("build"),
             fingerprint: root.join(".fingerprint"),
             examples: root.join("examples"),
             root: root,
-        }
+            _lock: lock,
+        })
     }
 
     pub fn prepare(&mut self) -> io::Result<()> {

src/cargo/ops/cargo_rustc/mod.rs

@@ -3,12 +3,12 @@ use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fs;
 use std::io::prelude::*;
-use std::path::{self, PathBuf, Path};
+use std::path::{self, PathBuf};
 use std::sync::Arc;
 
 use core::{Package, PackageId, PackageSet, Target, Resolve};
 use core::{Profile, Profiles};
-use util::{self, CargoResult, human, Filesystem};
+use util::{self, CargoResult, human};
 use util::{Config, internal, ChainError, profile, join_paths};
 
 use self::job::{Job, Work};
@@ -80,17 +80,13 @@ pub fn compile_targets<'a, 'cfg: 'a>(pkg_targets: &'a PackagesToBuild<'a>,
 
     let dest = if build_config.release {"release"} else {"debug"};
     let root = try!(packages.get(resolve.root()));
-    let host_layout = Layout::new(config, root, None, &dest);
-    let target_layout = build_config.requested_target.as_ref().map(|target| {
-        layout::Layout::new(config, root, Some(&target), &dest)
-    });
-
-    // For now we don't do any more finer-grained locking on the artifact
-    // directory, so just lock the entire thing for the duration of this
-    // compile.
-    let fs = Filesystem::new(host_layout.root().to_path_buf());
-    let path = Path::new(".cargo-lock");
-    let _lock = try!(fs.open_rw(path, config, "build directory"));
+    let host_layout = try!(Layout::new(config, root, None, &dest));
+    let target_layout = match build_config.requested_target.as_ref() {
+        Some(target) => {
+            Some(try!(layout::Layout::new(config, root, Some(&target), &dest)))
+        }
+        None => None,
+    };
 
     let mut cx = try!(Context::new(resolve, packages, config,
                                    host_layout, target_layout,

src/cargo/ops/registry.rs

@@ -1,6 +1,6 @@
 use std::collections::HashMap;
 use std::env;
-use std::fs;
+use std::fs::{self, File};
 use std::io::prelude::*;
 use std::iter::repeat;
 use std::path::{Path, PathBuf};
@@ -51,7 +51,7 @@ pub fn publish(manifest_path: &Path,
 
     // Upload said tarball to the specified destination
     try!(config.shell().status("Uploading", pkg.package_id().to_string()));
-    try!(transmit(&pkg, &tarball, &mut registry));
+    try!(transmit(&pkg, tarball.file(), &mut registry));
 
     Ok(())
 }
@@ -74,7 +74,7 @@ fn verify_dependencies(pkg: &Package, registry_src: &SourceId)
     Ok(())
 }
 
-fn transmit(pkg: &Package, tarball: &Path, registry: &mut Registry)
+fn transmit(pkg: &Package, tarball: &File, registry: &mut Registry)
             -> CargoResult<()> {
     let deps = pkg.dependencies().iter().map(|dep| {
         NewCrateDependency {

src/cargo/util/config.rs

@@ -30,7 +30,7 @@ pub struct Config {
     cwd: PathBuf,
     rustc: PathBuf,
     rustdoc: PathBuf,
-    target_dir: RefCell<Option<PathBuf>>,
+    target_dir: RefCell<Option<Filesystem>>,
 }
 
 impl Config {
@@ -110,14 +110,14 @@ impl Config {
 
     pub fn cwd(&self) -> &Path { &self.cwd }
 
-    pub fn target_dir(&self, pkg: &Package) -> PathBuf {
+    pub fn target_dir(&self, pkg: &Package) -> Filesystem {
         self.target_dir.borrow().clone().unwrap_or_else(|| {
-            pkg.root().join("target")
+            Filesystem::new(pkg.root().join("target"))
         })
     }
 
-    pub fn set_target_dir(&self, path: &Path) {
-        *self.target_dir.borrow_mut() = Some(path.to_owned());
+    pub fn set_target_dir(&self, path: Filesystem) {
+        *self.target_dir.borrow_mut() = Some(path);
     }
 
     fn get(&self, key: &str) -> CargoResult<Option<ConfigValue>> {
@@ -327,9 +327,9 @@ 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(self.cwd.join(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")) {
-            *self.target_dir.borrow_mut() = Some(val.val);
+            *self.target_dir.borrow_mut() = Some(Filesystem::new(val.val));
         }
         Ok(())
     }

src/cargo/util/flock.rs

@@ -1,7 +1,7 @@
 use std::fs::{self, File, OpenOptions};
 use std::io::*;
 use std::io;
-use std::path::{Path, PathBuf};
+use std::path::{Path, PathBuf, Display};
 
 use term::color::CYAN;
 use fs2::{FileExt, lock_contended_error};
@@ -102,7 +102,7 @@ impl Drop for FileLock {
 /// The `Path` of a filesystem cannot be learned unless it's done in a locked
 /// fashion, and otherwise functions on this structure are prepared to handle
 /// concurrent invocations across multiple instances of Cargo.
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub struct Filesystem {
     root: PathBuf,
 }
@@ -119,6 +119,11 @@ impl Filesystem {
         Filesystem::new(self.root.join(other))
     }
 
+    /// Like `Path::push`, pushes a new path component onto this filesystem.
+    pub fn push<T: AsRef<Path>>(&mut self, other: T) {
+        self.root.push(other);
+    }
+
     /// Consumes this filesystem and returns the underlying `PathBuf`.
     ///
     /// Note that this is a relatively dangerous operation and should be used
@@ -135,6 +140,12 @@ impl Filesystem {
         return create_dir_all(&self.root);
     }
 
+    /// Returns an adaptor that can be used to print the path of this
+    /// filesystem.
+    pub fn display(&self) -> Display {
+        self.root.display()
+    }
+
     /// Opens exclusive access to a file, returning the locked version of a
     /// file.
     ///