Skip to content

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
Contributor

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
Contributor

bors commented Jan 31, 2022

⌛ Testing commit b7e5186 with merge bb3d4e1c3ed75f32057d2e858f042f9fa00da94d...

@bors
Copy link
Contributor

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
Contributor

bors commented Jan 31, 2022

⌛ Testing commit b7e5186 with merge ea259a0...

@bors
Copy link
Contributor

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