Skip to content

Commit

Permalink
Track source incompatibilities
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 29, 2024
1 parent 9c63412 commit 66d17a0
Show file tree
Hide file tree
Showing 11 changed files with 192 additions and 65 deletions.
112 changes: 90 additions & 22 deletions crates/distribution-types/src/prioritized_distribution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ pub struct PrioritizedDist(Box<PrioritizedDistInner>);
/// [`PrioritizedDist`] is boxed because [`Dist`] is large.
#[derive(Debug, Default, Clone)]
struct PrioritizedDistInner {
/// An arbitrary source distribution for the package version.
source: Option<DistMetadata>,
/// The most-compatible source distribution for the package version.
source: Option<(DistMetadata, SourceCompatibility)>,
/// The most-compatible wheel distribution for the package version.
wheel: Option<(DistMetadata, WheelCompatibility)>,
/// The hashes for each distribution.
Expand All @@ -36,19 +36,36 @@ pub enum CompatibleDist<'a> {
},
}

#[derive(Debug, PartialEq, Eq, Clone)]
#[derive(Debug, PartialEq, Eq, Ord, PartialOrd, Clone, Copy)]
pub enum SourceCompatibility {
Incompatible(IncompatibleSource),
Compatible,
}

#[derive(Debug, PartialEq, Eq, Ord, PartialOrd, Clone, Copy)]
pub enum IncompatibleSource {
NoBuild,
}

#[derive(Debug, PartialEq, Eq, Clone, Copy)]
pub enum WheelCompatibility {
Incompatible(IncompatibleWheel),
Compatible(TagPriority),
}

#[derive(Debug, PartialEq, Eq, Ord, PartialOrd, Clone)]
#[derive(Debug, PartialEq, Eq, Ord, PartialOrd, Clone, Copy)]
pub enum IncompatibleWheel {
Tag(IncompatibleTag),
RequiresPython,
NoBinary,
}

#[derive(Debug, Clone, Copy)]
pub enum Incompatible {
Source(IncompatibleSource),
Wheel(IncompatibleWheel),
}

