From fe80a308b9eedc563f222d059d6af645b98ee998 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 5 Oct 2018 12:47:17 +0200 Subject: [PATCH 01/15] Add `dep_files_for_unit` API --- src/cargo/core/compiler/output_depinfo.rs | 56 +++++++++++++++-------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index df4f3d417a5..b4ee7c12dcb 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -7,8 +7,15 @@ use super::{fingerprint, Context, Unit}; use util::paths; use util::{internal, CargoResult}; -fn render_filename>(path: P, basedir: Option<&str>) -> CargoResult { - let path = path.as_ref(); +fn dep_info_basedir(cx: &Context<'_, '_>) -> CargoResult> { + cx.bcx + .config + .get_string("build.dep-info-basedir") + .map(|option| option.map(|o| o.val)) +} + +fn render_filename(path: impl AsRef, basedir: Option>) -> CargoResult { + let (path, basedir) = (path.as_ref(), basedir.as_ref()); let relpath = match basedir { None => path, Some(base) => match path.strip_prefix(base) { @@ -69,33 +76,44 @@ fn add_deps_for_unit<'a, 'b>( Ok(()) } -pub fn output_depinfo<'a, 'b>(cx: &mut Context<'a, 'b>, unit: &Unit<'a>) -> CargoResult<()> { - let bcx = cx.bcx; +/// Returns a list of file dependencies for a given compilation `Unit`. +/// +/// Inner `Result` type can fail if the actual dep-info generation failed; the +/// outer one is supposed to catch other, internal Cargo errors. +pub fn dep_files_for_unit<'a, 'b>( + cx: &mut Context<'a, 'b>, + unit: &Unit<'a>, +) -> CargoResult, ()>> { + let basedir = dep_info_basedir(cx)?; + let mut deps = BTreeSet::new(); let mut visited = HashSet::new(); - let success = add_deps_for_unit(&mut deps, cx, unit, &mut visited).is_ok(); - let basedir_string; - let basedir = match bcx.config.get_path("build.dep-info-basedir")? { - Some(value) => { - basedir_string = value - .val - .as_os_str() - .to_str() - .ok_or_else(|| internal("build.dep-info-basedir path not utf-8"))? - .to_string(); - Some(basedir_string.as_str()) - } - None => None, + Ok(match add_deps_for_unit(&mut deps, cx, unit, &mut visited) { + Ok(_) => Ok(deps + .iter() + .map(|f| render_filename(f, basedir.as_ref())) + .collect::>>()?), + Err(_) => Err(()), + }) +} + +pub fn output_depinfo<'a, 'b>(cx: &mut Context<'a, 'b>, unit: &Unit<'a>) -> CargoResult<()> { + let basedir = dep_info_basedir(cx)?; + + let (success, deps) = match dep_files_for_unit(cx, unit)? { + Ok(deps) => (true, deps), + _ => (false, vec![]) }; + let deps = deps.iter() - .map(|f| render_filename(f, basedir)) + .map(|f| render_filename(f, basedir.as_ref())) .collect::>>()?; for output in cx.outputs(unit)?.iter() { if let Some(ref link_dst) = output.hardlink { let output_path = link_dst.with_extension("d"); if success { - let target_fn = render_filename(link_dst, basedir)?; + let target_fn = render_filename(link_dst, basedir.as_ref())?; // If nothing changed don't recreate the file which could alter // its mtime From 32719525b4829634a9f57ea7086ca1ea03198dd6 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 12 Oct 2018 11:43:52 +0200 Subject: [PATCH 02/15] Add build plan inputs This only populates `input` JSON value when appropriate .d. files exist already (e.g. after a succesful `cargo check` run). --- src/cargo/core/compiler/build_plan.rs | 4 ++ src/cargo/core/compiler/custom_build.rs | 5 ++- src/cargo/core/compiler/job_queue.rs | 9 ++-- src/cargo/core/compiler/mod.rs | 5 ++- tests/testsuite/build_plan.rs | 56 +++++++++++++++++++++++++ tests/testsuite/metabuild.rs | 5 +++ 6 files changed, 77 insertions(+), 7 deletions(-) diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index 5813e29906b..8d6409b2cdd 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -23,6 +23,7 @@ struct Invocation { target_kind: TargetKind, kind: Kind, deps: Vec, + inputs: Vec, outputs: Vec, links: BTreeMap, program: String, @@ -52,6 +53,7 @@ impl Invocation { kind: unit.kind, target_kind: unit.target.kind().clone(), deps, + inputs: Vec::new(), outputs: Vec::new(), links: BTreeMap::new(), program: String::new(), @@ -122,6 +124,7 @@ impl BuildPlan { &mut self, invocation_name: &str, cmd: &ProcessBuilder, + inputs: &[String], outputs: &[OutputFile], ) -> CargoResult<()> { let id = self.invocation_map[invocation_name]; @@ -131,6 +134,7 @@ impl BuildPlan { .ok_or_else(|| internal(format!("couldn't find invocation for {}", invocation_name)))?; invocation.update_cmd(cmd)?; + invocation.inputs.extend(inputs.iter().cloned()); for output in outputs.iter() { invocation.add_output(&output.path, &output.hardlink); } diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index cb4a4537ce6..8e41fdb3e47 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -322,7 +322,10 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes // And now finally, run the build command itself! if build_plan { - state.build_plan(invocation_name, cmd.clone(), Arc::new(Vec::new())); + // NOTE: It's impossible to accurately detect file inputs/outputs for + // the *execution* of an arbitrary build script and so we pass empty file lists here. + let (inputs, outputs) = (vec![], Arc::default()); + state.build_plan(invocation_name, cmd.clone(), inputs, outputs); } else { state.running(&cmd); let output = if extra_verbose { diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 806412bcda0..4e495285fe4 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -86,7 +86,7 @@ pub struct JobState<'a> { enum Message<'a> { Run(String), - BuildPlanMsg(String, ProcessBuilder, Arc>), + BuildPlanMsg(String, ProcessBuilder, Vec, Arc>), Stdout(String), Stderr(String), FixDiagnostic(diagnostic_server::Message), @@ -103,10 +103,11 @@ impl<'a> JobState<'a> { &self, module_name: String, cmd: ProcessBuilder, + inputs: Vec, filenames: Arc>, ) { let _ = self.tx - .send(Message::BuildPlanMsg(module_name, cmd, filenames)); + .send(Message::BuildPlanMsg(module_name, cmd, inputs, filenames)); } pub fn capture_output( @@ -290,8 +291,8 @@ impl<'a> JobQueue<'a> { .shell() .verbose(|c| c.status("Running", &cmd))?; } - Message::BuildPlanMsg(module_name, cmd, filenames) => { - plan.update(&module_name, &cmd, &filenames)?; + Message::BuildPlanMsg(module_name, cmd, inputs, filenames) => { + plan.update(&module_name, &cmd, &inputs, &filenames)?; } Message::Stdout(out) => { println!("{}", out); diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index d371dc68216..476f9690375 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -21,7 +21,7 @@ use self::build_plan::BuildPlan; use self::job::{Job, Work}; use self::job_queue::JobQueue; -use self::output_depinfo::output_depinfo; +use self::output_depinfo::{dep_files_for_unit, output_depinfo}; pub use self::build_context::{BuildContext, FileFlavor, TargetConfig, TargetInfo}; pub use self::build_config::{BuildConfig, CompileMode, MessageFormat}; @@ -201,6 +201,7 @@ fn rustc<'a, 'cfg>( add_cap_lints(cx.bcx, unit, &mut rustc); + let inputs = dep_files_for_unit(cx, unit)?.unwrap_or_default(); let outputs = cx.outputs(unit)?; let root = cx.files().out_dir(unit); let kind = unit.kind; @@ -302,7 +303,7 @@ fn rustc<'a, 'cfg>( .map_err(internal_if_simple_exit_code) .chain_err(|| format!("Could not compile `{}`.", name))?; } else if build_plan { - state.build_plan(buildkey, rustc.clone(), outputs.clone()); + state.build_plan(buildkey, rustc.clone(), inputs, outputs.clone()); } else { exec.exec_and_capture_output(rustc, &package_id, &target, mode, state) .map_err(internal_if_simple_exit_code) diff --git a/tests/testsuite/build_plan.rs b/tests/testsuite/build_plan.rs index fa17a79067d..22c96a662c9 100644 --- a/tests/testsuite/build_plan.rs +++ b/tests/testsuite/build_plan.rs @@ -24,6 +24,7 @@ fn cargo_build_plan_simple() { "env": "{...}", "kind": "Host", "links": "{...}", + "inputs": [], "outputs": "{...}", "package_name": "foo", "package_version": "0.5.0", @@ -80,6 +81,7 @@ fn cargo_build_plan_single_dep() { "env": "{...}", "kind": "Host", "links": "{...}", + "inputs": [], "outputs": [ "[..]/foo/target/debug/deps/libbar-[..].rlib" ], @@ -95,6 +97,7 @@ fn cargo_build_plan_single_dep() { "env": "{...}", "kind": "Host", "links": "{...}", + "inputs": [], "outputs": [ "[..]/foo/target/debug/deps/libfoo-[..].rlib" ], @@ -142,6 +145,7 @@ fn cargo_build_plan_build_script() { "env": "{...}", "kind": "Host", "links": "{...}", + "inputs": [], "outputs": [ "[..]/foo/target/debug/build/[..]/build_script_build-[..]" ], @@ -157,6 +161,7 @@ fn cargo_build_plan_build_script() { "env": "{...}", "kind": "Host", "links": "{...}", + "inputs": [], "outputs": [], "package_name": "foo", "package_version": "0.5.0", @@ -170,6 +175,7 @@ fn cargo_build_plan_build_script() { "env": "{...}", "kind": "Host", "links": "{...}", + "inputs": [], "outputs": "{...}", "package_name": "foo", "package_version": "0.5.0", @@ -206,3 +212,53 @@ fn build_plan_with_dev_dep() { .masquerade_as_nightly_cargo() .run(); } + +#[test] +fn build_plan_with_inputs() { + let p = project() + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("data/some.txt", "o hai there") + .file("src/foo.rs", "mod module1; mod module2; fn main() {}") + .file("src/module1.rs", "mod nested;") + .file("src/module1/nested.rs", "") + .file("src/module2/mod.rs", r#" + const DATA: &str = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/data/some.txt"));"#) + .file("src/not_included_in_inputs.rs", "") + .build(); + // Currently we need to populate .d files which are later used to include "inputs" field + p.cargo("build").masquerade_as_nightly_cargo().run(); + + p.cargo("build --build-plan -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_json( + r#" + { + "inputs": [ + "[..]/foo/Cargo.toml" + ], + "invocations": [ + { + "args": "{...}", + "cwd": "[..]/cit/[..]/foo", + "deps": [], + "env": "{...}", + "kind": "Host", + "links": "{...}", + "inputs": [ + "[..]/foo/data/some.txt", + "[..]/foo/src/foo.rs", + "[..]/foo/src/module1/nested.rs", + "[..]/foo/src/module1.rs", + "[..]/foo/src/module2/mod.rs" + ], + "outputs": "{...}", + "package_name": "foo", + "package_version": "0.5.0", + "program": "rustc", + "target_kind": ["bin"] + } + ] + } + "#, + ).run(); +} diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs index c96f0c4b6b6..76112d69076 100644 --- a/tests/testsuite/metabuild.rs +++ b/tests/testsuite/metabuild.rs @@ -435,6 +435,7 @@ fn metabuild_build_plan() { "target_kind": ["lib"], "kind": "Host", "deps": [], + "inputs": [], "outputs": ["[..]/target/debug/deps/libmb-[..].rlib"], "links": {}, "program": "rustc", @@ -448,6 +449,7 @@ fn metabuild_build_plan() { "target_kind": ["lib"], "kind": "Host", "deps": [], + "inputs": [], "outputs": ["[..]/target/debug/deps/libmb_other-[..].rlib"], "links": {}, "program": "rustc", @@ -461,6 +463,7 @@ fn metabuild_build_plan() { "target_kind": ["custom-build"], "kind": "Host", "deps": [0, 1], + "inputs": [], "outputs": ["[..]/target/debug/build/foo-[..]/metabuild_foo-[..][EXE]"], "links": "{...}", "program": "rustc", @@ -474,6 +477,7 @@ fn metabuild_build_plan() { "target_kind": ["custom-build"], "kind": "Host", "deps": [2], + "inputs": [], "outputs": [], "links": {}, "program": "[..]/foo/target/debug/build/foo-[..]/metabuild-foo", @@ -487,6 +491,7 @@ fn metabuild_build_plan() { "target_kind": ["lib"], "kind": "Host", "deps": [3], + "inputs": [], "outputs": ["[..]/foo/target/debug/deps/libfoo-[..].rlib"], "links": "{...}", "program": "rustc", From b37752c7e557cf146272e309fd427b085c6cca50 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sun, 21 Oct 2018 21:21:50 +0200 Subject: [PATCH 03/15] Update file deps after build --- src/cargo/core/compiler/build_plan.rs | 15 +++++++++++++++ src/cargo/core/compiler/context/mod.rs | 11 ++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index 8d6409b2cdd..53e21a071a0 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -11,6 +11,7 @@ use std::collections::BTreeMap; use core::TargetKind; use super::{Context, Kind, Unit}; use super::context::OutputFile; +use super::output_depinfo::dep_files_for_unit; use util::{internal, CargoResult, ProcessBuilder}; use std::path::PathBuf; use serde_json; @@ -142,6 +143,20 @@ impl BuildPlan { Ok(()) } + pub fn update_file_deps<'a, 'b>(&mut self, cx: &mut Context<'a, 'b>, units: &[Unit<'a>]) -> CargoResult<()> { + for unit in units { + let id = self.invocation_map[&unit.buildkey()]; + let invocation = self.plan + .invocations + .get_mut(id) + .ok_or_else(|| internal(format!("couldn't find invocation for {}", unit.buildkey())))?; + + invocation.inputs = dep_files_for_unit(cx, unit)?.unwrap_or_default(); + } + + Ok(()) + } + pub fn set_inputs(&mut self, inputs: Vec) { self.plan.inputs = inputs; } diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 3bc78fd8b38..15621fe6e07 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -152,11 +152,6 @@ impl<'a, 'cfg> Context<'a, 'cfg> { // Now that we've figured out everything that we're going to do, do it! queue.execute(&mut self, &mut plan)?; - if build_plan { - plan.set_inputs(self.build_plan_inputs()?); - plan.output_plan(); - } - for unit in units.iter() { for output in self.outputs(unit)?.iter() { if output.flavor == FileFlavor::DebugInfo { @@ -254,6 +249,12 @@ impl<'a, 'cfg> Context<'a, 'cfg> { super::output_depinfo(&mut self, unit)?; } + if build_plan { + plan.update_file_deps(&mut self, units)?; + plan.set_inputs(self.build_plan_inputs()?); + plan.output_plan(); + } + for (&(ref pkg, _), output) in self.build_state.outputs.lock().unwrap().iter() { self.compilation .cfgs From 7a4f14b8dadfeaa0d57e3b723e94ddfd18490449 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sun, 21 Oct 2018 22:45:22 +0200 Subject: [PATCH 04/15] WIPWIP: Support detailed plan The detailed plan doesn't work fully yet - we try and mimick actual `cargo build` but we only get inputs for the targets we execute work for? --- src/bin/cargo/command_prelude.rs | 23 +++++++++--- src/cargo/core/compiler/build_config.rs | 24 +++++++++++-- src/cargo/core/compiler/build_plan.rs | 3 +- src/cargo/core/compiler/context/mod.rs | 2 +- src/cargo/core/compiler/custom_build.rs | 6 ++-- src/cargo/core/compiler/job_queue.rs | 8 ++--- src/cargo/core/compiler/mod.rs | 48 +++++++++++++++---------- 7 files changed, 80 insertions(+), 34 deletions(-) diff --git a/src/bin/cargo/command_prelude.rs b/src/bin/cargo/command_prelude.rs index c8f2fc824b0..5d847907b07 100644 --- a/src/bin/cargo/command_prelude.rs +++ b/src/bin/cargo/command_prelude.rs @@ -4,7 +4,7 @@ use std::fs; use clap::{self, SubCommand}; use cargo::CargoResult; use cargo::core::Workspace; -use cargo::core::compiler::{BuildConfig, MessageFormat}; +use cargo::core::compiler::{BuildConfig, BuildPlanMode, MessageFormat}; use cargo::ops::{CompileFilter, CompileOptions, NewOptions, Packages, VersionControl}; use cargo::sources::CRATES_IO_REGISTRY; use cargo::util::paths; @@ -134,7 +134,11 @@ pub trait AppExt: Sized { } fn arg_build_plan(self) -> Self { - self._arg(opt("build-plan", "Output the build plan in JSON")) + self._arg( + opt("build-plan", "Output the build plan in JSON") + .takes_value(true) + .min_values(0), + ) } fn arg_new_opts(self) -> Self { @@ -288,8 +292,19 @@ pub trait ArgMatchesExt { let mut build_config = BuildConfig::new(config, self.jobs()?, &self.target(), mode)?; build_config.message_format = message_format; build_config.release = self._is_present("release"); - build_config.build_plan = self._is_present("build-plan"); - if build_config.build_plan && !config.cli_unstable().unstable_options { + build_config.build_plan = match (self._is_present("build-plan"), self._value_of("build-plan")) { + (false, _) => None, + (true, Some(val)) if val.eq_ignore_ascii_case("detailed") => { + config.shell().warn(format!("REMOVEME: Detailed"))?; + Some(BuildPlanMode::Detailed) + }, + (true, _) => { + config.shell().warn(format!("REMOVEME: Commands only"))?; + Some(BuildPlanMode::CommandsOnly) + }, + }; + + if build_config.build_plan.is_some() && !config.cli_unstable().unstable_options { Err(format_err!( "`--build-plan` flag is unstable, pass `-Z unstable-options` to enable it" ))?; diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 1add81d0e58..c493b50bd0d 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -19,7 +19,7 @@ pub struct BuildConfig { /// Force cargo to do a full rebuild and treat each target as changed. pub force_rebuild: bool, /// Output a build plan to stdout instead of actually compiling. - pub build_plan: bool, + pub build_plan: Option, /// Use Cargo itself as the wrapper around rustc, only used for `cargo fix` pub cargo_as_rustc_wrapper: bool, /// Extra env vars to inject into rustc commands @@ -82,7 +82,7 @@ impl BuildConfig { mode, message_format: MessageFormat::Human, force_rebuild: false, - build_plan: false, + build_plan: None, cargo_as_rustc_wrapper: false, extra_rustc_env: Vec::new(), extra_rustc_args: Vec::new(), @@ -99,6 +99,26 @@ impl BuildConfig { } } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum BuildPlanMode { + /// Quick mode of generating build plan (contains only commands to execute) + CommandsOnly, + /// In addition to containing build commands, it also includes detailed list + /// of input files used for each compilation rule. + /// Because rustc can only emit dep-info for successful compilation, this + /// potentially has to rerun an entire build to get that information. + Detailed, +} + +impl BuildPlanMode { + pub fn is_detailed(&self) -> bool { + match self { + BuildPlanMode::Detailed => true, + _ => false, + } + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum MessageFormat { Human, diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index 53e21a071a0..2b12fa9b612 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -151,7 +151,8 @@ impl BuildPlan { .get_mut(id) .ok_or_else(|| internal(format!("couldn't find invocation for {}", unit.buildkey())))?; - invocation.inputs = dep_files_for_unit(cx, unit)?.unwrap_or_default(); + let dep_files = dep_files_for_unit(cx, unit)?.unwrap_or_default(); + invocation.inputs.extend(dep_files); } Ok(()) diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 15621fe6e07..9944c30737a 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -249,7 +249,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { super::output_depinfo(&mut self, unit)?; } - if build_plan { + if build_plan.is_some() { plan.update_file_deps(&mut self, units)?; plan.set_inputs(self.build_plan_inputs()?); plan.output_plan(); diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 8e41fdb3e47..47376e2a1c0 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -94,7 +94,7 @@ pub fn prepare<'a, 'cfg>( build_work(cx, unit)? }; - if cx.bcx.build_config.build_plan { + if cx.bcx.build_config.build_plan.map_or(false, |x| !x.is_detailed()) { Ok((work_dirty, work_fresh, Freshness::Dirty)) } else { // Now that we've prep'd our work, build the work needed to manage the @@ -296,7 +296,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes // along to this custom build command. We're also careful to augment our // dynamic library search path in case the build script depended on any // native dynamic libraries. - if !build_plan { + if build_plan.map_or(true, |x| x.is_detailed()) { let build_state = build_state.outputs.lock().unwrap(); for (name, id) in lib_deps { let key = (id.clone(), kind); @@ -321,7 +321,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes } // And now finally, run the build command itself! - if build_plan { + if build_plan.map_or(false, |x| !x.is_detailed()) { // NOTE: It's impossible to accurately detect file inputs/outputs for // the *execution* of an arbitrary build script and so we pass empty file lists here. let (inputs, outputs) = (vec![], Arc::default()); diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 4e495285fe4..a05a24a8825 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -21,7 +21,7 @@ use util::{Progress, ProgressStyle}; use util::diagnostic_server::{self, DiagnosticPrinter}; use super::job::Job; -use super::{BuildContext, BuildPlan, CompileMode, Context, Kind, Unit}; +use super::{BuildContext, BuildPlan, BuildPlanMode, CompileMode, Context, Kind, Unit}; use super::context::OutputFile; /// A management structure of the entire dependency graph to compile. @@ -370,7 +370,7 @@ impl<'a> JobQueue<'a> { "{} [{}] target(s) in {}", build_type, opt_type, time_elapsed ); - if !build_plan { + if build_plan.map_or(true, |x| x.is_detailed()) { cx.bcx.config.shell().status("Finished", message)?; } Ok(()) @@ -391,7 +391,7 @@ impl<'a> JobQueue<'a> { job: Job, config: &Config, scope: &Scope<'a>, - build_plan: bool, + build_plan: Option, ) -> CargoResult<()> { info!("start: {:?}", key); @@ -404,7 +404,7 @@ impl<'a> JobQueue<'a> { my_tx.send(Message::Finish(key, res)).unwrap(); }; - if !build_plan { + if build_plan.map_or(true, |x| x.is_detailed()) { // Print out some nice progress information self.note_working_on(config, &key, fresh)?; } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 476f9690375..ebaea29ac4e 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -24,7 +24,7 @@ use self::job_queue::JobQueue; use self::output_depinfo::{dep_files_for_unit, output_depinfo}; pub use self::build_context::{BuildContext, FileFlavor, TargetConfig, TargetInfo}; -pub use self::build_config::{BuildConfig, CompileMode, MessageFormat}; +pub use self::build_config::{BuildConfig, BuildPlanMode, CompileMode, MessageFormat}; pub use self::compilation::{Compilation, Doctest}; pub use self::context::{Context, Unit}; pub use self::custom_build::{BuildMap, BuildOutput, BuildScripts}; @@ -148,7 +148,7 @@ fn compile<'a, 'cfg: 'a>( } else if unit.mode == CompileMode::Doctest { // we run these targets later, so this is just a noop for now (Work::noop(), Work::noop(), Freshness::Fresh) - } else if build_plan { + } else if build_plan.map_or(false, |x| !x.is_detailed()) { ( rustc(cx, unit, &exec.clone())?, Work::noop(), @@ -156,6 +156,12 @@ fn compile<'a, 'cfg: 'a>( ) } else { let (mut freshness, dirty, fresh) = fingerprint::prepare_target(cx, unit)?; + // TODO: For detailed build plan we always want to call rustc() (to + // prepare the command) but actually *run* it/link_targets/prep_fingerprint + // only when the fingerprint is missing (target is *actually* dirty) + // But again in rustc() we check against build_plan and as above, sometimes + // for detailed plan we want to execute it and sometimes we don't + // TODO: Pass "fresh" to rustc? that makes not that much sense... let work = if unit.mode.is_doc() { rustdoc(cx, unit)? } else { @@ -178,7 +184,7 @@ fn compile<'a, 'cfg: 'a>( for unit in cx.dep_targets(unit).iter() { compile(cx, jobs, plan, unit, exec, false)?; } - if build_plan { + if build_plan.is_some() { plan.add(cx, unit)?; } @@ -252,7 +258,7 @@ fn rustc<'a, 'cfg>( // previous build scripts, we include them in the rustc invocation. if let Some(build_deps) = build_deps { let build_state = build_state.outputs.lock().unwrap(); - if !build_plan { + if build_plan.map_or(true, |x| x.is_detailed()) { add_native_deps( &mut rustc, &build_state, @@ -291,23 +297,27 @@ fn rustc<'a, 'cfg>( } state.running(&rustc); - if json_messages { - exec.exec_json( - rustc, - &package_id, - &target, - mode, - &mut assert_is_empty, - &mut |line| json_stderr(line, &package_id, &target), - ) - .map_err(internal_if_simple_exit_code) - .chain_err(|| format!("Could not compile `{}`.", name))?; - } else if build_plan { + + if build_plan.is_some() { state.build_plan(buildkey, rustc.clone(), inputs, outputs.clone()); - } else { - exec.exec_and_capture_output(rustc, &package_id, &target, mode, state) - .map_err(internal_if_simple_exit_code) + } + + if build_plan.map_or(true, |x| x.is_detailed()) { + if json_messages { + exec.exec_json( + rustc, + &package_id, + &target, + mode, + &mut assert_is_empty, + &mut |line| json_stderr(line, &package_id, &target), + ).map_err(internal_if_simple_exit_code) .chain_err(|| format!("Could not compile `{}`.", name))?; + } else { + exec.exec_and_capture_output(rustc, &package_id, &target, mode, state) + .map_err(internal_if_simple_exit_code) + .chain_err(|| format!("Could not compile `{}`.", name))?; + } } if do_rename && real_name != crate_name { From 30f46fd65dccd15833cdb8f1c798c04937eaa7a0 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sun, 21 Oct 2018 23:13:20 +0200 Subject: [PATCH 05/15] Eventually add file deps for *transitive* deps as well --- src/cargo/core/compiler/build_plan.rs | 31 +++++++++++++---------- src/cargo/core/compiler/output_depinfo.rs | 1 + 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index 2b12fa9b612..e1324a95aa9 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -128,11 +128,7 @@ impl BuildPlan { inputs: &[String], outputs: &[OutputFile], ) -> CargoResult<()> { - let id = self.invocation_map[invocation_name]; - let invocation = self.plan - .invocations - .get_mut(id) - .ok_or_else(|| internal(format!("couldn't find invocation for {}", invocation_name)))?; + let invocation = self.get_invocation_mut(invocation_name)?; invocation.update_cmd(cmd)?; invocation.inputs.extend(inputs.iter().cloned()); @@ -145,14 +141,15 @@ impl BuildPlan { pub fn update_file_deps<'a, 'b>(&mut self, cx: &mut Context<'a, 'b>, units: &[Unit<'a>]) -> CargoResult<()> { for unit in units { - let id = self.invocation_map[&unit.buildkey()]; - let invocation = self.plan - .invocations - .get_mut(id) - .ok_or_else(|| internal(format!("couldn't find invocation for {}", unit.buildkey())))?; - - let dep_files = dep_files_for_unit(cx, unit)?.unwrap_or_default(); - invocation.inputs.extend(dep_files); + { + let mut invocation = self.get_invocation_mut(&unit.buildkey())?; + let dep_files = dep_files_for_unit(cx, unit)?.unwrap_or_default(); + ::log::debug!("update_file_deps: dep_files: {:#?}", dep_files); + invocation.inputs.extend(dep_files); + } + + let deps = cx.dep_targets(unit); + self.update_file_deps(cx, &deps)?; } Ok(()) @@ -166,6 +163,14 @@ impl BuildPlan { let encoded = serde_json::to_string(&self.plan).unwrap(); println!("{}", encoded); } + + fn get_invocation_mut(&mut self, buildkey: impl AsRef) -> CargoResult<&mut Invocation> { + let id = self.invocation_map[buildkey.as_ref()]; + self.plan + .invocations + .get_mut(id) + .ok_or_else(|| internal(format!("couldn't find invocation for {}", buildkey.as_ref()))) + } } impl SerializedBuildPlan { diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index b4ee7c12dcb..740712f661a 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -84,6 +84,7 @@ pub fn dep_files_for_unit<'a, 'b>( cx: &mut Context<'a, 'b>, unit: &Unit<'a>, ) -> CargoResult, ()>> { + ::log::debug!("dep_files_for_unit: {}", unit.buildkey()); let basedir = dep_info_basedir(cx)?; let mut deps = BTreeSet::new(); From bd9e35f252a7c564319aa0c21546796ece72044f Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Mon, 22 Oct 2018 12:33:36 +0200 Subject: [PATCH 06/15] Fix caching rustc invocations for fresh detailed build plan --- src/cargo/core/compiler/mod.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index ebaea29ac4e..455de4947b4 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -21,7 +21,7 @@ use self::build_plan::BuildPlan; use self::job::{Job, Work}; use self::job_queue::JobQueue; -use self::output_depinfo::{dep_files_for_unit, output_depinfo}; +use self::output_depinfo::output_depinfo; pub use self::build_context::{BuildContext, FileFlavor, TargetConfig, TargetInfo}; pub use self::build_config::{BuildConfig, BuildPlanMode, CompileMode, MessageFormat}; @@ -150,7 +150,7 @@ fn compile<'a, 'cfg: 'a>( (Work::noop(), Work::noop(), Freshness::Fresh) } else if build_plan.map_or(false, |x| !x.is_detailed()) { ( - rustc(cx, unit, &exec.clone())?, + rustc(cx, unit, &exec.clone(), false)?, Work::noop(), Freshness::Dirty, ) @@ -165,13 +165,13 @@ fn compile<'a, 'cfg: 'a>( let work = if unit.mode.is_doc() { rustdoc(cx, unit)? } else { - rustc(cx, unit, exec)? + rustc(cx, unit, exec, freshness == Freshness::Fresh)? }; // Need to link targets on both the dirty and fresh let dirty = work.then(link_targets(cx, unit, false)?).then(dirty); let fresh = link_targets(cx, unit, true)?.then(fresh); - if exec.force_rebuild(unit) || force_rebuild { + if exec.force_rebuild(unit) || force_rebuild || build_plan.map_or(false, |x| x.is_detailed()) { freshness = Freshness::Dirty; } @@ -195,6 +195,7 @@ fn rustc<'a, 'cfg>( cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>, exec: &Arc, + fresh: bool, ) -> CargoResult { let mut rustc = prepare_rustc(cx, &unit.target.rustc_crate_types(), unit)?; if cx.is_primary_package(unit) { @@ -207,7 +208,6 @@ fn rustc<'a, 'cfg>( add_cap_lints(cx.bcx, unit, &mut rustc); - let inputs = dep_files_for_unit(cx, unit)?.unwrap_or_default(); let outputs = cx.outputs(unit)?; let root = cx.files().out_dir(unit); let kind = unit.kind; @@ -299,10 +299,10 @@ fn rustc<'a, 'cfg>( state.running(&rustc); if build_plan.is_some() { - state.build_plan(buildkey, rustc.clone(), inputs, outputs.clone()); + state.build_plan(buildkey, rustc.clone(), vec![], outputs.clone()); } - if build_plan.map_or(true, |x| x.is_detailed()) { + if build_plan.map_or(true, |x| x.is_detailed()) && !fresh { if json_messages { exec.exec_json( rustc, From f2ba2757fc8cad73b9ffc1a1686c3f5298354a2f Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 10:13:54 +0200 Subject: [PATCH 07/15] Fix caching build script invocations for detailed build plans --- src/cargo/core/compiler/custom_build.rs | 50 ++++++++++++++++++++----- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 47376e2a1c0..a5837b0d3f8 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -4,6 +4,7 @@ use std::fs; use std::path::{Path, PathBuf}; use std::str; use std::sync::{Arc, Mutex}; +use std::sync::atomic::{AtomicBool, Ordering}; use core::PackageId; use util::errors::{CargoResult, CargoResultExt}; @@ -86,12 +87,14 @@ pub fn prepare<'a, 'cfg>( unit.target.name() )); + let is_fresh = Arc::new(AtomicBool::new(false)); + let key = (unit.pkg.package_id().clone(), unit.kind); let overridden = cx.build_script_overridden.contains(&key); let (work_dirty, work_fresh) = if overridden { (Work::noop(), Work::noop()) } else { - build_work(cx, unit)? + build_work(cx, unit, is_fresh.clone())? }; if cx.bcx.build_config.build_plan.map_or(false, |x| !x.is_detailed()) { @@ -100,7 +103,7 @@ pub fn prepare<'a, 'cfg>( // Now that we've prep'd our work, build the work needed to manage the // fingerprint and then start returning that upwards. let (freshness, dirty, fresh) = fingerprint::prepare_build_cmd(cx, unit)?; - + is_fresh.store(freshness == Freshness::Fresh, Ordering::Relaxed); Ok((work_dirty.then(dirty), work_fresh.then(fresh), freshness)) } } @@ -121,7 +124,11 @@ fn emit_build_output(output: &BuildOutput, id: &PackageId) { }); } -fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoResult<(Work, Work)> { +fn build_work<'a, 'cfg>( + cx: &mut Context<'a, 'cfg>, + unit: &Unit<'a>, + is_fresh: Arc, +) -> CargoResult<(Work, Work)> { assert!(unit.mode.is_run_custom_build()); let bcx = &cx.bcx; let dependencies = cx.dep_targets(unit); @@ -267,6 +274,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes .unwrap_or_else(|_| cmd.get_cwd().unwrap().to_path_buf()); let prev_output = BuildOutput::parse_file(&output_file, &pkg_name, &prev_root_output, &root_output).ok(); + let prev_output_clone = prev_output.clone(); + let prev_root_output_clone = prev_root_output.clone(); let deps = BuildDeps::new(&output_file, prev_output.as_ref()); cx.build_explicit_deps.insert(*unit, deps); @@ -279,6 +288,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes // Note that this has to do some extra work just before running the command // to determine extra environment variables and such. let dirty = Work::new(move |state| { + let is_fresh = is_fresh.load(Ordering::Relaxed); + // Make sure that OUT_DIR exists. // // If we have an old build directory, then just move it into place, @@ -296,7 +307,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes // along to this custom build command. We're also careful to augment our // dynamic library search path in case the build script depended on any // native dynamic libraries. - if build_plan.map_or(true, |x| x.is_detailed()) { + if build_plan.map_or(true, |x| x.is_detailed()) && !is_fresh { let build_state = build_state.outputs.lock().unwrap(); for (name, id) in lib_deps { let key = (id.clone(), kind); @@ -321,12 +332,7 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes } // And now finally, run the build command itself! - if build_plan.map_or(false, |x| !x.is_detailed()) { - // NOTE: It's impossible to accurately detect file inputs/outputs for - // the *execution* of an arbitrary build script and so we pass empty file lists here. - let (inputs, outputs) = (vec![], Arc::default()); - state.build_plan(invocation_name, cmd.clone(), inputs, outputs); - } else { + if build_plan.map_or(true, |x| x.is_detailed()) && !is_fresh { state.running(&cmd); let output = if extra_verbose { let prefix = format!("[{} {}] ", id.name(), id.version()); @@ -358,8 +364,32 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) -> CargoRes if json_messages { emit_build_output(&parsed_output, &id); } + build_state.insert(id, kind, parsed_output); + } else if is_fresh { + let output = match prev_output_clone { + Some(output) => output, + None => { + BuildOutput::parse_file(&output_file, &pkg_name, &prev_root_output_clone, &root_output)? + } + }; + + if json_messages { + emit_build_output(&output, &id); + } + + build_state.insert(id, kind, output); + } else { + // Build plan is configured to emit commands only, don't run anything. } + + if build_plan.is_some() { + // NOTE: It's impossible to accurately detect file inputs/outputs for + // the *execution* of an arbitrary build script and so we pass empty file lists here. + let (inputs, outputs) = (vec![], Arc::default()); + state.build_plan(invocation_name, cmd, inputs, outputs); + } + Ok(()) }); From f6f5bfffe10bd87f85a9759869870fdbef673d33 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 10:16:00 +0200 Subject: [PATCH 08/15] Tweak dep file calculation for each unit --- src/cargo/core/compiler/build_plan.rs | 12 ++++++++++-- tests/testsuite/build_plan.rs | 12 ++---------- tests/testsuite/metabuild.rs | 5 ----- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index e1324a95aa9..192e95d85e3 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -24,6 +24,8 @@ struct Invocation { target_kind: TargetKind, kind: Kind, deps: Vec, + // Inputs are only calculated and emitted in "detailed" build plan mode. + #[serde(skip_serializing_if = "Vec::is_empty")] inputs: Vec, outputs: Vec, links: BTreeMap, @@ -143,9 +145,15 @@ impl BuildPlan { for unit in units { { let mut invocation = self.get_invocation_mut(&unit.buildkey())?; - let dep_files = dep_files_for_unit(cx, unit)?.unwrap_or_default(); + let processed = !invocation.inputs.is_empty(); + if processed { + continue; + } + + let mut dep_files = dep_files_for_unit(cx, unit)?.unwrap_or_default(); + dep_files.sort(); ::log::debug!("update_file_deps: dep_files: {:#?}", dep_files); - invocation.inputs.extend(dep_files); + invocation.inputs = dep_files; } let deps = cx.dep_targets(unit); diff --git a/tests/testsuite/build_plan.rs b/tests/testsuite/build_plan.rs index 22c96a662c9..59281cd2d56 100644 --- a/tests/testsuite/build_plan.rs +++ b/tests/testsuite/build_plan.rs @@ -24,7 +24,6 @@ fn cargo_build_plan_simple() { "env": "{...}", "kind": "Host", "links": "{...}", - "inputs": [], "outputs": "{...}", "package_name": "foo", "package_version": "0.5.0", @@ -81,7 +80,6 @@ fn cargo_build_plan_single_dep() { "env": "{...}", "kind": "Host", "links": "{...}", - "inputs": [], "outputs": [ "[..]/foo/target/debug/deps/libbar-[..].rlib" ], @@ -97,7 +95,6 @@ fn cargo_build_plan_single_dep() { "env": "{...}", "kind": "Host", "links": "{...}", - "inputs": [], "outputs": [ "[..]/foo/target/debug/deps/libfoo-[..].rlib" ], @@ -145,7 +142,6 @@ fn cargo_build_plan_build_script() { "env": "{...}", "kind": "Host", "links": "{...}", - "inputs": [], "outputs": [ "[..]/foo/target/debug/build/[..]/build_script_build-[..]" ], @@ -161,7 +157,6 @@ fn cargo_build_plan_build_script() { "env": "{...}", "kind": "Host", "links": "{...}", - "inputs": [], "outputs": [], "package_name": "foo", "package_version": "0.5.0", @@ -175,7 +170,6 @@ fn cargo_build_plan_build_script() { "env": "{...}", "kind": "Host", "links": "{...}", - "inputs": [], "outputs": "{...}", "package_name": "foo", "package_version": "0.5.0", @@ -214,7 +208,7 @@ fn build_plan_with_dev_dep() { } #[test] -fn build_plan_with_inputs() { +fn build_plan_detailed_with_inputs() { let p = project() .file("Cargo.toml", &basic_bin_manifest("foo")) .file("data/some.txt", "o hai there") @@ -225,10 +219,8 @@ fn build_plan_with_inputs() { const DATA: &str = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/data/some.txt"));"#) .file("src/not_included_in_inputs.rs", "") .build(); - // Currently we need to populate .d files which are later used to include "inputs" field - p.cargo("build").masquerade_as_nightly_cargo().run(); - p.cargo("build --build-plan -Zunstable-options") + p.cargo("build --build-plan=detailed -Zunstable-options") .masquerade_as_nightly_cargo() .with_json( r#" diff --git a/tests/testsuite/metabuild.rs b/tests/testsuite/metabuild.rs index 76112d69076..c96f0c4b6b6 100644 --- a/tests/testsuite/metabuild.rs +++ b/tests/testsuite/metabuild.rs @@ -435,7 +435,6 @@ fn metabuild_build_plan() { "target_kind": ["lib"], "kind": "Host", "deps": [], - "inputs": [], "outputs": ["[..]/target/debug/deps/libmb-[..].rlib"], "links": {}, "program": "rustc", @@ -449,7 +448,6 @@ fn metabuild_build_plan() { "target_kind": ["lib"], "kind": "Host", "deps": [], - "inputs": [], "outputs": ["[..]/target/debug/deps/libmb_other-[..].rlib"], "links": {}, "program": "rustc", @@ -463,7 +461,6 @@ fn metabuild_build_plan() { "target_kind": ["custom-build"], "kind": "Host", "deps": [0, 1], - "inputs": [], "outputs": ["[..]/target/debug/build/foo-[..]/metabuild_foo-[..][EXE]"], "links": "{...}", "program": "rustc", @@ -477,7 +474,6 @@ fn metabuild_build_plan() { "target_kind": ["custom-build"], "kind": "Host", "deps": [2], - "inputs": [], "outputs": [], "links": {}, "program": "[..]/foo/target/debug/build/foo-[..]/metabuild-foo", @@ -491,7 +487,6 @@ fn metabuild_build_plan() { "target_kind": ["lib"], "kind": "Host", "deps": [3], - "inputs": [], "outputs": ["[..]/foo/target/debug/deps/libfoo-[..].rlib"], "links": "{...}", "program": "rustc", From 2a4a2adea30fe523a01c4301c761820e43c17032 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 13:52:25 +0200 Subject: [PATCH 09/15] Simplify BuildPlanMode-related logic --- src/bin/cargo/command_prelude.rs | 8 ++++---- src/cargo/core/compiler/build_config.rs | 16 ++++++++++++---- src/cargo/core/compiler/context/mod.rs | 2 +- src/cargo/core/compiler/custom_build.rs | 8 ++++---- src/cargo/core/compiler/job_queue.rs | 6 +++--- src/cargo/core/compiler/mod.rs | 12 ++++++------ 6 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/bin/cargo/command_prelude.rs b/src/bin/cargo/command_prelude.rs index 5d847907b07..6d925a6d576 100644 --- a/src/bin/cargo/command_prelude.rs +++ b/src/bin/cargo/command_prelude.rs @@ -293,18 +293,18 @@ pub trait ArgMatchesExt { build_config.message_format = message_format; build_config.release = self._is_present("release"); build_config.build_plan = match (self._is_present("build-plan"), self._value_of("build-plan")) { - (false, _) => None, + (false, _) => BuildPlanMode::NoEmit, (true, Some(val)) if val.eq_ignore_ascii_case("detailed") => { config.shell().warn(format!("REMOVEME: Detailed"))?; - Some(BuildPlanMode::Detailed) + BuildPlanMode::Detailed }, (true, _) => { config.shell().warn(format!("REMOVEME: Commands only"))?; - Some(BuildPlanMode::CommandsOnly) + BuildPlanMode::CommandsOnly }, }; - if build_config.build_plan.is_some() && !config.cli_unstable().unstable_options { + if build_config.build_plan.should_emit() && !config.cli_unstable().unstable_options { Err(format_err!( "`--build-plan` flag is unstable, pass `-Z unstable-options` to enable it" ))?; diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index c493b50bd0d..f4515345449 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -19,7 +19,7 @@ pub struct BuildConfig { /// Force cargo to do a full rebuild and treat each target as changed. pub force_rebuild: bool, /// Output a build plan to stdout instead of actually compiling. - pub build_plan: Option, + pub build_plan: BuildPlanMode, /// Use Cargo itself as the wrapper around rustc, only used for `cargo fix` pub cargo_as_rustc_wrapper: bool, /// Extra env vars to inject into rustc commands @@ -82,7 +82,7 @@ impl BuildConfig { mode, message_format: MessageFormat::Human, force_rebuild: false, - build_plan: None, + build_plan: BuildPlanMode::NoEmit, cargo_as_rustc_wrapper: false, extra_rustc_env: Vec::new(), extra_rustc_args: Vec::new(), @@ -101,6 +101,8 @@ impl BuildConfig { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum BuildPlanMode { + /// Don't emit any build plan. + NoEmit, /// Quick mode of generating build plan (contains only commands to execute) CommandsOnly, /// In addition to containing build commands, it also includes detailed list @@ -111,9 +113,15 @@ pub enum BuildPlanMode { } impl BuildPlanMode { - pub fn is_detailed(&self) -> bool { + pub fn should_emit(&self) -> bool { match self { - BuildPlanMode::Detailed => true, + BuildPlanMode::NoEmit => false, + _ => true, + } + } + pub fn commands_only(&self) -> bool { + match self { + BuildPlanMode::CommandsOnly => true, _ => false, } } diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 9944c30737a..44b8d21daa7 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -249,7 +249,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { super::output_depinfo(&mut self, unit)?; } - if build_plan.is_some() { + if build_plan.should_emit() { plan.update_file_deps(&mut self, units)?; plan.set_inputs(self.build_plan_inputs()?); plan.output_plan(); diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index a5837b0d3f8..34abff6d54c 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -97,7 +97,7 @@ pub fn prepare<'a, 'cfg>( build_work(cx, unit, is_fresh.clone())? }; - if cx.bcx.build_config.build_plan.map_or(false, |x| !x.is_detailed()) { + if cx.bcx.build_config.build_plan.commands_only() { Ok((work_dirty, work_fresh, Freshness::Dirty)) } else { // Now that we've prep'd our work, build the work needed to manage the @@ -307,7 +307,7 @@ fn build_work<'a, 'cfg>( // along to this custom build command. We're also careful to augment our // dynamic library search path in case the build script depended on any // native dynamic libraries. - if build_plan.map_or(true, |x| x.is_detailed()) && !is_fresh { + if !build_plan.commands_only() && !is_fresh { let build_state = build_state.outputs.lock().unwrap(); for (name, id) in lib_deps { let key = (id.clone(), kind); @@ -332,7 +332,7 @@ fn build_work<'a, 'cfg>( } // And now finally, run the build command itself! - if build_plan.map_or(true, |x| x.is_detailed()) && !is_fresh { + if !build_plan.commands_only() && !is_fresh { state.running(&cmd); let output = if extra_verbose { let prefix = format!("[{} {}] ", id.name(), id.version()); @@ -383,7 +383,7 @@ fn build_work<'a, 'cfg>( // Build plan is configured to emit commands only, don't run anything. } - if build_plan.is_some() { + if build_plan.should_emit() { // NOTE: It's impossible to accurately detect file inputs/outputs for // the *execution* of an arbitrary build script and so we pass empty file lists here. let (inputs, outputs) = (vec![], Arc::default()); diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index a05a24a8825..af9d7110613 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -370,7 +370,7 @@ impl<'a> JobQueue<'a> { "{} [{}] target(s) in {}", build_type, opt_type, time_elapsed ); - if build_plan.map_or(true, |x| x.is_detailed()) { + if !build_plan.commands_only() { cx.bcx.config.shell().status("Finished", message)?; } Ok(()) @@ -391,7 +391,7 @@ impl<'a> JobQueue<'a> { job: Job, config: &Config, scope: &Scope<'a>, - build_plan: Option, + build_plan: BuildPlanMode, ) -> CargoResult<()> { info!("start: {:?}", key); @@ -404,7 +404,7 @@ impl<'a> JobQueue<'a> { my_tx.send(Message::Finish(key, res)).unwrap(); }; - if build_plan.map_or(true, |x| x.is_detailed()) { + if !build_plan.commands_only() { // Print out some nice progress information self.note_working_on(config, &key, fresh)?; } diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 455de4947b4..2b172f8c22a 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -148,7 +148,7 @@ fn compile<'a, 'cfg: 'a>( } else if unit.mode == CompileMode::Doctest { // we run these targets later, so this is just a noop for now (Work::noop(), Work::noop(), Freshness::Fresh) - } else if build_plan.map_or(false, |x| !x.is_detailed()) { + } else if build_plan.commands_only() { ( rustc(cx, unit, &exec.clone(), false)?, Work::noop(), @@ -171,7 +171,7 @@ fn compile<'a, 'cfg: 'a>( let dirty = work.then(link_targets(cx, unit, false)?).then(dirty); let fresh = link_targets(cx, unit, true)?.then(fresh); - if exec.force_rebuild(unit) || force_rebuild || build_plan.map_or(false, |x| x.is_detailed()) { + if exec.force_rebuild(unit) || force_rebuild || build_plan.should_emit() { freshness = Freshness::Dirty; } @@ -184,7 +184,7 @@ fn compile<'a, 'cfg: 'a>( for unit in cx.dep_targets(unit).iter() { compile(cx, jobs, plan, unit, exec, false)?; } - if build_plan.is_some() { + if build_plan.should_emit() { plan.add(cx, unit)?; } @@ -258,7 +258,7 @@ fn rustc<'a, 'cfg>( // previous build scripts, we include them in the rustc invocation. if let Some(build_deps) = build_deps { let build_state = build_state.outputs.lock().unwrap(); - if build_plan.map_or(true, |x| x.is_detailed()) { + if !build_plan.commands_only() { add_native_deps( &mut rustc, &build_state, @@ -298,11 +298,11 @@ fn rustc<'a, 'cfg>( state.running(&rustc); - if build_plan.is_some() { + if build_plan.should_emit() { state.build_plan(buildkey, rustc.clone(), vec![], outputs.clone()); } - if build_plan.map_or(true, |x| x.is_detailed()) && !fresh { + if !build_plan.commands_only() && !fresh { if json_messages { exec.exec_json( rustc, From 728a8b1522c4f35695c07846a87da95518894451 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 14:09:13 +0200 Subject: [PATCH 10/15] Remove redundant inputs in BuildPlanMsg --- src/cargo/core/compiler/build_plan.rs | 3 --- src/cargo/core/compiler/custom_build.rs | 5 +---- src/cargo/core/compiler/job_queue.rs | 9 ++++----- src/cargo/core/compiler/mod.rs | 8 +------- src/cargo/core/compiler/output_depinfo.rs | 1 - 5 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index 192e95d85e3..4eb0ed04a6f 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -127,13 +127,11 @@ impl BuildPlan { &mut self, invocation_name: &str, cmd: &ProcessBuilder, - inputs: &[String], outputs: &[OutputFile], ) -> CargoResult<()> { let invocation = self.get_invocation_mut(invocation_name)?; invocation.update_cmd(cmd)?; - invocation.inputs.extend(inputs.iter().cloned()); for output in outputs.iter() { invocation.add_output(&output.path, &output.hardlink); } @@ -152,7 +150,6 @@ impl BuildPlan { let mut dep_files = dep_files_for_unit(cx, unit)?.unwrap_or_default(); dep_files.sort(); - ::log::debug!("update_file_deps: dep_files: {:#?}", dep_files); invocation.inputs = dep_files; } diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 34abff6d54c..4a589096773 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -384,10 +384,7 @@ fn build_work<'a, 'cfg>( } if build_plan.should_emit() { - // NOTE: It's impossible to accurately detect file inputs/outputs for - // the *execution* of an arbitrary build script and so we pass empty file lists here. - let (inputs, outputs) = (vec![], Arc::default()); - state.build_plan(invocation_name, cmd, inputs, outputs); + state.build_plan(invocation_name, cmd, Arc::default()); } Ok(()) diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index af9d7110613..02b386317c7 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -86,7 +86,7 @@ pub struct JobState<'a> { enum Message<'a> { Run(String), - BuildPlanMsg(String, ProcessBuilder, Vec, Arc>), + BuildPlanMsg(String, ProcessBuilder, Arc>), Stdout(String), Stderr(String), FixDiagnostic(diagnostic_server::Message), @@ -103,11 +103,10 @@ impl<'a> JobState<'a> { &self, module_name: String, cmd: ProcessBuilder, - inputs: Vec, filenames: Arc>, ) { let _ = self.tx - .send(Message::BuildPlanMsg(module_name, cmd, inputs, filenames)); + .send(Message::BuildPlanMsg(module_name, cmd, filenames)); } pub fn capture_output( @@ -291,8 +290,8 @@ impl<'a> JobQueue<'a> { .shell() .verbose(|c| c.status("Running", &cmd))?; } - Message::BuildPlanMsg(module_name, cmd, inputs, filenames) => { - plan.update(&module_name, &cmd, &inputs, &filenames)?; + Message::BuildPlanMsg(module_name, cmd, filenames) => { + plan.update(&module_name, &cmd, &filenames)?; } Message::Stdout(out) => { println!("{}", out); diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 2b172f8c22a..6f2528bb725 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -156,12 +156,6 @@ fn compile<'a, 'cfg: 'a>( ) } else { let (mut freshness, dirty, fresh) = fingerprint::prepare_target(cx, unit)?; - // TODO: For detailed build plan we always want to call rustc() (to - // prepare the command) but actually *run* it/link_targets/prep_fingerprint - // only when the fingerprint is missing (target is *actually* dirty) - // But again in rustc() we check against build_plan and as above, sometimes - // for detailed plan we want to execute it and sometimes we don't - // TODO: Pass "fresh" to rustc? that makes not that much sense... let work = if unit.mode.is_doc() { rustdoc(cx, unit)? } else { @@ -299,7 +293,7 @@ fn rustc<'a, 'cfg>( state.running(&rustc); if build_plan.should_emit() { - state.build_plan(buildkey, rustc.clone(), vec![], outputs.clone()); + state.build_plan(buildkey, rustc.clone(), outputs.clone()); } if !build_plan.commands_only() && !fresh { diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index 740712f661a..b4ee7c12dcb 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -84,7 +84,6 @@ pub fn dep_files_for_unit<'a, 'b>( cx: &mut Context<'a, 'b>, unit: &Unit<'a>, ) -> CargoResult, ()>> { - ::log::debug!("dep_files_for_unit: {}", unit.buildkey()); let basedir = dep_info_basedir(cx)?; let mut deps = BTreeSet::new(); From 498e4316f61f36adff41ee2adeb39b8d846a552a Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 15:27:50 +0200 Subject: [PATCH 11/15] Skip "dirty" work if possible only in build plan mode This is annoying, because we want to traverse the entire unit dependency graph and to do so we treat every work as `Freshness::Dirty`. However, for detailed build plan we do want to reuse the resulting artifacts/work from previous runs, but we can't just blindly skip work dirty work if it was fresh at construction time in the general case, since freshness can be transitively altered at job dequeue time. This makes us skip dirty work at construction time only if we are in a detailed build plan mode, effectively adding another layer of freshness tracking due to every node being considered as `Dirty`. --- src/cargo/core/compiler/build_config.rs | 6 ++++++ src/cargo/core/compiler/custom_build.rs | 20 +++++++++++--------- src/cargo/core/compiler/mod.rs | 5 +++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index f4515345449..cc8be9d65a8 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -125,6 +125,12 @@ impl BuildPlanMode { _ => false, } } + pub fn is_detailed(&self) -> bool { + match self { + BuildPlanMode::Detailed => true, + _ => false, + } + } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 4a589096773..609c02a8b39 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -87,14 +87,14 @@ pub fn prepare<'a, 'cfg>( unit.target.name() )); - let is_fresh = Arc::new(AtomicBool::new(false)); + let is_fresh_now = Arc::new(AtomicBool::new(false)); let key = (unit.pkg.package_id().clone(), unit.kind); let overridden = cx.build_script_overridden.contains(&key); let (work_dirty, work_fresh) = if overridden { (Work::noop(), Work::noop()) } else { - build_work(cx, unit, is_fresh.clone())? + build_work(cx, unit, is_fresh_now.clone())? }; if cx.bcx.build_config.build_plan.commands_only() { @@ -103,7 +103,7 @@ pub fn prepare<'a, 'cfg>( // Now that we've prep'd our work, build the work needed to manage the // fingerprint and then start returning that upwards. let (freshness, dirty, fresh) = fingerprint::prepare_build_cmd(cx, unit)?; - is_fresh.store(freshness == Freshness::Fresh, Ordering::Relaxed); + is_fresh_now.store(freshness == Freshness::Fresh, Ordering::Relaxed); Ok((work_dirty.then(dirty), work_fresh.then(fresh), freshness)) } } @@ -127,7 +127,7 @@ fn emit_build_output(output: &BuildOutput, id: &PackageId) { fn build_work<'a, 'cfg>( cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>, - is_fresh: Arc, + is_fresh_now: Arc, ) -> CargoResult<(Work, Work)> { assert!(unit.mode.is_run_custom_build()); let bcx = &cx.bcx; @@ -288,7 +288,7 @@ fn build_work<'a, 'cfg>( // Note that this has to do some extra work just before running the command // to determine extra environment variables and such. let dirty = Work::new(move |state| { - let is_fresh = is_fresh.load(Ordering::Relaxed); + let build_plan_can_skip_work = is_fresh_now.load(Ordering::Relaxed); // Make sure that OUT_DIR exists. // @@ -307,7 +307,7 @@ fn build_work<'a, 'cfg>( // along to this custom build command. We're also careful to augment our // dynamic library search path in case the build script depended on any // native dynamic libraries. - if !build_plan.commands_only() && !is_fresh { + if !build_plan.commands_only() { let build_state = build_state.outputs.lock().unwrap(); for (name, id) in lib_deps { let key = (id.clone(), kind); @@ -331,8 +331,9 @@ fn build_work<'a, 'cfg>( } } + let plan_can_skip = build_plan.is_detailed() && build_plan_can_skip_work; // And now finally, run the build command itself! - if !build_plan.commands_only() && !is_fresh { + if !build_plan.commands_only() && !plan_can_skip { state.running(&cmd); let output = if extra_verbose { let prefix = format!("[{} {}] ", id.name(), id.version()); @@ -366,7 +367,7 @@ fn build_work<'a, 'cfg>( } build_state.insert(id, kind, parsed_output); - } else if is_fresh { + } else if plan_can_skip { let output = match prev_output_clone { Some(output) => output, None => { @@ -380,11 +381,12 @@ fn build_work<'a, 'cfg>( build_state.insert(id, kind, output); } else { + assert!(build_plan.commands_only()); // Build plan is configured to emit commands only, don't run anything. } if build_plan.should_emit() { - state.build_plan(invocation_name, cmd, Arc::default()); + state.build_plan(invocation_name, cmd, Arc::default()); } Ok(()) diff --git a/src/cargo/core/compiler/mod.rs b/src/cargo/core/compiler/mod.rs index 6f2528bb725..22118e9c7e7 100644 --- a/src/cargo/core/compiler/mod.rs +++ b/src/cargo/core/compiler/mod.rs @@ -189,7 +189,7 @@ fn rustc<'a, 'cfg>( cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>, exec: &Arc, - fresh: bool, + is_fresh_now: bool, ) -> CargoResult { let mut rustc = prepare_rustc(cx, &unit.target.rustc_crate_types(), unit)?; if cx.is_primary_package(unit) { @@ -296,7 +296,8 @@ fn rustc<'a, 'cfg>( state.build_plan(buildkey, rustc.clone(), outputs.clone()); } - if !build_plan.commands_only() && !fresh { + let plan_can_skip = build_plan.is_detailed() && is_fresh_now; + if !build_plan.commands_only() && !plan_can_skip { if json_messages { exec.exec_json( rustc, From 14739f3673c5094793d27d2c1282d4148628a6bf Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 15:43:37 +0200 Subject: [PATCH 12/15] Clean up and rustfmt --- src/bin/cargo/command_prelude.rs | 19 ++++++++----------- src/cargo/core/compiler/build_plan.rs | 16 +++++++++++----- src/cargo/core/compiler/custom_build.rs | 11 +++++++---- src/cargo/core/compiler/output_depinfo.rs | 10 +++++++--- 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/bin/cargo/command_prelude.rs b/src/bin/cargo/command_prelude.rs index 6d925a6d576..fc78f89ca76 100644 --- a/src/bin/cargo/command_prelude.rs +++ b/src/bin/cargo/command_prelude.rs @@ -292,17 +292,14 @@ pub trait ArgMatchesExt { let mut build_config = BuildConfig::new(config, self.jobs()?, &self.target(), mode)?; build_config.message_format = message_format; build_config.release = self._is_present("release"); - build_config.build_plan = match (self._is_present("build-plan"), self._value_of("build-plan")) { - (false, _) => BuildPlanMode::NoEmit, - (true, Some(val)) if val.eq_ignore_ascii_case("detailed") => { - config.shell().warn(format!("REMOVEME: Detailed"))?; - BuildPlanMode::Detailed - }, - (true, _) => { - config.shell().warn(format!("REMOVEME: Commands only"))?; - BuildPlanMode::CommandsOnly - }, - }; + build_config.build_plan = + match (self._is_present("build-plan"), self._value_of("build-plan")) { + (false, _) => BuildPlanMode::NoEmit, + (true, Some(val)) if val.eq_ignore_ascii_case("detailed") => { + BuildPlanMode::Detailed + } + (true, _) => BuildPlanMode::CommandsOnly, + }; if build_config.build_plan.should_emit() && !config.cli_unstable().unstable_options { Err(format_err!( diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index 4eb0ed04a6f..7b7e6b13076 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -139,7 +139,11 @@ impl BuildPlan { Ok(()) } - pub fn update_file_deps<'a, 'b>(&mut self, cx: &mut Context<'a, 'b>, units: &[Unit<'a>]) -> CargoResult<()> { + pub fn update_file_deps<'a, 'b>( + &mut self, + cx: &mut Context<'a, 'b>, + units: &[Unit<'a>], + ) -> CargoResult<()> { for unit in units { { let mut invocation = self.get_invocation_mut(&unit.buildkey())?; @@ -171,10 +175,12 @@ impl BuildPlan { fn get_invocation_mut(&mut self, buildkey: impl AsRef) -> CargoResult<&mut Invocation> { let id = self.invocation_map[buildkey.as_ref()]; - self.plan - .invocations - .get_mut(id) - .ok_or_else(|| internal(format!("couldn't find invocation for {}", buildkey.as_ref()))) + self.plan.invocations.get_mut(id).ok_or_else(|| { + internal(format!( + "couldn't find invocation for {}", + buildkey.as_ref() + )) + }) } } diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 609c02a8b39..3b46f22deff 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -3,8 +3,8 @@ use std::collections::{BTreeSet, HashSet}; use std::fs; use std::path::{Path, PathBuf}; use std::str; -use std::sync::{Arc, Mutex}; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; use core::PackageId; use util::errors::{CargoResult, CargoResultExt}; @@ -370,9 +370,12 @@ fn build_work<'a, 'cfg>( } else if plan_can_skip { let output = match prev_output_clone { Some(output) => output, - None => { - BuildOutput::parse_file(&output_file, &pkg_name, &prev_root_output_clone, &root_output)? - } + None => BuildOutput::parse_file( + &output_file, + &pkg_name, + &prev_root_output_clone, + &root_output, + )?, }; if json_messages { diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index b4ee7c12dcb..13c77881338 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -14,7 +14,10 @@ fn dep_info_basedir(cx: &Context<'_, '_>) -> CargoResult> { .map(|option| option.map(|o| o.val)) } -fn render_filename(path: impl AsRef, basedir: Option>) -> CargoResult { +fn render_filename( + path: impl AsRef, + basedir: Option>, +) -> CargoResult { let (path, basedir) = (path.as_ref(), basedir.as_ref()); let relpath = match basedir { None => path, @@ -102,10 +105,11 @@ pub fn output_depinfo<'a, 'b>(cx: &mut Context<'a, 'b>, unit: &Unit<'a>) -> Carg let (success, deps) = match dep_files_for_unit(cx, unit)? { Ok(deps) => (true, deps), - _ => (false, vec![]) + _ => (false, vec![]), }; - let deps = deps.iter() + let deps = deps + .iter() .map(|f| render_filename(f, basedir.as_ref())) .collect::>>()?; From eccda0e23662bdf26e7729c325f9774765a51e3f Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 16:00:25 +0200 Subject: [PATCH 13/15] Simplify render_filename --- src/cargo/core/compiler/output_depinfo.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index 13c77881338..3c9b427c61c 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -19,14 +19,10 @@ fn render_filename( basedir: Option>, ) -> CargoResult { let (path, basedir) = (path.as_ref(), basedir.as_ref()); - let relpath = match basedir { - None => path, - Some(base) => match path.strip_prefix(base) { - Ok(relpath) => relpath, - _ => path, - }, - }; - relpath + + basedir + .and_then(|base| path.strip_prefix(base).ok()) + .unwrap_or(path) .to_str() .ok_or_else(|| internal("path not utf-8")) .map(|f| f.replace(" ", "\\ ")) From 7e27c741c3c72cc3e22c72ae1d303b14d0168851 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 23 Oct 2018 18:52:13 +0200 Subject: [PATCH 14/15] Collect file inputs per-unit (not per-package) This distinguishes file inputs for running build script, building build script and regular rustc compilations. --- src/cargo/core/compiler/build_plan.rs | 8 +- src/cargo/core/compiler/output_depinfo.rs | 44 +++--- tests/testsuite/build_plan.rs | 166 ++++++++++++++++++++++ 3 files changed, 196 insertions(+), 22 deletions(-) diff --git a/src/cargo/core/compiler/build_plan.rs b/src/cargo/core/compiler/build_plan.rs index 7b7e6b13076..03ffc7deeae 100644 --- a/src/cargo/core/compiler/build_plan.rs +++ b/src/cargo/core/compiler/build_plan.rs @@ -152,9 +152,11 @@ impl BuildPlan { continue; } - let mut dep_files = dep_files_for_unit(cx, unit)?.unwrap_or_default(); - dep_files.sort(); - invocation.inputs = dep_files; + let transitive = false; + invocation.inputs = dep_files_for_unit(cx, unit, transitive)? + .unwrap_or_default() + .into_iter() + .collect(); } let deps = cx.dep_targets(unit); diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index 3c9b427c61c..51c16154d12 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -33,11 +33,11 @@ fn add_deps_for_unit<'a, 'b>( context: &mut Context<'a, 'b>, unit: &Unit<'a>, visited: &mut HashSet>, + transitive: bool, ) -> CargoResult<()> { if !visited.insert(*unit) { return Ok(()); } - // units representing the execution of a build script don't actually // generate a dep info file, so we just keep on going below if !unit.mode.is_run_custom_build() { @@ -58,18 +58,28 @@ fn add_deps_for_unit<'a, 'b>( } // Add rerun-if-changed dependencies - let key = (unit.pkg.package_id().clone(), unit.kind); - if let Some(output) = context.build_state.outputs.lock().unwrap().get(&key) { - for path in &output.rerun_if_changed { - deps.insert(path.into()); + if transitive || unit.mode.is_run_custom_build() { + let key = (unit.pkg.package_id().clone(), unit.kind); + if let Some(output) = context.build_state.outputs.lock().unwrap().get(&key) { + for path in &output.rerun_if_changed { + let path = if !path.is_absolute() { + unit.pkg.root().join(path) + } else { + path.to_owned() + }; + + deps.insert(path); + } } } // Recursively traverse all transitive dependencies - for dep_unit in context.dep_targets(unit).iter() { - let source_id = dep_unit.pkg.package_id().source_id(); - if source_id.is_path() { - add_deps_for_unit(deps, context, dep_unit, visited)?; + if transitive { + for dep_unit in context.dep_targets(unit).iter() { + let source_id = dep_unit.pkg.package_id().source_id(); + if source_id.is_path() { + add_deps_for_unit(deps, context, dep_unit, visited, transitive)?; + } } } Ok(()) @@ -82,16 +92,17 @@ fn add_deps_for_unit<'a, 'b>( pub fn dep_files_for_unit<'a, 'b>( cx: &mut Context<'a, 'b>, unit: &Unit<'a>, -) -> CargoResult, ()>> { + transitive: bool, +) -> CargoResult, ()>> { let basedir = dep_info_basedir(cx)?; let mut deps = BTreeSet::new(); let mut visited = HashSet::new(); - Ok(match add_deps_for_unit(&mut deps, cx, unit, &mut visited) { + Ok(match add_deps_for_unit(&mut deps, cx, unit, &mut visited, transitive) { Ok(_) => Ok(deps .iter() .map(|f| render_filename(f, basedir.as_ref())) - .collect::>>()?), + .collect::>>()?), Err(_) => Err(()), }) } @@ -99,16 +110,11 @@ pub fn dep_files_for_unit<'a, 'b>( pub fn output_depinfo<'a, 'b>(cx: &mut Context<'a, 'b>, unit: &Unit<'a>) -> CargoResult<()> { let basedir = dep_info_basedir(cx)?; - let (success, deps) = match dep_files_for_unit(cx, unit)? { - Ok(deps) => (true, deps), + let (success, deps) = match dep_files_for_unit(cx, unit, true)? { + Ok(deps) => (true, deps.into_iter().collect()), _ => (false, vec![]), }; - let deps = deps - .iter() - .map(|f| render_filename(f, basedir.as_ref())) - .collect::>>()?; - for output in cx.outputs(unit)?.iter() { if let Some(ref link_dst) = output.hardlink { let output_path = link_dst.with_extension("d"); diff --git a/tests/testsuite/build_plan.rs b/tests/testsuite/build_plan.rs index 59281cd2d56..37df13034ac 100644 --- a/tests/testsuite/build_plan.rs +++ b/tests/testsuite/build_plan.rs @@ -254,3 +254,169 @@ fn build_plan_detailed_with_inputs() { "#, ).run(); } + +#[test] +fn build_plan_detailed_build_script() { + let p = project() + .file( + "Cargo.toml", + r#" + [project] + + name = "foo" + version = "0.5.0" + authors = ["wycats@example.com"] + build = "build.rs" + "#, + ) + .file("src/main.rs", r#"fn main() {}"#) + .file("build.rs", r#" + fn main() { + println!("cargo:rerun-if-changed=build_script_rerun_dep.txt"); + } + "#) + .file("build_script_rerun_dep.txt", "") + .build(); + + p.cargo("build --build-plan=detailed -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_json( + r#" + { + "inputs": [ + "[..]/foo/Cargo.toml" + ], + "invocations": [ + { + "args": "{...}", + "cwd": "[..]/cit/[..]/foo", + "deps": [], + "env": "{...}", + "kind": "Host", + "links": "{...}", + "inputs": [ + "[..]/foo/build.rs" + ], + "outputs": [ + "[..]/foo/target/debug/build/[..]/build_script_build-[..]" + ], + "package_name": "foo", + "package_version": "0.5.0", + "program": "rustc", + "target_kind": ["custom-build"] + }, + { + "args": "{...}", + "cwd": "[..]/cit/[..]/foo", + "deps": [0], + "env": "{...}", + "kind": "Host", + "links": "{...}", + "inputs": [ + "[..]/foo/build_script_rerun_dep.txt" + ], + "outputs": [], + "package_name": "foo", + "package_version": "0.5.0", + "program": "[..]/build-script-build", + "target_kind": ["custom-build"] + }, + { + "args": "{...}", + "cwd": "[..]/cit/[..]/foo", + "deps": [1], + "env": "{...}", + "kind": "Host", + "links": "{...}", + "inputs": [ + "[..]/foo/src/main.rs" + ], + "outputs": "{...}", + "package_name": "foo", + "package_version": "0.5.0", + "program": "rustc", + "target_kind": ["bin"] + } + ] + } + "#, + ).run(); +} + +#[test] +fn cargo_build_plan_detailed_path_dep() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + authors = [] + version = "0.5.0" + + [dependencies] + bar = { path = "bar" } + "#, + ).file( + "src/lib.rs", + r#" + extern crate bar; + pub fn foo() { bar::bar(); } + + #[test] + fn test() { foo(); } + "#, + ).file("bar/Cargo.toml", &basic_manifest("bar", "0.0.1")) + .file("bar/src/lib.rs", "pub fn bar() {}") + .build(); + p.cargo("build --build-plan=detailed -Zunstable-options") + .masquerade_as_nightly_cargo() + .with_json( + r#" + { + "inputs": [ + "[..]/foo/Cargo.toml", + "[..]/foo/bar/Cargo.toml" + ], + "invocations": [ + { + "args": "{...}", + "cwd": "[..]/cit/[..]/foo", + "deps": [], + "env": "{...}", + "kind": "Host", + "links": "{...}", + "inputs": [ + "[..]/foo/bar/src/lib.rs" + ], + "outputs": [ + "[..]/foo/target/debug/deps/libbar-[..].rlib" + ], + "package_name": "bar", + "package_version": "0.0.1", + "program": "rustc", + "target_kind": ["lib"] + }, + { + "args": "{...}", + "cwd": "[..]/cit/[..]/foo", + "deps": [0], + "env": "{...}", + "kind": "Host", + "links": "{...}", + "inputs": [ + "[..]/foo/src/lib.rs" + ], + "outputs": [ + "[..]/foo/target/debug/deps/libfoo-[..].rlib" + ], + "package_name": "foo", + "package_version": "0.5.0", + "program": "rustc", + "target_kind": ["lib"] + } + ] + } + "#, + ).run(); +} From 3e6be5d1640c8ec4323f81c737205d2faf2dce19 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 24 Oct 2018 13:39:56 +0200 Subject: [PATCH 15/15] Fix cargo +stable test --- src/cargo/core/compiler/output_depinfo.rs | 1 + tests/testsuite/build_plan.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/cargo/core/compiler/output_depinfo.rs b/src/cargo/core/compiler/output_depinfo.rs index 51c16154d12..2809c8b00dd 100644 --- a/src/cargo/core/compiler/output_depinfo.rs +++ b/src/cargo/core/compiler/output_depinfo.rs @@ -38,6 +38,7 @@ fn add_deps_for_unit<'a, 'b>( if !visited.insert(*unit) { return Ok(()); } + // units representing the execution of a build script don't actually // generate a dep info file, so we just keep on going below if !unit.mode.is_run_custom_build() { diff --git a/tests/testsuite/build_plan.rs b/tests/testsuite/build_plan.rs index 37df13034ac..7e0f615cca0 100644 --- a/tests/testsuite/build_plan.rs +++ b/tests/testsuite/build_plan.rs @@ -213,7 +213,7 @@ fn build_plan_detailed_with_inputs() { .file("Cargo.toml", &basic_bin_manifest("foo")) .file("data/some.txt", "o hai there") .file("src/foo.rs", "mod module1; mod module2; fn main() {}") - .file("src/module1.rs", "mod nested;") + .file("src/module1/mod.rs", "mod nested;") .file("src/module1/nested.rs", "") .file("src/module2/mod.rs", r#" const DATA: &str = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/data/some.txt"));"#) @@ -239,8 +239,8 @@ fn build_plan_detailed_with_inputs() { "inputs": [ "[..]/foo/data/some.txt", "[..]/foo/src/foo.rs", + "[..]/foo/src/module1/mod.rs", "[..]/foo/src/module1/nested.rs", - "[..]/foo/src/module1.rs", "[..]/foo/src/module2/mod.rs" ], "outputs": "{...}",