-
Notifications
You must be signed in to change notification settings - Fork 546
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
uv-resolver: filter out sibling dependencies in a fork #4415
Conversation
When a fork is created from a list of dependencies, we were previously adding all other sibling dependencies to every fork created. But this isn't actually quite right, since the fork created is always created by some marker expression. And while it is definitively disjoint from any directly conflicting dependency specification, it is also possibly disjoint with other dependencies. For example, as reported in #4414: ```toml dependencies = [ "anyio==4.4.0 ; python_version >= '3.12'", "anyio==4.3.0 ; python_version < '3.12'", "b1 ; python_version >= '3.12'", "b2 ; python_version < '3.12'", ] ``` The first two `anyio` requirements are conflicting with non-overlapping marker expressions, and so a fork is created. Prior to this commit, *both* `b1` and `b2` would be added to each fork. But of course, `b2` is impossible in the `anyio==4.4.0` fork because of disjoint marker expressions. So in this commit, we specifically filter out any sibling dependencies that could find their way into a fork that have disjoint markers with that fork. We are careful to do this both when a new fork is created from an existing set of dependencies, and when adding new dependencies to a fork. Fixes #4414
I added a packse test here: astral-sh/packse#197 |
source = "registry+https://astral-sh.github.io/packse/0.3.27/simple-html/" | ||
sdist = { url = "https://astral-sh.github.io/packse/0.3.27/files/fork_marker_inherit_combined_allowed_a-1.0.0.tar.gz#sha256=c7232306e8597d46c3fe53a3b1472f99b8ff36b3169f335ba0a5b625e193f7d4", hash = "sha256:c7232306e8597d46c3fe53a3b1472f99b8ff36b3169f335ba0a5b625e193f7d4" } | ||
wheels = [{ url = "https://astral-sh.github.io/packse/0.3.27/files/fork_marker_inherit_combined_allowed_a-1.0.0-py3-none-any.whl#sha256=609b50f1a9fdbea0cc5d77f3eaf5143900159d809c56db5839de138c3123d980", hash = "sha256:609b50f1a9fdbea0cc5d77f3eaf5143900159d809c56db5839de138c3123d980" }] | ||
source = "registry+https://astral-sh.github.io/packse/0.3.29/simple-html/" |
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.
Should we start censoring the packse version so an upgrade isn't a huge diff each time?
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.
cc @zanieb
I don't think I'm opposed to that. I do try to update the tests in a separate commit so that it's a little easier to review. But I agree there is a fair bit of noise in the diff.
Another possible mitigation here would be a way to update the existing tests without adding any new ones, commit that, and then do an update with the new tests. But I think that'd be a little annoying to do in practice.
When a fork is created from a list of dependencies, we were previously
adding all other sibling dependencies to every fork created. But this
isn't actually quite right, since the fork created is always created by
some marker expression. And while it is definitively disjoint from any
directly conflicting dependency specification, it is also possibly
disjoint with other dependencies. For example, as reported in #4414:
The first two
anyio
requirements are conflicting with non-overlappingmarker expressions, and so a fork is created. Prior to this commit,
both
b1
andb2
would be added to each fork. But of course,b2
isimpossible in the
anyio==4.4.0
fork because of disjoint markerexpressions.
So in this commit, we specifically filter out any sibling dependencies
that could find their way into a fork that have disjoint markers with
that fork. We are careful to do this both when a new fork is created
from an existing set of dependencies, and when adding new dependencies
to a fork.
Fixes #4414