Skip to content

Commit

Permalink
Respect all platforms
Browse files Browse the repository at this point in the history
Uhh trying

Fork on...

Add heuristic
  • Loading branch information
charliermarsh committed Dec 18, 2024
1 parent 4d3c1b3 commit 9a41883
Show file tree
Hide file tree
Showing 11 changed files with 1,475 additions and 444 deletions.
114 changes: 103 additions & 11 deletions crates/uv-distribution-types/src/prioritized_distribution.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::fmt::{Display, Formatter};
use uv_distribution_filename::BuildTag;
use uv_distribution_filename::{BuildTag, WheelFilename};

use uv_pep440::VersionSpecifiers;
use uv_pep508::{MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString};
use uv_platform_tags::{IncompatibleTag, TagPriority};
use uv_pypi_types::{HashDigest, Yanked};

Expand All @@ -14,7 +15,7 @@ use crate::{
pub struct PrioritizedDist(Box<PrioritizedDistInner>);

/// [`PrioritizedDist`] is boxed because [`Dist`] is large.
#[derive(Debug, Default, Clone)]
#[derive(Debug, Clone)]
struct PrioritizedDistInner {
/// The highest-priority source distribution. Between compatible source distributions this priority is arbitrary.
source: Option<(RegistrySourceDist, SourceDistCompatibility)>,
Expand All @@ -25,6 +26,20 @@ struct PrioritizedDistInner {
wheels: Vec<(RegistryBuiltWheel, WheelCompatibility)>,
/// The hashes for each distribution.
hashes: Vec<HashDigest>,
/// The set of supported platforms for the distribution, described in terms of their markers.
markers: MarkerTree,
}

impl Default for PrioritizedDistInner {
fn default() -> Self {
Self {
source: None,
best_wheel_index: None,
wheels: Vec::new(),
hashes: Vec::new(),
markers: MarkerTree::FALSE,
}
}
}

/// A distribution that can be used for both resolution and installation.
Expand Down Expand Up @@ -70,6 +85,16 @@ impl CompatibleDist<'_> {
CompatibleDist::IncompatibleWheel { sdist, .. } => sdist.file.requires_python.as_ref(),
}
}

/// Return the set of supported platform the distribution, in terms of their markers.
pub fn implied_markers(&self) -> MarkerTree {
match self {
CompatibleDist::InstalledDist(_) => MarkerTree::TRUE,
CompatibleDist::SourceDist { prioritized, .. } => prioritized.0.markers,
CompatibleDist::CompatibleWheel { prioritized, .. } => prioritized.0.markers,
CompatibleDist::IncompatibleWheel { prioritized, .. } => prioritized.0.markers,
}
}
}

