uv-resolver: refactor how we deal with requires-python

This commit refactors how deal with `requires-python` so that instead of
simplifying markers of dependencies inside the resolver, we do it at the
edges of our system. When writing markers to output, we simplify when
there's an obvious `requires-python` context. And when reading markers
as input, we complexity markers with the relevant `requires-python`
constraint.
This commit is contained in:
Andrew Gallant 2024-08-19 18:05:01 -04:00 committed by Andrew Gallant
parent 94a0a0f2ee
commit 4ff057e108
12 changed files with 544 additions and 164 deletions

View file

@ -222,6 +222,7 @@ impl std::fmt::Display for NoSolutionError {
// Transform the error tree for reporting
let mut tree = self.error.clone();
simplify_derivation_tree_markers(&self.python_requirement, &mut tree);
let should_display_tree = std::env::var_os("UV_INTERNAL__SHOW_DERIVATION_TREE").is_some()
|| tracing::enabled!(tracing::Level::TRACE);
@ -470,6 +471,52 @@ fn collapse_redundant_depends_on_no_versions_inner(
}
}
/// Simplifies the markers on pubgrub packages in the given derivation tree
/// according to the given Python requirement.
///
/// For example, when there's a dependency like `foo ; python_version >=
/// '3.11'` and `requires-python = '>=3.11'`, this simplification will remove
/// the `python_version >= '3.11'` marker since it's implied to be true by
/// the `requires-python` setting. This simplifies error messages by reducing
/// noise.
fn simplify_derivation_tree_markers(
python_requirement: &PythonRequirement,
tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
) {
match tree {
DerivationTree::External(External::NotRoot(ref mut pkg, _)) => {
pkg.simplify_markers(python_requirement);
}
DerivationTree::External(External::NoVersions(ref mut pkg, _)) => {
pkg.simplify_markers(python_requirement);
}
DerivationTree::External(External::FromDependencyOf(ref mut pkg1, _, ref mut pkg2, _)) => {
pkg1.simplify_markers(python_requirement);
pkg2.simplify_markers(python_requirement);
}
DerivationTree::External(External::Custom(ref mut pkg, _, _)) => {
pkg.simplify_markers(python_requirement);
}
DerivationTree::Derived(derived) => {
derived.terms = std::mem::take(&mut derived.terms)
.into_iter()
.map(|(mut pkg, term)| {
pkg.simplify_markers(python_requirement);
(pkg, term)
})
.collect();
simplify_derivation_tree_markers(
python_requirement,
Arc::make_mut(&mut derived.cause1),
);
simplify_derivation_tree_markers(
python_requirement,
Arc::make_mut(&mut derived.cause2),
);
}
}
}
/// Given a [`DerivationTree`], collapse incompatibilities for versions of a package that are
/// unavailable for the same reason to avoid repeating the same message for every unavailable
/// version.

View file

