mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-10-29 03:02:55 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

uv-resolver: propagate markers to sibling dependencies in forks

Browse source 
When a fork occurs, we divide not just the dependencies that
provoked a fork into distinct groups, but we also add the
corresponding sibling dependencies to each fork. Previously,
while we track markers on the fork itself, the individual
dependencies that had markers only corresponded to markers
written from the dependency specification.

This meant that the sibling dependencies that got added to
each fork would not themselves have markers attached to them.
This in turn meant they would not have markers associated with
them in the lock file.

In many cases, this is actually okay, because the resolver will
pick a version that is "universal" across all forks in most
cases. But in some cases, this just simply isn't possible as
the marker expressions in the fork can and do influence resolution.
In which case, it is possible for the same package with different
versions to show up in the lock file unconditionally. Which is a
big no-no.

So in this commit, after we determine the forks, we intersect the
markers on each fork with each of its dependencies.

This does seem to balloon the marker expressions in some cases.
I plucked one low hanging fruit to avoid doing `x and x` in
trivial cases. (And this eliminated a portion of the snapshot
diffs.) But some pretty gnarly diffs remain.

This commit also fixes another bug: previously, when we created a fork
to capture the "remaining" universe of an incomplete set of markers, we
left out dependencies that should be included in that fork. We rectify
that here.

Fixes #5086

Partially addresses #4732
This commit is contained in:
Andrew Gallant 2024-07-17 13:55:48 -04:00 • committed by Andrew Gallant
parent 412780fd99
commit 77b005244d
6 changed files with 133 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

5
crates/uv-resolver/src/pubgrub/dependencies.rs
View file

