Skip to content

Commit

Permalink
Restrict observed requirements to direct when --no-deps is specified (
Browse files Browse the repository at this point in the history
#3191)

## Summary

This PR avoids: (1) using the lookahead resolver when `--no-deps` is
specified (we'll never use those requirements), and (2) including any
transitive requirements when searching for allowed URLs, etc., when
`--no-deps` is specified.

Closes #3183.
  • Loading branch information
charliermarsh authored Apr 22, 2024
1 parent a4f125c commit 792a917
Show file tree
Hide file tree
Showing 11 changed files with 228 additions and 93 deletions.
4 changes: 3 additions & 1 deletion crates/uv-resolver/src/candidate_selector.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use pubgrub::range::Range;
use tracing::debug;

use distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource};
use distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist};
use pep440_rs::Version;
use pep508_rs::MarkerEnvironment;
use tracing::debug;
use uv_normalize::PackageName;
use uv_types::InstalledPackagesProvider;

Expand Down Expand Up @@ -32,11 +32,13 @@ impl CandidateSelector {
options.resolution_mode,
manifest,
markers,
options.dependency_mode,
),
prerelease_strategy: PreReleaseStrategy::from_mode(
options.prerelease_mode,
manifest,
markers,
options.dependency_mode,
),
}
}
Expand Down
136 changes: 88 additions & 48 deletions crates/uv-resolver/src/manifest.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use distribution_types::LocalEditable;
use either::Either;
use pep508_rs::{MarkerEnvironment, Requirement};
use pypi_types::Metadata23;
use uv_configuration::{Constraints, Overrides};
use uv_normalize::PackageName;
use uv_types::RequestedRequirements;

use crate::{preferences::Preference, Exclusions};
use crate::{preferences::Preference, DependencyMode, Exclusions};