@ -39,6 +39,7 @@ use uv_workspace::{InstallTarget, Workspace};
pub use crate::lock::requirements_txt::RequirementsTxtExport;
pub use crate::lock::tree::TreeDisplay;
use crate::requires_python::SimplifiedMarkerTree;
use crate::resolution::{AnnotatedDist, ResolutionGraphNode};
use crate::{ExcludeNewer, PrereleaseMode, RequiresPython, ResolutionGraph, ResolutionMode};
@ -108,7 +109,7 @@ impl Lock {
continue;
};
let marker = edge.weight().clone();
package.add_dependency(dependency_dist, marker, root)?;
package.add_dependency(&requires_python, dependency_dist, marker, root)?;
}
let id = package.id.clone();
@ -141,6 +142,7 @@ impl Lock {
};
let marker = edge.weight().clone();
package.add_optional_dependency(
&requires_python,
extra.clone(),
dependency_dist,
marker,
@ -163,7 +165,13 @@ impl Lock {
continue;
};
let marker = edge.weight().clone();
package.add_dev_dependency(group.clone(), dependency_dist, marker, root)?;
package.add_dev_dependency(
&requires_python,
group.clone(),
dependency_dist,
marker,
root,
)?;
}
}
}
@ -370,7 +378,19 @@ impl Lock {
/// Record the supported environments that were used to generate this lock.
#[must_use]
pub fn with_supported_environments(mut self, supported_environments: Vec<MarkerTree>) -> Self {
self.supported_environments = supported_environments;
// We "complexify" the markers given, since the supported
// environments given might be coming directly from what's written in
// `pyproject.toml`, and those are assumed to be simplified (i.e.,
// they assume `requires-python` is true). But a `Lock` always uses
// non-simplified markers internally, so we need to re-complexify them
// here.
//
// The nice thing about complexifying is that it's a no-op if the
// markers given have already been complexified.
self.supported_environments = supported_environments
.into_iter()
.map(|marker| self.requires_python.complexify_markers(marker))
.collect();
self
}
@ -424,6 +444,22 @@ impl Lock {
&self.manifest.members
}
/// Returns the supported environments that were used to generate this
/// lock.
///
/// The markers returned here are "simplified" with respect to the lock
/// file's `requires-python` setting. This means these should only be used
/// for direct comparison purposes with the supported environments written
/// by a human in `pyproject.toml`. (Think of "supported environments" in
/// `pyproject.toml` as having an implicit `and python_full_version >=
/// '{requires-python-bound}'` attached to each one.)
pub fn simplified_supported_environments(&self) -> Vec<MarkerTree> {
self.supported_environments()
.iter()
.map(|marker| self.requires_python.simplify_markers(marker.clone()))
.collect()
}
/// If this lockfile was built from a forking resolution with non-identical forks, return the
/// markers of those forks, otherwise `None`.
pub fn fork_markers(&self) -> &[MarkerTree] {
@ -512,7 +548,7 @@ impl Lock {
))
};
for dep in deps {
if dep.marker.evaluate(marker_env, &[]) {
if dep.complexified_marker.evaluate(marker_env, &[]) {
let dep_dist = self.find_by_id(&dep.package_id);
if seen.insert((&dep.package_id, None)) {
queue.push_back((dep_dist, None));
@ -547,8 +583,8 @@ impl Lock {
let fork_markers = each_element_on_its_line_array(
self.fork_markers
.iter()
.filter_map(MarkerTree::contents)
.map(|marker| marker.to_string()),
.map(|marker| SimplifiedMarkerTree::new(&self.requires_python, marker.clone()))
.filter_map(|marker| marker.try_to_string()),
);
doc.insert("resolution-markers", value(fork_markers));
}
@ -557,8 +593,8 @@ impl Lock {
let supported_environments = each_element_on_its_line_array(
self.supported_environments
.iter()
.filter_map(MarkerTree::contents)
.map(|marker| marker.to_string()),
.map(|marker| SimplifiedMarkerTree::new(&self.requires_python, marker.clone()))
.filter_map(|marker| marker.try_to_string()),
);
doc.insert("supported-markers", value(supported_environments));
}
@ -682,7 +718,7 @@ impl Lock {
let mut packages = ArrayOfTables::new();
for dist in &self.packages {
packages.push(dist.to_toml(&dist_count_by_name)?);
packages.push(dist.to_toml(&self.requires_python, &dist_count_by_name)?);
}
doc.insert("package", Item::ArrayOfTables(packages));
@ -1174,9 +1210,9 @@ struct LockWire {
/// If this lockfile was built from a forking resolution with non-identical forks, store the
/// forks in the lockfile so we can recreate them in subsequent resolutions.
#[serde(rename = "resolution-markers", default)]
fork_markers: Vec<MarkerTree>,
fork_markers: Vec<SimplifiedMarkerTree>,
#[serde(rename = "supported-markers", default)]
supported_environments: Vec<MarkerTree>,
supported_environments: Vec<SimplifiedMarkerTree>,
/// We discard the lockfile if these options match.
#[serde(default)]
options: ResolverOptions,
@ -1186,20 +1222,6 @@ struct LockWire {
packages: Vec<PackageWire>,
}
impl From<Lock> for LockWire {
fn from(lock: Lock) -> LockWire {
LockWire {
version: lock.version,
requires_python: lock.requires_python,
fork_markers: lock.fork_markers,
supported_environments: lock.supported_environments,
options: lock.options,
manifest: lock.manifest,
packages: lock.packages.into_iter().map(PackageWire::from).collect(),
}
}
}
impl TryFrom<LockWire> for Lock {
type Error = LockError;
@ -1224,17 +1246,62 @@ impl TryFrom<LockWire> for Lock {
let packages = wire
.packages
.into_iter()
.map(|dist| dist.unwire(&unambiguous_package_ids))
.map(|dist| dist.unwire(&wire.requires_python, &unambiguous_package_ids))
.collect::<Result<Vec<_>, _>>()?;
Lock::new(
let supported_environments = wire
.supported_environments
.into_iter()
.map(|simplified_marker| simplified_marker.into_marker(&wire.requires_python))
.collect();
let fork_markers = wire
.fork_markers
.into_iter()
.map(|simplified_marker| simplified_marker.into_marker(&wire.requires_python))
.collect();
let lock = Lock::new(
wire.version,
packages,
wire.requires_python,
wire.options,
wire.manifest,
wire.supported_environments,
wire.fork_markers,
)
supported_environments,
fork_markers,
)?;
/*
// TODO: Use the below in tests to validate we don't produce a
// trivially incorrect lock file.
let mut name_to_markers: BTreeMap<&PackageName, Vec<(&Version, &MarkerTree)>> =
BTreeMap::new();
for package in &lock.packages {
for dep in &package.dependencies {
name_to_markers
.entry(&dep.package_id.name)
.or_default()
.push((&dep.package_id.version, &dep.marker));
}
}
for (name, marker_trees) in name_to_markers {
for (i, (version1, marker1)) in marker_trees.iter().enumerate() {
for (version2, marker2) in &marker_trees[i + 1..] {
if version1 == version2 {
continue;
}
if !marker1.is_disjoint(marker2) {
eprintln!("{}", lock.to_toml().unwrap());
assert!(
false,
"[{marker1:?}] (for version {version1}) is not disjoint with \
[{marker2:?}] (for version {version2}) \
for package `{name}`",
);
}
}
}
}
*/
Ok(lock)
}
}
@ -1316,14 +1383,38 @@ impl Package {
/// Add the [`AnnotatedDist`] as a dependency of the [`Package`].
fn add_dependency(
&mut self,
requires_python: &RequiresPython,
annotated_dist: &AnnotatedDist,
marker: MarkerTree,
root: &Path,
) -> Result<(), LockError> {
let new_dep = Dependency::from_annotated_dist(annotated_dist, marker, root)?;
let new_dep =
Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
for existing_dep in &mut self.dependencies {
if existing_dep.package_id == new_dep.package_id
&& existing_dep.marker == new_dep.marker
// It's important that we do a comparison on
// *simplified* markers here. In particular, when
// we write markers out to the lock file, we use
// "simplified" markers, or markers that are simplified
// *given* that `requires-python` is satisfied. So if
// we don't do equality based on what the simplified
// marker is, we might wind up not merging dependencies
// that ought to be merged and thus writing out extra
// entries.
//
// For example, if `requires-python = '>=3.8'` and we
// have `foo==1` and
// `foo==1 ; python_version >= '3.8'` dependencies,
// then they don't have equivalent complexified
// markers, but their simplified markers are identical.
//
// NOTE: It does seem like perhaps this should
// be implemented semantically/algebraically on
// `MarkerTree` itself, but it wasn't totally clear
// how to do that. I think `pep508` would need to
// grow a concept of "requires python" and provide an
// operation specifically for that.
&& existing_dep.simplified_marker == new_dep.simplified_marker
{
existing_dep.extra.extend(new_dep.extra);
return Ok(());
@ -1337,15 +1428,20 @@ impl Package {
/// Add the [`AnnotatedDist`] as an optional dependency of the [`Package`].
fn add_optional_dependency(
&mut self,
requires_python: &RequiresPython,
extra: ExtraName,
annotated_dist: &AnnotatedDist,
marker: MarkerTree,
root: &Path,
) -> Result<(), LockError> {
let dep = Dependency::from_annotated_dist(annotated_dist, marker, root)?;
let dep = Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
let optional_deps = self.optional_dependencies.entry(extra).or_default();
for existing_dep in &mut *optional_deps {
if existing_dep.package_id == dep.package_id && existing_dep.marker == dep.marker {
if existing_dep.package_id == dep.package_id
// See note in add_dependency for why we use
// simplified markers here.
&& existing_dep.simplified_marker == dep.simplified_marker
{
existing_dep.extra.extend(dep.extra);
return Ok(());
}
@ -1358,15 +1454,20 @@ impl Package {
/// Add the [`AnnotatedDist`] as a development dependency of the [`Package`].
fn add_dev_dependency(
&mut self,
requires_python: &RequiresPython,
dev: GroupName,
annotated_dist: &AnnotatedDist,
marker: MarkerTree,
root: &Path,
) -> Result<(), LockError> {
let dep = Dependency::from_annotated_dist(annotated_dist, marker, root)?;
let dep = Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
let dev_deps = self.dev_dependencies.entry(dev).or_default();
for existing_dep in &mut *dev_deps {
if existing_dep.package_id == dep.package_id && existing_dep.marker == dep.marker {
if existing_dep.package_id == dep.package_id
// See note in add_dependency for why we use
// simplified markers here.
&& existing_dep.simplified_marker == dep.simplified_marker
{
existing_dep.extra.extend(dep.extra);
return Ok(());
}
@ -1634,7 +1735,11 @@ impl Package {
Ok(Some(sdist))
}
fn to_toml(&self, dist_count_by_name: &FxHashMap<PackageName, u64>) -> anyhow::Result<Table> {
fn to_toml(
&self,
requires_python: &RequiresPython,
dist_count_by_name: &FxHashMap<PackageName, u64>,
) -> anyhow::Result<Table> {
let mut table = Table::new();
self.id.to_toml(None, &mut table);
@ -1643,28 +1748,27 @@ impl Package {
let wheels = each_element_on_its_line_array(
self.fork_markers
.iter()
.filter_map(MarkerTree::contents)
.map(|marker| marker.to_string()),
.map(|marker| SimplifiedMarkerTree::new(requires_python, marker.clone()))
.filter_map(|marker| marker.try_to_string()),
);
table.insert("resolution-markers", value(wheels));
}
if !self.dependencies.is_empty() {
let deps = each_element_on_its_line_array(
self.dependencies
.iter()
.map(|dep| dep.to_toml(dist_count_by_name).into_inline_table()),
);
let deps = each_element_on_its_line_array(self.dependencies.iter().map(|dep| {
dep.to_toml(requires_python, dist_count_by_name)
.into_inline_table()
}));
table.insert("dependencies", value(deps));
}
if !self.optional_dependencies.is_empty() {
let mut optional_deps = Table::new();
for (extra, deps) in &self.optional_dependencies {
let deps = each_element_on_its_line_array(
deps.iter()
.map(|dep| dep.to_toml(dist_count_by_name).into_inline_table()),
);
let deps = each_element_on_its_line_array(deps.iter().map(|dep| {
dep.to_toml(requires_python, dist_count_by_name)
.into_inline_table()
}));
if !deps.is_empty() {
optional_deps.insert(extra.as_ref(), value(deps));
}
@ -1677,10 +1781,10 @@ impl Package {
if !self.dev_dependencies.is_empty() {
let mut dev_dependencies = Table::new();
for (extra, deps) in &self.dev_dependencies {
let deps = each_element_on_its_line_array(
deps.iter()
.map(|dep| dep.to_toml(dist_count_by_name).into_inline_table()),
);
let deps = each_element_on_its_line_array(deps.iter().map(|dep| {
dep.to_toml(requires_python, dist_count_by_name)
.into_inline_table()
}));
if !deps.is_empty() {
dev_dependencies.insert(extra.as_ref(), value(deps));
}
@ -1849,7 +1953,7 @@ struct PackageWire {
#[serde(default)]
wheels: Vec<Wheel>,
#[serde(default, rename = "resolution-markers")]
fork_markers: Vec<MarkerTree>,
fork_markers: Vec<SimplifiedMarkerTree>,
#[serde(default)]
dependencies: Vec<DependencyWire>,
#[serde(default)]
@ -1870,11 +1974,12 @@ struct PackageMetadata {
impl PackageWire {
fn unwire(
self,
requires_python: &RequiresPython,
unambiguous_package_ids: &FxHashMap<PackageName, PackageId>,
) -> Result<Package, LockError> {
let unwire_deps = |deps: Vec<DependencyWire>| -> Result<Vec<Dependency>, LockError> {
deps.into_iter()
.map(|dep| dep.unwire(unambiguous_package_ids))
.map(|dep| dep.unwire(requires_python, unambiguous_package_ids))
.collect()
};
Ok(Package {
@ -1882,7 +1987,11 @@ impl PackageWire {
metadata: self.metadata,
sdist: self.sdist,
wheels: self.wheels,
fork_markers: self.fork_markers,
fork_markers: self
.fork_markers
.into_iter()
.map(|simplified_marker| simplified_marker.into_marker(requires_python))
.collect(),
dependencies: unwire_deps(self.dependencies)?,
optional_dependencies: self
.optional_dependencies
@ -1898,32 +2007,6 @@ impl PackageWire {
}
}
impl From<Package> for PackageWire {
fn from(dist: Package) -> PackageWire {
let wire_deps = |deps: Vec<Dependency>| -> Vec<DependencyWire> {
deps.into_iter().map(DependencyWire::from).collect()
};
PackageWire {
id: dist.id,
metadata: dist.metadata,
sdist: dist.sdist,
wheels: dist.wheels,
fork_markers: dist.fork_markers,
dependencies: wire_deps(dist.dependencies),
optional_dependencies: dist
.optional_dependencies
.into_iter()
.map(|(extra, deps)| (extra, wire_deps(deps)))
.collect(),
dev_dependencies: dist
.dev_dependencies
.into_iter()
.map(|(group, deps)| (group, wire_deps(deps)))
.collect(),
}
}
}
/// Inside the lockfile, we match a dependency entry to a package entry through a key made up
/// of the name, the version and the source url.
#[derive(Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord, serde::Deserialize)]
@ -3131,26 +3214,70 @@ impl TryFrom<WheelWire> for Wheel {
struct Dependency {
package_id: PackageId,
extra: BTreeSet<ExtraName>,
marker: MarkerTree,
/// A marker simplified by assuming `requires-python` is satisfied.
/// So if `requires-python = '>=3.8'`, then
/// `python_version >= '3.8' and python_version < '3.12'`
/// gets simplfiied to `python_version < '3.12'`.
///
/// Generally speaking, this marker should not be exposed to
/// anything outside this module unless it's for a specialized use
/// case. But specifically, it should never be used to evaluate
/// against a marker environment or for disjointness checks or any
/// other kind of marker algebra.
///
/// It exists because there are some cases where we do actually
/// want to compare markers in their "simplified" form. For
/// example, when collapsing the extras on duplicate dependencies.
/// Even if a dependency has different complexified markers,
/// they might have identical markers once simplified. And since
/// `requires-python` applies to the entire lock file, it's
/// acceptable to do comparisons on the simplified form.
simplified_marker: SimplifiedMarkerTree,
/// The "complexified" marker is a marker that can stand on its
/// own independent of `requires-python`. It can be safely used
/// for any kind of marker algebra.
complexified_marker: MarkerTree,
}
impl Dependency {
fn new(
requires_python: &RequiresPython,
package_id: PackageId,
extra: BTreeSet<ExtraName>,
complexified_marker: MarkerTree,
) -> Dependency {
let simplified_marker =
SimplifiedMarkerTree::new(requires_python, complexified_marker.clone());
Dependency {
package_id,
extra,
simplified_marker,
complexified_marker,
}
}
fn from_annotated_dist(
requires_python: &RequiresPython,
annotated_dist: &AnnotatedDist,
marker: MarkerTree,
complexified_marker: MarkerTree,
root: &Path,
) -> Result<Dependency, LockError> {
let package_id = PackageId::from_annotated_dist(annotated_dist, root)?;
let extra = annotated_dist.extra.iter().cloned().collect();
Ok(Self {
Ok(Dependency::new(
requires_python,
package_id,
extra,
marker,
})
complexified_marker,
))
}
/// Returns the TOML representation of this dependency.
fn to_toml(&self, dist_count_by_name: &FxHashMap<PackageName, u64>) -> Table {
fn to_toml(
&self,
_requires_python: &RequiresPython,
dist_count_by_name: &FxHashMap<PackageName, u64>,
) -> Table {
let mut table = Table::new();
self.package_id
.to_toml(Some(dist_count_by_name), &mut table);
@ -3162,8 +3289,8 @@ impl Dependency {
.collect::<Array>();
table.insert("extra", value(extra_array));
}
if let Some(marker) = self.marker.contents() {
table.insert("marker", value(marker.to_string()));
if let Some(marker) = self.simplified_marker.try_to_string() {
table.insert("marker", value(marker));
}
table
@ -3199,32 +3326,25 @@ struct DependencyWire {
#[serde(default)]
extra: BTreeSet<ExtraName>,
#[serde(default)]
marker: MarkerTree,
marker: SimplifiedMarkerTree,
}
impl DependencyWire {
fn unwire(
self,
requires_python: &RequiresPython,
unambiguous_package_ids: &FxHashMap<PackageName, PackageId>,
) -> Result<Dependency, LockError> {
let complexified_marker = self.marker.clone().into_marker(requires_python);
Ok(Dependency {
package_id: self.package_id.unwire(unambiguous_package_ids)?,
extra: self.extra,
marker: self.marker,
simplified_marker: self.marker,
complexified_marker,
})
}
}
impl From<Dependency> for DependencyWire {
fn from(dependency: Dependency) -> DependencyWire {
DependencyWire {
package_id: PackageIdForDependency::from(dependency.package_id),
extra: dependency.extra,
marker: dependency.marker,
}
}
}
/// A single hash for a distribution artifact in a lockfile.
///
/// A hash is encoded as a single TOML string in the format

View file

@ -102,7 +102,11 @@ impl<'lock> RequirementsTxtExport<'lock> {
// Add the edge.
let dep_index = inverse[&dep.package_id];
petgraph.add_edge(index, dep_index, dep.marker.clone());
petgraph.add_edge(
index,
dep_index,
dep.simplified_marker.as_simplified_marker_tree().clone(),
);
// Push its dependencies on the queue.
if seen.insert((&dep.package_id, None)) {

View file

@ -62,7 +62,8 @@ impl<'env> TreeDisplay<'env> {
Cow::Owned(Dependency {
package_id: packages.id.clone(),
extra: dependency.extra.clone(),
marker: dependency.marker.clone(),
simplified_marker: dependency.simplified_marker.clone(),
complexified_marker: dependency.complexified_marker.clone(),
})
} else {
Cow::Borrowed(dependency)
@ -72,7 +73,10 @@ impl<'env> TreeDisplay<'env> {
// Skip dependencies that don't apply to the current environment.
if let Some(environment_markers) = markers {
if !dependency.marker.evaluate(environment_markers, &[]) {
if !dependency
.complexified_marker
.evaluate(environment_markers, &[])
{
continue;
}
}
@ -91,7 +95,8 @@ impl<'env> TreeDisplay<'env> {
Cow::Owned(Dependency {
package_id: packages.id.clone(),
extra: dependency.extra.clone(),
marker: dependency.marker.clone(),
simplified_marker: dependency.simplified_marker.clone(),
complexified_marker: dependency.complexified_marker.clone(),
})
} else {
Cow::Borrowed(dependency)
@ -101,7 +106,10 @@ impl<'env> TreeDisplay<'env> {
// Skip dependencies that don't apply to the current environment.
if let Some(environment_markers) = markers {
if !dependency.marker.evaluate(environment_markers, &[]) {
if !dependency
.complexified_marker
.evaluate(environment_markers, &[])
{
continue;
}
}
@ -126,7 +134,8 @@ impl<'env> TreeDisplay<'env> {
Cow::Owned(Dependency {
package_id: packages.id.clone(),
extra: dependency.extra.clone(),
marker: dependency.marker.clone(),
simplified_marker: dependency.simplified_marker.clone(),
complexified_marker: dependency.complexified_marker.clone(),
})
} else {
Cow::Borrowed(dependency)
@ -136,7 +145,10 @@ impl<'env> TreeDisplay<'env> {
// Skip dependencies that don't apply to the current environment.
if let Some(environment_markers) = markers {
if !dependency.marker.evaluate(environment_markers, &[]) {
if !dependency
.complexified_marker
.evaluate(environment_markers, &[])
{
continue;
}
}

View file

@ -4,6 +4,8 @@ use std::sync::Arc;
use pep508_rs::{MarkerTree, MarkerTreeContents};
use uv_normalize::{ExtraName, GroupName, PackageName};
use crate::python_requirement::PythonRequirement;
/// [`Arc`] wrapper around [`PubGrubPackageInner`] to make cloning (inside PubGrub) cheap.
#[derive(Debug, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)]
pub struct PubGrubPackage(Arc<PubGrubPackageInner>);
@ -85,7 +87,8 @@ pub enum PubGrubPackageInner {
/// rather than the `Marker` variant.
Marker {
name: PackageName,
marker: MarkerTreeContents,
/// The marker associated with this proxy package.
marker: MarkerTree,
},
}
@ -101,15 +104,16 @@ impl PubGrubPackage {
// extras end up having two distinct marker expressions, which in turn
// makes them two distinct packages. This results in PubGrub being
// unable to unify version constraints across such packages.
let marker = marker.simplify_extras_with(|_| true).contents();
let tree = marker.simplify_extras_with(|_| true);
let marker = tree.contents();
if let Some(extra) = extra {
Self(Arc::new(PubGrubPackageInner::Extra {
name,
extra,
marker,
}))
} else if let Some(marker) = marker {
Self(Arc::new(PubGrubPackageInner::Marker { name, marker }))
} else if marker.is_some() {
Self(Arc::new(PubGrubPackageInner::Marker { name, marker: tree }))
} else {
Self(Arc::new(PubGrubPackageInner::Package {
name,
@ -158,7 +162,7 @@ impl PubGrubPackage {
| PubGrubPackageInner::Dev { marker, .. } => {
marker.as_ref().map(MarkerTreeContents::as_ref)
}
PubGrubPackageInner::Marker { marker, .. } => Some(marker.as_ref()),
PubGrubPackageInner::Marker { marker, .. } => Some(marker),
}
}
@ -171,6 +175,47 @@ impl PubGrubPackage {
| PubGrubPackageInner::Marker { .. }
)
}
/// This simplifies the markers on this package (if any exist) using the
/// given Python requirement as assumed context.
///
/// See `RequiresPython::simplify_markers` for more details.
///
/// NOTE: This routine is kind of weird, because this should only really be
/// applied in contexts where the `PubGrubPackage` is printed as output.
/// So in theory, this should be a transformation into a new type with a
/// "printable" `PubGrubPackage` coupled with a `Requires-Python`. But at
/// time of writing, this was a larger refactor, particularly in the error
/// reporting where this routine is used.
pub(crate) fn simplify_markers(&mut self, python_requirement: &PythonRequirement) {
match *Arc::make_mut(&mut self.0) {
PubGrubPackageInner::Root(_) | PubGrubPackageInner::Python(_) => {}
PubGrubPackageInner::Package { ref mut marker, .. }
| PubGrubPackageInner::Extra { ref mut marker, .. }
| PubGrubPackageInner::Dev { ref mut marker, .. } => {
let Some(contents) = marker.as_mut() else {
return;
};
let tree = MarkerTree::from(contents.clone());
*marker = python_requirement.simplify_markers(tree).contents();
}
PubGrubPackageInner::Marker { ref mut marker, .. } => {
*marker = python_requirement.simplify_markers(marker.clone());
}
}
}
#[allow(dead_code)]
pub(crate) fn kind(&self) -> &'static str {
match &**self {
PubGrubPackageInner::Root(_) => "root",
PubGrubPackageInner::Python(_) => "python",
PubGrubPackageInner::Package { .. } => "package",
PubGrubPackageInner::Extra { .. } => "extra",
PubGrubPackageInner::Dev { .. } => "dev",
PubGrubPackageInner::Marker { .. } => "marker",
}
}
}
#[derive(Debug, Copy, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]
@ -234,7 +279,13 @@ impl std::fmt::Display for PubGrubPackageInner {
} => {
write!(f, "{name}[{dev}]{{{marker}}}")
}
Self::Marker { name, marker, .. } => write!(f, "{name}{{{marker}}}"),
Self::Marker { name, marker, .. } => {
if let Some(marker) = marker.contents() {
write!(f, "{name}{{{marker}}}")
} else {
write!(f, "{name}")
}
}
Self::Extra { name, extra, .. } => write!(f, "{name}[{extra}]"),
Self::Dev { name, dev, .. } => write!(f, "{name}:{dev}"),
// It is guaranteed that `extra` and `dev` are never set at the same time.

View file

@ -1,4 +1,5 @@
use pep440_rs::Version;
use pep508_rs::MarkerTree;
use uv_python::{Interpreter, PythonVersion};
use crate::{RequiresPython, RequiresPythonRange};
@ -89,6 +90,22 @@ impl PythonRequirement {
pub fn source(&self) -> PythonRequirementSource {
self.source
}
/// A wrapper around `RequiresPython::simplify_markers`. See its docs for
/// more info.
///
/// When this `PythonRequirement` isn't `RequiresPython`, the given markers
/// are returned unchanged.
pub(crate) fn simplify_markers(&self, marker: MarkerTree) -> MarkerTree {
self.target.simplify_markers(marker)
}
/// Return a [`MarkerTree`] representing the Python requirement.
///
/// See: [`RequiresPython::to_marker_tree`]
pub fn to_marker_tree(&self) -> MarkerTree {
self.target.to_marker_tree()
}
}
#[derive(Debug, Copy, Clone, Eq, PartialEq, PartialOrd, Hash, Ord)]

View file

@ -125,8 +125,19 @@ impl RequiresPython {
}
}
/// Returns the [`RequiresPython`] as a [`MarkerTree`].
pub fn markers(&self) -> MarkerTree {
/// Returns this `Requires-Python` specifier as an equivalent
/// [`MarkerTree`] utilizing the `python_full_version` marker field.
///
/// This is useful for comparing a `Requires-Python` specifier with
/// arbitrary marker expressions. For example, one can ask whether the
/// returned marker expression is disjoint with another marker expression.
/// If it is, then one can conclude that the `Requires-Python` specifier
/// excludes the dependency with that other marker expression.
///
/// If this `Requires-Python` specifier has no constraints, then this
/// returns a marker tree that evaluates to `true` for all possible marker
/// environments.
pub fn to_marker_tree(&self) -> MarkerTree {
match (self.range.0.as_ref(), self.range.1.as_ref()) {
(Bound::Included(lower), Bound::Included(upper)) => {
let mut lower = MarkerTree::expression(MarkerExpression::Version {
@ -283,6 +294,69 @@ impl RequiresPython {
&self.range
}
/// Simplifies the given markers in such a way as to assume that
/// the Python version is constrained by this Python version bound.
///
/// For example, with `requires-python = '>=3.8'`, a marker like this:
///
/// ```text
/// python_full_version >= '3.8' and python_full_version < '3.12'
/// ```
///
/// Will be simplified to:
///
/// ```text
/// python_full_version < '3.12'
/// ```
///
/// That is, `python_full_version >= '3.8'` is assumed to be true by virtue
/// of `requires-python`, and is thus not needed in the marker.
///
/// This should be used in contexts in which this assumption is valid to
/// make. Generally, this means it should not be used inside the resolver,
/// but instead near the boundaries of the system (like formatting error
/// messages and writing the lock file). The reason for this is that
/// this simplification fundamentally changes the meaning of the marker,
/// and the *only* correct way to interpret it is in a context in which
/// `requires-python` is known to be true. For example, when markers from
/// a lock file are deserialized and turned into a `ResolutionGraph`, the
/// markers are "complexified" to put the `requires-python` assumption back
/// into the marker explicitly.
pub(crate) fn simplify_markers(&self, marker: MarkerTree) -> MarkerTree {
let simplified = marker.simplify_python_versions(Range::from(self.range().clone()));
// FIXME: This is a hack to avoid the hidden state created by
// ADD's `restrict_versions`. I believe this is sound, but it's
// wasteful and silly.
simplified
.try_to_string()
.map(|s| s.parse().unwrap())
.unwrap_or(MarkerTree::TRUE)
}
/// The inverse of `simplify_markers`.
///
/// This should be applied near the boundaries of uv when markers are
/// deserialized from a context where `requires-python` is assumed. For
/// example, with `requires-python = '>=3.8'` and a marker like:
///
/// ```text
/// python_full_version < '3.12'
/// ```
///
/// It will be "complexified" to:
///
/// ```text
/// python_full_version >= '3.8' and python_full_version < '3.12'
/// ```
pub(crate) fn complexify_markers(&self, mut marker: MarkerTree) -> MarkerTree {
// PERF: There's likely a way to amortize this, particularly
// the construction of `to_marker_tree`. But at time of
// writing, it wasn't clear if this was an actual perf problem
// or not. If it is, try a `std::sync::OnceLock`.
marker.and(self.to_marker_tree());
marker
}
/// Returns `false` if the wheel's tags state it can't be used in the given Python version
/// range.
///
@ -477,6 +551,46 @@ impl Ord for RequiresPythonBound {
}
}
/// A simplified marker is just like a normal marker, except it has possibly
/// been simplified by `requires-python`.
///
/// A simplified marker should only exist in contexts where a `requires-python`
/// setting can be assumed. In order to get a "normal" marker out of
/// a simplified marker, one must re-contextualize it by adding the
/// `requires-python` constraint back to the marker.
#[derive(Clone, Debug, Default, Eq, PartialEq, PartialOrd, Ord, serde::Deserialize)]
pub(crate) struct SimplifiedMarkerTree(MarkerTree);
impl SimplifiedMarkerTree {
/// Simplifies the given markers by assuming the given `requires-python`
/// bound is true.
pub(crate) fn new(
requires_python: &RequiresPython,
marker: MarkerTree,
) -> SimplifiedMarkerTree {
SimplifiedMarkerTree(requires_python.simplify_markers(marker))
}
/// Complexifies the given markers by adding the given `requires-python` as
/// a constraint to these simplified markers.
pub(crate) fn into_marker(self, requires_python: &RequiresPython) -> MarkerTree {
requires_python.complexify_markers(self.0)
}
/// Attempts to convert this simplified marker to a string.
///
/// This only returns `None` when the underlying marker is always true,
/// i.e., it matches all possible marker environments.
pub(crate) fn try_to_string(&self) -> Option<String> {
self.0.try_to_string()
}
/// Returns the underlying marker tree without re-complexifying them.
pub(crate) fn as_simplified_marker_tree(&self) -> &MarkerTree {
&self.0
}
}
#[cfg(test)]
mod tests {
use std::cmp::Ordering;

View file

@ -187,7 +187,11 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> {
for (index, node) in nodes {
// Display the node itself.
let mut line = node
.to_requirements_txt(self.include_extras, self.include_markers)
.to_requirements_txt(
&self.resolution.requires_python,
self.include_extras,
self.include_markers,
)
.to_string();
// Display the distribution hashes, if any.

View file

@ -10,7 +10,10 @@ use pep508_rs::{split_scheme, MarkerTree, Scheme};
use pypi_types::HashDigest;
use uv_normalize::{ExtraName, PackageName};
use crate::resolution::AnnotatedDist;
use crate::{
requires_python::{RequiresPython, SimplifiedMarkerTree},
resolution::AnnotatedDist,
};
#[derive(Debug, Clone)]
/// A pinned package with its resolved distribution and all the extras that were pinned for it.
@ -31,6 +34,7 @@ impl RequirementsTxtDist {
/// supported in `requirements.txt`).
pub(crate) fn to_requirements_txt(
&self,
requires_python: &RequiresPython,
include_extras: bool,
include_markers: bool,
) -> Cow<str> {
@ -90,7 +94,9 @@ impl RequirementsTxtDist {
};
if let Some(given) = given {
return if let Some(markers) =
self.markers.contents().filter(|_| include_markers)
SimplifiedMarkerTree::new(requires_python, self.markers.clone())
.try_to_string()
.filter(|_| include_markers)
{
Cow::Owned(format!("{given} ; {markers}"))
} else {
@ -101,7 +107,10 @@ impl RequirementsTxtDist {
}
if self.extras.is_empty() || !include_extras {
if let Some(markers) = self.markers.contents().filter(|_| include_markers) {
if let Some(markers) = SimplifiedMarkerTree::new(requires_python, self.markers.clone())
.try_to_string()
.filter(|_| include_markers)
{
Cow::Owned(format!("{} ; {}", self.dist.verbatim(), markers))
} else {
self.dist.verbatim()
@ -110,7 +119,10 @@ impl RequirementsTxtDist {
let mut extras = self.extras.clone();
extras.sort_unstable();
extras.dedup();
if let Some(markers) = self.markers.contents().filter(|_| include_markers) {
if let Some(markers) = SimplifiedMarkerTree::new(requires_python, self.markers.clone())
.try_to_string()
.filter(|_| include_markers)
{
Cow::Owned(format!(
"{}[{}]{} ; {}",
self.name(),

View file

@ -562,6 +562,8 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
resolution.nodes.len()
);
}
}
for resolution in &resolutions {
Self::trace_resolution(resolution);
}
ResolutionGraph::from_state(
@ -1329,7 +1331,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
name: name.clone(),
extra: None,
dev: None,
marker: marker.cloned(),
marker: marker.and_then(MarkerTree::contents),
}),
version: Range::singleton(version.clone()),
specifier: None,
@ -1481,34 +1483,16 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
self.overrides
.apply(dependencies)
.filter_map(move |requirement| {
let python_marker = python_requirement.to_marker_tree();
// If the requirement would not be selected with any Python version
// supported by the root, skip it.
let requirement = if requirement.marker.is_true() {
requirement
} else {
let requires_python = python_requirement.target();
let marker = requirement.marker.clone().simplify_python_versions(
Range::from(requires_python.range().clone()),
if python_marker.is_disjoint(&requirement.marker) {
trace!(
"skipping {requirement} because of Requires-Python: {requires_python}",
requires_python = python_requirement.target(),
);
if marker.is_false() {
trace!("skipping {requirement} because of Requires-Python: {requires_python}");
return None;
}
if marker == requirement.marker {
requirement
} else {
Cow::Owned(Requirement {
name: requirement.name.clone(),
extras: requirement.extras.clone(),
source: requirement.source.clone(),
origin: requirement.origin.clone(),
marker
})
}
};
return None;
}
// If we're in a fork in universal mode, ignore any dependency that isn't part of
// this fork (but will be part of another fork).
@ -1572,10 +1556,9 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
} else {
let requires_python = python_requirement.target();
let python_marker = python_requirement.to_marker_tree();
let mut marker = constraint.marker.clone().simplify_python_versions(
Range::from(requires_python.range().clone()),
);
let mut marker = constraint.marker.clone();
marker.and(requirement.marker.clone());
// Additionally, if the requirement is `requests ; sys_platform == 'darwin'`
@ -1585,6 +1568,13 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
trace!("skipping {constraint} because of Requires-Python: {requires_python}");
return None;
}
if python_marker.is_disjoint(&marker) {
trace!(
"skipping constraint {requirement} because of Requires-Python: {requires_python}",
requires_python = python_requirement.target(),
);
return None;
}
if marker == constraint.marker {
Cow::Borrowed(constraint)
@ -2345,7 +2335,7 @@ impl ForkState {
to_url: to_url.cloned(),
to_extra: None,
to_dev: None,
marker: MarkerTree::from(dependency_marker.clone()),
marker: dependency_marker.clone(),
};
edges.insert(edge);
}
@ -2685,6 +2675,8 @@ impl Forks {
name_to_deps: BTreeMap<PackageName, Vec<PubGrubDependency>>,
python_requirement: &PythonRequirement,
) -> Forks {
let python_marker = python_requirement.to_marker_tree();
let mut forks = vec![Fork {
dependencies: vec![],
markers: MarkerTree::TRUE,
@ -2713,7 +2705,6 @@ impl Forks {
continue;
}
for dep in deps {
// We assume that the marker has already been Python-simplified.
let mut markers = dep.package.marker().cloned().unwrap_or(MarkerTree::TRUE);
if markers.is_false() {
// If the markers can never be satisfied, then we
@ -2740,17 +2731,25 @@ impl Forks {
continue;
}
let not_markers = simplify_python(markers.negate(), python_requirement);
let not_markers = markers.negate();
let mut new_markers = markers.clone();
new_markers.and(simplify_python(fork.markers.negate(), python_requirement));
new_markers.and(fork.markers.negate());
if !fork.markers.is_disjoint(&not_markers) {
let mut new_fork = fork.clone();
new_fork.intersect(not_markers);
new.push(new_fork);
// Filter out any forks we created that are disjoint with our
// Python requirement.
if !new_fork.markers.is_disjoint(&python_marker) {
new.push(new_fork);
}
}
fork.dependencies.push(dep.clone());
fork.intersect(markers);
new.push(fork);
// Filter out any forks we created that are disjoint with our
// Python requirement.
if !fork.markers.is_disjoint(&python_marker) {
new.push(fork);
}
markers = new_markers;
}
forks = new;
@ -2848,8 +2847,3 @@ impl PartialOrd for Fork {
Some(self.cmp(other))
}
}
/// Simplify a [`MarkerTree`] based on a [`PythonRequirement`].
fn simplify_python(marker: MarkerTree, python_requirement: &PythonRequirement) -> MarkerTree {
marker.simplify_python_versions(Range::from(python_requirement.target().range().clone()))
}

View file

@ -331,7 +331,7 @@ async fn do_lock(
.into_iter()
.flatten()
{
if requires_python.markers().is_disjoint(environment) {
if requires_python.to_marker_tree().is_disjoint(environment) {
return if let Some(contents) = environment.contents() {
Err(ProjectError::DisjointEnvironment(
contents,
@ -637,7 +637,7 @@ impl ValidatedLock {
}
// If the set of supported environments has changed, we have to perform a clean resolution.
if lock.supported_environments()
if lock.simplified_supported_environments()
!= environments
.map(SupportedEnvironments::as_markers)
.unwrap_or_default()

View file

@ -192,7 +192,12 @@ pub(super) async fn do_sync(
if !environments.is_empty() {
if !environments.iter().any(|env| env.evaluate(&markers, &[])) {
return Err(ProjectError::LockedPlatformIncompatibility(
environments
// For error reporting, we use the "simplified"
// supported environments, because these correspond to
// what the end user actually wrote. The non-simplified
// environments, by contrast, are explicitly
// constrained by `requires-python`.
lock.simplified_supported_environments()
.iter()
.filter_map(MarkerTree::contents)
.map(|env| format!("`{env}`"))