Apply extra to overrides and constraints #4829
Conversation
|
I think we do need to join extra markers specifically here... |
|
Another reasonable option would be to ignore markers in the overrides file and just respect the original markers, but that means you can't replace dependencies on a per-platform basis. |
konstin
force-pushed
the
konsti/join-markers-for-requirements
branch
from
388cdda
to
43762b2
Compare
konstin
changed the title
konstin
changed the title
konstin
force-pushed
the
konsti/join-markers-for-requirements
branch
from
43762b2
to
16a0a7c
Compare
|
Changed this to only apply extras, properly ready now. |
| @@ -1466,7 +1466,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag | |||
| true | |||
| }) | |||
| .flat_map(move |requirement| { | |||
| iter::once(Cow::Borrowed(requirement)).chain( | |||
| iter::once(requirement.clone()).chain( | |||
There was a problem hiding this comment.
This is now a Cow, so we're generally cloning a ref still. Trying to avoid this clone cause the borrow checker to yell a lot because we're using a reference to the requirement for the chained iterator and it doesn't like the closures with a reference to a value we're also moving.
| // Case 3: An optional dependency with constraint(s). | ||
| // | ||
| // When the original requirement is an optional dependency, the constraint(s) need to | ||
| // be optional for the same extra, otherwise we activate extras that should be inactive. |
There was a problem hiding this comment.
@konstin -- This is really random, but is this necessary for constraints? By adding a constraint, we know that the base package was added as a dependency, right?
There was a problem hiding this comment.
Also, we already merge markers for constraints.
There was a problem hiding this comment.
Hmm not sure, i imagine something like:
constraint: bar <2
requirement: root[foo] -> bar
In that case, we should only add bar<2 for root[foo], not for root.
But it could also be we only need this for overrides.
This is an attempt to solve https://github.com/astral-sh/uv/issues/ by applying the extra marker of the requirement to overrides and constraints.
Say in
awe have a requirementsand overrides
Our current strategy is to discard the markers in the original requirements. This means that on 3.12 for
awe installb==3, but it also means that we addctoawithouta[feature], causing #4826. With this PR, the new requirement become,allowing to override markers while preserving optional dependencies as such.
Fixes #4826