Skip to content

Commit

Permalink
Add convenience methods to Manifest to iterate over requirements (#…
Browse files Browse the repository at this point in the history
…2701)

## Summary

These are repeated a bunch. It's nice to DRY them up and ensure the
ordering is consistent.
  • Loading branch information
charliermarsh committed Mar 28, 2024
1 parent b6ab919 commit 4cc91cc
Show file tree
Hide file tree
Showing 7 changed files with 168 additions and 92 deletions.
76 changes: 75 additions & 1 deletion crates/uv-resolver/src/manifest.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use distribution_types::LocalEditable;
use pep508_rs::Requirement;
use pep508_rs::{MarkerEnvironment, Requirement};
use pypi_types::Metadata23;
use uv_normalize::PackageName;
use uv_types::RequestedRequirements;
Expand Down Expand Up @@ -74,4 +74,78 @@ impl Manifest {
lookaheads: Vec::new(),
}
}

/// Return an iterator over all requirements, constraints, and overrides, in priority order,
/// such that requirements come first, followed by constraints, followed by overrides.
///
/// At time of writing, this is used for:
/// - Determining which requirements should allow yanked versions.
/// - Determining which requirements should allow pre-release versions (e.g., `torch>=2.2.0a1`).
/// - Determining which requirements should allow direct URLs (e.g., `torch @ https://...`).
/// - Determining which requirements should allow local version specifiers (e.g., `torch==2.2.0+cpu`).
pub fn requirements<'a>(
&'a self,
markers: &'a MarkerEnvironment,
) -> impl Iterator<Item = &Requirement> {
self.lookaheads
.iter()
.flat_map(|lookahead| {
lookahead
.requirements()
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, lookahead.extras()))
})
.chain(self.editables.iter().flat_map(|(editable, metadata)| {
metadata
.requires_dist
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras))
}))
.chain(
self.requirements
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.constraints
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.overrides
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
}