/// A [`Dist`] and metadata about it required for downstream filtering.
#[derive(Debug, Clone)]
pub struct DistMetadata {
Expand Down Expand Up @@ -90,13 +107,17 @@ impl PrioritizedDist {
requires_python: Option<VersionSpecifiers>,
yanked: Yanked,
hash: Option<Hashes>,
compatibility: SourceCompatibility,
) -> Self {
Self(Box::new(PrioritizedDistInner {
source: Some(DistMetadata {
dist,
requires_python,
yanked,
}),
source: Some((
DistMetadata {
dist,
requires_python,
yanked,
},
compatibility,
)),
wheel: None,
hashes: hash.map(|hash| vec![hash]).unwrap_or_default(),
exclude_newer: false,
Expand Down Expand Up @@ -147,13 +168,29 @@ impl PrioritizedDist {
requires_python: Option<VersionSpecifiers>,
yanked: Yanked,
hash: Option<Hashes>,
compatibility: SourceCompatibility,
) {
if self.0.source.is_none() {
self.0.source = Some(DistMetadata {
dist,
requires_python,
yanked,
});
// Track the highest-priority source distribution.
if let Some((.., existing_compatibility)) = &self.0.source {
if compatibility > *existing_compatibility {
self.0.source = Some((
DistMetadata {
dist,
requires_python,
yanked,
},
compatibility,
));
}
} else {
self.0.source = Some((
DistMetadata {
dist,
requires_python,
yanked,
},
compatibility,
));
}

if let Some(hash) = hash {
Expand All @@ -172,18 +209,40 @@ impl PrioritizedDist {
// 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((wheel, _)), Some(source_dist)) => {
Some(CompatibleDist::IncompatibleWheel { source_dist, wheel })
(
Some((wheel, WheelCompatibility::Incompatible(_))),
Some((source_dist, SourceCompatibility::Compatible)),
) => Some(CompatibleDist::IncompatibleWheel { source_dist, wheel }),
// If we have a compatible source distribution and no wheel, return the source
// distribution.
(None, Some((source_dist, SourceCompatibility::Compatible))) => {
Some(CompatibleDist::SourceDist(source_dist))
}
// Otherwise, if we have a source distribution, return it.
(_, Some(source_dist)) => Some(CompatibleDist::SourceDist(source_dist)),
// If we have an incompatible source distribution and no wheel, return `None`.
_ => None,
}
}

/// Return the source distribution, if any.
pub fn source(&self) -> Option<&DistMetadata> {
self.0.source.as_ref()
/// Return the compatible source distribution, if any.
pub fn compatible_source(&self) -> Option<&DistMetadata> {
self.0
.source
.as_ref()
.and_then(|(dist, compatibility)| match compatibility {
SourceCompatibility::Compatible => Some(dist),
SourceCompatibility::Incompatible(_) => None,
})
}

/// Return the incompatible source distribution, if any.
pub fn incompatible_source(&self) -> Option<(&DistMetadata, &IncompatibleSource)> {
self.0
.source
.as_ref()
.and_then(|(dist, compatibility)| match compatibility {
SourceCompatibility::Compatible => None,
SourceCompatibility::Incompatible(incompatibility) => Some((dist, incompatibility)),
})
}

/// Return the compatible built distribution, if any.
Expand All @@ -208,6 +267,15 @@ impl PrioritizedDist {
})
}

pub fn incompatible(&self) -> Option<Incompatible> {
self.incompatible_source()
.map(|(_, incompatibility)| Incompatible::Source(*incompatibility))
.or_else(|| {
self.incompatible_wheel()
.map(|(_, incompatibility)| Incompatible::Wheel(*incompatibility))
})
}

/// Set the `exclude_newer` flag
pub fn set_exclude_newer(&mut self) {
self.0.exclude_newer = true;
Expand Down
2 changes: 1 addition & 1 deletion crates/platform-tags/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub enum TagsError {
InvalidPriority(usize, #[source] std::num::TryFromIntError),
}

#[derive(Debug, Eq, Ord, PartialEq, PartialOrd, Clone)]
#[derive(Debug, Eq, Ord, PartialEq, PartialOrd, Clone, Copy)]
pub enum IncompatibleTag {
Invalid,
Python,
Expand Down
13 changes: 9 additions & 4 deletions crates/uv-client/src/flat_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use url::Url;
use distribution_filename::DistFilename;
use distribution_types::{
BuiltDist, Dist, File, FileLocation, FlatIndexLocation, IndexUrl, PrioritizedDist,
RegistryBuiltDist, RegistrySourceDist, SourceDist,
RegistryBuiltDist, RegistrySourceDist, SourceCompatibility, SourceDist,
};
use pep440_rs::Version;
use pep508_rs::VerbatimUrl;
Expand Down Expand Up @@ -334,16 +334,21 @@ impl FlatIndex {
}));
match distributions.0.entry(filename.version) {
Entry::Occupied(mut entry) => {
entry
.get_mut()
.insert_source(dist, None, Yanked::default(), None);
entry.get_mut().insert_source(
dist,
None,
Yanked::default(),
None,
SourceCompatibility::Compatible,
);
}
Entry::Vacant(entry) => {
entry.insert(PrioritizedDist::from_source(
dist,
None,
Yanked::default(),
None,
SourceCompatibility::Compatible,
));
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/uv-dev/src/install_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ async fn install_chunk(
venv.interpreter(),
&FlatIndex::default(),
&NoBinary::None,
&NoBuild::None,
)
.resolve_stream(requirements)
.collect()
Expand Down
33 changes: 19 additions & 14 deletions crates/uv-resolver/src/candidate_selector.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
use pubgrub::range::Range;

use rustc_hash::FxHashMap;

use distribution_types::CompatibleDist;
use distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist};
use distribution_types::{CompatibleDist, Incompatible};
use distribution_types::{DistributionMetadata, Name, PrioritizedDist};
use pep440_rs::Version;
use pep508_rs::{Requirement, VersionOrUrl};
use uv_normalize::PackageName;

use crate::prerelease_mode::PreReleaseStrategy;

use crate::resolution_mode::ResolutionStrategy;
use crate::version_map::{VersionMap, VersionMapDistHandle};
use crate::{Manifest, Options};
Expand Down Expand Up @@ -248,7 +246,11 @@ impl CandidateSelector {
};

// Skip empty candidates due to exclude newer
if dist.exclude_newer() && dist.incompatible_wheel().is_none() && dist.get().is_none() {
if dist.exclude_newer()
&& dist.incompatible_wheel().is_none()
&& dist.incompatible_source().is_none()
&& dist.get().is_none()
{
continue;
}

Expand All @@ -274,23 +276,26 @@ impl CandidateSelector {
#[derive(Debug, Clone)]
pub(crate) enum CandidateDist<'a> {
Compatible(CompatibleDist<'a>),
Incompatible(Option<&'a IncompatibleWheel>),
Incompatible(Option<Incompatible>),
ExcludeNewer,
}

impl<'a> From<&'a PrioritizedDist> for CandidateDist<'a> {
fn from(value: &'a PrioritizedDist) -> Self {
if let Some(dist) = value.get() {
CandidateDist::Compatible(dist)
} else if value.exclude_newer() && value.incompatible_wheel().is_none() {
// If empty because of exclude-newer, mark as a special case
CandidateDist::ExcludeNewer
} else {
CandidateDist::Incompatible(
value
.incompatible_wheel()
.map(|(_, incompatibility)| incompatibility),
)
match value.incompatible() {
Some(incompatible) => CandidateDist::Incompatible(Some(incompatible)),
None => {
// If empty because of exclude-newer, mark as a special case.
if value.exclude_newer() {
CandidateDist::ExcludeNewer
} else {
CandidateDist::Incompatible(None)
}
}
}
}
}
}
Expand Down
16 changes: 13 additions & 3 deletions crates/uv-resolver/src/finder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use anyhow::Result;
use futures::{stream, Stream, StreamExt, TryStreamExt};
use rustc_hash::FxHashMap;
use uv_traits::NoBinary;
use uv_traits::{NoBinary, NoBuild};

use distribution_filename::DistFilename;
use distribution_types::{Dist, IndexUrl, Resolution};
Expand All @@ -26,6 +26,7 @@ pub struct DistFinder<'a> {
interpreter: &'a Interpreter,
flat_index: &'a FlatIndex,
no_binary: &'a NoBinary,
no_build: &'a NoBuild,
}

impl<'a> DistFinder<'a> {
Expand All @@ -36,6 +37,7 @@ impl<'a> DistFinder<'a> {
interpreter: &'a Interpreter,
flat_index: &'a FlatIndex,
no_binary: &'a NoBinary,
no_build: &'a NoBuild,
) -> Self {
Self {
tags,
Expand All @@ -44,6 +46,7 @@ impl<'a> DistFinder<'a> {
interpreter,
flat_index,
no_binary,
no_build,
}
}

Expand Down Expand Up @@ -135,6 +138,11 @@ impl<'a> DistFinder<'a> {
NoBinary::All => true,
NoBinary::Packages(packages) => packages.contains(&requirement.name),
};
let no_build = match self.no_build {
NoBuild::None => false,
NoBuild::All => true,
NoBuild::Packages(packages) => packages.contains(&requirement.name),
};

// Prioritize the flat index by initializing the "best" matches with its entries.
let matching_override = if let Some(flat_index) = flat_index {
Expand All @@ -155,7 +163,9 @@ impl<'a> DistFinder<'a> {
resolvable_dist
.compatible_wheel()
.map(|(dist, tag_priority)| (dist.dist.clone(), tag_priority)),
resolvable_dist.source().map(|dist| dist.dist.clone()),
resolvable_dist
.compatible_source()
.map(|dist| dist.dist.clone()),
)
} else {
(None, None, None)
Expand Down Expand Up @@ -213,7 +223,7 @@ impl<'a> DistFinder<'a> {
}

// Find the most-compatible sdist, if no wheel was found.
if best_wheel.is_none() {
if !no_build && best_wheel.is_none() {
for version_sdist in files.source_dists {
// Only add dists compatible with the python version.
// This is relevant for source dists which give no other indication of their
Expand Down
Loading

0 comments on commit 66d17a0

Please sign in to comment.