Skip to content

test: add more comprehensive workspace feature unification tests - #17466

Merged
weihanglo merged 1 commit into
rust-lang:masterfrom
aliu:feature-unification-tests
Sep 11, 2026
Merged

weihanglo merged 1 commit into
rust-lang:masterfrom
aliu:feature-unification-tests

Conversation

@aliu

@aliu aliu commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What does this PR try to resolve?

Followup to the discussion from #14774 (comment).

I added test cases to pin how workspace feature unification interacts with:

  • default-members workspace Cargo.toml
  • --exclude <package> flag
  • v1 resolver
  • v2 resolver
  • --all-features flag
  • --no-default-features flag

As for the ambiguous interactions mentioned in the comment, I think the following rule of thumb makes sense: since the goal of workspace feature unification is to avoid rebuilding artifacts, any interactions with flags/configs should not cause unnecessary rebuilds regardless of what packages are currently selected. That means for flags like --all-features and --no-default-features, the flag would apply to all members of the workspace despite what the documentation saying that it only applies to the selected packages. So I wrote all the tests with this in mind. Luckily this is already the current behavior of workspace feature unification and didn't require and changes to cargo.

How to test and review this PR?

New tests should make sense based on how we expect workspace feature unification to behave.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 11, 2026
@rustbot

rustbot commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @epage, @weihanglo
  • @epage, @weihanglo expanded to epage, weihanglo
  • Random selection from epage, weihanglo

@weihanglo weihanglo left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding these tests!

View changes since this review

.file("b/src/lib.rs", "")
.build();

p.cargo("check -p common --no-default-features")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

side note: a bit unsure about whether we want diagnostics for this kind of situation. I believe there will be some confusion when people forget they have set workspace unification, though it is a bit tricky to give good diagnostic for every case. Anyway not a blocker.

@weihanglo
weihanglo added this pull request to the merge queue Sep 11, 2026
Merged via the queue into rust-lang:master with commit fa45a17 Sep 11, 2026
28 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 11, 2026
rust-bors Bot pushed a commit to rust-lang/rust that referenced this pull request Sep 12, 2026
Update cargo submodule

6 commits in 3c0b534756e166d12eb9fd2e1abfe5b42ac6101e..7941be6fb416b4cd9666aef7b858dfea25587a8c
2026-09-04 17:03:10 +0000 to 2026-09-11 20:47:37 +0000
- fix(install): use packaged lockfile by default (rust-lang/cargo#17388)
- test: add more comprehensive workspace feature unification tests (rust-lang/cargo#17466)
- chore(deps): update crossbeam-epoch to 0.9.21 (rust-lang/cargo#17427)
- Fixed stale comment about fingerprint checking method (rust-lang/cargo#17450)
- docs(lints): how to configure Cargo lints (rust-lang/cargo#17441)
- Fix relative symlink handling in write_atomic (rust-lang/cargo#17362)

r? ghost
@rustbot rustbot added this to the 1.100.0 milestone Sep 12, 2026
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 this pull request may close these issues.

3 participants