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.

Sign up for GitHub

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

Jump to bottom

Confusing behavior when passing features to package #11415

Open
kamirr opened this issue Nov 24, 2022 · 2 comments
Open

Confusing behavior when passing features to package #11415

kamirr opened this issue Nov 24, 2022 · 2 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@kamirr
Copy link
Contributor

kamirr commented Nov 24, 2022 •
edited
Loading

Problem

If you have a package foo in your workspace, and it is not the default build target (cargo build builds bar), you build it via cargo build -p foo. Now, if you want to enable a feature, the intuitive approach would be to cargo build -p foo --features new-feat. This will cause an error if bar doesn't have such feature, but it will build just fine if bar also has the new-feat feautre.

This is immensly confusing, as the feature is enabled for a package which one isn't even attempting to build. The solution is to, of course, invoke cargo build -p foo --features "foo/new-feat", but finding out about this is not trivial.

Proposed Solution

Emit a warning when running cargo build -p MEMBER --features FEATURE if both MEMBER and the root package have this feature declared.

Notes

If this is deemed useful, I can tackle the implementation.
@kamirr kamirr added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Nov 24, 2022
epage added a commit to epage/cargo that referenced this issue Nov 25, 2022
@epage
fix: Move off atty to resolve soundness issue
dd31a47 
There is a soundness issue with atty when building on Windows with a
custom allocator.

This PR switches direct dependencies on atty to is-terminal.  New semver
compatible versions of clap and snapbox remove atty. rust-lang#11417 upgrades
env_logger to remove it from there.

Fixes rust-lang#11415
@epage epage mentioned this issue Nov 25, 2022
Merged
epage added a commit to epage/cargo that referenced this issue Nov 25, 2022
@epage
fix: Move off atty to resolve soundness issue
1f9ca5b 
There is a soundness issue with atty when building on Windows with a
custom allocator.

This PR switches direct dependencies on atty to is-terminal.  New semver
compatible versions of clap and snapbox remove atty. rust-lang#11417 upgrades
env_logger to remove it from there.

Fixes rust-lang#11415
@ehuss
Copy link
Contributor

ehuss commented Nov 25, 2022

I believe this has been fixed with the new resolver. Can you make sure you are using resolver="2" in your workspace? If that doesn't help, can you add a full reproduction here (such as a list of commands to generate the workspace, or a link or .ZIP to a demo repo).

@bors bors closed this as completed in 48895b1 Nov 25, 2022
@ehuss
Copy link
Contributor

ehuss commented Nov 25, 2022

This was inadvertently closed.

@ehuss ehuss reopened this Nov 25, 2022
@ehuss ehuss added the S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. label Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ehuss @kamirr