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

Allow an arbitrary, custom incompatibilities #152

Closed
zanieb opened this issue Nov 10, 2023 · 4 comments
Closed

Allow an arbitrary, custom incompatibilities #152

zanieb opened this issue Nov 10, 2023 · 4 comments

Comments

@zanieb
Copy link
Member

zanieb commented Nov 10, 2023

In a concrete example, pip allows users to define dependencies on packages via URLs. When resolving a dependency tree, we will only allow a transitive dependency to require a package with a URL if that URL is included as a direct dependency (for security). In this situation, we currently must raise an error and crash the solver. However, it'd be ideal for the solver to backtrack and attempt other versions of the transitive dependency as one may not have the URL package dependency.

Perhaps this problem could be simplified by thinking of a "banned list" of packages feature. If a transitive dependency version requires a package on the banned list, it should be marked as incompatible and other versions of the dependency should be checked.

This may be solvable by adding an Invalid incompatibility type that includes a custom message that describes why the package version was not acceptable. I'm not familiar with the design of incompatibilities though.

@Eh2406
Copy link
Member

Eh2406 commented Nov 10, 2023

I don't quite see why the URL situation requires crashing out, but I may not understand all the architectural decisions involved here. One solution that is already available is returning Dependencies::Unknown, although that doesn't let you customize the error message.

Users can implement the "banned list" themselves by simply filtering the versions out before providing them to PubGrub. Although this doesn't provide a way to customize the error message.

Another proposal that we have had is when returning a list of dependencies you could also return some kind of week dependency "I don't need X, but if it's present it must match this version range". See prior conversation at #120 and a draft PR at #124. This is currently held up on the additional complexity would add to the API, especially how even if you don't need the functionality you have to deal with the complexity. #148 was created to see if we could come up with an API so that users only deal with the complexity they need.

Allowing power users to provide arbitrary incompatibilities is discussed in ... I can't find the link. There will definitely be lots of ways a user can shoot themselves in the foot using this API. So I only want provided, when we're sure that the common use cases have ergonomics support.

I absolutely love the idea of allowing a "tag" | "span" | "label" to each response given to PubGrub by the user. Then the obvious answer to this is Dependencies::Unknown with a tag for whites unavailable. The challenge here is similar to #120/#124/#148 how can we add this to the API in such a way that it is not added complexity for the users who do not need the functionality.

@zanieb
Copy link
Member Author

zanieb commented Nov 10, 2023

Users can implement the "banned list" themselves by simply filtering the versions out before providing them to PubGrub. Although this doesn't provide a way to customize the error message.

This is the heart of the issue, as we cannot include information in the derivation tree and error messages as to why those versions were excluded.

One solution that is already available is returning Dependencies::Unknown, although that doesn't let you customize the error message.

Hm I'm not sure if we tried this, I'll look into that and see if it gets us further. I like the idea of attaching additional context to the response, if feasible.

@zanieb
Copy link
Member Author

zanieb commented Nov 10, 2023

Another use-case: We're filtering out package versions that are only available for other platforms (i.e. not published for the current/target platform) but PubGrub cannot be aware of this and consequently the information is excluded from the derivation tree.

zanieb added a commit to astral-sh/uv that referenced this issue 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]>
@konstin
Copy link
Member

konstin commented Jun 7, 2024

Solved by #208

@konstin konstin closed this as completed Jun 7, 2024
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

No branches or pull requests

3 participants