Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unit_for computation on proc-macros in shared workspace. #9059

Merged
merged 1 commit into from
Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -417,9 +414,7 @@ fn compute_deps_doc(unit: &Unit, state: &mut State<'_, '_>) -> CargoResult<Vec<U
// Rustdoc only needs rmeta files for regular dependencies.
// However, for plugins/proc macros, deps should be built like normal.
let mode = check_or_build_mode(unit.mode, lib);
let dep_unit_for = UnitFor::new_normal()
.with_for_host(lib.for_host())
.with_host_features(lib.proc_macro());
let dep_unit_for = UnitFor::new_normal().with_dependency(unit, lib);
let lib_unit_dep = new_unit_dep(
state,
unit,
Expand Down Expand Up @@ -466,12 +461,13 @@ fn maybe_lib(
.find(|t| t.is_linkable())
.map(|t| {
let mode = check_or_build_mode(unit.mode, t);
let dep_unit_for = unit_for.with_dependency(unit, t);
new_unit_dep(
state,
unit,
&unit.pkg,
t,
unit_for,
dep_unit_for,
unit.kind.for_target(t),
mode,
)
Expand Down
60 changes: 31 additions & 29 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::core::compiler::CompileMode;
use crate::core::compiler::{CompileMode, Unit};
use crate::core::resolver::features::FeaturesFor;
use crate::core::{Feature, Features, PackageId, PackageIdSpec, Resolve, Shell};
use crate::core::{Feature, Features, PackageId, PackageIdSpec, Resolve, Shell, Target};
use crate::util::errors::CargoResultExt;
use crate::util::interning::InternedString;
use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool};
Expand Down Expand Up @@ -976,36 +976,38 @@ impl UnitFor {
unit_for
}

/// Returns a new copy based on `for_host` setting.
/// Returns a new copy updated based on the target dependency.
///
/// When `for_host` is true, this clears `panic_abort_ok` in a sticky
/// fashion so that all its dependencies also have `panic_abort_ok=false`.
/// This'll help ensure that once we start compiling for the host platform
/// (build scripts, plugins, proc macros, etc) we'll share the same build
/// graph where everything is `panic=unwind`.
pub fn with_for_host(self, for_host: bool) -> 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
Expand Down
130 changes: 130 additions & 0 deletions tests/testsuite/features2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}