Track Markers via a PubGrub package variant
#4123
Conversation
crates/uv/tests/lock_scenarios.rs
Outdated
| @@ -727,7 +727,7 @@ fn fork_non_local_fork_marker_direct() -> Result<()> { | |||
| ----- stderr ----- | |||
| warning: `uv lock` is experimental and may change without warning. | |||
| × No solution found when resolving dependencies: | |||
| ╰─▶ Because package-b{sys_platform == 'darwin'}==1.0.0 depends on package-c>=2.0.0 and package-a{sys_platform == 'linux'}==1.0.0 depends on package-c<2.0.0, we can conclude that package-a{sys_platform == 'linux'}==1.0.0 and package-b{sys_platform == 'darwin'}==1.0.0 are incompatible. | |||
| ╰─▶ Because package-b==1.0.0 depends on package-c>=2.0.0 and package-a==1.0.0 depends on package-c<2.0.0, we can conclude that package-a==1.0.0 and package-b{sys_platform == 'darwin'}==1.0.0 are incompatible. | |||
There was a problem hiding this comment.
There is still some weirdness here -- we might want to throw out the markers entirely when reporting? I know how to do that.
| name, | ||
| marker, | ||
| url: Some(url), | ||
| })) |
There was a problem hiding this comment.
There is admittedly some weirdness here. If we have both a marker and an extra, I'm using the Extra variant. The Marker variant is being used only for bare packages with markers. In get_dependencies, if Extra has a marker field, we flatten it out to include the base package with and without markers.
There was a problem hiding this comment.
Hmmm yes that is odd. I think that was one of the reasons I ended up not going this route initially, because it seemed like we'd want an ExtraMarker variant too. But I guess the flattening in get_dependencies works around that?
I think I would find some comments about this, on the PubGrabInner variants, helpful.
There was a problem hiding this comment.
Yeah, the flattening works around it.
| ), | ||
| ])), | ||
|
|
||
| // Add a dependency on both the extra and base package, with and without the marker. |
There was a problem hiding this comment.
This is a bit strange. I think it's necessary?
There was a problem hiding this comment.
Would adding a dependency on the virtual marker package and having the virtual package then depend on the real package work to?
charliermarsh
force-pushed
the
charlie/marker-pkg
branch
from
84d8f58 to
e510e4e
Compare
charliermarsh
force-pushed
the
charlie/marker-pkg
branch
from
c872057 to
49b3b42
Compare
| name, | ||
| marker, | ||
| url: Some(url), | ||
| })) |
There was a problem hiding this comment.
Hmmm yes that is odd. I think that was one of the reasons I ended up not going this route initially, because it seemed like we'd want an ExtraMarker variant too. But I guess the flattening in get_dependencies works around that?
I think I would find some comments about this, on the PubGrabInner variants, helpful.
|
@BurntSushi -- I agree. We have the same patterns reused in a few places too, around matching on package name and URL. |
charliermarsh
force-pushed
the
charlie/marker-pkg
branch
from
49b3b42 to
f740560
Compare
Summary
This PR adds a lowering similar to that seen in #3100, but this time, for markers. Like
PubGrubPackageInner::Extra, we now havePubGrubPackageInner::Marker. The dependencies of theMarkerarePubGrubPackageInner::Packagewith and without the marker.As an example of why this is useful: assume we have
urllib3>=1.22.0as a direct dependency. Later, we seeurllib3 ; python_version > '3.7'as a transitive dependency. As-is, we might (for some reason) pick a very old version ofurllib3to satisfyurllib3 ; python_version > '3.7', then attempt to fetch its dependencies, which could even involve building a very old version ofurllib3 ; python_version > '3.7'. Once we fetch the dependencies, we would see thaturllib3at the same version is also a dependency (because we tack it on). In the new scheme though, as soon as we "choose" the very old version ofurllib3 ; python_version > '3.7', we'd then see thaturllib3(the base package) is also a dependency; so we see a conflict before we even fetch the dependencies of the old variant.With this, I can successfully resolve the case in #4099.
Closes #4099.