Skip to content

Commit

Permalink
Fix unit_for computation on proc-macros in shared workspace.
Browse files Browse the repository at this point in the history
  • Loading branch information
ehuss committed Jan 9, 2021
1 parent 94e21ad commit a846898
Show file tree
Hide file tree
Showing 3 changed files with 165 additions and 37 deletions.
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();
}

0 comments on commit a846898

Please sign in to comment.