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

cargo build --all --all-features in a non-virtual workspace ignores features not present in the root crate #5518

Closed
Ortham opened this issue May 11, 2018 · 5 comments · Fixed by #5556

Comments

@Ortham
Copy link

Ortham commented May 11, 2018

I've got a couple of projects (example) that are workspaces with a root crate and another member in an ffi subdirectory. The ffi member has an ffi-headers feature that uses cbindgen to generate C/C++ headers.

I expect to be able to run cargo build --release --all --all-features and have all crates in the workspace built with all their features. This was working in Rust 1.25.0, I was able to run that command and see C/C++ headers get generated.

I upgraded to the Rust 1.26.0 release and now the above command no longer generates headers, it's as if the feature in the ffi crate is being ignored (the issue title may be a little inaccurate, I haven't verified this is what's actually happening). If I run cargo +1.25.0 build --release --all --all-features, everything works as expected.

This might be related to #5362 and #5364, but they're more about --package than --all so I thought I'd file a new issue. If I run cargo +nightly build --release --all --all-features, that doesn't work, but cargo +nightly build --release --all --all-features -Z package-features does.

My current version of cargo is:

> cargo --version
cargo 1.26.0 (0e7c5a931 2018-04-06)

Running with the 1.25.0 toolchain:

> cargo +1.25.0 --version
cargo 0.26.0 (41480f5cc 2018-02-26)

Running with nightly:

> cargo +nightly --version
cargo 1.27.0-nightly (9e53ac6e6 2018-05-07)
Ortham added a commit to loot/libloot that referenced this issue May 11, 2018
I've filed rust-lang/cargo#5518 about this as the previous command worked as expected in Rust 1.25.0. It's probably not a bad idea to be more specific though.
Ortham added a commit to loot/libloot that referenced this issue May 11, 2018
I've filed rust-lang/cargo#5518 about this as the previous command worked as expected in Rust 1.25.0. It's probably not a bad idea to be more specific though.
@alexcrichton
Copy link
Member

Thanks for the report! Would it be possible to minimize this a bit perhaps to be easier to investigate?

@ehuss you may also be interested in this issue

@Ortham
Copy link
Author

Ortham commented May 20, 2018

Yes, I've uploaded a minimal example repository here. Running it (with cargo versions unchanged from what I gave above):

PS C:\...\cargo-features-issue> cargo +1.25.0 build --release --all --all-features
   Compiling cargo-features-issue v0.1.0 (file:///C:/.../cargo-features-issue)
    Finished release [optimized] target(s) in 0.44 secs
PS C:\...\cargo-features-issue> .\target\release\member.exe
I have member-feature active!
PS C:\...\cargo-features-issue> cargo build --release --all --all-features
   Compiling cargo-features-issue v0.1.0 (file:///C:/.../cargo-features-issue)
    Finished release [optimized] target(s) in 0.66 secs
PS C:\...\cargo-features-issue> .\target\release\member.exe
I do not have member-feature active!

alexcrichton added a commit to alexcrichton/cargo that referenced this issue May 21, 2018
This fixes an accidental regression introduced in rust-lang#5012 where the
`--all-features` CLI flag was only propagated to the "main crate" as opposed to
all workspace packages. This behavior has [already been deemed][pr] as
"basically not what you want", but for now it's best to avoid the regression.

Closes rust-lang#5518

[pr]: rust-lang#5353
@alexcrichton
Copy link
Member

Ok thanks for the reduction! I can indeed reproduce this locally now. I've bisected this regression to 7de30dd (part of #5012, cc @infinity0)

I believe this should be fixed in #5556

@infinity0
Copy link
Contributor

infinity0 commented May 23, 2018

Sorry about that, thanks for catching. In general I think it's an anti-pattern to have enums with options like { AllSettings: {records}, Default, Preset1, Preset2 } etc, I've seen far too much of this in various rust code. It would be better to have a single {records} type with functions called default() preset1() etc that return presets, with equality defined so one can still match against the presets if needed.

bors added a commit that referenced this issue May 24, 2018
Copy `--all-features` request to all workspace members

This fixes an accidental regression introduced in #5012 where the
`--all-features` CLI flag was only propagated to the "main crate" as opposed to
all workspace packages. This behavior has [already been deemed][pr] as
"basically not what you want", but for now it's best to avoid the regression.

Closes #5518

[pr]: #5353
alexcrichton added a commit to alexcrichton/cargo that referenced this issue May 24, 2018
This fixes an accidental regression introduced in rust-lang#5012 where the
`--all-features` CLI flag was only propagated to the "main crate" as opposed to
all workspace packages. This behavior has [already been deemed][pr] as
"basically not what you want", but for now it's best to avoid the regression.

Closes rust-lang#5518

[pr]: rust-lang#5353
@Ortham
Copy link
Author

Ortham commented May 25, 2018

I just pulled and built 22c0f22 and can confirm it doesn't have the issue, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants