Proc macro crates not given transitive features when they are enabled by other crates in the workspace

[cormacrelf]

Problem

Take the Cargo.toml examples below as a workspace. Due to #4463, cargo build --workspace gives the lib crate features=["feat"] when compiling a dependent crate that does not normally enable it. In this case, the proc macro used when compiling that dependent (here an integration test) is not compiled with features=["feat"], when it should.

This is a problem because it means you can get builds where lib, with the feature enabled, requires a proc macro to behave differently (e.g. implement an additional trait), but has to deal with output from a macro that didn't have the feature enabled (and doesn't implement the trait).

(I'm not sure this requires feat = ["lib-derive/feat"] to occur, it may be that proc macros simply do not participate in workspace-level resolution, but I would consider that a bug as well. I just don't have a repro handy. Also, I suspect this issue would survive changing #4463 behaviour to 'one lib per set of features used by workspace crates' as was suggested over there.)

# lib-derive/Cargo.toml
[lib]
proc-macro = true
[features]
feat = []

# lib/Cargo.toml
[features]
feat = ["lib-derive/feat"]
[dependencies]
lib-derive = { path = "../lib-derive" }

# other_dep_using_feat/Cargo.toml
[dependencies]
lib = { path = "../lib", features = ["feat"] }

# THE AFFECTED CRATE
# integration_test/Cargo.toml
[dependencies]
lib = { path = "../lib" }

Steps

1. Checkout https://github.com/cormacrelf/juniper/tree/cargo_resolver_issue (that branch fixes many unrelated compile errors)
2. Observe three cases:
   1. Observe cargo test -p juniper_tests compiling correctly
   2. With -Z package-features or cd integration_tests/juniper_tests, observe cargo test --features async also compiling
   3. Observe the compilation failure for juniper_tests when running with cargo test from the workspace root:

error[E0277]: the trait bound `i64: juniper::types::async_await::GraphQLTypeAsync<custom_scalar::MyScalarValue>` is not satisfied
   --> integration_tests/juniper_tests/src/custom_scalar.rs:163:1
    |
163 | / #[juniper::graphql_object(
164 | |     Scalar = MyScalarValue
165 | | )]
    | |__^ the trait `juniper::types::async_await::GraphQLTypeAsync<custom_scalar::MyScalarValue>` is not implemented for `i64`

Note that the failure occurs when compiling the macro output, essentially linking up juniper_codegen-provided trait implementations for MyScalarValue with bounds required by juniper. Those bounds include GraphQLTypeAsync when the juniper crate itself has feature = "async" enabled. GraphQLTypeAsync gets implemented by graphql_scalar! when feature = "async" is enabled when compiling the juniper_codegen proc macro crate.

Case (i) works because feature = "async" is disabled in both juniper and juniper_codegen, so the trait is neither implemented nor required. Case (ii) works because feature = "async" is enabled everywhere, so the trait gets implemented and required. But for case (iii), apparently the proc macro is not compiled with feature = "async", but the juniper crate is, hence the trait being required but not implemented.

(One note: juniper_tests does have its own [features] async = ["juniper/async", "futures"]. Not sure how that is affecting things.)

Output of cargo version

cargo 1.42.0-nightly (f6449ba23 2020-01-21)

[ehuss]

It's a bit hard to untangle your project, but I don't think it is directly related to how the proc_macro is being built. juniper_codegen has the async feature set in both cases. It looks like there is some kind of side-effect from including the futures crate. For example, if you cd into integration_tests/juniper_tests and run cargo test --features juniper/async you'll get the same error. It's not really evident to me how the absence of the futures crate causes that to fail.

If you can possibly rework the code to not require both features to be set at the same time, that might help. Another possibility is to write some scripts that will run the individual commands needed to run all the tests you need.

Otherwise, there are some other existing feature requests that could help (like better specification of features in a workspace or automatic features).

[ehuss]

I'm going to close this as a mixture of expected behavior, and other issues tracking similar enhancements. In this particular case, a package appears to require two features set at once, and fails to build if only one of them is set. Features fundamentally expect any combination to work. Other issues like #4463, "automatic" features, "mutually exclusive" features, and many others may also cover this.