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

Do not ignore --features when --all-features is present #10337

Merged
merged 2 commits into from
Jan 31, 2022

Conversation

WaffleLapkin
Copy link
Member

This allows to specify dependency features when using --all-features, for example:

$ cargo run --example example --all-features --features="tracing/log"

You can test this in this repository: https://github.com/WaffleLapkin/cargo_all_some_features, it contains an example with required-features = [..., "tracing/log"] that is impossible to run with --all-features without this patch.

An attempt to fix #10333

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 27, 2022
@WaffleLapkin
Copy link
Member Author

CI seems to fail due to changes in cli help formatting, but I didn't change anything there 🤔

@weihanglo
Copy link
Member

for fixing broken tests on CI, rebase to master 😄

ref: #10336

@WaffleLapkin WaffleLapkin force-pushed the merge_futures_with_all_features branch from 09ed620 to 3d74ad8 Compare January 27, 2022 23:49
This allows to specify dependency features when using `--all-features`,
for example:
```shell
$ cargo run --package a --example example --all-features --features="tracing/log"
```
@WaffleLapkin
Copy link
Member Author

@weihanglo thanks!

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me! Can you add a test for this feature as well?

@@ -849,7 +849,8 @@ foo v0.1.0 ([ROOT]/foo)
bar v1.0.0
└── bar feature \"default\"
└── foo v0.1.0 ([ROOT]/foo)
└── foo feature \"a\" (command-line)
├── foo feature \"a\" (command-line)
└── foo feature \"default\" (command-line)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I don't think that this is a bug with this PR per-se I think that this is a bug because neither of the packages here actually have a feature called default, so I don't think that this should be printed. I think this could probably be fixed by conditionally inserting the feature "default" in cargo tree depending on whether the package actually has that feature.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I open an issue about this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm yeah I think it's probably orthogonal-enough from this PR it's best to fix later, and yeah if you wouldn't mind opening an issue that'd be great!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #10351

@WaffleLapkin
Copy link
Member Author

@alexcrichton I've added a test

@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Collaborator

bors commented Jan 31, 2022

📌 Commit b7e5186 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2022
@bors
Copy link
Collaborator

bors commented Jan 31, 2022

⌛ Testing commit b7e5186 with merge bb3d4e1c3ed75f32057d2e858f042f9fa00da94d...

@bors
Copy link
Collaborator

bors commented Jan 31, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 31, 2022
@alexcrichton
Copy link
Member

@bors: retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2022
@bors
Copy link
Collaborator

bors commented Jan 31, 2022

⌛ Testing commit b7e5186 with merge ea259a0...

@bors
Copy link
Collaborator

bors commented Jan 31, 2022

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing ea259a0 to master...

@bors bors merged commit ea259a0 into rust-lang:master Jan 31, 2022
@WaffleLapkin WaffleLapkin deleted the merge_futures_with_all_features branch January 31, 2022 19:36
ehuss added a commit to ehuss/rust that referenced this pull request Feb 1, 2022
Update cargo

10 commits in 1c034752de0df744fcd7788fcbca158830b8bf85..25fcb135d02ea897ce894b67ae021f48107d522b
2022-01-25 22:36:53 +0000 to 2022-02-01 01:32:48 +0000
- fix(install): Keep v1 file formatting the same (rust-lang/cargo#10349)
- fix(vendor): Use tables for sample config (rust-lang/cargo#10348)
- Add bash completion for `cargo clippy` (rust-lang/cargo#10347)
- Do not ignore `--features` when `--all-features` is present (rust-lang/cargo#10337)
- test: Fix compatibilty with new toml_edit (rust-lang/cargo#10350)
- extra-link-arg-etc: support all link types (credit `@davidhewitt)` (rust-lang/cargo#10274)
- Make clippy happy (rust-lang/cargo#10340)
- Update publishing link for semver rules. (rust-lang/cargo#10338)
- Normalize --path when install bin outside current workspace (rust-lang/cargo#10335)
- Bump clap to v3.0.13 (rust-lang/cargo#10336)
@ehuss ehuss added this to the 1.60.0 milestone Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--features is silently ignored when --all-features is present
6 participants