-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 error messaging when a dependency is not found #1241
Conversation
2f90628
to
785c9e7
Compare
785c9e7
to
915bd38
Compare
I expect this to require some refactoring and polish, but it's ready for review. |
); | ||
return Ok(Dependencies::Unavailable( | ||
"The package is unavailable".to_string(), | ||
)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should never happen, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Wondering if the whole thing should be a debug assert so that we don't have to do this extra hash lookup in release, but what you have here is probably better, it's negligible.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this should never happen. I am just hesitant to panic in production.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks great.
Previously, whenever we encountered a missing package we would throw an error without information about why the package was requested. This meant that if a transitive dependency required a missing package, the user would have no idea why it was even selected. Here, we track
NotFound
andNoIndex
errors asNoVersions
incompatibilities with an attached reason. Improves our test coverage for--no-index
without--find-links
.The snapshots show a nice improvement.
I think this will also enable backtracking to another version if some version of transitive dependency has a missing dependency. I'll write a scenario for that next.
Requires astral-sh/pubgrub#22