Use PubGrubPython type in Python incompatibility reporting (#3992)

## Summary

Rather than re-testing compatibility, I think we can just rely on the
types directly.
This commit is contained in:
Charlie Marsh 2024-06-03 14:32:22 -04:00 committed by GitHub
parent a589ad5066
commit 1a60368ce4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 99 additions and 64 deletions

View file

@ -103,8 +103,8 @@ impl Display for IncompatibleDist {
Some(_) => f.write_str("was published after the exclude newer time"), Some(_) => f.write_str("was published after the exclude newer time"),
None => f.write_str("has no publish time"), None => f.write_str("has no publish time"),
}, },
IncompatibleWheel::RequiresPython(python) => { IncompatibleWheel::RequiresPython(python, _) => {
write!(f, "requires at python {python}") write!(f, "requires Python {python}")
} }
}, },
Self::Source(incompatibility) => match incompatibility { Self::Source(incompatibility) => match incompatibility {
@ -123,8 +123,8 @@ impl Display for IncompatibleDist {
Some(_) => f.write_str("was published after the exclude newer time"), Some(_) => f.write_str("was published after the exclude newer time"),
None => f.write_str("has no publish time"), None => f.write_str("has no publish time"),
}, },
IncompatibleSource::RequiresPython(python) => { IncompatibleSource::RequiresPython(python, _) => {
write!(f, "requires python {python}") write!(f, "requires Python {python}")
} }
}, },
Self::Unavailable => f.write_str("has no available distributions"), Self::Unavailable => f.write_str("has no available distributions"),
@ -132,6 +132,16 @@ impl Display for IncompatibleDist {
} }
} }
#[derive(Debug, PartialEq, Eq, Clone)]
pub enum PythonRequirementKind {
/// The installed version of Python.
Installed,
/// The target version of Python; that is, the version of Python for which we are resolving
/// dependencies. This is typically the same as the installed version, but may be different
/// when specifying an alternate Python version for the resolution.
Target,
}
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub enum WheelCompatibility { pub enum WheelCompatibility {
Incompatible(IncompatibleWheel), Incompatible(IncompatibleWheel),
@ -142,7 +152,7 @@ pub enum WheelCompatibility {
pub enum IncompatibleWheel { pub enum IncompatibleWheel {
ExcludeNewer(Option<i64>), ExcludeNewer(Option<i64>),
Tag(IncompatibleTag), Tag(IncompatibleTag),
RequiresPython(VersionSpecifiers), RequiresPython(VersionSpecifiers, PythonRequirementKind),
Yanked(Yanked), Yanked(Yanked),
NoBinary, NoBinary,
} }
@ -156,7 +166,7 @@ pub enum SourceDistCompatibility {
#[derive(Debug, PartialEq, Eq, Clone)] #[derive(Debug, PartialEq, Eq, Clone)]
pub enum IncompatibleSource { pub enum IncompatibleSource {
ExcludeNewer(Option<i64>), ExcludeNewer(Option<i64>),
RequiresPython(VersionSpecifiers), RequiresPython(VersionSpecifiers, PythonRequirementKind),
Yanked(Yanked), Yanked(Yanked),
NoBuild, NoBuild,
} }
@ -466,16 +476,16 @@ impl IncompatibleSource {
Self::ExcludeNewer(timestamp_self) => match other { Self::ExcludeNewer(timestamp_self) => match other {
// Smaller timestamps are closer to the cut-off time // Smaller timestamps are closer to the cut-off time
Self::ExcludeNewer(timestamp_other) => timestamp_other < timestamp_self, Self::ExcludeNewer(timestamp_other) => timestamp_other < timestamp_self,
Self::NoBuild | Self::RequiresPython(_) | Self::Yanked(_) => true, Self::NoBuild | Self::RequiresPython(_, _) | Self::Yanked(_) => true,
}, },
Self::RequiresPython(_) => match other { Self::RequiresPython(_, _) => match other {
Self::ExcludeNewer(_) => false, Self::ExcludeNewer(_) => false,
// Version specifiers cannot be reasonably compared // Version specifiers cannot be reasonably compared
Self::RequiresPython(_) => false, Self::RequiresPython(_, _) => false,
Self::NoBuild | Self::Yanked(_) => true, Self::NoBuild | Self::Yanked(_) => true,
}, },
Self::Yanked(_) => match other { Self::Yanked(_) => match other {
Self::ExcludeNewer(_) | Self::RequiresPython(_) => false, Self::ExcludeNewer(_) | Self::RequiresPython(_, _) => false,
// Yanks with a reason are more helpful for errors // Yanks with a reason are more helpful for errors
Self::Yanked(yanked_other) => matches!(yanked_other, Yanked::Reason(_)), Self::Yanked(yanked_other) => matches!(yanked_other, Yanked::Reason(_)),
Self::NoBuild => true, Self::NoBuild => true,
@ -497,21 +507,23 @@ impl IncompatibleWheel {
timestamp_other < timestamp_self timestamp_other < timestamp_self
} }
}, },
Self::NoBinary | Self::RequiresPython(_) | Self::Tag(_) | Self::Yanked(_) => true, Self::NoBinary | Self::RequiresPython(_, _) | Self::Tag(_) | Self::Yanked(_) => {
true
}
}, },
Self::Tag(tag_self) => match other { Self::Tag(tag_self) => match other {
Self::ExcludeNewer(_) => false, Self::ExcludeNewer(_) => false,
Self::Tag(tag_other) => tag_other > tag_self, Self::Tag(tag_other) => tag_other > tag_self,
Self::NoBinary | Self::RequiresPython(_) | Self::Yanked(_) => true, Self::NoBinary | Self::RequiresPython(_, _) | Self::Yanked(_) => true,
}, },
Self::RequiresPython(_) => match other { Self::RequiresPython(_, _) => match other {
Self::ExcludeNewer(_) | Self::Tag(_) => false, Self::ExcludeNewer(_) | Self::Tag(_) => false,
// Version specifiers cannot be reasonably compared // Version specifiers cannot be reasonably compared
Self::RequiresPython(_) => false, Self::RequiresPython(_, _) => false,
Self::NoBinary | Self::Yanked(_) => true, Self::NoBinary | Self::Yanked(_) => true,
}, },
Self::Yanked(_) => match other { Self::Yanked(_) => match other {
Self::ExcludeNewer(_) | Self::Tag(_) | Self::RequiresPython(_) => false, Self::ExcludeNewer(_) | Self::Tag(_) | Self::RequiresPython(_, _) => false,
// Yanks with a reason are more helpful for errors // Yanks with a reason are more helpful for errors
Self::Yanked(yanked_other) => matches!(yanked_other, Yanked::Reason(_)), Self::Yanked(yanked_other) => matches!(yanked_other, Yanked::Reason(_)),
Self::NoBinary => true, Self::NoBinary => true,

View file

@ -19,7 +19,7 @@ use crate::candidate_selector::CandidateSelector;
use crate::python_requirement::PythonRequirement; use crate::python_requirement::PythonRequirement;
use crate::resolver::{IncompletePackage, UnavailablePackage, UnavailableReason}; use crate::resolver::{IncompletePackage, UnavailablePackage, UnavailableReason};
use super::{PubGrubPackage, PubGrubPackageInner}; use super::{PubGrubPackage, PubGrubPackageInner, PubGrubPython};
#[derive(Debug)] #[derive(Debug)]
pub(crate) struct PubGrubReportFormatter<'a> { pub(crate) struct PubGrubReportFormatter<'a> {
@ -44,44 +44,29 @@ impl ReportFormatter<PubGrubPackage, Range<Version>, UnavailableReason>
format!("we are solving dependencies of {package} {version}") format!("we are solving dependencies of {package} {version}")
} }
External::NoVersions(package, set) => { External::NoVersions(package, set) => {
if matches!(&**package, PubGrubPackageInner::Python(_)) {
if let Some(python) = self.python_requirement { if let Some(python) = self.python_requirement {
if python.target() == python.installed() { if matches!(
// Simple case, the installed version is the same as the target version &**package,
return format!( PubGrubPackageInner::Python(PubGrubPython::Target)
"the current {package} version ({}) does not satisfy {}", ) {
python.target(),
PackageRange::compatibility(package, set)
);
}
// Complex case, the target was provided and differs from the installed one
// Determine which Python version requirement was not met
if !set.contains(python.target()) {
return format!( return format!(
"the requested {package} version ({}) does not satisfy {}", "the requested {package} version ({}) does not satisfy {}",
python.target(), python.target(),
PackageRange::compatibility(package, set) PackageRange::compatibility(package, set)
); );
} }
// TODO(zanieb): Explain to the user why the installed version is relevant if matches!(
// when they provided a target version; probably via a "hint" &**package,
debug_assert!( PubGrubPackageInner::Python(PubGrubPython::Installed)
!set.contains(python.installed()), ) {
"There should not be an incompatibility where the range is satisfied by both Python requirements"
);
return format!( return format!(
"the current {package} version ({}) does not satisfy {}", "the current {package} version ({}) does not satisfy {}",
python.installed(), python.installed(),
PackageRange::compatibility(package, set) PackageRange::compatibility(package, set)
); );
} }
// We should always have the required Python versions, if we don't we'll fall back
// to a less helpful message in production
debug_assert!(
false,
"Error reporting should always be provided with Python versions"
);
} }
let set = self.simplify_set(set, package); let set = self.simplify_set(set, package);
if set.as_ref() == &Range::full() { if set.as_ref() == &Range::full() {

View file

@ -20,7 +20,8 @@ use tracing::{debug, enabled, instrument, trace, warn, Level};
use distribution_types::{ use distribution_types::{
BuiltDist, Dist, DistributionMetadata, IncompatibleDist, IncompatibleSource, IncompatibleWheel, BuiltDist, Dist, DistributionMetadata, IncompatibleDist, IncompatibleSource, IncompatibleWheel,
InstalledDist, RemoteSource, ResolvedDist, ResolvedDistRef, SourceDist, VersionOrUrlRef, InstalledDist, PythonRequirementKind, RemoteSource, ResolvedDist, ResolvedDistRef, SourceDist,
VersionOrUrlRef,
}; };
pub(crate) use locals::Locals; pub(crate) use locals::Locals;
use pep440_rs::{Version, MIN_VERSION}; use pep440_rs::{Version, MIN_VERSION};
@ -406,9 +407,11 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
if let UnavailableVersion::IncompatibleDist( if let UnavailableVersion::IncompatibleDist(
IncompatibleDist::Source(IncompatibleSource::RequiresPython( IncompatibleDist::Source(IncompatibleSource::RequiresPython(
requires_python, requires_python,
kind,
)) ))
| IncompatibleDist::Wheel(IncompatibleWheel::RequiresPython( | IncompatibleDist::Wheel(IncompatibleWheel::RequiresPython(
requires_python, requires_python,
kind,
)), )),
) = reason ) = reason
{ {
@ -420,18 +423,25 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
})?; })?;
let package = &state.next; let package = &state.next;
for kind in [PubGrubPython::Installed, PubGrubPython::Target] { state
state.pubgrub.add_incompatibility( .pubgrub
Incompatibility::from_dependency( .add_incompatibility(Incompatibility::from_dependency(
package.clone(), package.clone(),
Range::singleton(version.clone()), Range::singleton(version.clone()),
( (
PubGrubPackage::from(PubGrubPackageInner::Python(kind)), PubGrubPackage::from(PubGrubPackageInner::Python(
match kind {
PythonRequirementKind::Installed => {
PubGrubPython::Installed
}
PythonRequirementKind::Target => {
PubGrubPython::Target
}
},
)),
python_version.clone(), python_version.clone(),
), ),
), ));
);
}
state state
.pubgrub .pubgrub
.partial_solution .partial_solution
@ -722,12 +732,29 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
// The version is incompatible due to its Python requirement. // The version is incompatible due to its Python requirement.
if let Some(requires_python) = metadata.requires_python.as_ref() { if let Some(requires_python) = metadata.requires_python.as_ref() {
let installed = self.python_requirement.installed();
let target = self.python_requirement.target(); let target = self.python_requirement.target();
if target != installed {
if !requires_python.contains(target) { if !requires_python.contains(target) {
return Ok(Some(ResolverVersion::Unavailable( return Ok(Some(ResolverVersion::Unavailable(
version.clone(), version.clone(),
UnavailableVersion::IncompatibleDist(IncompatibleDist::Source( UnavailableVersion::IncompatibleDist(IncompatibleDist::Source(
IncompatibleSource::RequiresPython(requires_python.clone()), IncompatibleSource::RequiresPython(
requires_python.clone(),
PythonRequirementKind::Target,
),
)),
)));
}
}
if !requires_python.contains(installed) {
return Ok(Some(ResolverVersion::Unavailable(
version.clone(),
UnavailableVersion::IncompatibleDist(IncompatibleDist::Source(
IncompatibleSource::RequiresPython(
requires_python.clone(),
PythonRequirementKind::Installed,
),
)), )),
))); )));
} }

View file

@ -8,7 +8,8 @@ use tracing::instrument;
use distribution_filename::{DistFilename, WheelFilename}; use distribution_filename::{DistFilename, WheelFilename};
use distribution_types::{ use distribution_types::{
HashComparison, IncompatibleSource, IncompatibleWheel, IndexUrl, PrioritizedDist, HashComparison, IncompatibleSource, IncompatibleWheel, IndexUrl, PrioritizedDist,
RegistryBuiltWheel, RegistrySourceDist, SourceDistCompatibility, WheelCompatibility, PythonRequirementKind, RegistryBuiltWheel, RegistrySourceDist, SourceDistCompatibility,
WheelCompatibility,
}; };
use pep440_rs::{Version, VersionSpecifiers}; use pep440_rs::{Version, VersionSpecifiers};
use platform_tags::{TagCompatibility, Tags}; use platform_tags::{TagCompatibility, Tags};
@ -465,11 +466,20 @@ impl VersionMapLazy {
// Source distributions must meet both the _target_ Python version and the // Source distributions must meet both the _target_ Python version and the
// _installed_ Python version (to build successfully) // _installed_ Python version (to build successfully)
if let Some(requires_python) = requires_python { if let Some(requires_python) = requires_python {
if !requires_python.contains(self.python_requirement.target()) if self.python_requirement.target() != self.python_requirement.installed() {
|| !requires_python.contains(self.python_requirement.installed()) if !requires_python.contains(self.python_requirement.target()) {
{ return SourceDistCompatibility::Incompatible(
IncompatibleSource::RequiresPython(
requires_python,
PythonRequirementKind::Target,
),
);
}
}
if !requires_python.contains(self.python_requirement.installed()) {
return SourceDistCompatibility::Incompatible(IncompatibleSource::RequiresPython( return SourceDistCompatibility::Incompatible(IncompatibleSource::RequiresPython(
requires_python, requires_python,
PythonRequirementKind::Installed,
)); ));
} }
} }
@ -526,6 +536,7 @@ impl VersionMapLazy {
if !requires_python.contains(self.python_requirement.target()) { if !requires_python.contains(self.python_requirement.target()) {
return WheelCompatibility::Incompatible(IncompatibleWheel::RequiresPython( return WheelCompatibility::Incompatible(IncompatibleWheel::RequiresPython(
requires_python, requires_python,
PythonRequirementKind::Target,
)); ));
} }
} }