Skip to content

Commit

Permalink
Move Requires-Python incompatibilities out of version map (#4705)
Browse files Browse the repository at this point in the history 
## Summary

This is required to solve #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.)
  • Loading branch information
@charliermarsh
charliermarsh authored Jul 2, 2024
1 parent 8dabc29 commit 89b3324
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 65 deletions.
78 changes: 74 additions & 4 deletions crates/uv-resolver/src/resolver/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
5 changes: 0 additions & 5 deletions crates/uv-resolver/src/resolver/provider.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
Expand All @@ -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>,
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
59 changes: 3 additions & 56 deletions crates/uv-resolver/src/version_map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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)]
Expand All @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -369,15 +362,13 @@ 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 {
DistFilename::WheelFilename(filename) => {
let compatibility = self.wheel_compatibility(
&filename,
&version,
requires_python,
&hashes,
yanked,
excluded,
Expand All @@ -393,7 +384,6 @@ impl VersionMapLazy {
DistFilename::SourceDistFilename(filename) => {
let compatibility = self.source_dist_compatibility(
&version,
requires_python,
&hashes,
yanked,
excluded,
Expand Down Expand Up @@ -422,7 +412,6 @@ impl VersionMapLazy {
fn source_dist_compatibility(
&self,
version: &Version,
requires_python: Option<VersionSpecifiers>,
hashes: &[HashDigest],
yanked: Option<Yanked>,
excluded: bool,
Expand All @@ -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
Expand All @@ -492,7 +459,6 @@ impl VersionMapLazy {
&self,
filename: &WheelFilename,
version: &Version,
requires_python: Option<VersionSpecifiers>,
hashes: &[HashDigest],
yanked: Option<Yanked>,
excluded: bool,
Expand All @@ -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) {
Expand Down

0 comments on commit 89b3324

Please sign in to comment.