From b07a1963b8556d6cf84b982be5a82e298d939284 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 27 Nov 2021 01:18:27 -0500 Subject: [PATCH 1/4] Pass compile mode to the build script Closes #4001 This PR adds code to track and pass compile mode to the build scripts. Since the unit's mode is always `RunCustomBuild`, I had to add tracking for the parent's compile mode, and pass it around. - [ ] env var naming. I used `CARGO_MODE`, but perhaps another name is better? - [ ] env var values naming. I used: `Test`, `Build`, `Check_test/Check`, `Bench`, `Doc_with_deps/Doc`, `Doctest`, `Docscrape`, `RunCustomBuild` - [ ] how to represent bool values for `Check` and `Doc`. I created two separate values, but perhaps the `test` and `deps` values should be ignored? - [ ] figure out why `cargo bench` sets env var to `Test` - [ ] add unit tests --- src/cargo/core/compiler/custom_build.rs | 14 ++++++++++++++ src/cargo/core/compiler/standard_lib.rs | 1 + src/cargo/core/compiler/unit.rs | 4 ++++ src/cargo/core/compiler/unit_dependencies.rs | 2 +- src/cargo/ops/cargo_compile.rs | 2 ++ 5 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index ae627c926f6..54acfebc334 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -1,5 +1,6 @@ use super::job::{Freshness, Job, Work}; use super::{fingerprint, Context, LinkType, Unit}; +use crate::core::compiler::CompileMode; use crate::core::compiler::context::Metadata; use crate::core::compiler::job_queue::JobState; use crate::core::{profiles::ProfileRoot, PackageId, Target}; @@ -201,6 +202,19 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { .env("HOST", &bcx.host_triple()) .env("RUSTC", &bcx.rustc().path) .env("RUSTDOC", &*bcx.config.rustdoc()?) + .env( + "CARGO_MODE", + match unit.root_mode { + CompileMode::Test => "Test", + CompileMode::Build => "Build", + CompileMode::Check { test } => if test { "Check_test" } else { "Check" }, + CompileMode::Bench => "Bench", + CompileMode::Doc { deps } => if deps { "Doc_with_deps" } else { "Doc" }, + CompileMode::Doctest => "Doctest", + CompileMode::Docscrape => "Docscrape", + CompileMode::RunCustomBuild => "RunCustomBuild", + }, + ) .inherit_jobserver(&cx.jobserver); if let Some(linker) = &bcx.target_data.target_config(unit.kind).linker { diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 6b76a5681be..7b0046b2d88 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -176,6 +176,7 @@ pub fn generate_std_roots( profile, *kind, mode, + mode, features.clone(), /*is_std*/ true, /*dep_hash*/ 0, diff --git a/src/cargo/core/compiler/unit.rs b/src/cargo/core/compiler/unit.rs index 71b4538c4f9..ab8eb5994bf 100644 --- a/src/cargo/core/compiler/unit.rs +++ b/src/cargo/core/compiler/unit.rs @@ -52,6 +52,8 @@ pub struct UnitInner { pub kind: CompileKind, /// The "mode" this unit is being compiled for. See [`CompileMode`] for more details. pub mode: CompileMode, + /// The "mode" of the root unit. Required when unit's mode is `CompileMode::RunCustomBuild` + pub root_mode: CompileMode, /// The `cfg` features to enable for this unit. /// This must be sorted. pub features: Vec, @@ -176,6 +178,7 @@ impl UnitInterner { profile: Profile, kind: CompileKind, mode: CompileMode, + root_mode: CompileMode, features: Vec, is_std: bool, dep_hash: u64, @@ -207,6 +210,7 @@ impl UnitInterner { profile, kind, mode, + root_mode, features, is_std, dep_hash, diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index d89d416b1a6..cc2b34bf8ed 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -641,7 +641,7 @@ fn new_unit_dep_with_profile( let features = state.activated_features(pkg.package_id(), features_for); let unit = state .interner - .intern(pkg, target, profile, kind, mode, features, state.is_std, 0); + .intern(pkg, target, profile, kind, mode, parent.mode, features, state.is_std, 0); Ok(UnitDep { unit, unit_for, diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 81b43124a18..bc1052d9adc 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -1048,6 +1048,7 @@ fn generate_targets( profile, kind.for_target(target), target_mode, + target_mode, features.clone(), /*is_std*/ false, /*dep_hash*/ 0, @@ -1609,6 +1610,7 @@ fn traverse_and_share( unit.profile, new_kind, unit.mode, + unit.root_mode, unit.features.clone(), unit.is_std, new_dep_hash, From be8c224a8c60ee3fa314c3cfaf710a1ef20ddf3a Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 27 Nov 2021 01:51:40 -0500 Subject: [PATCH 2/4] fix formatting errors --- src/cargo/core/compiler/custom_build.rs | 18 +++++++++++++++--- src/cargo/core/compiler/unit_dependencies.rs | 14 +++++++++++--- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 54acfebc334..7784f15f891 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -1,8 +1,8 @@ use super::job::{Freshness, Job, Work}; use super::{fingerprint, Context, LinkType, Unit}; -use crate::core::compiler::CompileMode; use crate::core::compiler::context::Metadata; use crate::core::compiler::job_queue::JobState; +use crate::core::compiler::CompileMode; use crate::core::{profiles::ProfileRoot, PackageId, Target}; use crate::util::errors::CargoResult; use crate::util::machine_message::{self, Message}; @@ -207,9 +207,21 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { match unit.root_mode { CompileMode::Test => "Test", CompileMode::Build => "Build", - CompileMode::Check { test } => if test { "Check_test" } else { "Check" }, + CompileMode::Check { test } => { + if test { + "Check_test" + } else { + "Check" + } + } CompileMode::Bench => "Bench", - CompileMode::Doc { deps } => if deps { "Doc_with_deps" } else { "Doc" }, + CompileMode::Doc { deps } => { + if deps { + "Doc_with_deps" + } else { + "Doc" + } + } CompileMode::Doctest => "Doctest", CompileMode::Docscrape => "Docscrape", CompileMode::RunCustomBuild => "RunCustomBuild", diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index cc2b34bf8ed..0ba15abb2b6 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -639,9 +639,17 @@ fn new_unit_dep_with_profile( .is_public_dep(parent.pkg.package_id(), pkg.package_id()); let features_for = unit_for.map_to_features_for(); let features = state.activated_features(pkg.package_id(), features_for); - let unit = state - .interner - .intern(pkg, target, profile, kind, mode, parent.mode, features, state.is_std, 0); + let unit = state.interner.intern( + pkg, + target, + profile, + kind, + mode, + parent.mode, + features, + state.is_std, + 0, + ); Ok(UnitDep { unit, unit_for, From 5b1862551230a1111768e1ba9e808cec5da3bbd5 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 27 Nov 2021 12:57:20 -0500 Subject: [PATCH 3/4] Per feedback: mode value, naming --- src/cargo/core/compiler/build_config.rs | 10 +++-- .../compiler/build_context/target_info.rs | 2 +- .../compiler/context/compilation_files.rs | 2 +- src/cargo/core/compiler/custom_build.rs | 25 ++----------- src/cargo/core/compiler/job_queue.rs | 2 +- src/cargo/core/compiler/standard_lib.rs | 1 - src/cargo/core/compiler/timings.rs | 2 +- src/cargo/core/compiler/unit.rs | 4 -- src/cargo/core/compiler/unit_dependencies.rs | 37 +++++++++++-------- src/cargo/core/profiles.rs | 4 +- src/cargo/ops/cargo_compile.rs | 8 ++-- 11 files changed, 43 insertions(+), 54 deletions(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 00733f38ab8..4c71a7768b0 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -152,7 +152,8 @@ pub enum CompileMode { /// An example or library that will be scraped for function calls by `rustdoc`. Docscrape, /// A marker for Units that represent the execution of a `build.rs` script. - RunCustomBuild, + /// `root_mode` tells build script which `cargo` command was used to call it. + RunCustomBuild { root_mode: &'static str }, } impl ser::Serialize for CompileMode { @@ -169,7 +170,7 @@ impl ser::Serialize for CompileMode { Doc { .. } => "doc".serialize(s), Doctest => "doctest".serialize(s), Docscrape => "docscrape".serialize(s), - RunCustomBuild => "run-custom-build".serialize(s), + RunCustomBuild { .. } => "run-custom-build".serialize(s), } } } @@ -217,6 +218,9 @@ impl CompileMode { /// Returns `true` if this is the *execution* of a `build.rs` script. pub fn is_run_custom_build(self) -> bool { - self == CompileMode::RunCustomBuild + match self { + CompileMode::RunCustomBuild { .. } => true, + _ => false, + } } } diff --git a/src/cargo/core/compiler/build_context/target_info.rs b/src/cargo/core/compiler/build_context/target_info.rs index f215014edc7..c66be1d294a 100644 --- a/src/cargo/core/compiler/build_context/target_info.rs +++ b/src/cargo/core/compiler/build_context/target_info.rs @@ -455,7 +455,7 @@ impl TargetInfo { CompileMode::Doc { .. } | CompileMode::Doctest | CompileMode::Docscrape - | CompileMode::RunCustomBuild => { + | CompileMode::RunCustomBuild { .. } => { panic!("asked for rustc output for non-rustc mode") } } diff --git a/src/cargo/core/compiler/context/compilation_files.rs b/src/cargo/core/compiler/context/compilation_files.rs index 37ab2520223..78635a12543 100644 --- a/src/cargo/core/compiler/context/compilation_files.rs +++ b/src/cargo/core/compiler/context/compilation_files.rs @@ -408,7 +408,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> { flavor: FileFlavor::Normal, }] } - CompileMode::RunCustomBuild => { + CompileMode::RunCustomBuild { .. } => { // At this time, this code path does not handle build script // outputs. vec![] diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 7784f15f891..0254ff2bfaa 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -204,27 +204,10 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { .env("RUSTDOC", &*bcx.config.rustdoc()?) .env( "CARGO_MODE", - match unit.root_mode { - CompileMode::Test => "Test", - CompileMode::Build => "Build", - CompileMode::Check { test } => { - if test { - "Check_test" - } else { - "Check" - } - } - CompileMode::Bench => "Bench", - CompileMode::Doc { deps } => { - if deps { - "Doc_with_deps" - } else { - "Doc" - } - } - CompileMode::Doctest => "Doctest", - CompileMode::Docscrape => "Docscrape", - CompileMode::RunCustomBuild => "RunCustomBuild", + if let CompileMode::RunCustomBuild { root_mode } = unit.mode { + root_mode + } else { + panic!("Unexpected mode {:?}", unit.mode) }, ) .inherit_jobserver(&cx.jobserver); diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs index 322205d6ebb..a68146026b8 100644 --- a/src/cargo/core/compiler/job_queue.rs +++ b/src/cargo/core/compiler/job_queue.rs @@ -934,7 +934,7 @@ impl<'cfg> DrainState<'cfg> { let target_name = unit.target.name(); match unit.mode { CompileMode::Doc { .. } => format!("{}(doc)", pkg_name), - CompileMode::RunCustomBuild => format!("{}(build)", pkg_name), + CompileMode::RunCustomBuild { .. } => format!("{}(build)", pkg_name), CompileMode::Test | CompileMode::Check { test: true } => match unit.target.kind() { TargetKind::Lib(_) => format!("{}(test)", target_name), TargetKind::CustomBuild => panic!("cannot test build script"), diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 7b0046b2d88..6b76a5681be 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -176,7 +176,6 @@ pub fn generate_std_roots( profile, *kind, mode, - mode, features.clone(), /*is_std*/ true, /*dep_hash*/ 0, diff --git a/src/cargo/core/compiler/timings.rs b/src/cargo/core/compiler/timings.rs index 33b46ce1671..172eea8c230 100644 --- a/src/cargo/core/compiler/timings.rs +++ b/src/cargo/core/compiler/timings.rs @@ -177,7 +177,7 @@ impl<'cfg> Timings<'cfg> { CompileMode::Doc { .. } => target.push_str(" (doc)"), CompileMode::Doctest => target.push_str(" (doc test)"), CompileMode::Docscrape => target.push_str(" (doc scrape)"), - CompileMode::RunCustomBuild => target.push_str(" (run)"), + CompileMode::RunCustomBuild { .. } => target.push_str(" (run)"), } let unit_time = UnitTime { unit, diff --git a/src/cargo/core/compiler/unit.rs b/src/cargo/core/compiler/unit.rs index ab8eb5994bf..71b4538c4f9 100644 --- a/src/cargo/core/compiler/unit.rs +++ b/src/cargo/core/compiler/unit.rs @@ -52,8 +52,6 @@ pub struct UnitInner { pub kind: CompileKind, /// The "mode" this unit is being compiled for. See [`CompileMode`] for more details. pub mode: CompileMode, - /// The "mode" of the root unit. Required when unit's mode is `CompileMode::RunCustomBuild` - pub root_mode: CompileMode, /// The `cfg` features to enable for this unit. /// This must be sorted. pub features: Vec, @@ -178,7 +176,6 @@ impl UnitInterner { profile: Profile, kind: CompileKind, mode: CompileMode, - root_mode: CompileMode, features: Vec, is_std: bool, dep_hash: u64, @@ -210,7 +207,6 @@ impl UnitInterner { profile, kind, mode, - root_mode, features, is_std, dep_hash, diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 0ba15abb2b6..87c78495c78 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -564,6 +564,21 @@ fn dep_build_script( // once because it would break a large number of scripts (they // would think they have the wrong set of features enabled). let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features()); + // Root mode is used to tell build script which cargo command invoked it + let root_mode = match &unit.mode { + CompileMode::Test => "test", + CompileMode::Build => "build", + CompileMode::Check { .. } => "check", + CompileMode::Bench => "bench", + CompileMode::Doc { .. } => "doc", + + // FIXME: Are Doctest and Docscrape possible here, and if so, what should the values be? + CompileMode::Doctest => "doctest", + CompileMode::Docscrape => "docscrape", + + CompileMode::RunCustomBuild { .. } => panic!("Nested {:?}", unit.mode), + }; + new_unit_dep_with_profile( state, unit, @@ -571,7 +586,7 @@ fn dep_build_script( t, script_unit_for, unit.kind, - CompileMode::RunCustomBuild, + CompileMode::RunCustomBuild { root_mode }, profile, ) }) @@ -639,17 +654,9 @@ fn new_unit_dep_with_profile( .is_public_dep(parent.pkg.package_id(), pkg.package_id()); let features_for = unit_for.map_to_features_for(); let features = state.activated_features(pkg.package_id(), features_for); - let unit = state.interner.intern( - pkg, - target, - profile, - kind, - mode, - parent.mode, - features, - state.is_std, - 0, - ); + let unit = state + .interner + .intern(pkg, target, profile, kind, mode, features, state.is_std, 0); Ok(UnitDep { unit, unit_for, @@ -682,7 +689,7 @@ fn connect_run_custom_build_deps(state: &mut State<'_, '_>) { let mut reverse_deps_map = HashMap::new(); for (unit, deps) in state.unit_dependencies.iter() { for dep in deps { - if dep.unit.mode == CompileMode::RunCustomBuild { + if dep.unit.mode.is_run_custom_build() { reverse_deps_map .entry(dep.unit.clone()) .or_insert_with(HashSet::new) @@ -703,7 +710,7 @@ fn connect_run_custom_build_deps(state: &mut State<'_, '_>) { for unit in state .unit_dependencies .keys() - .filter(|k| k.mode == CompileMode::RunCustomBuild) + .filter(|k| k.mode.is_run_custom_build()) { // This list of dependencies all depend on `unit`, an execution of // the build script. @@ -752,7 +759,7 @@ fn connect_run_custom_build_deps(state: &mut State<'_, '_>) { .filter_map(|other| { state.unit_dependencies[&other.unit] .iter() - .find(|other_dep| other_dep.unit.mode == CompileMode::RunCustomBuild) + .find(|other_dep| other_dep.unit.mode.is_run_custom_build()) .cloned() }) .collect::>(); diff --git a/src/cargo/core/profiles.rs b/src/cargo/core/profiles.rs index fe88e724e28..be69df6e967 100644 --- a/src/cargo/core/profiles.rs +++ b/src/cargo/core/profiles.rs @@ -312,7 +312,9 @@ impl Profiles { ) } } - CompileMode::Build | CompileMode::Check { .. } | CompileMode::RunCustomBuild => { + CompileMode::Build + | CompileMode::Check { .. } + | CompileMode::RunCustomBuild { .. } => { // Note: `RunCustomBuild` doesn't normally use this code path. // `build_unit_profiles` normally ensures that it selects the // ancestor's profile. However, `cargo clean -p` can hit this diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index bc1052d9adc..63f1a8ca1a8 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -348,7 +348,7 @@ pub fn create_bcx<'a, 'cfg>( | CompileMode::Build | CompileMode::Check { .. } | CompileMode::Bench - | CompileMode::RunCustomBuild => { + | CompileMode::RunCustomBuild { .. } => { if std::env::var("RUST_FLAGS").is_ok() { config.shell().warn( "Cargo does not read `RUST_FLAGS` environment variable. Did you mean `RUSTFLAGS`?", @@ -844,7 +844,7 @@ impl CompileFilter { .. } => examples.is_specific() || tests.is_specific() || benches.is_specific(), }, - CompileMode::RunCustomBuild => panic!("Invalid mode"), + CompileMode::RunCustomBuild { .. } => panic!("Invalid mode"), } } @@ -1048,7 +1048,6 @@ fn generate_targets( profile, kind.for_target(target), target_mode, - target_mode, features.clone(), /*is_std*/ false, /*dep_hash*/ 0, @@ -1434,7 +1433,7 @@ fn filter_default_targets(targets: &[Target], mode: CompileMode) -> Vec<&Target> }) .collect() } - CompileMode::Doctest | CompileMode::Docscrape | CompileMode::RunCustomBuild => { + CompileMode::Doctest | CompileMode::Docscrape | CompileMode::RunCustomBuild { .. } => { panic!("Invalid mode {:?}", mode) } } @@ -1610,7 +1609,6 @@ fn traverse_and_share( unit.profile, new_kind, unit.mode, - unit.root_mode, unit.features.clone(), unit.is_std, new_dep_hash, From 15c1393ff37f1537195df8efaf0d0445939cd48d Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 27 Nov 2021 13:39:34 -0500 Subject: [PATCH 4/4] remove ref to enum --- src/cargo/core/compiler/unit_dependencies.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 87c78495c78..710a579066d 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -565,7 +565,7 @@ fn dep_build_script( // would think they have the wrong set of features enabled). let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features()); // Root mode is used to tell build script which cargo command invoked it - let root_mode = match &unit.mode { + let root_mode = match unit.mode { CompileMode::Test => "test", CompileMode::Build => "build", CompileMode::Check { .. } => "check",