diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index 0216836d77..752116cbeb 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -11,12 +11,12 @@ use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use uv_configuration::ExtrasSpecificationWithDefaults; use uv_configuration::{BuildOptions, DependencyGroupsWithDefaults, InstallOptions}; -use uv_distribution_types::{Edge, Node, Resolution, ResolvedDist}; +use uv_distribution_types::{Edge, Node, Requirement, Resolution, ResolvedDist}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_platform_tags::Tags; use uv_pypi_types::ResolverMarkerEnvironment; -use crate::lock::{HashedDist, LockErrorKind, Package, TagPolicy}; +use crate::lock::{Dependency, HashedDist, LockErrorKind, Package, TagPolicy}; use crate::{Lock, LockError}; fn newly_activated_extras<'lock>( @@ -32,6 +32,31 @@ fn newly_activated_extras<'lock>( .collect() } +fn activated_requirement_extras<'lock>( + package: &'lock PackageName, + requirement: &'lock Requirement, + marker_env: &ResolverMarkerEnvironment, + activated_extras: &[(&'lock PackageName, &'lock ExtraName)], + next_activated_extras: &[(&'lock PackageName, &'lock ExtraName)], +) -> Option> { + let mut package_extras = activated_extras + .iter() + .filter_map(|(candidate, extra)| (*candidate == package).then_some((*extra).clone())) + .collect::>(); + if requirement.name == *package { + package_extras.extend(requirement.extras.iter().cloned()); + } + requirement + .evaluate_markers(Some(marker_env), &package_extras) + .then(|| { + newly_activated_extras( + &requirement.name, + requirement.extras.iter(), + next_activated_extras, + ) + }) +} + pub trait Installable<'lock> { /// Return the root install path. fn install_path(&self) -> &'lock Path; @@ -77,12 +102,6 @@ pub trait Installable<'lock> { let root = petgraph.add_node(Node::Root); // Determine the set of activated extras and groups, from the root. - // - // TODO(charlie): This isn't quite right. Below, when we add the dependency groups to the - // graph, we rely on the activated extras and dependency groups, to evaluate the conflict - // marker. But at that point, we don't know the full set of activated extras; this is only - // computed below. We somehow need to add the dependency groups _after_ we've computed all - // enabled extras, but the groups themselves could depend on the set of enabled extras. if needs_activation_context { for root_name in self.roots() { let dist = self @@ -116,6 +135,8 @@ pub trait Installable<'lock> { // Initialize the workspace roots. let mut roots = vec![]; + let mut group_dependencies: Vec<(_, &GroupName, &Dependency)> = vec![]; + let mut group_requirements: Vec<(&PackageName, &Requirement)> = vec![]; for root_name in self.roots() { let dist = self .lock() @@ -153,6 +174,16 @@ pub trait Installable<'lock> { } // Add any dev dependencies. + group_requirements.extend( + dist.dependency_groups() + .iter() + .filter(|(group, _)| groups.contains(group)) + .flat_map(|(_, requirements)| { + requirements + .iter() + .map(|requirement| (&dist.id.name, requirement)) + }), + ); for (group, dep) in dist .dependency_groups .iter() @@ -165,77 +196,7 @@ pub trait Installable<'lock> { }) .flatten() { - let additional_activated_extras = newly_activated_extras( - &dep.package_id.name, - dep.extra.iter(), - &activated_extras, - ); - if !dep.complexified_marker.evaluate( - marker_env, - activated_projects.iter().copied(), - activated_extras - .iter() - .chain(additional_activated_extras.iter()) - .copied(), - activated_groups.iter().copied(), - ) { - continue; - } - activated_extras.extend(additional_activated_extras); - - let dep_dist = self.lock().find_by_id(&dep.package_id); - - // Add the package to the graph. - let dep_index = match inverse.entry(&dep.package_id) { - Entry::Vacant(entry) => { - let index = petgraph.add_node(self.package_to_node( - dep_dist, - tags, - build_options, - install_options, - marker_env, - )?); - entry.insert(index); - index - } - Entry::Occupied(entry) => { - // Critically, if the package is already in the graph, then it's a workspace - // member. If it was omitted due to, e.g., `--only-dev`, but is itself - // referenced as a development dependency, then we need to re-enable it. - let index = *entry.get(); - let node = &mut petgraph[index]; - if !groups.prod() { - *node = self.package_to_node( - dep_dist, - tags, - build_options, - install_options, - marker_env, - )?; - } - index - } - }; - - petgraph.add_edge( - index, - dep_index, - // This is OK because we are resolving to a resolution for - // a specific marker environment and set of extras/groups. - // So at this point, we know the extras/groups have been - // satisfied, so we can safely drop the conflict marker. - Edge::Dev(group.clone()), - ); - - // Push its dependencies on the queue. - if seen.insert((&dep.package_id, None)) { - queue.push_back((dep_dist, None)); - } - for extra in &dep.extra { - if seen.insert((&dep.package_id, Some(extra))) { - queue.push_back((dep_dist, Some(extra))); - } - } + group_dependencies.push((index, group, dep)); } } @@ -405,6 +366,47 @@ pub trait Installable<'lock> { let mut next_activated_extras = root_activated_extras.clone(); let mut queue = queue.clone(); let mut seen = seen.clone(); + for (package, requirement) in &group_requirements { + let Some(additional_activated_extras) = activated_requirement_extras( + package, + requirement, + marker_env, + &activated_extras, + &next_activated_extras, + ) else { + continue; + }; + next_activated_extras.extend(additional_activated_extras); + } + for (_, _, dep) in &group_dependencies { + let additional_activated_extras = newly_activated_extras( + &dep.package_id.name, + dep.extra.iter(), + &next_activated_extras, + ); + if !dep.complexified_marker.evaluate( + marker_env, + activated_projects.iter().copied(), + activated_extras + .iter() + .chain(additional_activated_extras.iter()) + .copied(), + activated_groups.iter().copied(), + ) { + continue; + } + next_activated_extras.extend(additional_activated_extras); + + let dep_dist = self.lock().find_by_id(&dep.package_id); + if seen.insert((&dep.package_id, None)) { + queue.push_back((dep_dist, None)); + } + for extra in &dep.extra { + if seen.insert((&dep.package_id, Some(extra))) { + queue.push_back((dep_dist, Some(extra))); + } + } + } while let Some((package, extra)) = queue.pop_front() { let deps = if let Some(extra) = extra { Either::Left( @@ -463,6 +465,66 @@ pub trait Installable<'lock> { } } + for (index, group, dep) in group_dependencies { + let additional_activated_extras = + newly_activated_extras(&dep.package_id.name, dep.extra.iter(), &activated_extras); + if !dep.complexified_marker.evaluate( + marker_env, + activated_projects.iter().copied(), + activated_extras + .iter() + .chain(additional_activated_extras.iter()) + .copied(), + activated_groups.iter().copied(), + ) { + continue; + } + activated_extras.extend(additional_activated_extras); + + let dep_dist = self.lock().find_by_id(&dep.package_id); + let dep_index = match inverse.entry(&dep.package_id) { + Entry::Vacant(entry) => { + let dep_index = petgraph.add_node(self.package_to_node( + dep_dist, + tags, + build_options, + install_options, + marker_env, + )?); + entry.insert(dep_index); + dep_index + } + Entry::Occupied(entry) => { + // Critically, if the package is already in the graph, then it's a workspace + // member. If it was omitted due to, e.g., `--only-dev`, but is itself + // referenced as a development dependency, then we need to re-enable it. + let dep_index = *entry.get(); + let node = &mut petgraph[dep_index]; + if !groups.prod() { + *node = self.package_to_node( + dep_dist, + tags, + build_options, + install_options, + marker_env, + )?; + } + dep_index + } + }; + + petgraph.add_edge(index, dep_index, Edge::Dev(group.clone())); + + if seen.insert((&dep.package_id, None)) { + queue.push_back((dep_dist, None)); + } + for extra in &dep.extra { + if seen.insert((&dep.package_id, Some(extra))) { + queue.push_back((dep_dist, Some(extra))); + } + } + } + if !self.lock().conflicts().is_empty() { let activated_extras_set: BTreeSet<(&PackageName, &ExtraName)> = activated_extras.iter().copied().collect(); diff --git a/crates/uv/tests/it/sync.rs b/crates/uv/tests/it/sync.rs index 3278c7b47f..c410e025b4 100644 --- a/crates/uv/tests/it/sync.rs +++ b/crates/uv/tests/it/sync.rs @@ -10544,6 +10544,51 @@ fn sync_multiple_sources_group_activates_extra_url() -> Result<()> { Ok(()) } +/// Group dependencies must be evaluated after extras activated by the same +/// group have stabilized. +#[test] +fn sync_multiple_sources_group_extra_excludes_url() -> Result<()> { + let context = uv_test::test_context!("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str( + r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["iniconfig>=2"] + + [project.optional-dependencies] + foo = [] + + [dependency-groups] + use = ["project[foo]", "iniconfig"] + + [tool.uv.sources] + iniconfig = [ + { url = "https://files.pythonhosted.org/packages/9b/dd/b3c12c6d707058fa947864b67f0c4e0c39ef8610988d7baea9578f3c48f3/iniconfig-1.1.1-py2.py3-none-any.whl", group = "use", marker = "extra != 'foo'" }, + ] + "#, + )?; + + context.lock().assert().success(); + + uv_snapshot!(context.filters(), context.sync().arg("--group").arg("use"), @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==2.0.0 + "); + + Ok(()) +} + #[test] fn sync_script_manifest_extra_selects_url_source() -> Result<()> { let context = uv_test::test_context!("3.12").with_filtered_counts();