House all native build output in a per-package dir

In order to ensure there are no stale artifacts as part of a build, this commit
houses all output of native build commands in their own directories. Each
directory is on a per-package basis, and the output is preserved if the package
is fresh or discarded if it is not.

This does not remove the DEPS_DIR environment variable, it just wires it to the
same value as OUT_DIR.

src/cargo/ops/cargo_rustc/context.rs

@@ -1,5 +1,6 @@
-use std::str;
 use std::collections::{HashMap, HashSet};
+use std::os;
+use std::str;
 
 use core::{Package, PackageId, PackageSet, Resolve, Target};
 use util;
@@ -26,6 +27,7 @@ pub struct Context<'a, 'b> {
     host_dylib: (String, String),
     package_set: &'a PackageSet,
     target_dylib: (String, String),
+    target_exe: String,
     requirements: HashMap<(&'a PackageId, &'a str), PlatformRequirement>,
 }
 
@@ -34,11 +36,13 @@ impl<'a, 'b> Context<'a, 'b> {
                config: &'b mut Config<'b>,
                host: Layout, target: Option<Layout>)
                -> CargoResult<Context<'a, 'b>> {
-        let target_dylib = try!(Context::dylib_parts(config.target()));
+        let (target_dylib, target_exe) =
+                try!(Context::filename_parts(config.target()));
         let host_dylib = if config.target().is_none() {
             target_dylib.clone()
         } else {
-            try!(Context::dylib_parts(None))
+            let (dylib, _) = try!(Context::filename_parts(None));
+            dylib
         };
         Ok(Context {
             rustc_version: try!(Context::rustc_version()),
@@ -50,6 +54,7 @@ impl<'a, 'b> Context<'a, 'b> {
             package_set: deps,
             config: config,
             target_dylib: target_dylib,
+            target_exe: target_exe,
             host_dylib: host_dylib,
             requirements: HashMap::new(),
         })
@@ -63,8 +68,9 @@ impl<'a, 'b> Context<'a, 'b> {
     }
 
     /// Run `rustc` to discover the dylib prefix/suffix for the target
-    /// specified.
+    /// specified as well as the exe suffix
-    fn dylib_parts(target: Option<&str>) -> CargoResult<(String, String)> {
+    fn filename_parts(target: Option<&str>)
+                      -> CargoResult<((String, String), String)> {
         let process = util::process("rustc")
                            .arg("-")
                            .arg("--crate-name").arg("-")
@@ -77,10 +83,17 @@ impl<'a, 'b> Context<'a, 'b> {
         let output = try!(process.exec_with_output());
 
         let output = str::from_utf8(output.output.as_slice()).unwrap();
-        let parts: Vec<&str> = output.trim().split('-').collect();
+        let dylib_parts: Vec<&str> = output.trim().split('-').collect();
-        assert!(parts.len() == 2, "rustc --print-file-name output has changed");
+        assert!(dylib_parts.len() == 2,
+                "rustc --print-file-name output has changed");
+        let exe_suffix = match target {
+            None => os::consts::EXE_SUFFIX,
+            Some(s) if s.contains("win32") || s.contains("windows") => ".exe",
+            Some(_) => "",
+        };
 
-        Ok((parts[0].to_string(), parts[1].to_string()))
+        Ok(((dylib_parts[0].to_string(), dylib_parts[1].to_string()),
+            exe_suffix.to_string()))
     }
 
     /// Prepare this context, ensuring that all filesystem directories are in
@@ -170,7 +183,7 @@ impl<'a, 'b> Context<'a, 'b> {
             ret.push(format!("lib{}.rlib", stem));
         }
         if target.is_bin() {
-            ret.push(stem.to_string());
+            ret.push(format!("{}{}", stem, self.target_exe));
         }
         assert!(ret.len() > 0);
         return ret;
@@ -183,11 +196,7 @@ impl<'a, 'b> Context<'a, 'b> {
             None => return vec!(),
             Some(deps) => deps,
         };
-        deps.map(|pkg_id| {
+        deps.map(|pkg_id| self.get_package(pkg_id))
-            self.package_set.iter()
-                .find(|pkg| pkg_id == pkg.get_package_id())
-                .expect("Should have found package")
-        })
         .filter_map(|pkg| {
             pkg.get_targets().iter().find(|&t| self.is_relevant_target(t))
                .map(|t| (pkg, t))
@@ -195,6 +204,13 @@ impl<'a, 'b> Context<'a, 'b> {
         .collect()
     }
 
+    /// Gets a package for the given package id.
+    pub fn get_package(&self, id: &PackageId) -> &'a Package {
+        self.package_set.iter()
+            .find(|pkg| id == pkg.get_package_id())
+            .expect("Should have found package")
+    }
+
     pub fn is_relevant_target(&self, target: &Target) -> bool {
         target.is_lib() && match self.env {
             "test" => target.get_profile().is_compile(),

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -48,9 +48,12 @@ pub fn prepare(cx: &mut Context, pkg: &Package,
     let mut pairs = Vec::new();
     pairs.push((old_fingerprint_loc, new_fingerprint_loc));
     for &target in targets.iter() {
+        let layout = cx.layout(target.get_profile().is_plugin());
+        if pkg.get_manifest().get_build().len() > 0 {
+            pairs.push((layout.old_native(pkg), layout.native(pkg)));
+        }
         for filename in cx.target_filenames(target).iter() {
             let filename = filename.as_slice();
-            let layout = cx.layout(target.get_profile().is_plugin());
             pairs.push((layout.old_root().join(filename),
                         layout.root().join(filename)));
         }

src/cargo/ops/cargo_rustc/layout.rs

@@ -10,6 +10,18 @@
 //!         # This is the root directory for all output of *dependencies*
 //!         deps/
 //!
+//!         # This is the location at which the output of all custom build
+//!         # commands are rooted
+//!         native/
+//!
+//!             # Each package gets its own directory for where its output is
+//!             # placed. We can't track exactly what's getting put in here, so
+//!             # we just assume that all relevant output is in these
+//!             # directories.
+//!             $pkg1/
+//!             $pkg2/
+//!             $pkg3/
+//!
 //!         # This is a temporary directory as part of the build process. When a
 //!         # build starts, it initially moves the old `deps` directory to this
 //!         # location. This is done to ensure that there are no stale artifacts
@@ -23,16 +35,24 @@
 //!         # Similar to old-deps, this is where all of the output under
 //!         # `target/` is moved at the start of a build.
 //!         old-root/
+//!
+//!         # Same as the two above old directories
+//!         old-native/
 
 use std::io;
 use std::io::{fs, IoResult};
 
+use core::Package;
+use util::hex::short_hash;
+
 pub struct Layout {
     root: Path,
     deps: Path,
+    native: Path,
 
     old_deps: Path,
     old_root: Path,
+    old_native: Path,
 }
 
 pub struct LayoutProxy<'a> {
@@ -44,8 +64,10 @@ impl Layout {
     pub fn new(root: Path) -> Layout {
         Layout {
             deps: root.join("deps"),
+            native: root.join("native"),
             old_deps: root.join("old-deps"),
             old_root: root.join("old-root"),
+            old_native: root.join("old-native"),
             root: root,
         }
     }
@@ -61,11 +83,18 @@ impl Layout {
         if self.old_root.exists() {
             try!(fs::rmdir_recursive(&self.old_root));
         }
+        if self.old_native.exists() {
+            try!(fs::rmdir_recursive(&self.old_root));
+        }
         if self.deps.exists() {
             try!(fs::rename(&self.deps, &self.old_deps));
         }
+        if self.native.exists() {
+            try!(fs::rename(&self.native, &self.old_native));
+        }
 
         try!(fs::mkdir(&self.deps, io::UserRWX));
+        try!(fs::mkdir(&self.native, io::UserRWX));
         try!(fs::mkdir(&self.old_root, io::UserRWX));
 
         for file in try!(fs::readdir(&self.root)).iter() {
@@ -79,14 +108,26 @@ impl Layout {
 
     pub fn dest<'a>(&'a self) -> &'a Path { &self.root }
     pub fn deps<'a>(&'a self) -> &'a Path { &self.deps }
+    pub fn native(&self, package: &Package) -> Path {
+        self.native.join(self.native_name(package))
+    }
+
     pub fn old_dest<'a>(&'a self) -> &'a Path { &self.old_root }
     pub fn old_deps<'a>(&'a self) -> &'a Path { &self.old_deps }
+    pub fn old_native(&self, package: &Package) -> Path {
+        self.old_native.join(self.native_name(package))
+    }
+
+    fn native_name(&self, pkg: &Package) -> String {
+        format!("{}-{}", pkg.get_name(), short_hash(pkg.get_package_id()))
+    }
 }
 
 impl Drop for Layout {
     fn drop(&mut self) {
         let _ = fs::rmdir_recursive(&self.old_deps);
         let _ = fs::rmdir_recursive(&self.old_root);
+        let _ = fs::rmdir_recursive(&self.old_native);
     }
 }
 
@@ -103,7 +144,13 @@ impl<'a> LayoutProxy<'a> {
     }
     pub fn deps(&self) -> &'a Path { self.root.deps() }
 
+    pub fn native(&self, pkg: &Package) -> Path { self.root.native(pkg) }
+
     pub fn old_root(&self) -> &'a Path {
         if self.primary {self.root.old_dest()} else {self.root.old_deps()}
     }
+
+    pub fn old_native(&self, pkg: &Package) -> Path {
+        self.root.old_native(pkg)
+    }
 }

src/cargo/ops/cargo_rustc/mod.rs

@@ -1,7 +1,10 @@
-use core::{Package, PackageSet, Target, Resolve};
+use std::io::{fs, UserRWX};
+use std::collections::HashSet;
+
+use core::{Package, PackageId, PackageSet, Target, Resolve};
 use util;
 use util::{CargoResult, ProcessBuilder, CargoError, human};
-use util::{Config, Freshness};
+use util::{Config, Freshness, internal, ChainError};
 
 use self::job::Job;
 use self::job_queue::JobQueue;
@@ -94,7 +97,7 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package,
     // TODO: Should this be on the target or the package?
     let mut build_cmds = Vec::new();
     for build_cmd in pkg.get_manifest().get_build().iter() {
-        build_cmds.push(compile_custom(pkg, build_cmd.as_slice(), cx));
+        build_cmds.push(try!(compile_custom(pkg, build_cmd.as_slice(), cx)));
     }
 
     // After the custom command has run, execute rustc for all targets of our
@@ -135,26 +138,32 @@ fn compile<'a, 'b>(targets: &[&'a Target], pkg: &'a Package,
 }
 
 fn compile_custom(pkg: &Package, cmd: &str,
-                  cx: &Context) -> Job {
+                  cx: &Context) -> CargoResult<Job> {
     // TODO: this needs to be smarter about splitting
     let mut cmd = cmd.split(' ');
     // TODO: this shouldn't explicitly pass `false` for dest/deps_dir, we may
     //       be building a C lib for a plugin
     let layout = cx.layout(false);
+    let output = layout.native(pkg);
+    if !output.exists() {
+        try!(fs::mkdir(&output, UserRWX).chain_error(|| {
+            internal("failed to create output directory for build command")
+        }));
+    }
     let mut p = util::process(cmd.next().unwrap())
                      .cwd(pkg.get_root())
-                     .env("OUT_DIR", Some(layout.root().as_str()
+                     .env("OUT_DIR", Some(output.as_str()
                                                 .expect("non-UTF8 dest path")))
-                     .env("DEPS_DIR", Some(layout.deps().as_str()
+                     .env("DEPS_DIR", Some(output.as_str()
-                                                 .expect("non-UTF8 deps path")))
+                                                 .expect("non-UTF8 dest path")))
                      .env("TARGET", cx.config.target());
     for arg in cmd {
         p = p.arg(arg);
     }
-    Job::new(proc() {
+    Ok(Job::new(proc() {
         try!(p.exec_with_output().map(|_| ()).map_err(|e| e.mark_human()));
         Ok(Vec::new())
-    })
+    }))
 }
 
 fn rustc(package: &Package, target: &Target,
@@ -296,6 +305,10 @@ fn build_deps_args(dst: &mut Args, package: &Package, cx: &Context,
     dst.push("-L".to_string());
     dst.push(layout.deps().display().to_string());
 
+    // Traverse the entire dependency graph looking for -L paths to pass for
+    // native dependencies.
+    push_native_dirs(dst, &layout, package, cx, &mut HashSet::new());
+
     for &(_, target) in cx.dep_targets(package).iter() {
         let layout = cx.layout(target.get_profile().is_plugin());
         for filename in cx.target_filenames(target).iter() {
@@ -306,4 +319,25 @@ fn build_deps_args(dst: &mut Args, package: &Package, cx: &Context,
                      filename));
         }
     }
+
+    fn push_native_dirs(dst: &mut Args, layout: &layout::LayoutProxy,
+                        pkg: &Package, cx: &Context,
+                        visited: &mut HashSet<PackageId>) {
+        if !visited.insert(pkg.get_package_id().clone()) { return }
+
+        if pkg.get_manifest().get_build().len() > 0 {
+            dst.push("-L".to_string());
+            dst.push(layout.native(pkg).display().to_string());
+        }
+
+        match cx.resolve.deps(pkg.get_package_id()) {
+            Some(mut pkgids) => {
+                for dep_id in pkgids {
+                    let dep = cx.get_package(dep_id);
+                    push_native_dirs(dst, layout, dep, cx, visited);
+                }
+            }
+            None => {}
+        }
+    }
 }

tests/test_cargo_compile.rs

@@ -717,12 +717,12 @@ test!(custom_build_env_vars {
         .file("src/foo.rs", format!(r#"
             use std::os;
             fn main() {{
-                assert_eq!(os::getenv("OUT_DIR").unwrap(), r"{}".to_string());
+                let out = os::getenv("OUT_DIR").unwrap();
-                assert_eq!(os::getenv("DEPS_DIR").unwrap(), r"{}".to_string());
+                assert!(out.as_slice().starts_with(r"{}"));
+                assert!(Path::new(out).is_dir());
             }}
         "#,
-        p.root().join("target").display(),
+        p.root().join("target").join("native").join("foo-").display()));
-        p.root().join("target").join("deps").display()));
     assert_that(build.cargo_process("cargo-build"), execs().with_status(0));
 
 
@@ -762,12 +762,11 @@ test!(custom_build_in_dependency {
         .file("src/foo.rs", format!(r#"
             use std::os;
             fn main() {{
-                assert_eq!(os::getenv("OUT_DIR").unwrap(), r"{}".to_string());
+                assert!(os::getenv("OUT_DIR").unwrap().as_slice()
-                assert_eq!(os::getenv("DEPS_DIR").unwrap(), r"{}".to_string());
+                           .starts_with(r"{}"));
             }}
         "#,
-        p.root().join("target/deps").display(),
+        p.root().join("target/native/bar-").display()));
-        p.root().join("target/deps").display()));
     assert_that(build.cargo_process("cargo-build"), execs().with_status(0));