-
Notifications
You must be signed in to change notification settings - Fork 510
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
Throw an error if a requirement exists with conflicting URLs #284
Comments
charliermarsh
added a commit
that referenced
this issue
Nov 5, 2023
In the resolver, our current model for solving URL dependencies requires that we visit the URL dependency _before_ the registry-based dependency. This PR encodes a strict requirement that all URL dependencies be declared upfront, either as requirements or constraints. I wrote more about how it works and why it's necessary in documentation [here](https://github.com/astral-sh/puffin/pull/319/files#diff-2b1c4f36af0c62a2b7bebeae9473ae083588f2a6b18a3ec52393a24266adecbbR20). I think we could relax this constraint over time, but it requires a more sophisticated model -- and for now, I just want something that's (1) correct, (2) easy for us to reason about, and (3) easy for users to reason about. As additional motivation... allowing arbitrary URL dependencies anywhere in the tree creates some really confusing situations in which I'm not even sure what the right answers are. For example, assume you declare a direct dependency on `Werkzeug==2.0.0`. You then depend on a version of Flask that depends on a version of `Werkzeug` from some arbitrary URL. You build the source distribution at that arbitrary URL, and it turns out it _does_ build to a declared version of 2.0.0. What should happen? (And if it resolves to a version that _isn't_ 2.0.0, what should happen _then_?) I suspect different tools handle this differently, but it must lead to a lot of "silent" failures. In my testing of Poetry, it seems like Poetry just ignores the URL dependency, which seems wrong, but is also a behavior we could implement in the future. Closes #303. Closes #284.
zanieb
added a commit
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]>
Does anyone know a way to explicitly prefer one of the conflicting URLs or work around this error without updating the dependent packages? cc @charliermarsh |
@ayush-rathore-quartic -- You can use
|
@charliermarsh Thanks a whole lot for the prompt help, This worked! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
E.g., if you depend on Flask at one URL, and one of your dependencies depends on Flask at a different URL, we need to throw an error.
The text was updated successfully, but these errors were encountered: