Create complementary dependencies for extra-only sources#18272
Create complementary dependencies for extra-only sources#18272charliermarsh wants to merge 3 commits intomainfrom
Conversation
charliermarsh
temporarily deployed
to
uv-test-publish
GitHub Actions
Inactive
charliermarsh
force-pushed
the
charlie/extra
branch
from
779de94 to
5494a44
Compare
charliermarsh
force-pushed
the
charlie/extra
branch
2 times, most recently
from
c4bdcb8 to
ea4ea71
Compare
|
The net effects here seem quite good, though I don't fully understand the change yet. |
charliermarsh
force-pushed
the
charlie/extra
branch
from
ea4ea71 to
98151f7
Compare
charliermarsh
force-pushed
the
charlie/extra
branch
from
98151f7 to
4c23fd1
Compare
|
Before I go further on the inline comments, should this apply only to sources, or to more kinds of conditional dependencies? If the former, should we apply this in |
|
I can give that a try. |
|
I wasn't able to get that to work... Part of the problem is that this has to look across dependencies and optional dependencies to figure out whether to include the complement. |
konstin
left a comment
konstin
left a comment
There was a problem hiding this comment.
I gotta admit I have trouble following along with the logic, so this isn't a review, but instead some questions for my understanding.
| .collect(); | ||
|
|
||
| if extra.is_none() && group.is_none() { | ||
| self.add_complementary_source_dependencies( |
There was a problem hiding this comment.
Can we return an iterator here and chain that? iirc this is a hot path
| deps: &mut Vec<PubGrubDependency>, | ||
| ) { | ||
| let python_marker = python_requirement.to_marker_tree(); | ||
| for requirement in self.overrides.apply(requirements.iter()) { |
There was a problem hiding this comment.
Can you add a comment and a test why we need to apply overrides here? I removed it and the tests still pass.
| .collect() | ||
| .collect(); | ||
|
|
||
| if extra.is_none() && group.is_none() { |
There was a problem hiding this comment.
Why is that our condition, naively, I'd have expected the inverse condition for inverting
| python_marker: MarkerTree, | ||
| ) -> bool { | ||
| // Already included via `flatten_requirements`. | ||
| if requirement.evaluate_markers(env.marker_environment(), &[]) { |
| } | ||
|
|
||
| // A base dep (without a URL) must already exist; otherwise this is | ||
| // an extra-only dependency and should remain as-is. |
There was a problem hiding this comment.
Could the base dep be added to the dependency graph after the URL dep?
2 participants
Summary
When a
[tool.uv.sources]entry usesextra = "foo", the source should only apply when that extra is active. We now create a complementary dependency forextra != "foo"with the source un-applied.For example, given:
We now install from
example.comwhenfoois activated, but PyPI otherwise.Closes #17967.
Closes #18229.