Skip to content

Fix bug when with resolver = "1" non-virtual package was allowing unknown features#9437

Merged
bors merged 12 commits into
rust-lang:masterfrom
In-line:unknown-feature-resolver-1
May 24, 2021
Merged

Fix bug when with resolver = "1" non-virtual package was allowing unknown features#9437
bors merged 12 commits into
rust-lang:masterfrom
In-line:unknown-feature-resolver-1

Conversation

@In-line

@In-line In-line commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

No description provided.

@rust-highfive

This comment has been minimized.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 30, 2021
@In-line

In-line commented Apr 30, 2021

Copy link
Copy Markdown
Contributor Author

I accidently found, that if I remove resolver = "2" from this test if fails. It seems that doing cargo build --features package_name/some-nonsense is acceptable by cargo

@In-line

In-line commented Apr 30, 2021

Copy link
Copy Markdown
Contributor Author

Output with cargo ea0b5ca

thread 'package_features::virtual_typo_member_feature_default_resolver' panicked at '
Expected: execs
    but: differences:
  0 - |[ERROR] none of the selected packages contains these features: a/deny-warning|
    + |error: package `a v0.1.0 (/home/alik/Documents/projects/cargo/target/cit/t0/foo)` does not have a dependency named `a`|

But with d087aeb

thread 'package_features::virtual_typo_member_feature_default_resolver' panicked at '
Expected: execs
    but: exited with exit code: 0
--- stdout

--- stderr

@ehuss

ehuss commented Apr 30, 2021

Copy link
Copy Markdown
Contributor

Hm, looks like that slipped through. Would you be interested in trying to fix it?

@In-line

In-line commented May 3, 2021

Copy link
Copy Markdown
Contributor Author

@ehuss I will be looking to it.

@In-line In-line changed the title Add failing test Fix bug when with resolver = "1" non-virtual package was allowing unknown features May 3, 2021
Comment thread src/cargo/core/workspace.rs Outdated
@In-line

In-line commented May 3, 2021

Copy link
Copy Markdown
Contributor Author

r? @ehuss

@rust-highfive rust-highfive assigned ehuss and unassigned alexcrichton May 3, 2021
Comment thread src/cargo/core/workspace.rs Outdated
@ehuss

ehuss commented May 10, 2021

Copy link
Copy Markdown
Contributor

Hm, so I'm a bit uncertain about the approach here. If I understand correctly, the main concern is that with --features foo/featname, and foo is the "current" package, then featname is more or less ignored. Would it be possible to change it so that in that scenario, the feature is added to cwd_features instead? That should pass the feature to the normal machinery for checking if the feature exists. Then, the check for !member_specific_features.is_empty() can be treated more as a bug, since the two loops should end up matching one another.

@In-line

In-line commented May 13, 2021

Copy link
Copy Markdown
Contributor Author

@ehuss do you have any more ideas for test cases?

@ehuss

ehuss commented May 24, 2021

Copy link
Copy Markdown
Contributor

Thanks!

I think the tests are good enough, I can't think of anything specific to check.

I pushed a small change to remove masquerade_as_nightly_cargo which wasn't needed (it was an accident it wasn't removed in the other test).

@bors r+

@bors

bors commented May 24, 2021

Copy link
Copy Markdown
Contributor

📌 Commit 61b762b has been approved by ehuss

@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 May 24, 2021
@bors

bors commented May 24, 2021

Copy link
Copy Markdown
Contributor

⌛ Testing commit 61b762b with merge 07340c9...

@bors

bors commented May 24, 2021

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 07340c9 to master...

@bors bors merged commit 07340c9 into rust-lang:master May 24, 2021
@In-line In-line deleted the unknown-feature-resolver-1 branch May 25, 2021 11:24
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 26, 2021
Update cargo

7 commits in 070e459c2d8b79c5b2ac5218064e7603329c92ae..e931e4796b61de593aa1097649445e535c9c7ee0
2021-05-11 18:12:23 +0000 to 2021-05-24 16:17:27 +0000
- Add `cargo:rustc-link-arg-bin` flag. (rust-lang/cargo#9486)
- Add a cargo-doc.browser config option (rust-lang/cargo#9473)
- Fix bug when with resolver = "1" non-virtual package was allowing unknown features (rust-lang/cargo#9437)
- Add GitHub link to contributor guide. (rust-lang/cargo#9493)
- Add temporary fix for rustup on windows in CI. (rust-lang/cargo#9498)
- 3 typos and some capitalization (rust-lang/cargo#9495)
- fix 6 typos (rust-lang/cargo#9484)
JohnTitor added a commit to JohnTitor/rust that referenced this pull request May 26, 2021
Update cargo

7 commits in 070e459c2d8b79c5b2ac5218064e7603329c92ae..e931e4796b61de593aa1097649445e535c9c7ee0
2021-05-11 18:12:23 +0000 to 2021-05-24 16:17:27 +0000
- Add `cargo:rustc-link-arg-bin` flag. (rust-lang/cargo#9486)
- Add a cargo-doc.browser config option (rust-lang/cargo#9473)
- Fix bug when with resolver = "1" non-virtual package was allowing unknown features (rust-lang/cargo#9437)
- Add GitHub link to contributor guide. (rust-lang/cargo#9493)
- Add temporary fix for rustup on windows in CI. (rust-lang/cargo#9498)
- 3 typos and some capitalization (rust-lang/cargo#9495)
- fix 6 typos (rust-lang/cargo#9484)
@ehuss ehuss added this to the 1.54.0 milestone Feb 6, 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.

5 participants