-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 resolve error messages #6665
Improve resolve error messages #6665
Conversation
Looks like the commits are from before we moved to edition=2018, so the rebase may have left incoherent code. I like the use of the word |
The last commit should resolve the CI issues. Also, reading through the feedback in the other thread, I really quite like this suggestion for structuring the error output. I might give implementing that a go and see how it feels in pracise |
Hay anything I can do to help you with this? I just remembered another source of inspiration on resolver error messages. Dart's package manager has great error messages, we do not (yet) have the data structure to steal it wholesale, but it may be a great place to get inspiration. |
☔ The latest upstream changes (presumably #6653) made this pull request unmergeable. Please resolve the merge conflicts. |
I'm gonna close this to help clear out our queue, but it can of course be resubmitted! |
Improve resolver message to include dependency requirements 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 https://github.com/rust-lang/cargo/blob/216f915c46b8ada2323423d049314ba18247ef95/tests/testsuite/build.rs#L1104-L1106 to https://github.com/rust-lang/cargo/blob/0afd40b4de17a5c45145a0762beb4ef001720fe1/tests/testsuite/build.rs#L1104-L1106 Also provide different message for different source kinds, such like: https://github.com/rust-lang/cargo/blob/0afd40b4de17a5c45145a0762beb4ef001720fe1/tests/testsuite/build.rs#L2810-L2812 ## TODO? From #5452 (comment), there shall be at least one task left behind: > 3. 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`. https://github.com/rust-lang/cargo/blob/a5f8bc94f5d38539dd127f735ea4d3a515c230fd/tests/testsuite/build_script.rs#L1002-L1004 I am uncertain about how to resolve this. Besides the function`validate_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.
Continuation of PR #6374, including all previous commits
Now also includes the rebased changes from origin/master
Closes #6199
r? @Eh2406
cc/@Dylan-DPC