Support recursive extras (#1435)

## Summary

We had a guard in the resolve to avoid "self-dependencies" (as in
`gps3==0.33.3`), but this guard was _unintentionally_ filtering out
recursive extras.

Closes https://github.com/astral-sh/uv/issues/1342.

## Test Plan

Taken from https://github.com/astral-sh/uv/pull/1352.
This commit is contained in:
Charlie Marsh 2024-02-16 11:42:04 -05:00 committed by GitHub
parent e6c4c77ba1
commit f25781ff6c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 97 additions and 19 deletions

View file

@ -21,22 +21,16 @@ impl PubGrubDependencies {
requirements: &[Requirement], requirements: &[Requirement],
constraints: &[Requirement], constraints: &[Requirement],
overrides: &Overrides, overrides: &Overrides,
extra: Option<&ExtraName>, source_name: Option<&PackageName>,
source: Option<&PackageName>, source_extra: Option<&ExtraName>,
env: &MarkerEnvironment, env: &MarkerEnvironment,
) -> Result<Self, ResolveError> { ) -> Result<Self, ResolveError> {
let mut dependencies = DependencyConstraints::<PubGrubPackage, Range<Version>>::default(); let mut dependencies = DependencyConstraints::<PubGrubPackage, Range<Version>>::default();
// Iterate over all declared requirements. // Iterate over all declared requirements.
for requirement in overrides.apply(requirements) { for requirement in overrides.apply(requirements) {
// Avoid self-dependencies.
if source.is_some_and(|source| source == &requirement.name) {
warn!("{} has a dependency on itself", requirement.name);
continue;
}
// If the requirement isn't relevant for the current platform, skip it. // If the requirement isn't relevant for the current platform, skip it.
if let Some(extra) = extra { if let Some(extra) = source_extra {
if !requirement.evaluate_markers(env, std::slice::from_ref(extra)) { if !requirement.evaluate_markers(env, std::slice::from_ref(extra)) {
continue; continue;
} }
@ -56,6 +50,16 @@ impl PubGrubDependencies {
) { ) {
let (package, version) = result?; let (package, version) = result?;
// Ignore self-dependencies.
if let PubGrubPackage::Package(name, extra, None) = &package {
if source_name.is_some_and(|source_name| source_name == name)
&& source_extra == extra.as_ref()
{
warn!("{name} has a dependency on itself");
continue;
}
}
if let Some(entry) = dependencies.get_key_value(&package) { if let Some(entry) = dependencies.get_key_value(&package) {
// Merge the versions. // Merge the versions.
let version = merge_versions(&package, entry.1, &version)?; let version = merge_versions(&package, entry.1, &version)?;
@ -80,14 +84,8 @@ impl PubGrubDependencies {
continue; continue;
} }
// Avoid self-dependencies.
if source.is_some_and(|source| source == &constraint.name) {
warn!("{} has a dependency on itself", constraint.name);
continue;
}
// If the requirement isn't relevant for the current platform, skip it. // If the requirement isn't relevant for the current platform, skip it.
if let Some(extra) = extra { if let Some(extra) = source_extra {
if !constraint.evaluate_markers(env, std::slice::from_ref(extra)) { if !constraint.evaluate_markers(env, std::slice::from_ref(extra)) {
continue; continue;
} }
@ -107,6 +105,16 @@ impl PubGrubDependencies {
) { ) {
let (package, version) = result?; let (package, version) = result?;
// Ignore self-dependencies.
if let PubGrubPackage::Package(name, extra, None) = &package {
if source_name.is_some_and(|source_name| source_name == name)
&& source_extra == extra.as_ref()
{
warn!("{name} has a dependency on itself");
continue;
}
}
if let Some(entry) = dependencies.get_key_value(&package) { if let Some(entry) = dependencies.get_key_value(&package) {
// Merge the versions. // Merge the versions.
let version = merge_versions(&package, entry.1, &version)?; let version = merge_versions(&package, entry.1, &version)?;

View file

@ -861,8 +861,8 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
&metadata.requires_dist, &metadata.requires_dist,
&self.constraints, &self.constraints,
&self.overrides, &self.overrides,
extra.as_ref(),
Some(package_name), Some(package_name),
extra.as_ref(),
self.markers, self.markers,
)?; )?;

View file

@ -1,7 +1,7 @@
//! DO NOT EDIT //! DO NOT EDIT
//! //!
//! Generated with ./scripts/scenarios/update.py //! Generated with ./scripts/scenarios/update.py
//! Scenarios from <https://github.com/zanieb/packse/tree/de58b3e3f998486b6c0f3dd67b7341c880eb54b2/scenarios> //! Scenarios from <https://github.com/zanieb/packse/tree/64b4451b832cece378f6e773d326ea09efe8903d/scenarios>
//! //!
#![cfg(all(feature = "python", feature = "pypi"))] #![cfg(all(feature = "python", feature = "pypi"))]
@ -735,6 +735,76 @@ fn multiple_extras_required() {
assert_installed(&context.venv, "c_502cbb59", "1.0.0", &context.temp_dir); assert_installed(&context.venv, "c_502cbb59", "1.0.0", &context.temp_dir);
} }
/// all-extras-required
///
/// Multiple optional dependencies are requested for the via an 'all' extra.
///
/// ```text
/// 4cf56e90
/// ├── environment
/// │ └── python3.8
/// ├── root
/// │ └── requires a[all]
/// │ ├── satisfied by a-1.0.0
/// │ ├── satisfied by a-1.0.0[all]
/// │ ├── satisfied by a-1.0.0[extra_b]
/// │ └── satisfied by a-1.0.0[extra_c]
/// ├── a
/// │ ├── a-1.0.0
/// │ ├── a-1.0.0[all]
/// │ │ ├── requires a[extra_b]
/// │ │ │ ├── satisfied by a-1.0.0
/// │ │ │ ├── satisfied by a-1.0.0[all]
/// │ │ │ ├── satisfied by a-1.0.0[extra_b]
/// │ │ │ └── satisfied by a-1.0.0[extra_c]
/// │ │ └── requires a[extra_c]
/// │ │ ├── satisfied by a-1.0.0
/// │ │ ├── satisfied by a-1.0.0[all]
/// │ │ ├── satisfied by a-1.0.0[extra_b]
/// │ │ └── satisfied by a-1.0.0[extra_c]
/// │ ├── a-1.0.0[extra_b]
/// │ │ └── requires b
/// │ │ └── satisfied by b-1.0.0
/// │ └── a-1.0.0[extra_c]
/// │ └── requires c
/// │ └── satisfied by c-1.0.0
/// ├── b
/// │ └── b-1.0.0
/// └── c
/// └── c-1.0.0
/// ```
#[test]
fn all_extras_required() {
let context = TestContext::new("3.8");
// In addition to the standard filters, swap out package names for more realistic messages
let mut filters = INSTA_FILTERS.to_vec();
filters.push((r"a-4cf56e90", "albatross"));
filters.push((r"b-4cf56e90", "bluebird"));
filters.push((r"c-4cf56e90", "crow"));
filters.push((r"-4cf56e90", ""));
uv_snapshot!(filters, command(&context)
.arg("a-4cf56e90[all]")
, @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 3 packages in [TIME]
Downloaded 3 packages in [TIME]
Installed 3 packages in [TIME]
+ albatross==1.0.0
+ bluebird==1.0.0
+ crow==1.0.0
"###);
assert_installed(&context.venv, "a_4cf56e90", "1.0.0", &context.temp_dir);
assert_installed(&context.venv, "b_4cf56e90", "1.0.0", &context.temp_dir);
assert_installed(&context.venv, "c_4cf56e90", "1.0.0", &context.temp_dir);
}
/// extra-incompatible-with-extra /// extra-incompatible-with-extra
/// ///
/// Multiple optional dependencies are requested for the package, but they have /// Multiple optional dependencies are requested for the package, but they have

View file

@ -45,7 +45,7 @@ import textwrap
from pathlib import Path from pathlib import Path
PACKSE_COMMIT = "de58b3e3f998486b6c0f3dd67b7341c880eb54b2" PACKSE_COMMIT = "64b4451b832cece378f6e773d326ea09efe8903d"
TOOL_ROOT = Path(__file__).parent TOOL_ROOT = Path(__file__).parent
TEMPLATES = TOOL_ROOT / "templates" TEMPLATES = TOOL_ROOT / "templates"
INSTALL_TEMPLATE = TEMPLATES / "install.mustache" INSTALL_TEMPLATE = TEMPLATES / "install.mustache"