@ -5,6 +5,7 @@ use pubgrub::range::Range;
use tracing::warn;
use pep440_rs::{Version, VersionSpecifiers};
use pep508_rs::MarkerTree;
use pypi_types::{
ParsedArchiveUrl, ParsedDirectoryUrl, ParsedGitUrl, ParsedPathUrl, ParsedUrl, Requirement,
RequirementSource, VerbatimParsedUrl,
@ -29,6 +30,10 @@ pub(crate) struct PubGrubDependency {
}
impl PubGrubDependency {
pub(crate) fn and_markers(&mut self, marker: &MarkerTree) {
self.package.and_markers(marker);
}
pub(crate) fn from_requirement<'a>(
requirement: &'a Requirement,
source_name: Option<&'a PackageName>,

75
crates/uv-resolver/src/pubgrub/package.rs
View file

@ -1,3 +1,4 @@
use std::mem;
use std::ops::Deref;
use std::sync::Arc;
@ -42,6 +43,9 @@ pub enum PubGrubPackageInner {
/// A Python version.
Python(PubGrubPython),
/// A Python package.
///
/// Note that it is guaranteed that `extra` and `dev` are never both
/// `Some`. That is, if one is `Some` then the other must be `None`.
Package {
name: PackageName,
extra: Option<ExtraName>,
@ -117,6 +121,77 @@ impl PubGrubPackage {
}
}
/// Modifies this package in place such that it is associated with the
/// given markers by intersecting them any pre-existing markers.
///
/// That is, this causes the package to only be applicable to marker
/// environments corresponding to the intersection of what it previously
/// matched and the given markers.
///
/// This is useful when one needs to propagate markers from a fork to each
/// of its constituent dependencies. This is necessary because within a
/// fork, the resolver makes decisions based on the markers that created
/// that fork. While in many cases these decisions are universal, some may
/// not be! And so the markers from the fork must be propagated out to the
/// individual packages.
pub(crate) fn and_markers(&mut self, fork_marker: &MarkerTree) {
// We do a little dance here to pluck out as much existing
// information from this package as we can to avoid allocs.
// It is possible that the `Arc::make_mut` will do a deep
// clone here, thereby defeating our efforts, but I think it
// is likely that in most cases it will not. This is because
// this routine should be called almost immediately after a
// `PubGrubPackage` is created and _before_ it is passed to
// PubGrub itself.
match Arc::make_mut(&mut self.0) {
PubGrubPackageInner::Root(_) | PubGrubPackageInner::Python(_) => {}
// In this case, we may or may not already have a marker.
// If we don't, then this implies we will, and thus we may
// need to switch `Package` to some other representation.
PubGrubPackageInner::Package {
ref mut name,
ref mut extra,
ref dev,
ref mut marker,
} => {
// This case *should* never happen, I believe, because
// a `Package` with a non-None `dev` can only happen as
// a result of a `Dev` package, which we should have
// processed already by the time we get this package.
if dev.is_some() {
return;
}
let mut and = fork_marker.clone();
if let Some(marker) = marker.take() {
and.and(marker);
}
*self = PubGrubPackage::from_package(mem::take(name), extra.take(), Some(and));
}
// These cases are easy, because we can just modify the marker
// in place.
PubGrubPackageInner::Extra { ref mut marker, .. } => {
let mut and = fork_marker.clone();
if let Some(marker) = marker.take() {
and.and(marker);
}
*marker = Some(and);
}
PubGrubPackageInner::Dev { ref mut marker, .. } => {
let mut and = fork_marker.clone();
if let Some(marker) = marker.take() {
and.and(marker);
}
*marker = Some(and);
}
PubGrubPackageInner::Marker { ref mut marker, .. } => {
let mut and = fork_marker.clone();
and.and(mem::replace(marker, MarkerTree::And(vec![])));
*marker = and;
}
}
}
/// Returns the name of this PubGrub package, if it has one.
pub(crate) fn name(&self) -> Option<&PackageName> {
match &**self {

38
crates/uv-resolver/src/resolver/mod.rs
View file

@ -2735,10 +2735,22 @@ impl Dependencies {
let mut new_forks: Vec<Fork> = vec![];
if let Some(markers) = fork_groups.remaining_universe() {
trace!("Adding split to cover possibly incomplete markers: {markers}");
new_forks.push(Fork {
dependencies: vec![],
markers,
});
let mut new_forks_for_remaining_universe = forks.clone();
for fork in &mut new_forks_for_remaining_universe {
fork.markers.and(markers.clone());
fork.dependencies.retain(|dep| {
let Some(dep_marker) = dep.package.marker() else {
return true;
};
// After we constrain the markers on an existing
// fork, we should ensure that any existing
// dependencies that are no longer possible in this
// fork are removed. This mirrors the check we do in
// `add_nonfork_package`.
!crate::marker::is_disjoint(&fork.markers, dep_marker)
});
}
new_forks.extend(new_forks_for_remaining_universe);
}
for group in fork_groups.forks {
let mut new_forks_for_group = forks.clone();
@ -2752,6 +2764,9 @@ impl Dependencies {
forks = new_forks;
diverging_packages.push(name.clone());
}
for fork in &mut forks {
fork.propagate_markers();
}
ForkedDependencies::Forked {
forks,
diverging_packages,
@ -2879,6 +2894,21 @@ impl Fork {
.map_or(true, |pkg_marker| !is_disjoint(pkg_marker, fork_marker))
});
}
/// This attaches the marker in this fork to each of its dependencies.
///
/// In effect, this "propagates" the markers to each individual dependency
/// that was spawned as the result of a fork. While in many cases the
/// markers will be combined when multiple forks choose the same version of
/// a dependency, in some cases, the version chosen can be specific to a
/// particular set of marker environments. In this case, the dependencies
/// will be platform specific and thus require marker expressions to appear
/// in the lock file.
fn propagate_markers(&mut self) {
for dependency in &mut self.dependencies {
dependency.and_markers(&self.markers);
}
}
}
/// Intermediate state that represents a *possible* grouping of forks

6
crates/uv/tests/lock.rs
View file

@ -2901,9 +2901,9 @@ fn lock_python_version_marker_complement() -> Result<()> {
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "attrs" },
{ name = "iniconfig" },
{ name = "typing-extensions", marker = "python_full_version <= '3.10' or python_full_version > '3.10'" },
{ name = "attrs", marker = "(python_full_version <= '3.10' and python_version == '3.10') or (python_full_version <= '3.10' and python_version < '3.10') or (python_full_version <= '3.10' and python_version > '3.10') or (python_full_version > '3.10' and python_version == '3.10') or (python_full_version > '3.10' and python_version < '3.10') or (python_full_version > '3.10' and python_version > '3.10')" },
{ name = "iniconfig", marker = "(python_full_version <= '3.10' and python_version == '3.10') or (python_full_version <= '3.10' and python_version < '3.10') or (python_full_version <= '3.10' and python_version < '3.10' and python_version > '3.10') or (python_full_version <= '3.10' and python_version > '3.10') or (python_full_version > '3.10' and python_version == '3.10') or (python_full_version > '3.10' and python_version < '3.10') or (python_full_version > '3.10' and python_version < '3.10' and python_version > '3.10') or (python_full_version > '3.10' and python_version > '3.10')" },
{ name = "typing-extensions", marker = "(python_full_version <= '3.10' and python_version == '3.10') or (python_full_version <= '3.10' and python_version < '3.10') or (python_full_version <= '3.10' and python_version < '3.10' and python_version > '3.10') or (python_full_version <= '3.10' and python_version > '3.10') or (python_full_version > '3.10' and python_version == '3.10') or (python_full_version > '3.10' and python_version < '3.10') or (python_full_version > '3.10' and python_version < '3.10' and python_version > '3.10') or (python_full_version > '3.10' and python_version > '3.10')" },
]
[[distribution]]

8
crates/uv/tests/lock_scenarios.rs
View file

@ -723,7 +723,7 @@ fn fork_incomplete_markers() -> Result<()> {
dependencies = [
{ name = "package-a", version = "1.0.0", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "python_version < '3.10'" },
{ name = "package-a", version = "2.0.0", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "python_version >= '3.11'" },
{ name = "package-b" },
{ name = "package-b", marker = "python_version < '3.10' or python_version >= '3.11' or (python_version < '3.11' and python_version >= '3.10')" },
]
"###
);
@ -1804,7 +1804,7 @@ fn fork_marker_limited_inherit() -> Result<()> {
dependencies = [
{ name = "package-a", version = "1.0.0", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "sys_platform == 'darwin'" },
{ name = "package-a", version = "2.0.0", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "sys_platform == 'linux'" },
{ name = "package-b" },
{ name = "package-b", marker = "sys_platform == 'darwin' or sys_platform == 'linux' or (sys_platform != 'darwin' and sys_platform != 'linux')" },
]
"###
);
@ -1919,7 +1919,7 @@ fn fork_marker_selection() -> Result<()> {
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "package-a" },
{ name = "package-a", marker = "sys_platform == 'darwin' or sys_platform == 'linux' or (sys_platform != 'darwin' and sys_platform != 'linux')" },
{ name = "package-b", version = "1.0.0", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "sys_platform == 'darwin'" },
{ name = "package-b", version = "2.0.0", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "sys_platform == 'linux'" },
]
@ -2059,7 +2059,7 @@ fn fork_marker_track() -> Result<()> {
version = "0.1.0"
source = { editable = "." }
dependencies = [
{ name = "package-a" },
{ name = "package-a", marker = "sys_platform == 'darwin' or sys_platform == 'linux' or (sys_platform != 'darwin' and sys_platform != 'linux')" },
{ name = "package-b", version = "2.7", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "sys_platform == 'darwin'" },
{ name = "package-b", version = "2.8", source = { registry = "https://astral-sh.github.io/packse/0.3.30/simple-html/" }, marker = "sys_platform == 'linux'" },
]

24
crates/uv/tests/pip_compile.rs
View file

@ -7298,7 +7298,7 @@ fn universal_nested_overlapping_local_requirement() -> Result<()> {
# uv pip compile --cache-dir [CACHE_DIR] requirements.in --universal
cmake==3.28.4 ; platform_machine == 'x86_64' and platform_system == 'Linux'
# via triton
. ; os_name == 'Linux'
. ; (os_name == 'Linux' and platform_machine == 'x86_64') or (os_name == 'Linux' and platform_machine != 'x86_64')
# via -r requirements.in
filelock==3.13.1
# via
@ -7766,7 +7766,7 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
# uv pip compile --cache-dir [CACHE_DIR] requirements.in -p 3.8 --universal
alabaster==0.7.13
# via sphinx
astroid==3.1.0
astroid==3.1.0 ; python_version < '3.11' or python_version >= '3.12'
# via pylint
babel==2.14.0
# via sphinx
@ -7778,7 +7778,7 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
# via
# pylint
# sphinx
dill==0.3.8
dill==0.3.8 ; python_version < '3.11' or python_version >= '3.12'
# via pylint
docutils==0.20.1
# via sphinx
@ -7788,17 +7788,17 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
# via sphinx
importlib-metadata==7.1.0 ; python_version < '3.10'
# via sphinx
isort==5.13.2
isort==5.13.2 ; python_version < '3.11' or python_version >= '3.12'
# via pylint
jinja2==3.1.3
# via sphinx
markupsafe==2.1.5
# via jinja2
mccabe==0.7.0
mccabe==0.7.0 ; python_version < '3.11' or python_version >= '3.12'
# via pylint
packaging==24.0
# via sphinx
platformdirs==4.2.0
platformdirs==4.2.0 ; python_version < '3.11' or python_version >= '3.12'
# via pylint
pygments==2.17.2
# via sphinx
@ -7826,7 +7826,7 @@ fn universal_no_repeated_unconditional_distributions() -> Result<()> {
# via sphinx
tomli==2.0.1 ; python_version < '3.11'
# via pylint
tomlkit==0.12.4
tomlkit==0.12.4 ; python_version < '3.11' or python_version >= '3.12'
# via pylint
typing-extensions==4.10.0 ; python_version < '3.11'
# via
@ -7916,7 +7916,7 @@ fn universal_marker_propagation() -> Result<()> {
# via requests
filelock==3.13.1
# via torch
fsspec==2024.3.1
fsspec==2024.3.1 ; platform_machine != 'x86_64'
# via torch
idna==3.6
# via requests
@ -7936,17 +7936,17 @@ fn universal_marker_propagation() -> Result<()> {
# via torchvision
sympy==1.12
# via torch
torch==2.0.0
torch==2.0.0 ; platform_machine == 'x86_64'
# via
# -r requirements.in
# torchvision
torch==2.2.0
torch==2.2.0 ; platform_machine != 'x86_64'
# via
# -r requirements.in
# torchvision
torchvision==0.15.1+rocm5.4.2
torchvision==0.15.1+rocm5.4.2 ; platform_machine == 'x86_64'
# via -r requirements.in
torchvision==0.17.0+rocm5.7
torchvision==0.17.0+rocm5.7 ; platform_machine != 'x86_64'
# via -r requirements.in
typing-extensions==4.10.0
# via torch