diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index ef1a1689afae3..fc84e0e74d250 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -12,7 +12,7 @@ use tracing::trace; use uv_distribution_types::{ DerivationChain, DistErrorKind, IndexCapabilities, IndexLocations, IndexUrl, RequestedDist, }; -use uv_normalize::{ExtraName, PackageName}; +use uv_normalize::PackageName; use uv_pep440::{LocalVersionSlice, Version}; use uv_platform_tags::Tags; use uv_static::EnvVars; @@ -48,13 +48,6 @@ 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, - extra: ExtraName, - }, - #[error( "Requirements contain conflicting URLs for package `{package_name}`{}:\n- {}", if env.marker_environment().is_some() { diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index 97b59f2d0b776..6d1c6bbf71680 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -1,8 +1,11 @@ +use std::borrow::Cow; use std::collections::hash_map::Entry; +use std::collections::BTreeSet; use std::collections::VecDeque; use std::path::Path; use either::Either; +use itertools::Itertools; use petgraph::Graph; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; @@ -264,6 +267,8 @@ pub trait Installable<'lock> { } } + let mut all_activated_extras: BTreeSet<(&PackageName, &ExtraName)> = + activated_extras.iter().copied().collect(); while let Some((package, extra)) = queue.pop_front() { let deps = if let Some(extra) = extra { Either::Left( @@ -277,6 +282,15 @@ pub trait Installable<'lock> { Either::Right(package.dependencies.iter()) }; for dep in deps { + let mut activated_extras = Cow::Borrowed(&*activated_extras); + if !dep.extra.is_empty() { + let mut extended = activated_extras.to_vec(); + for extra in &dep.extra { + extended.push((&dep.package_id.name, extra)); + all_activated_extras.insert((&dep.package_id.name, extra)); + } + activated_extras = Cow::Owned(extended); + } if !dep.complexified_marker.evaluate( marker_env, &activated_extras, @@ -329,6 +343,32 @@ pub trait Installable<'lock> { } } + // At time of writing, it's somewhat expected that the set of + // conflicting extras is pretty small. With that said, the + // time complexity of the following routine is pretty gross. + // Namely, `set.contains` is linear in the size of the set, + // iteration over all conflicts is also obviously linear in + // the number of conflicting sets and then for each of those, + // we visit every possible pair of activated extra from above, + // which is quadratic in the total number of extras enabled. I + // believe the simplest improvement here, if it's necessary, is + // to adjust the `Conflicts` internals to own these sorts of + // checks. ---AG + for set in self.lock().conflicts().iter() { + for ((pkg1, extra1), (pkg2, extra2)) in all_activated_extras.iter().tuple_combinations() + { + if set.contains(pkg1, *extra1) && set.contains(pkg2, *extra2) { + return Err(LockErrorKind::ConflictingExtra { + package1: (*pkg1).clone(), + extra1: (*extra1).clone(), + package2: (*pkg2).clone(), + extra2: (*extra2).clone(), + } + .into()); + } + } + } + Ok(Resolution::new(petgraph)) } diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index cd40d920ff305..fa58050496307 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -4903,6 +4903,16 @@ enum LockErrorKind { #[source] err: uv_distribution::Error, }, + #[error( + "Found conflicting extras `{package1}[{extra1}]` \ + and `{package2}[{extra2}]` enabled simultaneously" + )] + ConflictingExtra { + package1: PackageName, + extra1: ExtraName, + package2: PackageName, + extra2: ExtraName, + }, } /// An error that occurs when a source string could not be parsed. diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index d5f424e72efbf..d479ec9fb2250 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -1682,16 +1682,6 @@ impl ResolverState, } -/// 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: &Conflicts, reqs: &[Requirement]) -> Option { - 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 -} - #[derive(Debug, Default, Clone)] struct ConflictTracker { /// How often a decision on the package was discarded due to another package decided earlier. diff --git a/crates/uv/tests/it/lock_conflict.rs b/crates/uv/tests/it/lock_conflict.rs index c69d8cb9876f5..a0143635f2cdb 100644 --- a/crates/uv/tests/it/lock_conflict.rs +++ b/crates/uv/tests/it/lock_conflict.rs @@ -1016,10 +1016,6 @@ fn extra_unconditional() -> Result<()> { [tool.uv.sources] proxy1 = { workspace = true } - - [build-system] - requires = ["hatchling"] - build-backend = "hatchling.build" "#, )?; @@ -1047,15 +1043,23 @@ fn extra_unconditional() -> Result<()> { )?; uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 6 packages in [TIME] + "###); + // This should error since we're enabling two conflicting extras. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" success: false exit_code: 2 ----- stdout ----- ----- stderr ----- - error: Found conflicting extra `extra1` unconditionally enabled in `proxy1[extra1,extra2] @ file://[TEMP_DIR]/proxy1` + error: Found conflicting extras `proxy1[extra1]` and `proxy1[extra2]` enabled simultaneously "###); - // An error should occur even when only one conflicting extra is enabled. root_pyproject_toml.write_str( r#" [project] @@ -1071,19 +1075,29 @@ fn extra_unconditional() -> Result<()> { [tool.uv.sources] proxy1 = { workspace = true } - - [build-system] - requires = ["hatchling"] - build-backend = "hatchling.build" "#, )?; uv_snapshot!(context.filters(), context.lock(), @r###" - success: false - exit_code: 2 + success: true + exit_code: 0 ----- stdout ----- ----- stderr ----- - error: Found conflicting extra `extra1` unconditionally enabled in `proxy1[extra1] @ file://[TEMP_DIR]/proxy1` + Resolved 6 packages in [TIME] + "###); + // This is fine because we are only enabling one + // extra, and thus, there is no conflict. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==4.1.0 + + idna==3.6 + + sniffio==1.3.1 "###); // And same thing for the other extra. @@ -1102,20 +1116,324 @@ fn extra_unconditional() -> Result<()> { [tool.uv.sources] proxy1 = { workspace = true } + "#, + )?; + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- - [build-system] - requires = ["hatchling"] - build-backend = "hatchling.build" + ----- stderr ----- + Resolved 6 packages in [TIME] + "###); + // This is fine because we are only enabling one + // extra, and thus, there is no conflict. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 1 package in [TIME] + Uninstalled 1 package in [TIME] + Installed 1 package in [TIME] + - anyio==4.1.0 + + anyio==4.2.0 + "###); + + Ok(()) +} + +/// A regression tests for unconditionally installing an extra that is +/// marked as conflicting. Previously, the `proxy1[extra1]` dependency +/// would be completely ignored here. +#[test] +fn extra_unconditional_non_conflicting() -> 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[extra1]" + ] + + [tool.uv.workspace] + members = ["proxy1"] + [tool.uv.sources] + proxy1 = { workspace = true } "#, )?; + + 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] + extra1 = ["anyio==4.1.0"] + extra2 = [] + extra3 = [] + + [tool.uv] + conflicts = [ + [ + { extra = "extra1" }, + { extra = "extra3" }, + ], + ] + "#, + )?; + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 5 packages in [TIME] + "###); + + // This *should* install `anyio==4.1.0`, but when this + // test was initially written, it didn't. This was because + // `uv sync` wasn't correctly propagating extras in a way + // that would satisfy the conflict markers that got added + // to the `proxy1[extra1]` dependency. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==4.1.0 + + idna==3.6 + + sniffio==1.3.1 + "###); + + Ok(()) +} + +#[test] +fn extra_unconditional_in_optional() -> 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 = "foo" + version = "0.1.0" + description = "Add your description here" + readme = "README.md" + requires-python = ">=3.10.0" + dependencies = [] + + [tool.uv.workspace] + members = ["proxy1"] + + [tool.uv.sources] + proxy1 = { workspace = true } + + [project.optional-dependencies] + x1 = ["proxy1[nested-x1]"] + x2 = ["proxy1[nested-x2]"] + "#, + )?; + + 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.10.0" + dependencies = [] + + [project.optional-dependencies] + nested-x1 = ["sortedcontainers==2.3.0"] + nested-x2 = ["sortedcontainers==2.4.0"] + + [tool.uv] + conflicts = [ + [ + { extra = "nested-x1" }, + { extra = "nested-x2" }, + ], + ] + "#, + )?; + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 4 packages in [TIME] + "###); + + // This shouldn't install anything. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Audited in [TIME] + "###); + + // This should install `sortedcontainers==2.3.0`. + uv_snapshot!(context.filters(), context.sync().arg("--frozen").arg("--extra=x1"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + sortedcontainers==2.3.0 + "###); + + // This should install `sortedcontainers==2.4.0`. + uv_snapshot!(context.filters(), context.sync().arg("--frozen").arg("--extra=x2"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Prepared 1 package in [TIME] + Uninstalled 1 package in [TIME] + Installed 1 package in [TIME] + - sortedcontainers==2.3.0 + + sortedcontainers==2.4.0 + "###); + + // This should error! + uv_snapshot!( + context.filters(), + context.sync().arg("--frozen").arg("--extra=x1").arg("--extra=x2"), + @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Found conflicting extras `proxy1[nested-x1]` and `proxy1[nested-x2]` enabled simultaneously + "###); + + Ok(()) +} + +#[test] +fn extra_unconditional_non_local_conflict() -> 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 = "foo" + version = "0.1.0" + description = "Add your description here" + readme = "README.md" + requires-python = ">=3.10.0" + dependencies = ["a", "b"] + + [tool.uv.workspace] + members = ["a", "b", "c"] + + [tool.uv.sources] + a = { workspace = true } + b = { workspace = true } + c = { workspace = true } + "#, + )?; + + let a_pyproject_toml = context.temp_dir.child("a").child("pyproject.toml"); + a_pyproject_toml.write_str( + r#" + [project] + name = "a" + version = "0.1.0" + requires-python = ">=3.10.0" + dependencies = ["c[x1]"] + + [tool.uv.sources] + c = { workspace = true } + "#, + )?; + + let b_pyproject_toml = context.temp_dir.child("b").child("pyproject.toml"); + b_pyproject_toml.write_str( + r#" + [project] + name = "b" + version = "0.1.0" + requires-python = ">=3.10.0" + dependencies = ["c[x2]"] + + [tool.uv.sources] + c = { workspace = true } + "#, + )?; + + let c_pyproject_toml = context.temp_dir.child("c").child("pyproject.toml"); + c_pyproject_toml.write_str( + r#" + [project] + name = "c" + version = "0.1.0" + requires-python = ">=3.10.0" + dependencies = [] + + [project.optional-dependencies] + x1 = ["sortedcontainers==2.3.0"] + x2 = ["sortedcontainers==2.4.0"] + + [tool.uv] + conflicts = [ + [ + { extra = "x1" }, + { extra = "x2" }, + ], + ] + "#, + )?; + + // Regrettably, this produces a lock file, and it is one + // that can never be installed! Namely, because two different + // conflicting extras are enabled unconditionally in all + // configurations. + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 6 packages in [TIME] + "###); + + // This should fail. If it doesn't and we generated a lock + // file above, then this will likely result in the installation + // of two different versions of the same package. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" success: false exit_code: 2 ----- stdout ----- ----- stderr ----- - error: Found conflicting extra `extra2` unconditionally enabled in `proxy1[extra2] @ file://[TEMP_DIR]/proxy1` + error: Found conflicting extras `c[x1]` and `c[x2]` enabled simultaneously "###); Ok(())