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

Use UnusableDependencies for URL dependency conflicts #425

Merged
merged 4 commits into from
Nov 17, 2023

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Nov 14, 2023

Extends #424 with support for URL dependency incompatibilities.

Requires changes to miette to prevent URLs from being word wrapped; accepted upstream in zkat/miette#321

@zanieb
Copy link
Member Author

zanieb commented Nov 15, 2023

It'd be nice to add a backtracking test case.

Cargo.toml Outdated Show resolved Hide resolved
Base automatically changed from zanie/new-incompatibility to main November 16, 2023 20:02
zanieb added a commit that referenced this pull request Nov 16, 2023
…cting versions (#424)

Addresses
#309 (comment)

Similar to #338 this throws an error when merging versions results in an
empty set. Instead of propagating that error, we capture it and return a
new dependency type of `Unusable`. Unusable dependencies are a new
incompatibility kind which includes an arbitrary "reason" string that we
present to the user. Adding a new incompatibility kind requires changes
to the vendored pubgrub crate.

We could use this same incompatibility kind for conflicting urls as in
#284 which should allow the solver to backtrack to another valid version
instead of failing (see #425).

Unlike #383 this does not require changes to PubGrub's package mapping
model. I think in the long run we'll want PubGrub to accept multiple
versions per package to solve this specific issue, but we're interested
in it being merged upstream first. This pull request is just using the
issue as a simple case to explore adding a new incompatibility type.

We may or may not be able convince them to add this new incompatibility
type upstream. As discussed in
pubgrub-rs/pubgrub#152, we may want a more
general incompatibility kind instead which can be used for arbitrary
problems. An upstream pull request has been opened for discussion at
pubgrub-rs/pubgrub#153.

Related to:
- pubgrub-rs/pubgrub#152
- #338 
- #383

---------

Co-authored-by: konsti <[email protected]>
@@ -47,6 +47,16 @@ pub(crate) async fn pip_compile(
cache: &Path,
mut printer: Printer,
) -> Result<ExitStatus> {
miette::set_hook(Box::new(|_| {
Copy link
Member Author

Choose a reason for hiding this comment

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

We may want to consider using this elsewhere, but let's start small

@zanieb zanieb marked this pull request as ready for review November 16, 2023 20:07
@zanieb zanieb merged commit 2217514 into main Nov 17, 2023
3 checks passed
@zanieb zanieb deleted the zanie/new-incompat-urls branch November 17, 2023 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants