diff options
| author | Alex Crichton <alex@alexcrichton.com> | 2018-03-15 10:58:02 -0700 |
|---|---|---|
| committer | Alex Crichton <alex@alexcrichton.com> | 2018-03-26 13:07:12 -0700 |
| commit | faebcc108768210cda97a74f0407cd7b3c3348c9 (patch) | |
| tree | b328cf4f781391a0d35d1f06c7e719bb0a3235a4 /src/bootstrap | |
| parent | ab8b961677ac5c74762dcea955aa0ff4d7fe4915 (diff) | |
| download | rust-faebcc108768210cda97a74f0407cd7b3c3348c9.tar.gz rust-faebcc108768210cda97a74f0407cd7b3c3348c9.zip | |
rustbuild: Fail the build if we build Cargo twice
This commit updates the `ToolBuild` step to stream Cargo's JSON messages, parse them, and record all libraries built. If we build anything twice (aka Cargo) it'll most likely happen due to dependencies being recompiled which is caught by this check.
Diffstat (limited to 'src/bootstrap')
| -rw-r--r-- | src/bootstrap/compile.rs | 121 | ||||
| -rw-r--r-- | src/bootstrap/lib.rs | 5 | ||||
| -rw-r--r-- | src/bootstrap/tool.rs | 75 |
3 files changed, 152 insertions, 49 deletions
diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index a1318086af7..9f33935b6e9 100644 --- a/src/bootstrap/compile.rs +++ b/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>, + } +} diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index b2c8ac24d72..833faf3618d 100644 --- a/src/bootstrap/lib.rs +++ b/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(), } } diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index 669308c8dd0..2bb46cc5171 100644 --- a/src/bootstrap/tool.rs +++ b/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 { |
