Skip to content

Commit

Permalink
uv-resolver: error on unconditional conflicting extras
Browse files Browse the repository at this point in the history
If we don't do this, then it would be like permitting
`uv sync --extra x1 --extra x2` even when `x1` and `x2` are declared as
conflicting.

Technically, we should only report an error when 2 or more conflicting
extras are unconditionally enabled. Instead, here, we report an error
if just 1 is found. The reason is that it seems tricky to detect all
possible cases of 2 or more since I believe it would require looking at
the full dependency tree.
  • Loading branch information
BurntSushi committed Nov 13, 2024
1 parent ed12f93 commit f49d6b4
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 1 deletion.
9 changes: 8 additions & 1 deletion crates/uv-resolver/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use tracing::trace;
use uv_distribution_types::{
BuiltDist, IndexCapabilities, IndexLocations, IndexUrl, InstalledDist, SourceDist,
};
use uv_normalize::PackageName;
use uv_normalize::{ExtraName, PackageName};
use uv_pep440::{LocalVersionSlice, Version};
use uv_static::EnvVars;

Expand Down Expand Up @@ -41,6 +41,13 @@ pub enum ResolveError {
#[error("Attempted to wait on an unregistered task: `{_0}`")]
UnregisteredTask(String),

#[error("Found conflicting extra `{extra}` unconditionally enabled in `{requirement}`")]
ConflictingExtra {
// Boxed because `Requirement` is large.
requirement: Box<uv_pypi_types::Requirement>,
extra: ExtraName,
},

#[error("Overrides contain conflicting URLs for package `{0}`:\n- {1}\n- {2}")]
ConflictingOverrideUrls(PackageName, String, String),

Expand Down
45 changes: 45 additions & 0 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1333,6 +1333,18 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
}
};

if let Some(err) =
find_conflicting_extra(&self.conflicting_groups, &metadata.requires_dist)
{
return Err(err);
}
for dependencies in metadata.dependency_groups.values() {
if let Some(err) =
find_conflicting_extra(&self.conflicting_groups, dependencies)
{
return Err(err);
}
}
let requirements = self.flatten_requirements(
&metadata.requires_dist,
&metadata.dependency_groups,
Expand Down Expand Up @@ -3075,3 +3087,36 @@ impl PartialOrd for Fork {
Some(self.cmp(other))
}
}

/// Returns an error if a conflicting extra is found in the given requirements.
///
/// Specifically, if there is any conflicting extra (just one is enough) that
/// is unconditionally enabled as part of a dependency specification, then this
/// returns an error.
///
/// The reason why we're so conservative here is because it avoids us needing
/// the look at the entire dependency tree at once.
///
/// For example, consider packages `root`, `a`, `b` and `c`, where `c` has
/// declared conflicting extras of `x1` and `x2`.
///
/// Now imagine `root` depends on `a` and `b`, `a` depends on `c[x1]` and `b`
/// depends on `c[x2]`. That's a conflict, but not easily detectable unless
/// you reject either `c[x1]` or `c[x2]` on the grounds that `x1` and `x2` are
/// conflicting and thus cannot be enabled unconditionally.
fn find_conflicting_extra(
conflicting: &ConflictingGroupList,
reqs: &[Requirement],
) -> Option<ResolveError> {
for req in reqs {
for extra in &req.extras {
if conflicting.contains(&req.name, extra) {
return Some(ResolveError::ConflictingExtra {
requirement: Box::new(req.clone()),
extra: extra.clone(),
});
}
}
}
None
}
136 changes: 136 additions & 0 deletions crates/uv/tests/it/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3243,6 +3243,142 @@ fn lock_conflicting_extra_config_change_ignore_lockfile() -> Result<()> {
Ok(())
}

/// This tests that we report an error when a requirement unconditionally
/// enables a conflicting extra.
#[test]
fn lock_conflicting_extra_unconditional() -> Result<()> {
let context = TestContext::new("3.12");

let root_pyproject_toml = context.temp_dir.child("pyproject.toml");
root_pyproject_toml.write_str(
r#"
[project]
name = "dummy"
version = "0.1.0"
requires-python = "==3.12.*"
dependencies = [
"proxy1[project1,project2]"
]

[tool.uv]
conflicting-groups = [
[
{ package = "proxy1", extra = "project1" },
{ package = "proxy1", extra = "project2" },
],
]

[tool.uv.sources]
proxy1 = { path = "./proxy1" }

[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"

"#,
)?;

let proxy1_pyproject_toml = context.temp_dir.child("proxy1").child("pyproject.toml");
proxy1_pyproject_toml.write_str(
r#"
[project]
name = "proxy1"
version = "0.1.0"
requires-python = "==3.12.*"
dependencies = []

[project.optional-dependencies]
project1 = ["anyio==4.1.0"]
project2 = ["anyio==4.2.0"]
"#,
)?;

uv_snapshot!(context.filters(), context.lock(), @r###"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: Found conflicting extra `project1` unconditionally enabled in `proxy1[project1,project2] @ file://[TEMP_DIR]/proxy1`
"###);

// An error should occur even when only one conflicting extra is enabled.
root_pyproject_toml.write_str(
r#"
[project]
name = "dummy"
version = "0.1.0"
requires-python = "==3.12.*"
dependencies = [
"proxy1[project1]"
]

[tool.uv]
conflicting-groups = [
[
{ package = "proxy1", extra = "project1" },
{ package = "proxy1", extra = "project2" },
],
]

[tool.uv.sources]
proxy1 = { path = "./proxy1" }

[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"

"#,
)?;
uv_snapshot!(context.filters(), context.lock(), @r###"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: Found conflicting extra `project1` unconditionally enabled in `proxy1[project1] @ file://[TEMP_DIR]/proxy1`
"###);

// And same thing for the other extra.
root_pyproject_toml.write_str(
r#"
[project]
name = "dummy"
version = "0.1.0"
requires-python = "==3.12.*"
dependencies = [
"proxy1[project2]"
]

[tool.uv]
conflicting-groups = [
[
{ package = "proxy1", extra = "project1" },
{ package = "proxy1", extra = "project2" },
],
]

[tool.uv.sources]
proxy1 = { path = "./proxy1" }

[build-system]
requires = ["hatchling"]
build-backend = "hatchling.build"

"#,
)?;
uv_snapshot!(context.filters(), context.lock(), @r###"
success: false
exit_code: 2
----- stdout -----

----- stderr -----
error: Found conflicting extra `project2` unconditionally enabled in `proxy1[project2] @ file://[TEMP_DIR]/proxy1`
"###);

Ok(())
}

/// Show updated dependencies on `lock --upgrade`.
#[test]
fn lock_upgrade_log() -> Result<()> {
Expand Down

0 comments on commit f49d6b4

Please sign in to comment.