Skip to content

Commit

Permalink
Avoid building incompatible distributions
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 3, 2024
1 parent 6a0aa4f commit 4aff917
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 66 deletions.
54 changes: 52 additions & 2 deletions crates/puffin-resolver/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use distribution_types::{
BuiltDist, Dist, DistributionMetadata, IndexUrl, LocalEditable, Name, PackageId, SourceDist,
VersionOrUrl,
};
use pep440_rs::VersionSpecifiers;
use pep508_rs::{MarkerEnvironment, Requirement};
use platform_tags::Tags;
use puffin_client::RegistryClient;
Expand Down Expand Up @@ -494,6 +495,20 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
return Ok(());
};

// If the version is incompatible, short-circuit.
if let Some(requires_python) = candidate.resolve().requires_python.as_ref() {
let interpreter_version = self.interpreter.version();
let marker_version = &self.markers.python_version.version;
if !requires_python.contains(interpreter_version)
|| !requires_python.contains(marker_version)
{
self.index
.incompatibilities
.done(candidate.package_id(), requires_python.clone());
continue;
}
}

// Emit a request to fetch the metadata for this version.
if self.index.distributions.register(&candidate.package_id()) {
let distribution = candidate.into_distribution(index.clone(), base.clone());
Expand Down Expand Up @@ -592,6 +607,20 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
return Ok(None);
};

// If the version is incompatible, short-circuit.
if let Some(requires_python) = candidate.resolve().requires_python.as_ref() {
let interpreter_version = self.interpreter.version();
let marker_version = &self.markers.python_version.version;
if !requires_python.contains(interpreter_version)
|| !requires_python.contains(marker_version)
{
self.index
.incompatibilities
.done(candidate.package_id(), requires_python.clone());
return Ok(Some(candidate.version().clone()));
}
}