#[derive(Debug, PartialEq, Eq, Clone)]
Expand All @@ -94,6 +119,9 @@ impl IncompatibleDist {
None => format!("has {self}"),
},
IncompatibleWheel::RequiresPython(..) => format!("requires {self}"),
IncompatibleWheel::MissingPlatform(_) => {
format!("has {self}")
}
},
Self::Source(incompatibility) => match incompatibility {
IncompatibleSource::NoBuild => format!("has {self}"),
Expand Down Expand Up @@ -121,6 +149,9 @@ impl IncompatibleDist {
None => format!("have {self}"),
},
IncompatibleWheel::RequiresPython(..) => format!("require {self}"),
IncompatibleWheel::MissingPlatform(_) => {
format!("have {self}")
}
},
Self::Source(incompatibility) => match incompatibility {
IncompatibleSource::NoBuild => format!("have {self}"),
Expand Down Expand Up @@ -171,6 +202,9 @@ impl Display for IncompatibleDist {
IncompatibleWheel::RequiresPython(python, _) => {
write!(f, "Python {python}")
}
IncompatibleWheel::MissingPlatform(platform) => {
write!(f, "no {platform}-compatible wheels")
}
},
Self::Source(incompatibility) => match incompatibility {
IncompatibleSource::NoBuild => f.write_str("no usable wheels"),
Expand Down Expand Up @@ -223,6 +257,8 @@ pub enum IncompatibleWheel {
Yanked(Yanked),
/// The use of binary wheels is disabled.
NoBinary,
/// The distribution is missing support for the target platform.
MissingPlatform(String),
}

#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -257,6 +293,7 @@ impl PrioritizedDist {
compatibility: WheelCompatibility,
) -> Self {
Self(Box::new(PrioritizedDistInner {
markers: implied_markers(&dist.filename),
best_wheel_index: Some(0),
wheels: vec![(dist, compatibility)],
source: None,
Expand All @@ -271,6 +308,7 @@ impl PrioritizedDist {
compatibility: SourceDistCompatibility,
) -> Self {
Self(Box::new(PrioritizedDistInner {
markers: MarkerTree::TRUE,
best_wheel_index: None,
wheels: vec![],
source: Some((dist, compatibility)),
Expand All @@ -293,8 +331,11 @@ impl PrioritizedDist {
} else {
self.0.best_wheel_index = Some(self.0.wheels.len());
}
self.0.wheels.push((dist, compatibility));
self.0.hashes.extend(hashes);
if !self.0.markers.is_true() {
self.0.markers.or(implied_markers(&dist.filename));
}
self.0.wheels.push((dist, compatibility));
}

/// Insert the given source distribution into the [`PrioritizedDist`].
Expand All @@ -312,7 +353,9 @@ impl PrioritizedDist {
} else {
self.0.source = Some((dist, compatibility));
}

if !self.0.markers.is_true() {
self.0.markers.or(MarkerTree::TRUE);
}
self.0.hashes.extend(hashes);
}

Expand Down Expand Up @@ -563,6 +606,7 @@ impl IncompatibleSource {
}

impl IncompatibleWheel {
#[allow(clippy::match_like_matches_macro)]
fn is_more_compatible(&self, other: &Self) -> bool {
match self {
Self::ExcludeNewer(timestamp_self) => match other {
Expand All @@ -574,28 +618,76 @@ impl IncompatibleWheel {
timestamp_other < timestamp_self
}
},
Self::NoBinary | Self::RequiresPython(_, _) | Self::Tag(_) | Self::Yanked(_) => {
true
}
Self::MissingPlatform(_)
| Self::NoBinary
| Self::RequiresPython(_, _)
| Self::Tag(_)
| Self::Yanked(_) => true,
},
Self::Tag(tag_self) => match other {
Self::ExcludeNewer(_) => false,
Self::Tag(tag_other) => tag_self > tag_other,
Self::NoBinary | Self::RequiresPython(_, _) | Self::Yanked(_) => true,
Self::MissingPlatform(_)
| Self::NoBinary
| Self::RequiresPython(_, _)
| Self::Yanked(_) => true,
},
Self::RequiresPython(_, _) => match other {
Self::ExcludeNewer(_) | Self::Tag(_) => false,
// Version specifiers cannot be reasonably compared
Self::RequiresPython(_, _) => false,
Self::NoBinary | Self::Yanked(_) => true,
Self::MissingPlatform(_) | Self::NoBinary | Self::Yanked(_) => true,
},
Self::Yanked(_) => match other {
Self::ExcludeNewer(_) | Self::Tag(_) | Self::RequiresPython(_, _) => false,
// Yanks with a reason are more helpful for errors
Self::Yanked(yanked_other) => matches!(yanked_other, Yanked::Reason(_)),
Self::NoBinary => true,
Self::MissingPlatform(_) | Self::NoBinary => true,
},
Self::NoBinary => match other {
Self::MissingPlatform(_) => true,
_ => false,
},
Self::MissingPlatform(platform_other) => match other {
Self::MissingPlatform(platform_self) => platform_self > platform_other,
_ => false,
},
Self::NoBinary => false,
}
}
}

/// Given a wheel filename, determine the set of supported platforms, in terms of their markers.
pub fn implied_markers(filename: &WheelFilename) -> MarkerTree {
let mut marker = MarkerTree::FALSE;
for platform_tag in &filename.platform_tag {
match platform_tag.as_str() {
"any" => marker.or(MarkerTree::TRUE),
tag if tag.starts_with("win") => {
marker.or(MarkerTree::expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "win32".to_string(),
}));
}
tag if tag.starts_with("macosx") => {
marker.or(MarkerTree::expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "darwin".to_string(),
}));
}
tag if tag.starts_with("manylinux")
|| tag.starts_with("musllinux")
|| tag.starts_with("linux") =>
{
marker.or(MarkerTree::expression(MarkerExpression::String {
key: MarkerValueString::SysPlatform,
operator: MarkerOperator::Equal,
value: "linux".to_string(),
}));
}
_ => {}
}
}
marker
}
47 changes: 46 additions & 1 deletion crates/uv-resolver/src/resolver/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,7 @@ impl<'d> Forker<'d> {
}

/// Fork the resolver based on a `Requires-Python` specifier.
pub(crate) fn fork_python_requirement(
pub(crate) fn fork_version_by_python_requirement(
requires_python: &VersionSpecifiers,
python_requirement: &PythonRequirement,
env: &ResolverEnvironment,
Expand Down Expand Up @@ -555,6 +555,51 @@ pub(crate) fn fork_python_requirement(
envs
}

/// Fork the resolver based on a marker.
pub(crate) fn fork_version_by_marker(
env: &ResolverEnvironment,
marker: MarkerTree,
) -> Vec<ResolverEnvironment> {
let Kind::Universal {
markers: ref env_marker,
..
} = env.kind
else {
panic!("resolver must be in universal mode for forking")
};

// Attempt to split based on the marker.
//
// For example, given `python_version >= '3.10'` and the split marker `sys_platform == 'linux'`,
// the result will be:
//
// `python_version >= '3.10' and sys_platform == 'linux'`
// `python_version >= '3.10' and sys_platform != 'linux'`
//
// If the marker is disjoint with the current environment, then we should return an empty list.
// If the marker complement is disjoint with the current environment, then we should also return
// an empty list.
//
// For example, given `python_version >= '3.10' and sys_platform == 'linux'` and the split marker
// `sys_platform == 'win32'`, return an empty list, since the following isn't satisfiable:
//
// python_version >= '3.10' and sys_platform == 'linux' and sys_platform == 'win32'

let mut envs = vec![];
if env_marker.is_disjoint(marker) {
return vec![];
}
envs.push(env.narrow_environment(marker));

let complement = marker.negate();
if env_marker.is_disjoint(complement) {
return vec![];
}
envs.push(env.narrow_environment(complement));

envs
}

#[cfg(test)]
mod tests {
use std::ops::Bound;
Expand Down
30 changes: 30 additions & 0 deletions crates/uv-resolver/src/resolver/markers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use std::sync::Arc;

use rustc_hash::FxHashMap;

use uv_normalize::PackageName;
use uv_pep508::MarkerTree;

/// The superset of markers for which a package is known to be relevant.
///
/// These markers may not represent the exact set of relevant environments, as they aren't adjusted
/// when backtracking; instead, we only _add_ to this set over the course of the resolution. As
/// such, the marker value represents a superset of the environments in which the package is known
/// to be included, but it may include environments in which the package is ultimately excluded.
#[derive(Debug, Default, Clone)]
pub(crate) struct KnownMarkers(Arc<FxHashMap<PackageName, MarkerTree>>);

impl KnownMarkers {
/// Inserts the given [`MarkerTree`] for the given package name.
pub(crate) fn insert(&mut self, package_name: PackageName, marker_tree: MarkerTree) {
Arc::make_mut(&mut self.0)
.entry(package_name)
.or_insert(MarkerTree::FALSE)
.or(marker_tree);
}

/// Returns the [`MarkerTree`] for the given package name, if it exists.
pub(crate) fn get(&self, package_name: &PackageName) -> Option<&MarkerTree> {
self.0.get(package_name)
}
}
Loading

0 comments on commit 9a41883

Please sign in to comment.