Help us test the breaking bug-fix to Cargo features

[matklad]

TL;DR: if you use any of --package, --feature, --all-features, --no-default-features flags in your build, could you check that on the latest nightly, if you add -Z package-features flag as well, everything continues to work?

Hi! We’ve recently noticed a rather surprising interaction of Cargo --feature and --package flags. For command like cargo build --package foo --feature feat, the feature would be activated not for the foo package, but for the package at current working directory. Moreover, features of current package were always activated, so cargo build --package foo might have activated more features than strictly necessary for building foo! We would like to make this behavior more intuitive, and always apply features to the explicitly selected package. This is a change in behavior, so, for now, it is hidden behind an opt-in -Z package-features flag. We would like to access the real-world impact of this change, so please try building your projects with this opt-in!

Tracking issue: https://github.com/rust-lang/cargo/issues/5364 implementation PR: https://github.com/rust-lang/cargo/pull/5353 (includes more details about what exactly has changed)

[matklad]

Oh, as usual, we are interested in both “my build is broken!!!111” and “everything works!” reports

[Nemo157]

Does this also affect virtual manifest handling? I’ve definitely noticed when working with futures that there is no way to activate features for sub-packages.

If yes, I assume this would allow changing all the --manifest-path <package>/Cargo.toml in https://github.com/rust-lang-nursery/futures-rs/blob/master/.travis.yml#L21 to just --package <package> (at least on nightly).

[matklad]

Yep, -Z package-features -p package should work with virtual manifests just fine!

[Nemo157]

And testing it myself, it seems to work perfectly for futures, having --no-default-features apply to all packages and --features actually error out insteads of just being ignored is great

Even playing round with a non-virtual workspace I noticed that --features without --package correctly detects when there’s a single or multiple default-members to apply to

One potential addition: allow specifying features on packages in the current workspace when doing a full build by referring to them via <package>/<feature>, e.g. instead of having to run cargo build --package futures --features nightly it could be possible to run cargo build --features futures/nightly and have that enable the correct features on all packages based on the features specified on the command line.

Not sure how useful that would be in practice, just an idea I had while thinking about how futures Travis config could be changed with this.

[matklad]

One potential addition:

Yeah, it is true that today there’s simply no syntax for specifying features for different packages, and that we should probably add it. However, we might need to fix https://github.com/rust-lang/cargo/issues/4463 first, which would be not so trivial

[dhardy]

Testing Rand since we have quite a few feature configurations and multiple crates.

I see this is no longer possible (though the command may not have worked correctly before anyway — I guess we have to test each crate individually):

$ cargo test --all --features=alloc -Z package-features
error: cannot specify features for more than one package

This happens to fix a “bug” we had when testing with the following command, plus “std” enabled by default in rand_core:

cargo test --tests --no-default-features --features=alloc --all

Other than that, no problems. I guess we just have to update our Travis scripts.

[matklad]

I see this is no longer possible

Indeed. The previous semantics for this was … curious, now it just errors :0)

[parasyte]

Yep, -Z package-features -p package should work with virtual manifests just fine!

This fixes the behavior in my case! I was also using the --manifest-path workaround.

[kevinaboos]

Thanks! It seems to work properly in my virtual workspace (for a no_std OS project).

However, I can’t figure out how to specify features for multiple packages within my workspace. For example, I’d like to do the following:

cargo build -Z package-features --package crate1 --feature featA --package crate2 --feature featB

This gives me the error:

error: The argument '--features <FEATURES>' was provided more than once, but cannot be used multiple times

or if I try to enable the same feature for multiple packages, like so:

cargo build -Z package-features --package crate1 --package crate2 --feature myfeat

I get the error:

error: cannot specify features for more than one package

I assume there’s a way to do this, but it’s not clear from the new package-features discussion.

[matklad]

I assume there’s a way to do this, but it’s not clear from the new package-features discussion.

No, there’s no syntax to enable several specific features for several packages, you’ll need to invoke cargo build multiple times.

[kevinaboos]