if let Some(extra) = extra {
debug!(
"Selecting: {}[{}]=={} ({})",
Expand Down Expand Up @@ -684,7 +713,25 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
Some(url) => PubGrubDistribution::from_url(package_name, url),
None => PubGrubDistribution::from_registry(package_name, version),
};
let entry = self.index.distributions.wait(&dist.package_id()).await;
let package_id = dist.package_id();

// If the package is known to be incompatible, return the Python version as an
// incompatibility, and skip fetching the metadata.
if let Some(entry) = self.index.incompatibilities.get(&package_id) {
let requires_python = entry.value();
let version = requires_python
.iter()
.map(PubGrubSpecifier::try_from)
.fold_ok(Range::full(), |range, specifier| {
range.intersection(&specifier.into())
})?;

let mut constraints = DependencyConstraints::default();
constraints.insert(PubGrubPackage::Python, version.clone());
return Ok(Dependencies::Known(constraints));
}

let entry = self.index.distributions.wait(&package_id).await;
let metadata = entry.value();

let mut constraints = PubGrubDependencies::from_requirements(
Expand Down Expand Up @@ -901,9 +948,12 @@ pub(crate) struct Index {
/// came from.
pub(crate) packages: OnceMap<PackageName, (IndexUrl, BaseUrl, VersionMap)>,

/// A map from distribution SHA to metadata for that distribution.
/// A map from package ID to metadata for that distribution.
pub(crate) distributions: OnceMap<PackageId, Metadata21>,

/// A map from package ID to required Python version.
pub(crate) incompatibilities: OnceMap<PackageId, VersionSpecifiers>,

/// A map from source URL to precise URL.
pub(crate) redirects: OnceMap<Url, Url>,
}
Expand Down
98 changes: 34 additions & 64 deletions crates/puffin-resolver/src/version_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,24 +67,24 @@ impl VersionMap {
}
}

// When resolving, include `requires_python` when determining version compatibility.
let is_compatible = file
.requires_python
.as_ref()
.map_or(true, |requires_python| {
// The interpreter and marker version are often the same, but can differ.
// For example, if the user is resolving against a target Python version
// passed in via the command line, that version will differ from the
// interpreter version.
let interpreter_version = interpreter.version();
let marker_version = &markers.python_version.version;
requires_python.contains(interpreter_version)
&& requires_python.contains(marker_version)
});

match filename {
DistFilename::WheelFilename(filename) => {
let priority = filename.compatibility(tags).filter(|_| is_compatible);
// To be compatible, the wheel must both have compatible tags _and_ have a
// compatible Python version marker.
let priority = filename.compatibility(tags).filter(|_| {
file.requires_python
.as_ref()
.map_or(true, |requires_python| {
// The interpreter and marker version are often the same, but can differ.
// For example, if the user is resolving against a target Python version
// passed in via the command line, that version will differ from the
// interpreter version.
let interpreter_version = interpreter.version();
let marker_version = &markers.python_version.version;
requires_python.contains(interpreter_version)
&& requires_python.contains(marker_version)
})
});
match version_map.entry(version.clone().into()) {
Entry::Occupied(mut entry) => {
entry.get_mut().insert_built(WheelFile(file), priority);
Expand All @@ -100,15 +100,10 @@ impl VersionMap {
DistFilename::SourceDistFilename(_) => {
match version_map.entry(version.clone().into()) {
Entry::Occupied(mut entry) => {
entry
.get_mut()
.insert_source(SdistFile(file), is_compatible);
entry.get_mut().insert_source(SdistFile(file));
}
Entry::Vacant(entry) => {
entry.insert(PrioritizedDistribution::from_source(
SdistFile(file),
is_compatible,
));
entry.insert(PrioritizedDistribution::from_source(SdistFile(file)));
}
}
}
Expand Down Expand Up @@ -137,9 +132,7 @@ impl VersionMap {
#[derive(Debug)]
struct PrioritizedDistribution {
/// An arbitrary source distribution for the package version.
compatible_source: Option<DistFile>,
/// An arbitrary source distribution for the package version.
incompatible_source: Option<DistFile>,
source: Option<DistFile>,
/// The highest-priority, platform-compatible wheel for the package version.
compatible_wheel: Option<(DistFile, TagPriority)>,
/// An arbitrary, platform-incompatible wheel for the package version.
Expand All @@ -151,37 +144,25 @@ impl PrioritizedDistribution {
fn from_built(dist: WheelFile, priority: Option<TagPriority>) -> Self {
if let Some(priority) = priority {
Self {
compatible_source: None,
incompatible_source: None,
source: None,
compatible_wheel: Some((dist.into(), priority)),
incompatible_wheel: None,
}
} else {
Self {
compatible_source: None,
incompatible_source: None,
source: None,
compatible_wheel: None,
incompatible_wheel: Some(dist.into()),
}
}
}

/// Create a new [`PrioritizedDistribution`] from the given source distribution.
fn from_source(dist: SdistFile, is_compatible: bool) -> Self {
if is_compatible {
Self {
compatible_source: Some(dist.into()),
incompatible_source: None,
compatible_wheel: None,
incompatible_wheel: None,
}
} else {
Self {
compatible_source: None,
incompatible_source: Some(dist.into()),
compatible_wheel: None,
incompatible_wheel: None,
}
fn from_source(dist: SdistFile) -> Self {
Self {
source: Some(dist.into()),
compatible_wheel: None,
incompatible_wheel: None,
}
}

Expand All @@ -202,39 +183,28 @@ impl PrioritizedDistribution {
}

/// Insert the given source distribution into the [`PrioritizedDistribution`].
fn insert_source(&mut self, file: SdistFile, is_compatible: bool) {
if is_compatible {
if self.compatible_source.is_none() {
self.compatible_source = Some(file.into());
}
} else if self.incompatible_source.is_none() {
self.incompatible_source = Some(file.into());
fn insert_source(&mut self, file: SdistFile) {
if self.source.is_none() {
self.source = Some(file.into());
}
}

/// Return the highest-priority distribution for the package version, if any.
fn get(&self) -> Option<ResolvableFile> {
match (
&self.compatible_wheel,
&self.compatible_source,
&self.source,
&self.incompatible_wheel,
&self.incompatible_source,
) {
// Prefer the highest-priority, platform-compatible wheel.
(Some((wheel, _)), _, _, _) => Some(ResolvableFile::CompatibleWheel(wheel)),
(Some((wheel, _)), _, _) => Some(ResolvableFile::CompatibleWheel(wheel)),
// If we have a compatible source distribution and an incompatible wheel, return the
// wheel. We assume that all distributions have the same metadata for a given package
// version. If a compatible source distribution exists, we assume we can build it, but
// using the wheel is faster.
(_, Some(sdist), Some(wheel), _) => {
Some(ResolvableFile::IncompatibleWheel(sdist, wheel))
}
// Otherwise, if we have a compatible source distribution, return it.
(_, Some(sdist), _, _) => Some(ResolvableFile::SourceDist(sdist)),
// Otherwise, if we have an incompatible source distribution, return it. We should
// ultimately reject it when resolving, since the incompatibility _must_ be due to a
// mismatch in Python version.
(_, _, _, Some(sdist)) => Some(ResolvableFile::SourceDist(sdist)),
(_, Some(sdist), Some(wheel)) => Some(ResolvableFile::IncompatibleWheel(sdist, wheel)),
// Otherwise, if we have a source distribution, return it.
(_, Some(sdist), _) => Some(ResolvableFile::SourceDist(sdist)),
_ => None,
}
}
Expand Down
1 change: 1 addition & 0 deletions requirements.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
numpy

0 comments on commit 4aff917

Please sign in to comment.