Move Requires-Python incompatibilities out of version map (#4705)

## Summary

This is required to solve https://github.com/astral-sh/uv/issues/4669,
because the `Requires-Python` version can now vary across a resolution.
For example, within certain forks, we might have a more narrow range,
which would allow us to use distributions that would not be allowed for
the global resolution.

This should be fine because `requires-python` is part of the package
metadata, so it should be consistent between files within a package
version. As such, there shouldn't be any risk that we incorrectly
prioritize distributions by omitting this information.

(To be more specific, the risk is something like: we prioritize some
wheel over a source distribution within a package-version, so we don't
track the source distribution at all. Then, later, when we choose a
candidate, we see that the wheel doesn't meet the `Requires-Python`
requirement, even though the source distribution _would've_ met it. If
files within a distribution could have varied support, this would be a
real risk.)
This commit is contained in:
Charlie Marsh 2024-07-02 08:15:39 -04:00 committed by GitHub
parent 8dabc29d80
commit 89b3324ae1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 77 additions and 65 deletions

View file

@ -23,9 +23,9 @@ use tokio_stream::wrappers::ReceiverStream;
use tracing::{debug, enabled, instrument, trace, warn, Level};
use distribution_types::{
BuiltDist, Dist, DistributionMetadata, IncompatibleDist, IncompatibleSource, IncompatibleWheel,
InstalledDist, PythonRequirementKind, RemoteSource, ResolvedDist, ResolvedDistRef, SourceDist,
VersionOrUrlRef,
BuiltDist, CompatibleDist, Dist, DistributionMetadata, IncompatibleDist, IncompatibleSource,
IncompatibleWheel, InstalledDist, PythonRequirementKind, RemoteSource, ResolvedDist,
ResolvedDistRef, SourceDist, VersionOrUrlRef,
};
pub(crate) use locals::Locals;
use pep440_rs::{Version, MIN_VERSION};
@ -155,7 +155,6 @@ impl<'a, Context: BuildContext, InstalledPackages: InstalledPackagesProvider>
database,
flat_index,
tags,
python_requirement.clone(),
AllowedYanks::from_manifest(&manifest, markers, options.dependency_mode),
hasher,
options.exclude_newer,
@ -922,6 +921,77 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
};
let incompatibility = match dist {
CompatibleDist::InstalledDist(_) => None,
CompatibleDist::SourceDist { sdist, .. }
| CompatibleDist::IncompatibleWheel { sdist, .. } => {
// Source distributions must meet both the _target_ Python version and the
// _installed_ Python version (to build successfully).
sdist
.file
.requires_python
.as_ref()
.and_then(|requires_python| {
if let Some(target) = self.python_requirement.target() {
if !target.is_compatible_with(requires_python) {
return Some(IncompatibleDist::Source(
IncompatibleSource::RequiresPython(
requires_python.clone(),
PythonRequirementKind::Target,
),
));
}
}
if !requires_python.contains(self.python_requirement.installed()) {
return Some(IncompatibleDist::Source(
IncompatibleSource::RequiresPython(
requires_python.clone(),
PythonRequirementKind::Installed,
),
));
}
None
})
}
CompatibleDist::CompatibleWheel { wheel, .. } => {
// Wheels must meet the _target_ Python version.
wheel
.file
.requires_python
.as_ref()
.and_then(|requires_python| {
if let Some(target) = self.python_requirement.target() {
if !target.is_compatible_with(requires_python) {
return Some(IncompatibleDist::Wheel(
IncompatibleWheel::RequiresPython(
requires_python.clone(),
PythonRequirementKind::Target,
),
));
}
} else {
if !requires_python.contains(self.python_requirement.installed()) {
return Some(IncompatibleDist::Wheel(
IncompatibleWheel::RequiresPython(
requires_python.clone(),
PythonRequirementKind::Installed,
),
));
}
}
None
})
}
};
// The version is incompatible due to its Python requirement.
if let Some(incompatibility) = incompatibility {
return Ok(Some(ResolverVersion::Unavailable(
candidate.version().clone(),
UnavailableVersion::IncompatibleDist(incompatibility),
)));
}
let filename = match dist.for_installation() {
ResolvedDistRef::InstallableRegistrySourceDist { sdist, .. } => sdist
.filename()

View file

@ -8,7 +8,6 @@ use uv_normalize::PackageName;
use uv_types::{BuildContext, HashStrategy};
use crate::flat_index::FlatIndex;
use crate::python_requirement::PythonRequirement;
use crate::version_map::VersionMap;
use crate::yanks::AllowedYanks;
use crate::ExcludeNewer;
@ -77,7 +76,6 @@ pub struct DefaultResolverProvider<'a, Context: BuildContext> {
/// These are the entries from `--find-links` that act as overrides for index responses.
flat_index: FlatIndex,
tags: Option<Tags>,
python_requirement: PythonRequirement,
allowed_yanks: AllowedYanks,
hasher: HashStrategy,
exclude_newer: Option<ExcludeNewer>,
@ -90,7 +88,6 @@ impl<'a, Context: BuildContext> DefaultResolverProvider<'a, Context> {
fetcher: DistributionDatabase<'a, Context>,
flat_index: &'a FlatIndex,
tags: Option<&'a Tags>,
python_requirement: PythonRequirement,
allowed_yanks: AllowedYanks,
hasher: &'a HashStrategy,
exclude_newer: Option<ExcludeNewer>,
@ -100,7 +97,6 @@ impl<'a, Context: BuildContext> DefaultResolverProvider<'a, Context> {
fetcher,
flat_index: flat_index.clone(),
tags: tags.cloned(),
python_requirement,
allowed_yanks,
hasher: hasher.clone(),
exclude_newer,
@ -131,7 +127,6 @@ impl<'a, Context: BuildContext> ResolverProvider for DefaultResolverProvider<'a,
package_name,
&index,
self.tags.as_ref(),
&self.python_requirement,
&self.allowed_yanks,
&self.hasher,
self.exclude_newer.as_ref(),

View file

@ -8,10 +8,9 @@ use tracing::instrument;
use distribution_filename::{DistFilename, WheelFilename};
use distribution_types::{
HashComparison, IncompatibleSource, IncompatibleWheel, IndexUrl, PrioritizedDist,
PythonRequirementKind, RegistryBuiltWheel, RegistrySourceDist, SourceDistCompatibility,
WheelCompatibility,
RegistryBuiltWheel, RegistrySourceDist, SourceDistCompatibility, WheelCompatibility,
};
use pep440_rs::{Version, VersionSpecifiers};
use pep440_rs::Version;
use platform_tags::{TagCompatibility, Tags};
use pypi_types::{HashDigest, Yanked};
use uv_client::{OwnedArchive, SimpleMetadata, VersionFiles};
@ -21,7 +20,7 @@ use uv_types::HashStrategy;
use uv_warnings::warn_user_once;
use crate::flat_index::FlatDistributions;
use crate::{python_requirement::PythonRequirement, yanks::AllowedYanks, ExcludeNewer};
use crate::{yanks::AllowedYanks, ExcludeNewer};
/// A map from versions to distributions.
#[derive(Debug)]
@ -45,7 +44,6 @@ impl VersionMap {
package_name: &PackageName,
index: &IndexUrl,
tags: Option<&Tags>,
python_requirement: &PythonRequirement,
allowed_yanks: &AllowedYanks,
hasher: &HashStrategy,
exclude_newer: Option<&ExcludeNewer>,
@ -107,7 +105,6 @@ impl VersionMap {
no_build: build_options.no_build_package(package_name),
index: index.clone(),
tags: tags.cloned(),
python_requirement: python_requirement.clone(),
exclude_newer: exclude_newer.copied(),
allowed_yanks,
required_hashes,
@ -285,10 +282,6 @@ struct VersionMapLazy {
/// The set of compatibility tags that determines whether a wheel is usable
/// in the current environment.
tags: Option<Tags>,
/// The version of Python active in the current environment. This is used
/// to determine whether a package's Python version constraint (if one
/// exists) is satisfied or not.
python_requirement: PythonRequirement,
/// Whether files newer than this timestamp should be excluded or not.
exclude_newer: Option<ExcludeNewer>,
/// Which yanked versions are allowed
@ -369,7 +362,6 @@ impl VersionMapLazy {
// Prioritize amongst all available files.
let version = filename.version().clone();
let requires_python = file.requires_python.clone();
let yanked = file.yanked.clone();
let hashes = file.hashes.clone();
match filename {
@ -377,7 +369,6 @@ impl VersionMapLazy {
let compatibility = self.wheel_compatibility(
&filename,
&version,
requires_python,
&hashes,
yanked,
excluded,
@ -393,7 +384,6 @@ impl VersionMapLazy {
DistFilename::SourceDistFilename(filename) => {
let compatibility = self.source_dist_compatibility(
&version,
requires_python,
&hashes,
yanked,
excluded,
@ -422,7 +412,6 @@ impl VersionMapLazy {
fn source_dist_compatibility(
&self,
version: &Version,
requires_python: Option<VersionSpecifiers>,
hashes: &[HashDigest],
yanked: Option<Yanked>,
excluded: bool,
@ -447,28 +436,6 @@ impl VersionMapLazy {
}
}
// Check if Python version is supported
// Source distributions must meet both the _target_ Python version and the
// _installed_ Python version (to build successfully)
if let Some(requires_python) = requires_python {
if let Some(target) = self.python_requirement.target() {
if !target.is_compatible_with(&requires_python) {
return SourceDistCompatibility::Incompatible(
IncompatibleSource::RequiresPython(
requires_python,
PythonRequirementKind::Target,
),
);
}
}
if !requires_python.contains(self.python_requirement.installed()) {
return SourceDistCompatibility::Incompatible(IncompatibleSource::RequiresPython(
requires_python,
PythonRequirementKind::Installed,
));
}
}
// Check if hashes line up. If hashes aren't required, they're considered matching.
let hash = if self.required_hashes.is_empty() {
HashComparison::Matched
@ -492,7 +459,6 @@ impl VersionMapLazy {
&self,
filename: &WheelFilename,
version: &Version,
requires_python: Option<VersionSpecifiers>,
hashes: &[HashDigest],
yanked: Option<Yanked>,
excluded: bool,
@ -515,25 +481,6 @@ impl VersionMapLazy {
}
}
// Check for a Python version incompatibility
if let Some(requires_python) = requires_python {
if let Some(target) = self.python_requirement.target() {
if !target.is_compatible_with(&requires_python) {
return WheelCompatibility::Incompatible(IncompatibleWheel::RequiresPython(
requires_python,
PythonRequirementKind::Target,
));
}
} else {
if !requires_python.contains(self.python_requirement.installed()) {
return WheelCompatibility::Incompatible(IncompatibleWheel::RequiresPython(
requires_python,
PythonRequirementKind::Installed,
));
}
}
}
// Determine a compatibility for the wheel based on tags.
let priority = match &self.tags {
Some(tags) => match filename.compatibility(tags) {