From a84689870f669acee11786cf20fe5bd42e931608 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sat, 9 Jan 2021 15:18:28 -0800 Subject: [PATCH] Fix unit_for computation on proc-macros in shared workspace. --- src/cargo/core/compiler/unit_dependencies.rs | 12 +- src/cargo/core/profiles.rs | 60 ++++----- tests/testsuite/features2.rs | 130 +++++++++++++++++++ 3 files changed, 165 insertions(+), 37 deletions(-) diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 40f179898e3..e58d2dfc053 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -265,10 +265,7 @@ fn compute_deps( None => continue, }; let mode = check_or_build_mode(unit.mode, lib); - let dep_unit_for = unit_for - .with_for_host(lib.for_host()) - // If it is a custom build script, then it *only* has build dependencies. - .with_host_features(unit.target.is_custom_build() || lib.proc_macro()); + let dep_unit_for = unit_for.with_dependency(unit, lib); if state.config.cli_unstable().dual_proc_macros && lib.proc_macro() && !unit.kind.is_host() { @@ -417,9 +414,7 @@ fn compute_deps_doc(unit: &Unit, state: &mut State<'_, '_>) -> CargoResult UnitFor { + /// This is where the magic happens that the host/host_features settings + /// transition in a sticky fashion. As the dependency graph is being + /// built, once those flags are set, they stay set for the duration of + /// that portion of tree. + pub fn with_dependency(self, parent: &Unit, dep_target: &Target) -> UnitFor { + // A build script or proc-macro transitions this to being built for the host. + let dep_for_host = dep_target.for_host(); + // This is where feature decoupling of host versus target happens. + // + // Once host features are desired, they are always desired. + // + // A proc-macro should always use host features. + // + // Dependencies of a build script should use host features (subtle + // point: the build script itself does *not* use host features, that's + // why the parent is checked here, and not the dependency). + let host_features = + self.host_features || parent.target.is_custom_build() || dep_target.proc_macro(); + // Build scripts and proc macros, and all of their dependencies are + // AlwaysUnwind. + let panic_setting = if dep_for_host { + PanicSetting::AlwaysUnwind + } else { + self.panic_setting + }; UnitFor { - host: self.host || for_host, - host_features: self.host_features, - panic_setting: if for_host { - PanicSetting::AlwaysUnwind - } else { - self.panic_setting - }, - } - } - - /// Returns a new copy updating it whether or not it should use features - /// for build dependencies and proc-macros. - /// - /// This is part of the machinery responsible for handling feature - /// decoupling for build dependencies in the new feature resolver. - pub fn with_host_features(mut self, host_features: bool) -> UnitFor { - if host_features { - assert!(self.host); + host: self.host || dep_for_host, + host_features, + panic_setting, } - self.host_features = self.host_features || host_features; - self } /// Returns `true` if this unit is for a build script or any of its diff --git a/tests/testsuite/features2.rs b/tests/testsuite/features2.rs index 3f943d8aa44..cb0a57b672a 100644 --- a/tests/testsuite/features2.rs +++ b/tests/testsuite/features2.rs @@ -2147,3 +2147,133 @@ foo v0.1.0 ([ROOT]/foo) .run(); clear(); } + +#[cargo_test] +fn pm_with_int_shared() { + // This is a somewhat complex scenario of a proc-macro in a workspace with + // an integration test where the proc-macro is used for other things, and + // *everything* is built at once (`--workspace --all-targets + // --all-features`). There was a bug where the UnitFor settings were being + // incorrectly computed based on the order that the graph was traversed. + // + // There are some uncertainties about exactly how proc-macros should behave + // with `--workspace`, see https://github.com/rust-lang/cargo/issues/8312. + // + // This uses a const-eval hack to do compile-time feature checking. + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + members = ["foo", "pm", "shared"] + resolver = "2" + "#, + ) + .file( + "foo/Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2018" + + [dependencies] + pm = { path = "../pm" } + shared = { path = "../shared", features = ["norm-feat"] } + "#, + ) + .file( + "foo/src/lib.rs", + r#" + // foo->shared always has both features set + const _CHECK: [(); 0] = [(); 0-!(shared::FEATS==3) as usize]; + "#, + ) + .file( + "pm/Cargo.toml", + r#" + [package] + name = "pm" + version = "0.1.0" + + [lib] + proc-macro = true + + [dependencies] + shared = { path = "../shared", features = ["host-feat"] } + "#, + ) + .file( + "pm/src/lib.rs", + r#" + // pm->shared always has just host + const _CHECK: [(); 0] = [(); 0-!(shared::FEATS==1) as usize]; + "#, + ) + .file( + "pm/tests/pm_test.rs", + r#" + // integration test gets both set + const _CHECK: [(); 0] = [(); 0-!(shared::FEATS==3) as usize]; + "#, + ) + .file( + "shared/Cargo.toml", + r#" + [package] + name = "shared" + version = "0.1.0" + + [features] + norm-feat = [] + host-feat = [] + "#, + ) + .file( + "shared/src/lib.rs", + r#" + pub const FEATS: u32 = { + if cfg!(feature="norm-feat") && cfg!(feature="host-feat") { + 3 + } else if cfg!(feature="norm-feat") { + 2 + } else if cfg!(feature="host-feat") { + 1 + } else { + 0 + } + }; + "#, + ) + .build(); + + p.cargo("build --workspace --all-targets --all-features -v") + .with_stderr_unordered( + "\ +[COMPILING] shared [..] +[RUNNING] `rustc --crate-name shared [..]--crate-type lib [..] +[RUNNING] `rustc --crate-name shared [..]--crate-type lib [..] +[RUNNING] `rustc --crate-name shared [..]--test[..] +[COMPILING] pm [..] +[RUNNING] `rustc --crate-name pm [..]--crate-type proc-macro[..] +[RUNNING] `rustc --crate-name pm [..]--test[..] +[COMPILING] foo [..] +[RUNNING] `rustc --crate-name foo [..]--test[..] +[RUNNING] `rustc --crate-name pm_test [..]--test[..] +[RUNNING] `rustc --crate-name foo [..]--crate-type lib[..] +[FINISHED] [..] +", + ) + .run(); + + // And again, should stay fresh. + p.cargo("build --workspace --all-targets --all-features -v") + .with_stderr_unordered( + "\ +[FRESH] shared [..] +[FRESH] pm [..] +[FRESH] foo [..] +[FINISHED] [..]", + ) + .run(); +}