mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-11-01 12:24:15 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Use separate types to represent raw vs. resolver markers (#6646)

Browse source 
## Summary

This is similar to https://github.com/astral-sh/uv/pull/6171 but more
expansive... _Anywhere_ that we test requirements for platform
compatibility, we _need_ to respect the resolver-friendly markers. In
fixing the motivating issue (#6621), I also realized that we had a bunch
of bugs here around `pip install` with `--python-platform` and
`--python-version`, because we always performed our `satisfy` and `Plan`
operations on the interpreter's markers, not the adjusted markers!

Closes https://github.com/astral-sh/uv/issues/6621.
This commit is contained in:
Charlie Marsh 2024-08-26 14:00:21 -04:00 • committed by GitHub
parent 6220532373
commit a7850d2a1c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
37 changed files with 422 additions and 247 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/uv-resolver/src/candidate_selector.rs
View file

@ -6,7 +6,7 @@ use tracing::{debug, trace};
use distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource};
use distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist};
use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, MarkerTree};
use pep508_rs::MarkerTree;
use uv_configuration::IndexStrategy;
use uv_normalize::PackageName;
use uv_types::InstalledPackagesProvider;
@ -30,7 +30,7 @@ impl CandidateSelector {
pub(crate) fn for_resolution(
options: Options,
manifest: &Manifest,
markers: Option<&MarkerEnvironment>,
markers: &ResolverMarkers,
) -> Self {
Self {
resolution_strategy: ResolutionStrategy::from_mode(

6
crates/uv-resolver/src/lock.rs
View file

@ -27,7 +27,7 @@ use pep508_rs::{split_scheme, MarkerEnvironment, MarkerTree, VerbatimUrl, Verbat
use platform_tags::{TagCompatibility, TagPriority, Tags};
use pypi_types::{
redact_git_credentials, HashDigest, ParsedArchiveUrl, ParsedGitUrl, Requirement,
RequirementSource,
RequirementSource, ResolverMarkerEnvironment,
};
use uv_configuration::ExtrasSpecification;
use uv_distribution::DistributionDatabase;
@ -419,7 +419,7 @@ impl Lock {
pub fn to_resolution(
&self,
project: &VirtualProject,
marker_env: &MarkerEnvironment,
marker_env: &ResolverMarkerEnvironment,
tags: &Tags,
extras: &ExtrasSpecification,
dev: &[GroupName],
@ -3641,7 +3641,7 @@ impl<'env> TreeDisplay<'env> {
/// Create a new [`DisplayDependencyGraph`] for the set of installed packages.
pub fn new(
lock: &'env Lock,
markers: Option<&'env MarkerEnvironment>,
markers: Option<&'env ResolverMarkerEnvironment>,
depth: usize,
prune: Vec<PackageName>,
package: Vec<PackageName>,

134
crates/uv-resolver/src/manifest.rs
View file

@ -1,15 +1,15 @@
use either::Either;
use std::borrow::Cow;
use std::collections::BTreeSet;
use pep508_rs::MarkerEnvironment;
use either::Either;
use pypi_types::Requirement;
use uv_configuration::{Constraints, Overrides};
use uv_normalize::{GroupName, PackageName};
use uv_types::RequestedRequirements;
use crate::preferences::Preferences;
use crate::{DependencyMode, Exclusions};
use crate::{DependencyMode, Exclusions, ResolverMarkers};
/// A manifest of requirements, constraints, and preferences.
#[derive(Clone, Debug)]
@ -109,7 +109,7 @@ impl Manifest {
/// - Determining which requirements should allow local version specifiers (e.g., `torch==2.2.0+cpu`).
pub fn requirements<'a>(
&'a self,
markers: Option<&'a MarkerEnvironment>,
markers: &'a ResolverMarkers,
mode: DependencyMode,
) -> impl Iterator<Item = Cow<'a, Requirement>> + 'a {
self.requirements_no_overrides(markers, mode)
@ -119,39 +119,48 @@ impl Manifest {
/// Like [`Self::requirements`], but without the overrides.
pub fn requirements_no_overrides<'a>(
&'a self,
markers: Option<&'a MarkerEnvironment>,
markers: &'a ResolverMarkers,
mode: DependencyMode,
) -> impl Iterator<Item = Cow<'a, Requirement>> + 'a {
match mode {
// Include all direct and transitive requirements, with constraints and overrides applied.
DependencyMode::Transitive => Either::Left(
self.lookaheads
.iter()
.flat_map(move |lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(move |requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
})
.chain(
self.overrides
.apply(&self.requirements)
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.constraints
.requirements()
.filter(move |requirement| requirement.evaluate_markers(markers, &[]))
.map(Cow::Borrowed),
),
),
DependencyMode::Transitive => {
Either::Left(
self.lookaheads
.iter()
.flat_map(move |lookahead| {
self.overrides.apply(lookahead.requirements()).filter(
move |requirement| {
requirement.evaluate_markers(
markers.marker_environment(),
lookahead.extras(),
)
},
)
})
.chain(self.overrides.apply(&self.requirements).filter(
move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
},
))
.chain(
self.constraints
.requirements()
.filter(move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
})
.map(Cow::Borrowed),
),
)
}
// Include direct requirements, with constraints and overrides applied.
DependencyMode::Direct => Either::Right(
self.overrides
.apply(&self.requirements)
.chain(self.constraints.requirements().map(Cow::Borrowed))
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
.filter(move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
}),
),
}
}
@ -159,7 +168,7 @@ impl Manifest {
/// Only the overrides from [`Self::requirements`].
pub fn overrides<'a>(
&'a self,
markers: Option<&'a MarkerEnvironment>,
markers: &'a ResolverMarkers,
mode: DependencyMode,
) -> impl Iterator<Item = Cow<'a, Requirement>> + 'a {
match mode {
@ -167,14 +176,18 @@ impl Manifest {
DependencyMode::Transitive => Either::Left(
self.overrides
.requirements()
.filter(move |requirement| requirement.evaluate_markers(markers, &[]))
.filter(move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
})
.map(Cow::Borrowed),
),
// Include direct requirements, with constraints and overrides applied.
DependencyMode::Direct => Either::Right(
self.overrides
.requirements()
.filter(move |requirement| requirement.evaluate_markers(markers, &[]))
.filter(move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
})
.map(Cow::Borrowed),
),
}
@ -192,36 +205,43 @@ impl Manifest {
/// the `lowest-direct` strategy is in use.
pub fn user_requirements<'a>(
&'a self,
markers: Option<&'a MarkerEnvironment>,
markers: &'a ResolverMarkers,
mode: DependencyMode,
) -> impl Iterator<Item = Cow<'a, Requirement>> + 'a {
match mode {
// Include direct requirements, dependencies of editables, and transitive dependencies
// of local packages.
DependencyMode::Transitive => Either::Left(
self.lookaheads
.iter()
.filter(|lookahead| lookahead.direct())
.flat_map(move |lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(move |requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
})
.chain(
self.overrides
.apply(&self.requirements)
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
),
),
DependencyMode::Transitive => {
Either::Left(
self.lookaheads
.iter()
.filter(|lookahead| lookahead.direct())
.flat_map(move |lookahead| {
self.overrides.apply(lookahead.requirements()).filter(
move |requirement| {
requirement.evaluate_markers(
markers.marker_environment(),
lookahead.extras(),
)
},
)
})
.chain(self.overrides.apply(&self.requirements).filter(
move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
},
)),
)
}
// Restrict to the direct requirements.
DependencyMode::Direct => Either::Right(
self.overrides
.apply(self.requirements.iter())
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
),
DependencyMode::Direct => {
Either::Right(self.overrides.apply(self.requirements.iter()).filter(
move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
},
))
}
}
}
@ -232,11 +252,13 @@ impl Manifest {
/// resolution (assuming the user enabled development dependencies).
pub fn direct_requirements<'a>(
&'a self,
markers: Option<&'a MarkerEnvironment>,
markers: &'a ResolverMarkers,
) -> impl Iterator<Item = Cow<'a, Requirement>> + 'a {
self.overrides
.apply(self.requirements.iter())
.filter(move |requirement| requirement.evaluate_markers(markers, &[]))
.filter(move |requirement| {
requirement.evaluate_markers(markers.marker_environment(), &[])
})
}
/// Apply the overrides and constraints to a set of requirements.

8
crates/uv-resolver/src/preferences.rs
View file

@ -5,11 +5,13 @@ use tracing::trace;
use distribution_types::{InstalledDist, InstalledMetadata, InstalledVersion, Name};
use pep440_rs::{Operator, Version};
use pep508_rs::{MarkerEnvironment, MarkerTree, VersionOrUrl};
use pep508_rs::{MarkerTree, VersionOrUrl};
use pypi_types::{HashDigest, HashError};
use requirements_txt::{RequirementEntry, RequirementsTxtRequirement};
use uv_normalize::PackageName;
use crate::ResolverMarkers;
#[derive(thiserror::Error, Debug)]
pub enum PreferenceError {
#[error(transparent)]
@ -121,12 +123,12 @@ impl Preferences {
/// to an applicable subset.
pub fn from_iter<PreferenceIterator: IntoIterator<Item = Preference>>(
preferences: PreferenceIterator,
markers: Option<&MarkerEnvironment>,
markers: &ResolverMarkers,
) -> Self {
let mut slf = Self::default();
for preference in preferences {
// Filter non-matching preferences when resolving for an environment.
if let Some(markers) = markers {
if let Some(markers) = markers.marker_environment() {
if !preference.marker.evaluate(markers, &[]) {
trace!("Excluding {preference} from preferences due to unmatched markers");
continue;

3
crates/uv-resolver/src/prerelease.rs
View file

@ -1,6 +1,5 @@
use pypi_types::RequirementSource;
use pep508_rs::MarkerEnvironment;
use uv_normalize::PackageName;
use crate::resolver::ForkSet;
@ -68,7 +67,7 @@ impl PrereleaseStrategy {
pub(crate) fn from_mode(
mode: PrereleaseMode,
manifest: &Manifest,
markers: Option<&MarkerEnvironment>,
markers: &ResolverMarkers,
dependencies: DependencyMode,
) -> Self {
let mut packages = ForkSet::default();

5
crates/uv-resolver/src/resolution_mode.rs
View file

@ -1,9 +1,8 @@
use rustc_hash::FxHashSet;
use pep508_rs::MarkerEnvironment;
use uv_normalize::PackageName;
use crate::{DependencyMode, Manifest};
use crate::{DependencyMode, Manifest, ResolverMarkers};
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Deserialize, serde::Serialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
@ -47,7 +46,7 @@ impl ResolutionStrategy {
pub(crate) fn from_mode(
mode: ResolutionMode,
manifest: &Manifest,
markers: Option<&MarkerEnvironment>,
markers: &ResolverMarkers,
dependencies: DependencyMode,
) -> Self {
match mode {

5
crates/uv-resolver/src/resolver/groups.rs
View file

@ -1,9 +1,8 @@
use rustc_hash::FxHashMap;
use pep508_rs::MarkerEnvironment;
use uv_normalize::{GroupName, PackageName};
use crate::Manifest;
use crate::{Manifest, ResolverMarkers};
/// A map of package names to their activated dependency groups.
#[derive(Debug, Default, Clone)]
@ -11,7 +10,7 @@ pub(crate) struct Groups(FxHashMap<PackageName, Vec<GroupName>>);
impl Groups {
/// Determine the set of enabled dependency groups in the [`Manifest`].
pub(crate) fn from_manifest(manifest: &Manifest, markers: Option<&MarkerEnvironment>) -> Self {
pub(crate) fn from_manifest(manifest: &Manifest, markers: &ResolverMarkers) -> Self {
let mut groups = FxHashMap::default();
// Enable the groups for all direct dependencies. In practice, this tends to mean: when

4
crates/uv-resolver/src/resolver/locals.rs
View file

@ -3,7 +3,7 @@ use std::str::FromStr;
use distribution_filename::{SourceDistFilename, WheelFilename};
use distribution_types::RemoteSource;
use pep440_rs::{Operator, Version, VersionSpecifier, VersionSpecifierBuildError};
use pep508_rs::{MarkerEnvironment, PackageName};
use pep508_rs::PackageName;
use pypi_types::RequirementSource;
use crate::resolver::ForkMap;
@ -17,7 +17,7 @@ impl Locals {
/// Determine the set of permitted local versions in the [`Manifest`].
pub(crate) fn from_manifest(
manifest: &Manifest,
markers: Option<&MarkerEnvironment>,
markers: &ResolverMarkers,
dependencies: DependencyMode,
) -> Self {
let mut locals = ForkMap::default();

27
crates/uv-resolver/src/resolver/mod.rs
View file

@ -157,11 +157,7 @@ impl<'a, Context: BuildContext, InstalledPackages: InstalledPackagesProvider>
python_requirement
.target()
.and_then(|target| target.as_requires_python()),
AllowedYanks::from_manifest(
&manifest,
markers.marker_environment(),
options.dependency_mode,
),
AllowedYanks::from_manifest(&manifest, &markers, options.dependency_mode),
hasher,
options.exclude_newer,
build_context.build_options(),
@ -199,24 +195,11 @@ impl<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>
let state = ResolverState {
index: index.clone(),
git: git.clone(),
selector: CandidateSelector::for_resolution(
options,
&manifest,
markers.marker_environment(),
),
selector: CandidateSelector::for_resolution(options, &manifest, &markers),
dependency_mode: options.dependency_mode,
urls: Urls::from_manifest(
&manifest,
markers.marker_environment(),
git,
options.dependency_mode,
)?,
locals: Locals::from_manifest(
&manifest,
markers.marker_environment(),
options.dependency_mode,
),
groups: Groups::from_manifest(&manifest, markers.marker_environment()),
urls: Urls::from_manifest(&manifest, &markers, git, options.dependency_mode)?,
locals: Locals::from_manifest(&manifest, &markers, options.dependency_mode),
groups: Groups::from_manifest(&manifest, &markers),
project: manifest.project,
workspace_members: manifest.workspace_members,
requirements: manifest.requirements,

49
crates/uv-resolver/src/resolver/resolver_markers.rs
View file

@ -1,7 +1,7 @@
use std::fmt::{Display, Formatter};
use tracing::debug;
use pep508_rs::{MarkerEnvironment, MarkerTree};
use pypi_types::ResolverMarkerEnvironment;
#[derive(Debug, Clone)]
/// Whether we're solving for a specific environment, universally or for a specific fork.
@ -20,8 +20,8 @@ pub enum ResolverMarkers {
impl ResolverMarkers {
/// Set the resolver to perform a resolution for a specific environment.
pub fn specific_environment(markers: MarkerEnvironment) -> Self {
Self::SpecificEnvironment(ResolverMarkerEnvironment::from(markers))
pub fn specific_environment(markers: ResolverMarkerEnvironment) -> Self {
Self::SpecificEnvironment(markers)
}
/// Set the resolver to perform a universal resolution.
@ -71,46 +71,3 @@ impl Display for ResolverMarkers {
}
}
}
/// A wrapper type around [`MarkerEnvironment`] that ensures the Python version markers are
/// release-only, to match the resolver's semantics.
#[derive(Debug, Clone)]
pub struct ResolverMarkerEnvironment(MarkerEnvironment);
impl From<MarkerEnvironment> for ResolverMarkerEnvironment {
fn from(value: MarkerEnvironment) -> Self {
// Strip `python_version`.
let python_version = value.python_version().only_release();
let value = if python_version == **value.python_version() {
value
} else {
debug!(
"Stripping pre-release from `python_version`: {}",
value.python_version()
);
value.with_python_version(python_version)
};
// Strip `python_full_version`.
let python_full_version = value.python_full_version().only_release();
let value = if python_full_version == **value.python_full_version() {
value
} else {
debug!(
"Stripping pre-release from `python_full_version`: {}",
value.python_full_version()
);
value.with_python_full_version(python_full_version)
};
Self(value)
}
}
impl std::ops::Deref for ResolverMarkerEnvironment {
type Target = MarkerEnvironment;
fn deref(&self) -> &Self::Target {
&self.0
}
}

6
crates/uv-resolver/src/resolver/urls.rs
View file

@ -6,12 +6,12 @@ use tracing::debug;
use cache_key::CanonicalUrl;
use distribution_types::Verbatim;
use pep508_rs::{MarkerEnvironment, VerbatimUrl};
use pep508_rs::VerbatimUrl;
use pypi_types::{ParsedDirectoryUrl, ParsedUrl, VerbatimParsedUrl};
use uv_git::GitResolver;
use uv_normalize::PackageName;
use crate::{DependencyMode, Manifest, ResolveError};
use crate::{DependencyMode, Manifest, ResolveError, ResolverMarkers};
/// The URLs that are allowed for packages.
///
@ -36,7 +36,7 @@ pub(crate) struct Urls {
impl Urls {
pub(crate) fn from_manifest(
manifest: &Manifest,
markers: Option<&MarkerEnvironment>,
markers: &ResolverMarkers,
git: &GitResolver,
dependencies: DependencyMode,
) -> Result<Self, ResolveError> {

10
crates/uv-resolver/src/yanks.rs
View file

@ -1,12 +1,12 @@
use pypi_types::RequirementSource;
use rustc_hash::{FxHashMap, FxHashSet};
use std::sync::Arc;
use rustc_hash::{FxHashMap, FxHashSet};
use pep440_rs::Version;
use pep508_rs::MarkerEnvironment;
use pypi_types::RequirementSource;
use uv_normalize::PackageName;
use crate::{DependencyMode, Manifest};
use crate::{DependencyMode, Manifest, ResolverMarkers};
/// A set of package versions that are permitted, even if they're marked as yanked by the
/// relevant index.
@ -16,7 +16,7 @@ pub struct AllowedYanks(Arc<FxHashMap<PackageName, FxHashSet<Version>>>);
impl AllowedYanks {
pub fn from_manifest(
manifest: &Manifest,
markers: Option<&MarkerEnvironment>,
markers: &ResolverMarkers,
dependencies: DependencyMode,
) -> Self {
let mut allowed_yanks = FxHashMap::<PackageName, FxHashSet<Version>>::default();