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

Improve resolver message to include dependency requirements #9827

Merged
merged 4 commits into from
Aug 25, 2021

Conversation

weihanglo
Copy link
Member

@weihanglo weihanglo commented Aug 23, 2021

Resolves #6199.

Thanks for previous efforts: #5452, #6374, #6665, which are great but somehow outdated, so I tweak them and create this PR. This will also be obsolete if we ship pubgrub-rs with cargo in the future 😃 But before that happens, IMO these changes are still helpful.


This PR changes the resolver error message from

cargo/tests/testsuite/build.rs

Lines 1104 to 1106 in 216f915

previously selected package `bad v1.0.0`
... which is depended on by `baz v0.1.0`
... which is depended on by `foo v0.0.1 ([..])`

to

cargo/tests/testsuite/build.rs

Lines 1104 to 1106 in 0afd40b

previously selected package `bad v1.0.0`
... which satisfies dependency `bad = \"=1.0.0\"` of package `baz v0.1.0`
... which satisfies dependency `baz = \"^0.1.0\"` of package `foo v0.0.1 ([..])`

Also provide different message for different source kinds, such like:

cargo/tests/testsuite/build.rs

Lines 2810 to 2812 in 0afd40b

package `a v0.0.1 ([CWD]/a)`
... which satisfies path dependency `a` of package `foo v0.0.1 ([CWD])`
... which satisfies path dependency `foo` of package `a v0.0.1 ([..])`",

TODO?

From #5452 (comment), there shall be at least one task left behind:

  1. Special case pind by a lock file and not a "=1.1.2" in a dependency. Also add a "note: try cargo update" to the end.

In this PR, validate_links also faces this issue that a dependency requirement is locked into a precise version =0.1.0.

package `bar v0.1.0`
... which satisfies dependency `bar = \"=0.1.0\"` of package `foo v0.1.0 ([..]foo)`
links to native library `a`

I am uncertain about how to resolve this. Besides the functionvalidate_links, is this problem really a thing that may happen? If not, since validate_links only handles old validation logic, it may be ok to drop the commit a5f8bc9 and leave it as is.

@rust-highfive
Copy link

r? @alexcrichton

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2021
@weihanglo
Copy link
Member Author

weihanglo commented Aug 23, 2021

r? @Eh2406
I guess you would be interested in resolver stuff but no pressure. 🙂

@rust-highfive rust-highfive assigned Eh2406 and unassigned alexcrichton Aug 23, 2021
@Eh2406
Copy link
Contributor

Eh2406 commented Aug 23, 2021

Thank you so much! This is very exciting!

Handling lock files looking like "=1.1.2" can be left for a follow up. Or we can flag all "=" reqs with something like "maybe from a lockfile" for now. (I do have a plan for a long term fix, if you want to take it on.)

Why is validate_links getting =? But overall it should be rare, and I don't much care if it is confusing. We can leave it with less info or have a consistent look with confusing reqs, whichever.

@weihanglo
Copy link
Member Author

Why is validate_links getting =?

Because it is called after ops::resolve_ws_with_opts? But yeah I guess the way to address the issue is similar to addressing lockfile. Anyway, if the current status of this PR is acceptable, then I think it's ready for review. Thanks!

@Eh2406
Copy link
Contributor

Eh2406 commented Aug 25, 2021

@bors r+

Thank you!

@bors
Copy link
Collaborator

bors commented Aug 25, 2021

📌 Commit a5f8bc9 has been approved by Eh2406

@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 Aug 25, 2021
@bors
Copy link
Collaborator

bors commented Aug 25, 2021

⌛ Testing commit a5f8bc9 with merge 77a0379...

@bors
Copy link
Collaborator

bors commented Aug 25, 2021

☀️ Test successful - checks-actions
Approved by: Eh2406
Pushing 77a0379 to master...

@bors bors merged commit 77a0379 into rust-lang:master Aug 25, 2021
@weihanglo weihanglo deleted the issue-6199 branch August 25, 2021 23:49
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 27, 2021
Update cargo

19 commits in e96bdb0c3d0a418e7fcd7fbd69be08abf830b4bc..f559c109cc79fe413a8535fb620a5a58b3823d94
2021-08-17 22:58:47 +0000 to 2021-08-26 22:54:55 +0000
- Fix test not to rely on `cargo` in PATH. (rust-lang/cargo#9843)
- Improve resolver message to include dependency requirements (rust-lang/cargo#9827)
- Add hint for cargo metadata in environment section (rust-lang/cargo#9836)
- Fix panic with build-std of a proc-macro. (rust-lang/cargo#9834)
- Fix typos “a”→“an” (rust-lang/cargo#9821)
- Fix typo in git-authentication.md (rust-lang/cargo#9832)
- Add some debug logging for `cargo fix` (rust-lang/cargo#9831)
- Add documentation about third-party registries. (rust-lang/cargo#9830)
- unset the FIX_ENV when executing the real rustc (rust-lang/cargo#9818)
- Allow crate download by checksum (rust-lang/cargo#9801)
- Emit warning for migrating to unstable edition in stable channel (rust-lang/cargo#9792)
- Warning for no lib dependencies (rust-lang/cargo#9771)
- Temporarily disable extern-html-root-url test. (rust-lang/cargo#9824)
- Move `tmp` test directory. (rust-lang/cargo#9814)
- Fix test incorrectly validating CARGO_PKG_LICENSE_FILE. (rust-lang/cargo#9813)
- Implement `[future-incompat-report]` config section (rust-lang/cargo#9774)
- Bump curl. (rust-lang/cargo#9809)
- Determine packages to install prior to installing (rust-lang/cargo#9793)
- Show feature resolver differences for dev-dependencies. (rust-lang/cargo#9803)
@ehuss ehuss added this to the 1.56.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.

Resolvers error messages should include the version requirements.
6 participants