Skip to content

Commit

Permalink
Make direct dependency detection respect markers
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Mar 5, 2024
1 parent 7f07ada commit 1f7e8be
Show file tree
Hide file tree
Showing 6 changed files with 135 additions and 57 deletions.
39 changes: 27 additions & 12 deletions crates/uv-resolver/src/candidate_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_hash::FxHashMap;
use distribution_types::CompatibleDist;
use distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist};
use pep440_rs::Version;
use pep508_rs::{Requirement, VersionOrUrl};
use pep508_rs::{MarkerEnvironment, Requirement, VersionOrUrl};
use uv_normalize::PackageName;

use crate::prerelease_mode::PreReleaseStrategy;
Expand All @@ -21,11 +21,23 @@ pub(crate) struct CandidateSelector {

impl CandidateSelector {
/// Return a [`CandidateSelector`] for the given [`Manifest`].
pub(crate) fn for_resolution(manifest: &Manifest, options: Options) -> Self {
pub(crate) fn for_resolution(
options: Options,
manifest: &Manifest,
markers: &MarkerEnvironment,
) -> Self {
Self {
resolution_strategy: ResolutionStrategy::from_mode(options.resolution_mode, manifest),
prerelease_strategy: PreReleaseStrategy::from_mode(options.prerelease_mode, manifest),
preferences: Preferences::from(manifest.preferences.as_slice()),
resolution_strategy: ResolutionStrategy::from_mode(
options.resolution_mode,
manifest,
markers,
),
prerelease_strategy: PreReleaseStrategy::from_mode(
options.prerelease_mode,
manifest,
markers,
),
preferences: Preferences::from_requirements(manifest.preferences.as_slice(), markers),
}
}

Expand All @@ -47,17 +59,15 @@ impl CandidateSelector {
struct Preferences(FxHashMap<PackageName, Version>);

impl Preferences {
fn get(&self, package_name: &PackageName) -> Option<&Version> {
self.0.get(package_name)
}
}

impl From<&[Requirement]> for Preferences {
fn from(requirements: &[Requirement]) -> Self {
/// Create a set of [`Preferences`] from a set of requirements.
fn from_requirements(requirements: &[Requirement], markers: &MarkerEnvironment) -> Self {
Self(
requirements
.iter()
.filter_map(|requirement| {
if !requirement.evaluate_markers(markers, &[]) {
return None;
}
let Some(VersionOrUrl::VersionSpecifier(version_specifiers)) =
requirement.version_or_url.as_ref()
else {
Expand All @@ -74,6 +84,11 @@ impl From<&[Requirement]> for Preferences {
.collect(),
)
}

/// Return the pinned version for a package, if any.
fn get(&self, package_name: &PackageName) -> Option<&Version> {
self.0.get(package_name)
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
Expand Down
32 changes: 18 additions & 14 deletions crates/uv-resolver/src/prerelease_mode.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc_hash::FxHashSet;

use pep508_rs::VersionOrUrl;
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
use uv_normalize::PackageName;

use crate::Manifest;
Expand Down Expand Up @@ -50,7 +50,11 @@ pub(crate) enum PreReleaseStrategy {
}

impl PreReleaseStrategy {
pub(crate) fn from_mode(mode: PreReleaseMode, manifest: &Manifest) -> Self {
pub(crate) fn from_mode(
mode: PreReleaseMode,
manifest: &Manifest,
markers: &MarkerEnvironment,
) -> Self {
match mode {
PreReleaseMode::Disallow => Self::Disallow,
PreReleaseMode::Allow => Self::Allow,
Expand All @@ -61,12 +65,12 @@ impl PreReleaseStrategy {
.iter()
.chain(manifest.constraints.iter())
.chain(manifest.overrides.iter())
.chain(
manifest
.editables
.iter()
.flat_map(|(_editable, metadata)| metadata.requires_dist.iter()),
)
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
.filter(|requirement| {
let Some(version_or_url) = &requirement.version_or_url else {
return false;
Expand All @@ -90,12 +94,12 @@ impl PreReleaseStrategy {
.iter()
.chain(manifest.constraints.iter())
.chain(manifest.overrides.iter())
.chain(
manifest
.editables
.iter()
.flat_map(|(_editable, metadata)| metadata.requires_dist.iter()),
)
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
.filter(|requirement| {
let Some(version_or_url) = &requirement.version_or_url else {
return false;
Expand Down
19 changes: 12 additions & 7 deletions crates/uv-resolver/src/resolution_mode.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustc_hash::FxHashSet;

use pep508_rs::MarkerEnvironment;
use uv_normalize::PackageName;

use crate::Manifest;
Expand Down Expand Up @@ -31,7 +32,11 @@ pub(crate) enum ResolutionStrategy {
}

impl ResolutionStrategy {
pub(crate) fn from_mode(mode: ResolutionMode, manifest: &Manifest) -> Self {
pub(crate) fn from_mode(
mode: ResolutionMode,
manifest: &Manifest,
markers: &MarkerEnvironment,
) -> Self {
match mode {
ResolutionMode::Highest => Self::Highest,
ResolutionMode::Lowest => Self::Lowest,
Expand All @@ -40,12 +45,12 @@ impl ResolutionStrategy {
manifest
.requirements
.iter()
.chain(
manifest
.editables
.iter()
.flat_map(|(_editable, metadata)| metadata.requires_dist.iter()),
)
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
.map(|requirement| requirement.name.clone())
.collect(),
),
Expand Down
13 changes: 2 additions & 11 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,21 +158,12 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
index: &'a InMemoryIndex,
provider: Provider,
) -> Result<Self, ResolveError> {
let selector = CandidateSelector::for_resolution(&manifest, options);

// Determine the allowed yanked package versions
let allowed_yanks = manifest
.requirements
.iter()
.chain(manifest.constraints.iter())
.collect();

Ok(Self {
index,
unavailable_packages: DashMap::default(),
visited: DashSet::default(),
selector,
allowed_yanks,
selector: CandidateSelector::for_resolution(options, &manifest, markers),
allowed_yanks: AllowedYanks::from_manifest(&manifest, markers),
dependency_mode: options.dependency_mode,
urls: Urls::from_manifest(&manifest, markers)?,
project: manifest.project,
Expand Down
39 changes: 26 additions & 13 deletions crates/uv-resolver/src/yanks.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,33 @@
use rustc_hash::{FxHashMap, FxHashSet};

use pep440_rs::Version;
use pep508_rs::Requirement;
use pep508_rs::MarkerEnvironment;
use uv_normalize::PackageName;

use crate::Manifest;

/// A set of package versions that are permitted, even if they're marked as yanked by the
/// relevant index.
#[derive(Debug, Default)]
pub(crate) struct AllowedYanks(FxHashMap<PackageName, FxHashSet<Version>>);

impl AllowedYanks {
/// Returns `true` if the given package version is allowed, even if it's marked as yanked by
/// the relevant index.
pub(crate) fn allowed(&self, package_name: &PackageName, version: &Version) -> bool {
self.0
.get(package_name)
.is_some_and(|allowed_yanks| allowed_yanks.contains(version))
}
}

impl<'a> FromIterator<&'a Requirement> for AllowedYanks {
fn from_iter<T: IntoIterator<Item = &'a Requirement>>(iter: T) -> Self {
pub(crate) fn from_manifest(manifest: &Manifest, markers: &MarkerEnvironment) -> Self {
let mut allowed_yanks = FxHashMap::<PackageName, FxHashSet<Version>>::default();
for requirement in iter {
for requirement in manifest
.requirements
.iter()
.chain(manifest.constraints.iter())
.chain(manifest.overrides.iter())
.chain(manifest.preferences.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata
.requires_dist
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras))
}))
{
let Some(pep508_rs::VersionOrUrl::VersionSpecifier(specifiers)) =
&requirement.version_or_url
else {
Expand All @@ -43,4 +48,12 @@ impl<'a> FromIterator<&'a Requirement> for AllowedYanks {
}
Self(allowed_yanks)
}

/// Returns `true` if the given package version is allowed, even if it's marked as yanked by
/// the relevant index.
pub(crate) fn allowed(&self, package_name: &PackageName, version: &Version) -> bool {
self.0
.get(package_name)
.is_some_and(|allowed_yanks| allowed_yanks.contains(version))
}
}
50 changes: 50 additions & 0 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4804,3 +4804,53 @@ dev = [

Ok(())
}

/// Under `--resolution=lowest-direct`, ignore optional dependencies.
///
/// In the below example, ensure that `setuptools` does not resolve to the lowest-available version.
#[test]
fn editable_optional_lowest_direct() -> Result<()> {
let context = TestContext::new("3.12");

// Create an editable package with an optional URL dependency.
let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"[project]
name = "example"
version = "0.0.0"
dependencies = ["setuptools-scm>=8.0.0"]
requires-python = '>=3.8'
[project.optional-dependencies]
dev = ["setuptools"]
"#,
)?;

// Write to a requirements file.
let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str("-e .")?;

uv_snapshot!(context.compile()
.arg("requirements.in")
.arg("--resolution=lowest-direct"), @r###"
success: true
exit_code: 0
----- stdout -----
# This file was autogenerated by uv via the following command:
# uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2023-11-18T12:00:00Z requirements.in --resolution=lowest-direct
-e .
packaging==23.2
# via setuptools-scm
setuptools==68.2.2
# via setuptools-scm
setuptools-scm==8.0.1
# via example
----- stderr -----
Built 1 editable in [TIME]
Resolved 4 packages in [TIME]
"###
);

Ok(())
}

0 comments on commit 1f7e8be

Please sign in to comment.