rustbuild: Fail the build if we build Cargo twice

src/bootstrap/compile.rs

@@ -16,6 +16,7 @@
 //! compiler. This module is also responsible for assembling the sysroot as it
 //! goes along from the output of the previous stage.
 
+use std::borrow::Cow;
 use std::env;
 use std::fs::{self, File};
 use std::io::BufReader;
@@ -996,24 +997,6 @@ fn stderr_isatty() -> bool {
 pub fn run_cargo(build: &Build, cargo: &mut Command, stamp: &Path, is_check: bool)
     -> Vec<PathBuf>
 {
-    // Instruct Cargo to give us json messages on stdout, critically leaving
-    // stderr as piped so we can get those pretty colors.
-    cargo.arg("--message-format").arg("json")
-         .stdout(Stdio::piped());
-
-    if stderr_isatty() && build.ci_env == CiEnv::None {
-        // since we pass message-format=json to cargo, we need to tell the rustc
-        // wrapper to give us colored output if necessary. This is because we
-        // only want Cargo's JSON output, not rustcs.
-        cargo.env("RUSTC_COLOR", "1");
-    }
-
-    build.verbose(&format!("running: {:?}", cargo));
-    let mut child = match cargo.spawn() {
-        Ok(child) => child,
-        Err(e) => panic!("failed to execute command: {:?}\nerror: {}", cargo, e),
-    };
-
     // `target_root_dir` looks like $dir/$target/release
     let target_root_dir = stamp.parent().unwrap();
     // `target_deps_dir` looks like $dir/$target/release/deps
@@ -1028,46 +1011,33 @@ pub fn run_cargo(build: &Build, cargo: &mut Command, stamp: &Path, is_check: boo
     // files we need to probe for later.
     let mut deps = Vec::new();
     let mut toplevel = Vec::new();
-    let stdout = BufReader::new(child.stdout.take().unwrap());
-    for line in stdout.lines() {
-        let line = t!(line);
-        let json: serde_json::Value = if line.starts_with("{") {
-            t!(serde_json::from_str(&line))
-        } else {
-            // If this was informational, just print it out and continue
-            println!("{}", line);
-            continue
+    let ok = stream_cargo(build, cargo, &mut |msg| {
+        let filenames = match msg {
+            CargoMessage::CompilerArtifact { filenames, .. } => filenames,
+            _ => return,
         };
-        if json["reason"].as_str() != Some("compiler-artifact") {
-            if build.config.rustc_error_format.as_ref().map_or(false, |e| e == "json") {
-                // most likely not a cargo message, so let's send it out as well
-                println!("{}", line);
-            }
-            continue
-        }
-        for filename in json["filenames"].as_array().unwrap() {
-            let filename = filename.as_str().unwrap();
+        for filename in filenames {
             // Skip files like executables
             if !filename.ends_with(".rlib") &&
                !filename.ends_with(".lib") &&
                !is_dylib(&filename) &&
                !(is_check && filename.ends_with(".rmeta")) {
-                continue
+                return;
             }
 
-            let filename = Path::new(filename);
+            let filename = Path::new(&*filename);
 
             // If this was an output file in the "host dir" we don't actually
             // worry about it, it's not relevant for us.
             if filename.starts_with(&host_root_dir) {
-                continue;
+                return;
             }
 
             // If this was output in the `deps` dir then this is a precise file
             // name (hash included) so we start tracking it.
             if filename.starts_with(&target_deps_dir) {
                 deps.push(filename.to_path_buf());
-                continue;
+                return;
             }
 
             // Otherwise this was a "top level artifact" which right now doesn't
@@ -1088,15 +1058,10 @@ pub fn run_cargo(build: &Build, cargo: &mut Command, stamp: &Path, is_check: boo
 
             toplevel.push((file_stem, extension, expected_len));
         }
-    }
+    });
 
-    // Make sure Cargo actually succeeded after we read all of its stdout.
-    let status = t!(child.wait());
-    if !status.success() {
-        panic!("command did not execute successfully: {:?}\n\
-                expected success, got: {}",
-               cargo,
-               status);
+    if !ok {
+        panic!("cargo must succeed");
     }
 
     // Ok now we need to actually find all the files listed in `toplevel`. We've
@@ -1167,3 +1132,63 @@ pub fn run_cargo(build: &Build, cargo: &mut Command, stamp: &Path, is_check: boo
     t!(t!(File::create(stamp)).write_all(&new_contents));
     deps
 }
+
+pub fn stream_cargo(
+    build: &Build,
+    cargo: &mut Command,
+    cb: &mut FnMut(CargoMessage),
+) -> bool {
+    // Instruct Cargo to give us json messages on stdout, critically leaving
+    // stderr as piped so we can get those pretty colors.
+    cargo.arg("--message-format").arg("json")
+         .stdout(Stdio::piped());
+
+    if stderr_isatty() && build.ci_env == CiEnv::None {
+        // since we pass message-format=json to cargo, we need to tell the rustc
+        // wrapper to give us colored output if necessary. This is because we
+        // only want Cargo's JSON output, not rustcs.
+        cargo.env("RUSTC_COLOR", "1");
+    }
+
+    build.verbose(&format!("running: {:?}", cargo));
+    let mut child = match cargo.spawn() {
+        Ok(child) => child,
+        Err(e) => panic!("failed to execute command: {:?}\nerror: {}", cargo, e),
+    };
+
+    // Spawn Cargo slurping up its JSON output. We'll start building up the
+    // `deps` array of all files it generated along with a `toplevel` array of
+    // files we need to probe for later.
+    let stdout = BufReader::new(child.stdout.take().unwrap());
+    for line in stdout.lines() {
+        let line = t!(line);
+        match serde_json::from_str::<CargoMessage>(&line) {
+            Ok(msg) => cb(msg),
+            // If this was informational, just print it out and continue
+            Err(_) => println!("{}", line)
+        }
+    }
+
+    // Make sure Cargo actually succeeded after we read all of its stdout.
+    let status = t!(child.wait());
+    if !status.success() {
+        println!("command did not execute successfully: {:?}\n\
+                  expected success, got: {}",
+                 cargo,
+                 status);
+    }
+    status.success()
+}
+
+#[derive(Deserialize)]
+#[serde(tag = "reason", rename_all = "kebab-case")]
+pub enum CargoMessage<'a> {
+    CompilerArtifact {
+        package_id: Cow<'a, str>,
+        features: Vec<Cow<'a, str>>,
+        filenames: Vec<Cow<'a, str>>,
+    },
+    BuildScriptExecuted {
+        package_id: Cow<'a, str>,
+    }
+}

src/bootstrap/lib.rs

@@ -254,6 +254,10 @@ pub struct Build {
     ci_env: CiEnv,
     delayed_failures: RefCell<Vec<String>>,
     prerelease_version: Cell<Option<u32>>,
+    tool_artifacts: RefCell<HashMap<
+        Interned<String>,
+        HashMap<String, (&'static str, PathBuf, Vec<String>)>
+    >>,
 }
 
 #[derive(Debug)]
@@ -353,6 +357,7 @@ impl Build {
             ci_env: CiEnv::current(),
             delayed_failures: RefCell::new(Vec::new()),
             prerelease_version: Cell::new(None),
+            tool_artifacts: Default::default(),
         }
     }
 

src/bootstrap/tool.rs

@@ -117,7 +117,80 @@ impl Step for ToolBuild {
 
         let _folder = build.fold_output(|| format!("stage{}-{}", compiler.stage, tool));
         println!("Building stage{} tool {} ({})", compiler.stage, tool, target);
-        let is_expected = build.try_run(&mut cargo);
+        let mut duplicates = Vec::new();
+        let is_expected = compile::stream_cargo(build, &mut cargo, &mut |msg| {
+            // Only care about big things like the RLS/Cargo for now
+            if tool != "rls" && tool != "cargo" {
+                return
+            }
+            let (id, features, filenames) = match msg {
+                compile::CargoMessage::CompilerArtifact {
+                    package_id,
+                    features,
+                    filenames
+                } => {
+                    (package_id, features, filenames)
+                }
+                _ => return,
+            };
+            let features = features.iter().map(|s| s.to_string()).collect::<Vec<_>>();
+
+            for path in filenames {
+                let val = (tool, PathBuf::from(&*path), features.clone());
+                // we're only interested in deduplicating rlibs for now
+                if val.1.extension().and_then(|s| s.to_str()) != Some("rlib") {
+                    continue
+                }
+
+                // Don't worry about libs that turn out to be host dependencies
+                // or build scripts, we only care about target dependencies that
+                // are in `deps`.
+                if let Some(maybe_target) = val.1
+                    .parent()                   // chop off file name
+                    .and_then(|p| p.parent())   // chop off `deps`
+                    .and_then(|p| p.parent())   // chop off `release`
+                    .and_then(|p| p.file_name())
+                    .and_then(|p| p.to_str())
+                {
+                    if maybe_target != &*target {
+                        continue
+                    }
+                }
+
+                let mut artifacts = build.tool_artifacts.borrow_mut();
+                let prev_artifacts = artifacts
+                    .entry(target)
+                    .or_insert_with(Default::default);
+                if let Some(prev) = prev_artifacts.get(&*id) {
+                    if prev.1 != val.1 {
+                        duplicates.push((
+                            id.to_string(),
+                            val,
+                            prev.clone(),
+                        ));
+                    }
+                    return
+                }
+                prev_artifacts.insert(id.to_string(), val);
+            }
+        });
+
+        if is_expected && duplicates.len() != 0 {
+            println!("duplicate artfacts found when compiling a tool, this \
+                      typically means that something was recompiled because \
+                      a transitive dependency has different features activated \
+                      than in a previous build:\n");
+            for (id, cur, prev) in duplicates {
+                println!("  {}", id);
+                println!("    `{}` enabled features {:?} at {:?}",
+                         cur.0, cur.2, cur.1);
+                println!("    `{}` enabled features {:?} at {:?}",
+                         prev.0, prev.2, prev.1);
+            }
+            println!("");
+            panic!("tools should not compile multiple copies of the same crate");
+        }
+
         build.save_toolstate(tool, if is_expected {
             ToolState::TestFail
         } else {

src/librustc/Cargo.toml

@@ -56,3 +56,4 @@ byteorder = { version = "1.1", features = ["i128"]}
 #        later crate stop compiling. If you can remove this and everything
 #        compiles, then please feel free to do so!
 flate2 = "1.0"
+tempdir = "0.3"

src/tools/unstable-book-gen/Cargo.toml

@@ -7,3 +7,7 @@ license = "MIT/Apache-2.0"
 
 [dependencies]
 tidy = { path = "../tidy" }
+
+# not actually needed but required for now to unify the feature selection of
+# `num-traits` between this and `rustbook`
+num-traits = "0.2"