Skip to content

Commit

Permalink
Auto merge of #8742 - ehuss:proc-macro-test-feature, r=alexcrichton
Browse files Browse the repository at this point in the history
Properly set for_host for proc-macro tests.

Proc-macro tests are currently forced to run for the host target (by [this line of code](https://github.com/rust-lang/cargo/blob/898ccde7ac867ecdb62184714b379c4328409399/src/cargo/ops/cargo_compile.rs#L819)). However, the code that builds the dependency graph wasn't playing by the same rules, and was building the proc-macro test as-if it was "not for_host".  This would cause the proc-macro test to pull in the wrong set of dependencies/features with the new feature resolver.

Forcing proc-macro tests to run on host isn't 100% required, since most targets have the proc_macro crate available. However, I feel like it simplifies things to build for-host. I was thinking we could relax that requirement, but I'm not really sure.  See also #4336 where there's an bug if you do specify `--target`.

Tested with the wasmtime repo running `cargo test -Zfeatures=all -p wiggle-macro` with `doctest = false` commented out of `crates/wiggle/macro/Cargo.toml`.

Fixes #8563.
  • Loading branch information
bors committed Sep 29, 2020
2 parents 898ccde + 6fe6595 commit 0863077
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,13 @@ fn deps_of_roots(roots: &[Unit], mut state: &mut State<'_, '_>) -> CargoResult<(
// without, once for `--test`). In particular, the lib included for
// Doc tests and examples are `Build` mode here.
let unit_for = if unit.mode.is_any_test() || state.global_mode.is_rustc_test() {
UnitFor::new_test(state.config)
if unit.target.proc_macro() {
// Special-case for proc-macros, which are forced to for-host
// since they need to link with the proc_macro crate.
UnitFor::new_host_test(state.config)
} else {
UnitFor::new_test(state.config)
}
} else if unit.target.is_custom_build() {
// This normally doesn't happen, except `clean` aggressively
// generates all units.
Expand Down
10 changes: 10 additions & 0 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,16 @@ impl UnitFor {
}
}

/// This is a special case for unit tests of a proc-macro.
///
/// Proc-macro unit tests are forced to be run on the host.
pub fn new_host_test(config: &Config) -> UnitFor {
let mut unit_for = UnitFor::new_test(config);
unit_for.host = true;
unit_for.host_features = true;
unit_for
}

/// Returns a new copy based on `for_host` setting.
///
/// When `for_host` is true, this clears `panic_abort_ok` in a sticky
Expand Down
82 changes: 82 additions & 0 deletions tests/testsuite/features2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1864,3 +1864,85 @@ warning: feat: enabled
)
.run();
}

#[cargo_test]
fn test_proc_macro() {
// Running `cargo test` on a proc-macro, with a shared dependency that has
// different features.
//
// There was a bug where `shared` was built twice (once with feature "B"
// and once without), and both copies linked into the unit test. This
// would cause a type failure when used in an intermediate dependency
// (the-macro-support).
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "runtime"
version = "0.1.0"
[dependencies]
the-macro = { path = "the-macro", features = ['a'] }
[build-dependencies]
shared = { path = "shared", features = ['b'] }
"#,
)
.file("src/lib.rs", "")
.file(
"the-macro/Cargo.toml",
r#"
[package]
name = "the-macro"
version = "0.1.0"
[lib]
proc-macro = true
test = false
[dependencies]
the-macro-support = { path = "../the-macro-support" }
shared = { path = "../shared" }
[dev-dependencies]
runtime = { path = ".." }
[features]
a = []
"#,
)
.file(
"the-macro/src/lib.rs",
"
fn _test() {
the_macro_support::foo(shared::Foo);
}
",
)
.file(
"the-macro-support/Cargo.toml",
r#"
[package]
name = "the-macro-support"
version = "0.1.0"
[dependencies]
shared = { path = "../shared" }
"#,
)
.file(
"the-macro-support/src/lib.rs",
"
pub fn foo(_: shared::Foo) {}
",
)
.file(
"shared/Cargo.toml",
r#"
[package]
name = "shared"
version = "0.1.0"
[features]
b = []
"#,
)
.file("shared/src/lib.rs", "pub struct Foo;")
.build();
p.cargo("test -Zfeatures=all --manifest-path the-macro/Cargo.toml")
.masquerade_as_nightly_cargo()
.run();
}

0 comments on commit 0863077

Please sign in to comment.