mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-08-03 18:38:21 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Remove uses of Option<MarkerTree> (#5978)

Browse source 
## Summary

Follow up to https://github.com/astral-sh/uv/pull/5898. This should fix
some of the failures in https://github.com/astral-sh/uv/pull/5887 where
`uv lock --locked` is failing due to `Some(true)` and `None` markers not
comparing equal.
This commit is contained in:
Ibraheem Ahmed 2024-08-10 13:23:29 -04:00 • committed by GitHub
parent 4eced1bd0c
commit f5110f7b5e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
45 changed files with 228 additions and 330 deletions
Download patch file Download diff file Expand all files Collapse all files

107
crates/uv-resolver/src/lock.rs
View file

@ -94,30 +94,24 @@ impl Lock {
for package in &mut lock.packages {
for dep in &mut package.dependencies {
if let Some(marker) = &mut dep.marker {
*marker = marker.clone().simplify_python_versions(
python_version.clone(),
python_full_version.clone(),
);
}
dep.marker = dep.marker.clone().simplify_python_versions(
python_version.clone(),
python_full_version.clone(),
);
}
for dep in package.optional_dependencies.values_mut().flatten() {
if let Some(marker) = &mut dep.marker {
*marker = marker.clone().simplify_python_versions(
python_version.clone(),
python_full_version.clone(),
);
}
dep.marker = dep.marker.clone().simplify_python_versions(
python_version.clone(),
python_full_version.clone(),
);
}
for dep in package.dev_dependencies.values_mut().flatten() {
if let Some(marker) = &mut dep.marker {
*marker = marker.clone().simplify_python_versions(
python_version.clone(),
python_full_version.clone(),
);
}
dep.marker = dep.marker.clone().simplify_python_versions(
python_version.clone(),
python_full_version.clone(),
);
}
}
}
@ -146,8 +140,8 @@ impl Lock {
else {
continue;
};
let marker = edge.weight().as_ref();
locked_dist.add_dependency(dependency_dist, marker);
let marker = edge.weight().clone();
locked_dist.add_dependency(dependency_dist, marker.unwrap_or_default());
}
let id = locked_dist.id.clone();
if let Some(locked_dist) = locked_dists.insert(id, locked_dist) {
@ -178,8 +172,12 @@ impl Lock {
else {
continue;
};
let marker = edge.weight().as_ref();
locked_dist.add_optional_dependency(extra.clone(), dependency_dist, marker);
let marker = edge.weight().clone();
locked_dist.add_optional_dependency(
extra.clone(),
dependency_dist,
marker.unwrap_or_default(),
);
}
}
if let Some(group) = dist.dev.as_ref() {
@ -196,8 +194,12 @@ impl Lock {
else {
continue;
};
let marker = edge.weight().as_ref();
locked_dist.add_dev_dependency(group.clone(), dependency_dist, marker);
let marker = edge.weight().clone();
locked_dist.add_dev_dependency(
group.clone(),
dependency_dist,
marker.unwrap_or_default(),
);
}
}
}
@ -488,11 +490,7 @@ impl Lock {
))
};
for dep in deps {
if dep
.marker
.as_ref()
.map_or(true, |marker| marker.evaluate(marker_env, &[]))
{
if dep.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));
@ -772,7 +770,7 @@ impl Package {
}
/// Add the [`AnnotatedDist`] as a dependency of the [`Package`].
fn add_dependency(&mut self, annotated_dist: &AnnotatedDist, marker: Option<&MarkerTree>) {
fn add_dependency(&mut self, annotated_dist: &AnnotatedDist, marker: MarkerTree) {
let new_dep = Dependency::from_annotated_dist(annotated_dist, marker);
for existing_dep in &mut self.dependencies {
if existing_dep.package_id == new_dep.package_id
@ -790,7 +788,7 @@ impl Package {
&mut self,
extra: ExtraName,
annotated_dist: &AnnotatedDist,
marker: Option<&MarkerTree>,
marker: MarkerTree,
) {
self.optional_dependencies
.entry(extra)
@ -803,7 +801,7 @@ impl Package {
&mut self,
dev: GroupName,
annotated_dist: &AnnotatedDist,
marker: Option<&MarkerTree>,
marker: MarkerTree,
) {
self.dev_dependencies
.entry(dev)
@ -1072,15 +1070,11 @@ impl Package {
for dep in deps {
if let Some(mut dep) = dep.to_requirement(workspace_root, &mut dependency_extras)? {
// Add back the extra marker expression.
let marker = MarkerTree::expression(MarkerExpression::Extra {
operator: ExtraOperator::Equal,
name: extra.clone(),
});
match dep.marker {
Some(ref mut tree) => tree.and(marker),
None => dep.marker = Some(marker),
}
dep.marker
.and(MarkerTree::expression(MarkerExpression::Extra {
operator: ExtraOperator::Equal,
name: extra.clone(),
}));
requires_dist.push(dep);
}
@ -2282,17 +2276,13 @@ impl TryFrom<WheelWire> for Wheel {
struct Dependency {
package_id: PackageId,
extra: BTreeSet<ExtraName>,
marker: Option<MarkerTree>,
marker: MarkerTree,
}
impl Dependency {
fn from_annotated_dist(
annotated_dist: &AnnotatedDist,
marker: Option<&MarkerTree>,
) -> Dependency {
fn from_annotated_dist(annotated_dist: &AnnotatedDist, marker: MarkerTree) -> Dependency {
let package_id = PackageId::from_annotated_dist(annotated_dist);
let extra = annotated_dist.extra.iter().cloned().collect();
let marker = marker.cloned();
Dependency {
package_id,
extra,
@ -2389,7 +2379,7 @@ impl Dependency {
.collect::<Array>();
table.insert("extra", value(extra_array));
}
if let Some(marker) = self.marker.as_ref().and_then(MarkerTree::contents) {
if let Some(marker) = self.marker.contents() {
table.insert("marker", value(marker.to_string()));
}
@ -2425,7 +2415,8 @@ struct DependencyWire {
package_id: PackageIdForDependency,
#[serde(default)]
extra: BTreeSet<ExtraName>,
marker: Option<MarkerTree>,
#[serde(default)]
marker: MarkerTree,
}
impl DependencyWire {
@ -2816,10 +2807,8 @@ impl<'env> TreeDisplay<'env> {
// Skip dependencies that don't apply to the current environment.
if let Some(environment_markers) = markers {
if let Some(dependency_markers) = dependency.marker.as_ref() {
if !dependency_markers.evaluate(environment_markers, &[]) {
continue;
}
if !dependency.marker.evaluate(environment_markers, &[]) {
continue;
}
}
@ -2847,10 +2836,8 @@ impl<'env> TreeDisplay<'env> {
// Skip dependencies that don't apply to the current environment.
if let Some(environment_markers) = markers {
if let Some(dependency_markers) = dependency.marker.as_ref() {
if !dependency_markers.evaluate(environment_markers, &[]) {
continue;
}
if !dependency.marker.evaluate(environment_markers, &[]) {
continue;
}
}
@ -2884,10 +2871,8 @@ impl<'env> TreeDisplay<'env> {
// Skip dependencies that don't apply to the current environment.
if let Some(environment_markers) = markers {
if let Some(dependency_markers) = dependency.marker.as_ref() {
if !dependency_markers.evaluate(environment_markers, &[]) {
continue;
}
if !dependency.marker.evaluate(environment_markers, &[]) {
continue;
}
}

12
crates/uv-resolver/src/preferences.rs
View file

@ -23,7 +23,7 @@ pub struct Preference {
name: PackageName,
version: Version,
/// The markers on the requirement itself (those after the semicolon).
marker: Option<MarkerTree>,
marker: MarkerTree,
/// If coming from a package with diverging versions, the markers of the forks this preference
/// is part of, otherwise `None`.
fork_markers: Option<BTreeSet<MarkerTree>>,
@ -77,7 +77,7 @@ impl Preference {
Self {
name: dist.name().clone(),
version: version.clone(),
marker: None,
marker: MarkerTree::TRUE,
// Installed distributions don't have fork annotations.
fork_markers: None,
hashes: Vec::new(),
@ -89,7 +89,7 @@ impl Preference {
Self {
name: package.id.name.clone(),
version: package.id.version.clone(),
marker: None,
marker: MarkerTree::TRUE,
fork_markers: package.fork_markers().cloned(),
hashes: Vec::new(),
}
@ -128,11 +128,7 @@ impl Preferences {
for preference in preferences {
// Filter non-matching preferences when resolving for an environment.
if let Some(markers) = markers {
if !preference
.marker
.as_ref()
.map_or(true, |marker| marker.evaluate(markers, &[]))
{
if !preference.marker.evaluate(markers, &[]) {
trace!("Excluding {preference} from preferences due to unmatched markers");
continue;
}

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

@ -94,16 +94,14 @@ impl PubGrubPackage {
pub(crate) fn from_package(
name: PackageName,
extra: Option<ExtraName>,
marker: Option<MarkerTree>,
marker: MarkerTree,
) -> Self {
// Remove all extra expressions from the marker, since we track extras
// separately. This also avoids an issue where packages added via
// 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
.map(|m| m.simplify_extras_with(|_| true))
.and_then(|marker| marker.contents());
let marker = marker.simplify_extras_with(|_| true).contents();
if let Some(extra) = extra {
Self(Arc::new(PubGrubPackageInner::Extra {
name,

10
crates/uv-resolver/src/resolution/graph.rs
View file

@ -587,10 +587,7 @@ impl ResolutionGraph {
.constraints
.apply(self.overrides.apply(archive.metadata.requires_dist.iter()))
{
let Some(ref marker_tree) = req.marker else {
continue;
};
add_marker_params_from_tree(marker_tree, &mut seen_marker_values);
add_marker_params_from_tree(&req.marker, &mut seen_marker_values);
}
}
@ -599,10 +596,7 @@ impl ResolutionGraph {
.constraints
.apply(self.overrides.apply(self.requirements.iter()))
{
let Some(ref marker_tree) = direct_req.marker else {
continue;
};
add_marker_params_from_tree(marker_tree, &mut seen_marker_values);
add_marker_params_from_tree(&direct_req.marker, &mut seen_marker_values);
}
// Generate the final marker expression as a conjunction of

9
crates/uv-resolver/src/resolver/fork_map.rs
View file

@ -15,7 +15,7 @@ pub(crate) struct ForkMap<T>(FxHashMap<PackageName, Vec<Entry<T>>>);
#[derive(Debug, Clone)]
struct Entry<T> {
value: T,
marker: Option<MarkerTree>,
marker: MarkerTree,
}
impl<T> Default for ForkMap<T> {
@ -67,12 +67,7 @@ impl<T> ForkMap<T> {
// with the current fork, i.e. the markers are not disjoint.
ResolverMarkers::Fork(fork) => values
.iter()
.filter(|entry| {
!entry
.marker
.as_ref()
.is_some_and(|marker| fork.is_disjoint(marker))
})
.filter(|entry| !fork.is_disjoint(&entry.marker))
.map(|entry| &entry.value)
.collect(),

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

@ -1598,12 +1598,11 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// If the requirement is `requests ; sys_platform == 'darwin'` and the
// constraint is `requests ; python_version == '3.6'`, the constraint
// should only apply when _both_ markers are true.
if let Some(marker) = requirement.marker.as_ref() {
let marker = constraint.marker.as_ref().map(|m| {
let mut marker = marker.clone();
marker.and(m.clone());
marker
}).or_else(|| Some(marker.clone()));
if requirement.marker.is_true() {
Cow::Borrowed(constraint)
} else {
let mut marker = constraint.marker.clone();
marker.and(requirement.marker.clone());
Cow::Owned(Requirement {
name: constraint.name.clone(),
@ -1612,8 +1611,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
origin: constraint.origin.clone(),
marker
})
} else {
Cow::Borrowed(constraint)
}
})
)
@ -3120,8 +3117,5 @@ fn satisfies_requires_python(
/// Returns true if and only if the given requirement's marker expression has a
/// possible true value given the `markers` expression given.
fn possible_to_satisfy_markers(markers: &MarkerTree, requirement: &Requirement) -> bool {
let Some(marker) = requirement.marker.as_ref() else {
return true;
};
!markers.is_disjoint(marker)
!markers.is_disjoint(&requirement.marker)
}

2
crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap
View file

@ -99,7 +99,7 @@ Ok(
),
},
extra: {},
marker: None,
marker: true,
},
],
optional_dependencies: {},

2
crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap
View file

@ -99,7 +99,7 @@ Ok(
),
},
extra: {},
marker: None,
marker: true,
},
],
optional_dependencies: {},

2
crates/uv-resolver/src/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap
View file

@ -99,7 +99,7 @@ Ok(
),
},
extra: {},
marker: None,
marker: true,
},
],
optional_dependencies: {},