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

Fix dep/feat syntax with hidden implicit optional dependencies #12130

Merged
merged 2 commits into from
May 13, 2023

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented May 11, 2023

This fixes an issue with dep/feat syntax in the [features] table where it wouldn't work if the optional dependency had its implicit feature removed via the dep: syntax.

The problem is that both resolvers were expecting that dep/feat would be able to activate a feature named "dep". But if that implicit feature wasn't created, then it would fail with an error.

This was just an oversight (which probably happened in #9574).

Fixes #10788

@rustbot
Copy link
Collaborator

rustbot commented May 11, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver A-features2 Area: issues specifically related to the v2 feature resolver S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2023
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

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

Based on the tests this seems fine but I don't feel the resolver changes are clear enough without digging a lot more into the implementation for me to feel comfortable merging this. The change causes us to skip some parts but its not obvious from this that this allows the rest to work

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

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

The change is good. r=weihanglo when you're ready.

src/cargo/core/resolver/features.rs Show resolved Hide resolved
@ehuss
Copy link
Contributor Author

ehuss commented May 12, 2023

@bors r=weihanglo

@bors
Copy link
Contributor

bors commented May 12, 2023

📌 Commit 996bca2 has been approved by weihanglo

It is now in the queue for this repository.

@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 12, 2023
@bors
Copy link
Contributor

bors commented May 12, 2023

⌛ Testing commit 996bca2 with merge e41605b...

@bors
Copy link
Contributor

bors commented May 13, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing e41605b to master...

@bors bors merged commit e41605b into rust-lang:master May 13, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 17, 2023
Update cargo

8 commits in 13413c64ff88dd6c2824e9eb9374fc5f10895d28..09276c703a473ab33daaeb94917232e80eefd628
2023-05-10 13:46:18 +0000 to 2023-05-16 21:43:35 +0000
- docs: Clarify that crates.io doesn't link to docs.rs right away. (rust-lang/cargo#12146)
- docs(ref): Clarify MSRV is generally minor (rust-lang/cargo#12122)
- Fix `check_for_file_and_add`'s check for conflict file (rust-lang/cargo#12135)
- Fixes: Incorrect document link (rust-lang/cargo#12143)
- doc: intra-doc links and doc comments for build script (rust-lang/cargo#12133)
- Add Cargo team charter. (rust-lang/cargo#12010)
- Remove useless drop of copy type (rust-lang/cargo#12136)
- Fix dep/feat syntax with hidden implicit optional dependencies (rust-lang/cargo#12130)

r? ghost
@ehuss ehuss added this to the 1.71.0 milestone May 30, 2023
ydirson added a commit to ydirson/xen-guest-agent that referenced this pull request Feb 2, 2024
…is set

The syntax used was equivalent to "dep:xenstore-rs/static", and a bug in
Rust 1.70 (fixed by rust-lang/cargo#12130 in 1.71)
was triggered.
ydirson added a commit to ydirson/xen-guest-agent that referenced this pull request Feb 2, 2024
Our dependency `env_logger` suffers from a bug in versions of Cargo
older than 1.71, fixed by rust-lang/cargo#12130
(the one we suffered from before the "Cargo: don't let feature
"static" pull xenstore-rs unless "xenstore" is set" fix).

Note that some of our dependencies already required 1.70 or later.

Signed-off-by: Yann Dirson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-features2 Area: issues specifically related to the v2 feature resolver S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
5 participants