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

Add incompatibility for unusable dependencies #153

Closed
wants to merge 1 commit into from

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Nov 15, 2023

Adds a new incompatibility kind of unusable dependencies, which is a distinct case from unknown dependencies in that we know the dependencies of the package but we are unwilling to use them.

This incompatibility is useful for some of the cases described in #152 allowing versions of a package to be marked as unusable due to a problem in their direct dependencies.

In my fork, I attach a reason: Option<String> to the UnusableDependencies types which allows the resolver to explain why the dependencies are unusable. I have opted not to include that here yet, as I know there is interest in adding such labels to all incompatibility kinds which feels like a separate discussion.

@zanieb zanieb changed the base branch from release to dev November 15, 2023 19:39
@zanieb zanieb changed the title zanie/unusable dependencies Add incompatibility for unusable dependencies Nov 15, 2023
@Eh2406
Copy link
Member

Eh2406 commented Nov 16, 2023

We talked about this PR in office hours and the plan is to experiment with the larger change to allow adding arbitrary metadata to every user provided incompatibility.

@mpizenberg
Copy link
Member

Makes sense that the dependency providers can attach metadata to its responses. These are then useful with custom error reporting, as long as that data is stored and re-shared with the derivation tree.

@zanieb
Copy link
Member Author

zanieb commented Nov 16, 2023

Makes sense that the dependency providers can attach metadata to its responses. These are then useful with custom error reporting, as long as that data is stored and re-shared with the derivation tree.

That's my plan to explore!

Also another note that Dependencies::Unknown should probably be renamed to Dependencies::Unavailable and additional metadata can be used to describe if they are irretrievable (and therefore unknown) or invalid (unusable).

@zanieb
Copy link
Member Author

zanieb commented Nov 16, 2023

I've made my internal copy public at astral-sh#4 so I'll close this one out.

I'll follow-up with some drafts of the discussed changes :)

@zanieb zanieb closed this Nov 16, 2023
zanieb added a commit to astral-sh/uv 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 <konstin@mailbox.org>
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.

3 participants