/// A manifest of requirements, constraints, and preferences.
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -97,64 +98,103 @@ impl Manifest {
pub fn requirements<'a>(
&'a self,
markers: &'a MarkerEnvironment,
mode: DependencyMode,
) -> impl Iterator<Item = &Requirement> {
self.lookaheads
.iter()
.flat_map(|lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(|requirement| requirement.evaluate_markers(markers, lookahead.extras()))
})
.chain(self.editables.iter().flat_map(|(editable, metadata)| {
self.overrides
.apply(&metadata.requires_dist)
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras))
}))
.chain(
self.overrides
.apply(&self.requirements)
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.constraints
.requirements()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.overrides
.requirements()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
match mode {
// Include all direct and transitive requirements, with constraints and overrides applied.
DependencyMode::Transitive => Either::Left( self
.lookaheads
.iter()
.flat_map(|lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(|requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
})
.chain(self.editables.iter().flat_map(|(editable, metadata)| {
self.overrides
.apply(&metadata.requires_dist)
.filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
.chain(
self.overrides
.apply(&self.requirements)
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.constraints
.requirements()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.overrides
.requirements()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
))
,

// Include direct requirements, with constraints and overrides applied.
DependencyMode::Direct => Either::Right(
self.overrides.apply(& self.requirements)
.chain(self.constraints.requirements())
.chain(self.overrides.requirements())
.filter(|requirement| requirement.evaluate_markers(markers, &[]))),
}
}

/// Return an iterator over the names of all direct dependency requirements.
/// Return an iterator over the names of all user-provided requirements.
///
/// This includes:
/// - Direct requirements
/// - Dependencies of editable requirements
/// - Transitive dependencies of local package requirements
///
/// At time of writing, this is used for:
/// - Determining which packages should use the "lowest-compatible version" of a package, when
/// the `lowest-direct` strategy is in use.
pub fn direct_dependencies<'a>(
pub fn user_requirements<'a>(
&'a self,
markers: &'a MarkerEnvironment,
) -> impl Iterator<Item = &PackageName> {
self.lookaheads
.iter()
.filter(|lookahead| lookahead.direct())
.flat_map(|lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(|requirement| requirement.evaluate_markers(markers, lookahead.extras()))
})
.chain(self.editables.iter().flat_map(|(editable, metadata)| {
self.overrides
.apply(&metadata.requires_dist)
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras))
}))
.chain(
mode: DependencyMode,
) -> impl Iterator<Item = &Requirement> {
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(|lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(|requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
})
.chain(self.editables.iter().flat_map(|(editable, metadata)| {
self.overrides
.apply(&metadata.requires_dist)
.filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
.chain(
self.overrides
.apply(&self.requirements)
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
),
),

// Restrict to the direct requirements.
DependencyMode::Direct => Either::Right(
self.overrides
.apply(&self.requirements)
.apply(self.requirements.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.map(|requirement| &requirement.name)
),
}
}

/// Apply the overrides and constraints to a set of requirements.
Expand Down
7 changes: 4 additions & 3 deletions crates/uv-resolver/src/prerelease_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rustc_hash::FxHashSet;
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
use uv_normalize::PackageName;

use crate::Manifest;
use crate::{DependencyMode, Manifest};

#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
Expand Down Expand Up @@ -60,14 +60,15 @@ impl PreReleaseStrategy {
mode: PreReleaseMode,
manifest: &Manifest,
markers: &MarkerEnvironment,
dependencies: DependencyMode,
) -> Self {
match mode {
PreReleaseMode::Disallow => Self::Disallow,
PreReleaseMode::Allow => Self::Allow,
PreReleaseMode::IfNecessary => Self::IfNecessary,
PreReleaseMode::Explicit => Self::Explicit(
manifest
.requirements(markers)
.requirements(markers, dependencies)
.filter(|requirement| {
let Some(version_or_url) = &requirement.version_or_url else {
return false;
Expand All @@ -87,7 +88,7 @@ impl PreReleaseStrategy {
),
PreReleaseMode::IfNecessaryOrExplicit => Self::IfNecessaryOrExplicit(
manifest
.requirements(markers)
.requirements(markers, dependencies)
.filter(|requirement| {
let Some(version_or_url) = &requirement.version_or_url else {
return false;
Expand Down
12 changes: 8 additions & 4 deletions crates/uv-resolver/src/resolution_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use rustc_hash::FxHashSet;
use pep508_rs::MarkerEnvironment;
use uv_normalize::PackageName;

use crate::Manifest;
use crate::{DependencyMode, Manifest};

#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
Expand Down Expand Up @@ -42,13 +42,17 @@ impl ResolutionStrategy {
mode: ResolutionMode,
manifest: &Manifest,
markers: &MarkerEnvironment,
dependencies: DependencyMode,
) -> Self {
match mode {
ResolutionMode::Highest => Self::Highest,
ResolutionMode::Lowest => Self::Lowest,
ResolutionMode::LowestDirect => {
Self::LowestDirect(manifest.direct_dependencies(markers).cloned().collect())
}
ResolutionMode::LowestDirect => Self::LowestDirect(
manifest
.user_requirements(markers, dependencies)
.map(|requirement| requirement.name.clone())
.collect(),
),
}
}
}
10 changes: 7 additions & 3 deletions crates/uv-resolver/src/resolver/locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use pep440_rs::{Operator, Version, VersionSpecifier, VersionSpecifierBuildError}
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
use uv_normalize::PackageName;

use crate::Manifest;
use crate::{DependencyMode, Manifest};

#[derive(Debug, Default)]
pub(crate) struct Locals {
Expand All @@ -19,12 +19,16 @@ pub(crate) struct Locals {

impl Locals {
/// Determine the set of permitted local versions in the [`Manifest`].
pub(crate) fn from_manifest(manifest: &Manifest, markers: &MarkerEnvironment) -> Self {
pub(crate) fn from_manifest(
manifest: &Manifest,
markers: &MarkerEnvironment,
dependencies: DependencyMode,
) -> Self {
let mut required: FxHashMap<PackageName, Version> = FxHashMap::default();

// Add all direct requirements and constraints. There's no need to look for conflicts,
// since conflicts will be enforced by the solver.
for requirement in manifest.requirements(markers) {
for requirement in manifest.requirements(markers, dependencies) {
if let Some(version_or_url) = requirement.version_or_url.as_ref() {
for local in iter_locals(version_or_url) {
required.insert(requirement.name.clone(), local);
Expand Down
6 changes: 3 additions & 3 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl<
flat_index,
tags,
PythonRequirement::new(interpreter, markers),
AllowedYanks::from_manifest(&manifest, markers),
AllowedYanks::from_manifest(&manifest, markers, options.dependency_mode),
hasher,
options.exclude_newer,
build_context.no_binary(),
Expand Down Expand Up @@ -210,8 +210,8 @@ impl<
visited: DashSet::default(),
selector: CandidateSelector::for_resolution(options, &manifest, markers),
dependency_mode: options.dependency_mode,
urls: Urls::from_manifest(&manifest, markers)?,
locals: Locals::from_manifest(&manifest, markers),
urls: Urls::from_manifest(&manifest, markers, options.dependency_mode)?,
locals: Locals::from_manifest(&manifest, markers, options.dependency_mode),
project: manifest.project,
requirements: manifest.requirements,
constraints: manifest.constraints,
Expand Down
5 changes: 3 additions & 2 deletions crates/uv-resolver/src/resolver/urls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use pep508_rs::{MarkerEnvironment, VerbatimUrl};
use uv_distribution::is_same_reference;
use uv_normalize::PackageName;

use crate::{Manifest, ResolveError};
use crate::{DependencyMode, Manifest, ResolveError};

/// A map of package names to their associated, required URLs.
#[derive(Debug, Default)]
Expand All @@ -16,6 +16,7 @@ impl Urls {
pub(crate) fn from_manifest(
manifest: &Manifest,
markers: &MarkerEnvironment,
dependencies: DependencyMode,
) -> Result<Self, ResolveError> {
let mut urls: FxHashMap<PackageName, VerbatimUrl> = FxHashMap::default();

Expand All @@ -37,7 +38,7 @@ impl Urls {
}

// Add all direct requirements and constraints. If there are any conflicts, return an error.
for requirement in manifest.requirements(markers) {
for requirement in manifest.requirements(markers, dependencies) {
if let Some(pep508_rs::VersionOrUrl::Url(url)) = &requirement.version_or_url {
if let Some(previous) = urls.insert(requirement.name.clone(), url.clone()) {
if !is_equal(&previous, url) {
Expand Down
10 changes: 7 additions & 3 deletions crates/uv-resolver/src/yanks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,23 @@ use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
use uv_normalize::PackageName;

use crate::{Manifest, Preference};
use crate::{DependencyMode, Manifest, Preference};

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

impl AllowedYanks {
pub fn from_manifest(manifest: &Manifest, markers: &MarkerEnvironment) -> Self {
pub fn from_manifest(
manifest: &Manifest,
markers: &MarkerEnvironment,
dependencies: DependencyMode,
) -> Self {
let mut allowed_yanks = FxHashMap::<PackageName, FxHashSet<Version>>::default();

for requirement in manifest
.requirements(markers)
.requirements(markers, dependencies)
.chain(manifest.preferences.iter().map(Preference::requirement))
{
let Some(VersionOrUrl::VersionSpecifier(specifiers)) = &requirement.version_or_url
Expand Down
31 changes: 18 additions & 13 deletions crates/uv/src/commands/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,19 +402,24 @@ pub(crate) async fn pip_compile(
};

// Determine any lookahead requirements.
let lookaheads = LookaheadResolver::new(
&requirements,
&constraints,
&overrides,
&editables,
&hasher,
&build_dispatch,
&client,
&top_level_index,
)
.with_reporter(ResolverReporter::from(printer))
.resolve(&markers)
.await?;
let lookaheads = match dependency_mode {
DependencyMode::Transitive => {
LookaheadResolver::new(
&requirements,
&constraints,
&overrides,
&editables,
&hasher,
&build_dispatch,
&client,
&top_level_index,
)
.with_reporter(ResolverReporter::from(printer))
.resolve(&markers)
.await?
}
DependencyMode::Direct => Vec::new(),
};

// Create a manifest of the requirements.
let manifest = Manifest::new(
Expand Down
Loading

0 comments on commit 792a917

Please sign in to comment.