diff --git a/crates/uv-resolver/src/pubgrub/dependencies.rs b/crates/uv-resolver/src/pubgrub/dependencies.rs index 97a1a1e4a..ab9243213 100644 --- a/crates/uv-resolver/src/pubgrub/dependencies.rs +++ b/crates/uv-resolver/src/pubgrub/dependencies.rs @@ -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>, diff --git a/crates/uv-resolver/src/pubgrub/package.rs b/crates/uv-resolver/src/pubgrub/package.rs index e677b4d0d..82fe2eb3e 100644 --- a/crates/uv-resolver/src/pubgrub/package.rs +++ b/crates/uv-resolver/src/pubgrub/package.rs @@ -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, @@ -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 { diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 19cb4be6c..8d94fcef7 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -2735,10 +2735,22 @@ impl Dependencies { let mut new_forks: Vec = 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 diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index ffd99710c..0fcc3a58a 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -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]] diff --git a/crates/uv/tests/lock_scenarios.rs b/crates/uv/tests/lock_scenarios.rs index 5b7cf9871..553ab3136 100644 --- a/crates/uv/tests/lock_scenarios.rs +++ b/crates/uv/tests/lock_scenarios.rs @@ -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'" }, ] diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index d6fe40a6a..2fc3b5072 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -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