No, there’s no syntax to enable several specific features for several packages, you’ll need to invoke cargo build multiple times.

I see, thanks. BTW, it also doesn’t work at all with cargo build --all, whereas the old way of doing things with --manifest-path did work.

[matklad]

Could you elaborate what exactly stopped working? If you specify -all, then only -all-features and -no-default-features flags are supported. —features does not work, because it is ambiguous as to which package the feature should apply.

I guess previously it was possible to do something like Cargo build —all —manifest-path foo/Cargo.toml —feature bar, but it doesn’t look that useful?

[kevinaboos]

Could you elaborate what exactly stopped working? If you specify -all, then only -all-features and -no-default-features flags are supported. —features does not work, because it is ambiguous as to which package the feature should apply.

Yes, it does make sense that --features wouldn’t work by itself with --all, that’s why I was hoping to use the -Z package-features flag to specify which packages a given feature should apply to.

I guess previously it was possible to do something like Cargo build —all —manifest-path foo/Cargo.toml —feature bar, but it doesn’t look that useful?

Yep, that’s exactly it. It’s useful to me because I can (currently) issue a single very quick cargo build command form a top-level Makefile while specifying one feature in a subcrate of my workspace (the makefile is doing other non-Rust stuff for my OS). It’d be even better to issue a single cargo build command with multiple features specified for multiple packages, but I understand if y’all don’t wish to support that.

[matklad]

It’d be even better to issue a single cargo build command with multiple features specified for multiple packages, but I understand if y’all don’t wish to support that.

We definitely wish to support that! However, we need to fix one small and one big problem before it is possible

The small problem is that we just don’t have syntax for it yet. --package foo --features "foo/bar" seems a plausible option though!

The bigger problem is that features selection works weirdly depending on the precise set of packages that you are compiling. That end result is that cargo build -p foo and cargo build -p foo -p bar can produce different artifacts for foo, because, currently, Cargo unions all the features for packages being compiled.

We’ve also decided to go ahead and flip the behavior hear, without explicit opt-in, so, when Cargo is updated at rust-lang/rust, the next nightly would have the new behavior. If it turns out to break code in the wild, we’ll have time to revert the behavior.

[dherman]

I can’t seem to get my docs build working. My previous build command was

RUSTDOCFLAGS=--document-private-items cargo doc --features universal-docs --all --no-deps

It used to successfully build docs for my virtual workspace, and has now started erroring. When I tried adding -Z package-features I get the error error: unknown -Z flag specified: package-features. The full command I tried was:

RUSTDOCFLAGS=--document-private-items cargo doc -Z package-features --features universal-docs --all --no-deps

Any suggestions?

Thanks, Dave

[CAD97]

-Z package-features is default now.

The fact that this worked previously is sort of by accident. I assume that your base package has a universal-docs feature that activates all the children’s universal-docs features? Because otherwise the feature wasn’t getting activated before.

To get the previous behavior, you should just be able to remove --features universal-docs. If you want to enable universal-docs on all packages, for the time being (including on stable), you unfortunately need to iterate through all of the packages. Alternatively, add a root package that passes through --features universal-docs, don’t use --all --no-deps, and live with documenting your dependencies as well.

@matklad and the rest of the rustdoc team: any chance of a --no-foreign-crates alternative to --no-deps that travels through path dependencies but not crates-io or git dependencies?

I just realized that the change actually broke UNIC’s tests as well, but that’s because they weren’t doing what the author thought they were to begin with. Overall this change is good, because it makes it harder to expect one thing to happen and get a different one.

[matklad]

Quick update: we’ve enabled this on nightly for a while, and unfortunately it did break tons of code in the wild, so we are rolling this change back now!

[tcsc]

At the risk of reviving a long dead conversation, is there any reason why, instead of disallowing it all-together, cargo {test,build} --all --features $feat doesn’t apply the feature to, err, all? Similarly for multiple packages. That seems like it would fix a lot of issues people would have in this thread (as well as issues I have with cargo’s behavior here, and will continue to have after the fix).

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.