/// Return an iterator over the names of all direct dependency 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>(
&'a self,
markers: &'a MarkerEnvironment,
) -> impl Iterator<Item = &PackageName> {
self.lookaheads
.iter()
.flat_map(|lookahead| {
lookahead
.requirements()
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, lookahead.extras()))
})
.chain(self.editables.iter().flat_map(|(editable, metadata)| {
metadata
.requires_dist
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras))
}))
.chain(
self.requirements
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[])),
)
.map(|requirement| &requirement.name)
}
}
32 changes: 2 additions & 30 deletions crates/uv-resolver/src/prerelease_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,7 @@ impl PreReleaseStrategy {
PreReleaseMode::IfNecessary => Self::IfNecessary,
PreReleaseMode::Explicit => Self::Explicit(
manifest
.requirements
.iter()
.chain(manifest.constraints.iter())
.chain(manifest.overrides.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.lookaheads.iter().flat_map(|lookahead| {
lookahead.requirements().iter().filter(|requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
}))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
.requirements(markers)
.filter(|requirement| {
let Some(version_or_url) = &requirement.version_or_url else {
return false;
Expand All @@ -95,21 +81,7 @@ impl PreReleaseStrategy {
),
PreReleaseMode::IfNecessaryOrExplicit => Self::IfNecessaryOrExplicit(
manifest
.requirements
.iter()
.chain(manifest.constraints.iter())
.chain(manifest.overrides.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.lookaheads.iter().flat_map(|lookahead| {
lookahead.requirements().iter().filter(|requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
}))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
.requirements(markers)
.filter(|requirement| {
let Some(version_or_url) = &requirement.version_or_url else {
return false;
Expand Down
22 changes: 3 additions & 19 deletions crates/uv-resolver/src/resolution_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,25 +40,9 @@ impl ResolutionStrategy {
match mode {
ResolutionMode::Highest => Self::Highest,
ResolutionMode::Lowest => Self::Lowest,
ResolutionMode::LowestDirect => Self::LowestDirect(
// Consider `requirements` and dependencies of any local requirements to be "direct" dependencies.
manifest
.requirements
.iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.lookaheads.iter().flat_map(|lookahead| {
lookahead.requirements().iter().filter(|requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
}))
.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(),
),
ResolutionMode::LowestDirect => {
Self::LowestDirect(manifest.direct_dependencies(markers).cloned().collect())
}
}
}
}
21 changes: 2 additions & 19 deletions crates/uv-resolver/src/resolver/locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,8 @@ impl Locals {
let mut required: FxHashMap<PackageName, Version> = FxHashMap::default();

// Add all direct requirements and constraints. There's no need to look for conflicts,
// since conflicting versions will be tracked upstream.
for requirement in
manifest
.requirements
.iter()
.chain(manifest.constraints.iter())
.chain(manifest.overrides.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.lookaheads.iter().flat_map(|lookahead| {
lookahead.requirements().iter().filter(|requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
}))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
{
// since conflicts will be enforced by the solver.
for requirement in manifest.requirements(markers) {
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/version_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ impl VersionMap {
/// stored in this map. For example, the versions `1.2.0` and `1.2` are
/// semantically equivalent, but when converted to strings, they are
/// distinct.
pub(crate) fn get_with_version<'a>(
&'a self,
pub(crate) fn get_with_version(
&self,
version: &Version,
) -> Option<(&'a Version, &'a PrioritizedDist)> {
) -> Option<(&Version, &PrioritizedDist)> {
match self.inner {
VersionMapInner::Eager(ref map) => map.get_key_value(version),
VersionMapInner::Lazy(ref lazy) => lazy.get_with_version(version),
Expand Down
24 changes: 4 additions & 20 deletions crates/uv-resolver/src/yanks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, VersionOrUrl};
use uv_normalize::PackageName;

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

/// A set of package versions that are permitted, even if they're marked as yanked by the
/// relevant index.
Expand All @@ -15,24 +14,9 @@ pub struct AllowedYanks(FxHashMap<PackageName, FxHashSet<Version>>);
impl AllowedYanks {
pub fn from_manifest(manifest: &Manifest, markers: &MarkerEnvironment) -> Self {
let mut allowed_yanks = FxHashMap::<PackageName, FxHashSet<Version>>::default();
for requirement in
manifest
.requirements
.iter()
.chain(manifest.constraints.iter())
.chain(manifest.overrides.iter())
.chain(manifest.preferences.iter().map(Preference::requirement))
.filter(|requirement| requirement.evaluate_markers(markers, &[]))
.chain(manifest.lookaheads.iter().flat_map(|lookahead| {
lookahead.requirements().iter().filter(|requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
}))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras)
})
}))
for requirement in manifest
.requirements(markers)
.chain(manifest.preferences.iter().map(Preference::requirement))
{
let Some(VersionOrUrl::VersionSpecifier(specifiers)) = &requirement.version_or_url
else {
Expand Down
79 changes: 79 additions & 0 deletions crates/uv/tests/pip_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1815,6 +1815,85 @@ fn allowed_transitive_url_path_dependency() -> Result<()> {
Ok(())
}

/// A dependency with conflicting URLs in `requirements.in` and `constraints.txt` should arguably
/// be ignored if the dependency has an override. However, we currently error in this case.
#[test]
fn requirement_constraint_override_url() -> Result<()> {
let context = TestContext::new("3.12");

let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str("anyio @ https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz")?;

let constraints_txt = context.temp_dir.child("constraints.txt");
constraints_txt.write_str("anyio @ https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24bd4afdf1143390242c0b33774da0e2e34f/anyio-4.3.0-py3-none-any.whl")?;

let overrides_txt = context.temp_dir.child("overrides.txt");
overrides_txt.write_str("anyio==3.7.0")?;

uv_snapshot!(context.compile()
.arg("requirements.in")
.arg("--constraint")
.arg("constraints.txt")
.arg("--override")
.arg("overrides.txt"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: Requirements contain conflicting URLs for package `anyio`:
- https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz
- https://files.pythonhosted.org/packages/14/fd/2f20c40b45e4fb4324834aea24bd4afdf1143390242c0b33774da0e2e34f/anyio-4.3.0-py3-none-any.whl
"###
);

Ok(())
}

/// A dependency that uses a pre-release marker in `requirements.in` should be overridden by a
/// non-pre-release version in `overrides.txt`. We currently allow Flask to use a pre-release below,
/// but probably shouldn't.
#[test]
fn requirement_override_prerelease() -> Result<()> {
let context = TestContext::new("3.12");

let requirements_in = context.temp_dir.child("requirements.in");
requirements_in.write_str("flask<2.0.0rc4")?;

let overrides_txt = context.temp_dir.child("overrides.txt");
overrides_txt.write_str("flask<2.0.1,!=2.0.0")?;

uv_snapshot!(context.compile()
.arg("requirements.in")
.arg("--override")
.arg("overrides.txt"), @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 2024-03-25T00:00:00Z requirements.in --override overrides.txt
click==8.1.7
# via flask
flask==2.0.0rc2
itsdangerous==2.1.2
# via flask
jinja2==3.1.3
# via flask
markupsafe==2.1.5
# via
# jinja2
# werkzeug
werkzeug==3.0.1
# via flask
----- stderr -----
Resolved 6 packages in [TIME]
"###
);

Ok(())
}

/// Resolve packages from all optional dependency groups in a `pyproject.toml` file.
#[test]
fn compile_pyproject_toml_all_extras() -> Result<()> {
Expand Down

0 comments on commit 4cc91cc

Please sign in to comment.