From d50d1ac2bd632146c3cf399f44d73873df0ed0dc Mon Sep 17 00:00:00 2001 From: Zsolt Dollenstein Date: Thu, 18 Jun 2026 10:00:44 +0000 Subject: [PATCH 1/4] Add locked subgraph materialization primitives --- crates/uv-resolver/src/lock/installable.rs | 663 +++++++++++++++--- crates/uv-resolver/src/lock/mod.rs | 6 + crates/uv/src/commands/project/environment.rs | 45 +- .../uv/src/commands/project/install_target.rs | 64 +- crates/uv/tests/sync/sync.rs | 11 + .../uv/tests/workspace/workspace_metadata.rs | 36 + 6 files changed, 736 insertions(+), 89 deletions(-) diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index 94ce42a3d35..f9deccc04c0 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -54,6 +54,122 @@ pub trait Installable<'lock> { groups: &DependencyGroupsWithDefaults, build_options: &BuildOptions, install_options: &InstallOptions, + ) -> Result { + let roots = self + .roots() + .map(|root_name| { + self.lock() + .find_by_name(root_name) + .map_err(|_| LockErrorKind::MultipleRootPackages { + name: root_name.clone(), + })? + .ok_or_else(|| { + LockError::from(LockErrorKind::MissingRootPackage { + name: root_name.clone(), + }) + }) + }) + .collect::, LockError>>()?; + + InstallableExt::to_resolution_from_packages( + self, + &roots, + true, + marker_env, + tags, + extras, + groups, + build_options, + install_options, + ) + } + + /// Create an installable [`Node`] from a [`Package`]. + fn installable_node( + &self, + package: &Package, + tags: &Tags, + marker_env: &ResolverMarkerEnvironment, + build_options: &BuildOptions, + ) -> Result { + let tag_policy = TagPolicy::Required(tags); + let HashedDist { dist, hashes } = + package.to_dist(self.install_path(), tag_policy, build_options, marker_env)?; + let version = package.version().cloned(); + let dist = ResolvedDist::Installable { + dist: Arc::new(dist), + version, + }; + Ok(Node::Dist { + dist, + hashes, + install: true, + }) + } + + /// Create a non-installable [`Node`] from a [`Package`]. + fn non_installable_node( + &self, + package: &Package, + tags: &Tags, + marker_env: &ResolverMarkerEnvironment, + ) -> Result { + let HashedDist { dist, .. } = package.to_dist( + self.install_path(), + TagPolicy::Preferred(tags), + &BuildOptions::default(), + marker_env, + )?; + let version = package.version().cloned(); + let dist = ResolvedDist::Installable { + dist: Arc::new(dist), + version, + }; + let hashes = package.hashes(); + Ok(Node::Dist { + dist, + hashes, + install: false, + }) + } + + /// Convert a lockfile entry to a graph [`Node`]. + fn package_to_node( + &self, + package: &Package, + tags: &Tags, + build_options: &BuildOptions, + install_options: &InstallOptions, + marker_env: &ResolverMarkerEnvironment, + ) -> Result { + if install_options.include_package( + package.as_install_target(), + self.project_name(), + self.lock().members(), + ) { + self.installable_node(package, tags, marker_env, build_options) + } else { + self.non_installable_node(package, tags, marker_env) + } + } +} + +/// Internal lock-to-resolution implementation shared by [`Installable`] and [`Lock`]. +trait InstallableExt<'lock>: Installable<'lock> { + /// Convert concrete locked packages to a [`Resolution`]. + /// + /// `include_manifest` controls whether requirements attached directly to the lock target are + /// included in addition to `roots`. + fn to_resolution_from_packages( + &self, + roots: &[&Package], + include_manifest: bool, + marker_env: &ResolverMarkerEnvironment, + tags: &Tags, + extras: &ExtrasSpecificationWithDefaults, + groups: &DependencyGroupsWithDefaults, + build_options: &BuildOptions, + install_options: &InstallOptions, ) -> Result { let size_guess = self.lock().packages.len(); let mut petgraph = Graph::with_capacity(size_guess, size_guess); @@ -75,17 +191,7 @@ pub trait Installable<'lock> { // 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 !self.lock().conflicts().is_empty() { - for root_name in self.roots() { - let dist = self - .lock() - .find_by_name(root_name) - .map_err(|_| LockErrorKind::MultipleRootPackages { - name: root_name.clone(), - })? - .ok_or_else(|| LockErrorKind::MissingRootPackage { - name: root_name.clone(), - })?; - + for dist in roots.iter().copied() { // Track the activated extras. if groups.prod() { activated_projects.push(&dist.id.name); @@ -106,18 +212,8 @@ pub trait Installable<'lock> { } // Initialize the workspace roots. - let mut roots = vec![]; - for root_name in self.roots() { - let dist = self - .lock() - .find_by_name(root_name) - .map_err(|_| LockErrorKind::MultipleRootPackages { - name: root_name.clone(), - })? - .ok_or_else(|| LockErrorKind::MissingRootPackage { - name: root_name.clone(), - })?; - + let mut initialized_roots = vec![]; + for dist in roots.iter().copied() { // Add the workspace package to the graph. let index = petgraph.add_node(if groups.prod() { self.package_to_node(dist, tags, build_options, install_options, marker_env)? @@ -130,11 +226,11 @@ pub trait Installable<'lock> { petgraph.add_edge(root, index, Edge::Prod); // Push the package onto the queue. - roots.push((dist, index)); + initialized_roots.push((dist, index)); } // Add the workspace dependencies to the queue. - for (dist, index) in roots { + for (dist, index) in initialized_roots { if groups.prod() { // Push its dependencies onto the queue. queue.push_back((dist, None)); @@ -227,7 +323,12 @@ pub trait Installable<'lock> { // Add any requirements that are exclusive to the workspace root (e.g., dependencies in // PEP 723 scripts). - for dependency in self.lock().requirements() { + for dependency in self + .lock() + .requirements() + .iter() + .filter(|_| include_manifest) + { if !dependency.marker.evaluate(marker_env, &[]) { continue; } @@ -271,6 +372,7 @@ pub trait Installable<'lock> { .lock() .dependency_groups() .iter() + .filter(|_| include_manifest) .filter_map(|(group, deps)| { if groups.contains(group) { Some(deps.iter().map(move |dep| (group, dep))) @@ -530,73 +632,468 @@ pub trait Installable<'lock> { Ok(Resolution::new(petgraph)) } +} - /// Create an installable [`Node`] from a [`Package`]. - fn installable_node( - &self, - package: &Package, - tags: &Tags, +impl<'lock, T> InstallableExt<'lock> for T where T: Installable<'lock> + ?Sized {} + +/// An [`Installable`] adapter for materializing concrete packages directly from a [`Lock`]. +struct LockedPackages<'lock> { + lock: &'lock Lock, + install_path: &'lock Path, + project_name: Option<&'lock PackageName>, +} + +impl<'lock> Installable<'lock> for LockedPackages<'lock> { + fn install_path(&self) -> &'lock Path { + self.install_path + } + + fn lock(&self) -> &'lock Lock { + self.lock + } + + fn roots(&self) -> impl Iterator { + std::iter::empty() + } + + fn project_name(&self) -> Option<&PackageName> { + self.project_name + } +} + +impl Lock { + /// Materialize the exact dependency subgraph reachable from concrete locked `roots`. + /// + /// Each root must be a [`Package`] from this lock. Unlike [`Installable::to_resolution`], this + /// method does not include requirements or dependency groups attached directly to the lock + /// manifest. Extras and dependency groups on the concrete roots are still included according + /// to `extras` and `groups`. `project_name` identifies the project for project-specific + /// [`InstallOptions`] filters, if applicable. Callers are responsible for selecting roots that + /// apply to `marker_env`. + pub fn to_resolution<'lock>( + &'lock self, + install_path: &'lock Path, + roots: impl IntoIterator, + project_name: Option<&'lock PackageName>, marker_env: &ResolverMarkerEnvironment, + tags: &Tags, + extras: &ExtrasSpecificationWithDefaults, + groups: &DependencyGroupsWithDefaults, build_options: &BuildOptions, - ) -> Result { - let tag_policy = TagPolicy::Required(tags); - let HashedDist { dist, hashes } = - package.to_dist(self.install_path(), tag_policy, build_options, marker_env)?; - let version = package.version().cloned(); - let dist = ResolvedDist::Installable { - dist: Arc::new(dist), - version, - }; - Ok(Node::Dist { - dist, - hashes, - install: true, + install_options: &InstallOptions, + ) -> Result { + let mut seen = FxHashSet::default(); + let mut concrete_roots = Vec::new(); + for root in roots { + let Some(index) = self.by_id.get(&root.id) else { + return Err(LockErrorKind::RootPackageMissingFromLock { + id: root.id.clone(), + } + .into()); + }; + if seen.insert(&root.id) { + let Some(root) = self.packages.get(*index) else { + return Err(LockErrorKind::RootPackageMissingFromLock { + id: root.id.clone(), + } + .into()); + }; + concrete_roots.push(root); + } + } + + LockedPackages { + lock: self, + install_path, + project_name, + } + .to_resolution_from_packages( + &concrete_roots, + false, + marker_env, + tags, + extras, + groups, + build_options, + install_options, + ) + } +} + +#[cfg(test)] +mod tests { + use std::cell::Cell; + use std::sync::LazyLock; + + use petgraph::visit::EdgeRef; + use uv_configuration::{DependencyGroups, ExtrasSpecification}; + use uv_distribution_types::Name; + use uv_normalize::{DefaultExtras, DefaultGroups}; + use uv_pep508::{MarkerEnvironment, MarkerEnvironmentBuilder}; + use uv_platform_tags::{Arch, Os, Platform, TagsOptions}; + + use super::*; + + static TAGS: LazyLock = LazyLock::new(|| { + Tags::from_env( + &Platform::new( + Os::Macos { + major: 14, + minor: 0, + }, + Arch::Aarch64, + ), + (3, 11), + "cpython", + (3, 11), + TagsOptions::default(), + ) + .expect("valid tags") + }); + + static DARWIN_MARKERS: LazyLock = + LazyLock::new(|| ResolverMarkerEnvironment::from(marker_environment("darwin", "Darwin"))); + + static LINUX_MARKERS: LazyLock = + LazyLock::new(|| ResolverMarkerEnvironment::from(marker_environment("linux", "Linux"))); + + fn marker_environment( + sys_platform: &'static str, + platform_system: &'static str, + ) -> MarkerEnvironment { + MarkerEnvironment::try_from(MarkerEnvironmentBuilder { + implementation_name: "cpython", + implementation_version: "3.11.5", + os_name: "posix", + platform_machine: "arm64", + platform_python_implementation: "CPython", + platform_release: "23.0.0", + platform_system, + platform_version: "test", + python_full_version: "3.11.5", + python_version: "3.11", + sys_platform, }) + .expect("valid marker environment") } - /// Create a non-installable [`Node`] from a [`Package`]. - fn non_installable_node( - &self, - package: &Package, - tags: &Tags, + fn lock() -> Lock { + toml::from_str( + r#" +version = 1 +revision = 3 +requires-python = ">=3.11" +resolution-markers = [ + "sys_platform == 'darwin'", + "sys_platform != 'darwin'", +] + +[manifest] +requirements = [{ name = "unrelated" }] + +[[package]] +name = "dev-dependency" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/dev_dependency-1.0.0.tar.gz", hash = "sha256:1111111111111111111111111111111111111111111111111111111111111111" } + +[[package]] +name = "forked" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +resolution-markers = ["sys_platform == 'darwin'"] +sdist = { url = "https://example.com/forked-1.0.0.tar.gz", hash = "sha256:2222222222222222222222222222222222222222222222222222222222222222" } + +[[package]] +name = "forked" +version = "2.0.0" +source = { registry = "https://example.com/simple" } +resolution-markers = ["sys_platform != 'darwin'"] +sdist = { url = "https://example.com/forked-2.0.0.tar.gz", hash = "sha256:3333333333333333333333333333333333333333333333333333333333333333" } + +[[package]] +name = "optional-dependency" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/optional_dependency-1.0.0.tar.gz", hash = "sha256:4444444444444444444444444444444444444444444444444444444444444444" } + +[[package]] +name = "root-a" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +dependencies = [ + { name = "forked", version = "1.0.0", source = { registry = "https://example.com/simple" }, marker = "sys_platform == 'darwin'" }, + { name = "forked", version = "2.0.0", source = { registry = "https://example.com/simple" }, marker = "sys_platform != 'darwin'" }, + { name = "shared" }, +] +sdist = { url = "https://example.com/root_a-1.0.0.tar.gz", hash = "sha256:5555555555555555555555555555555555555555555555555555555555555555" } + +[package.optional-dependencies] +feature = [{ name = "optional-dependency" }] + +[package.dependency-groups] +dev = [{ name = "dev-dependency" }] + +[package.metadata] +provides-extras = ["feature"] + +[[package]] +name = "root-b" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +dependencies = [{ name = "shared" }] +sdist = { url = "https://example.com/root_b-1.0.0.tar.gz", hash = "sha256:6666666666666666666666666666666666666666666666666666666666666666" } + +[[package]] +name = "shared" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/shared-1.0.0.tar.gz", hash = "sha256:7777777777777777777777777777777777777777777777777777777777777777" } + +[[package]] +name = "unrelated" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/unrelated-1.0.0.tar.gz", hash = "sha256:8888888888888888888888888888888888888888888888888888888888888888" } +"#, + ) + .expect("valid lock") + } + + fn package<'lock>(lock: &'lock Lock, name: &str, version: &str) -> &'lock Package { + lock.packages() + .iter() + .find(|package| { + package.name().as_ref() == name + && package + .version() + .is_some_and(|package_version| package_version.to_string() == version) + }) + .expect("locked package") + } + + fn materialize( + lock: &Lock, + roots: &[&Package], marker_env: &ResolverMarkerEnvironment, - ) -> Result { - let HashedDist { dist, .. } = package.to_dist( - self.install_path(), - TagPolicy::Preferred(tags), - &BuildOptions::default(), + ) -> Resolution { + let extras = ExtrasSpecification::from_all_extras().with_defaults(DefaultExtras::default()); + let groups = DependencyGroups::from_all_groups().with_defaults(DefaultGroups::default()); + lock.to_resolution( + Path::new("."), + roots.iter().copied(), + None, marker_env, - )?; - let version = package.version().cloned(); - let dist = ResolvedDist::Installable { - dist: Arc::new(dist), - version, - }; - let hashes = package.hashes(); - Ok(Node::Dist { - dist, - hashes, - install: false, - }) + &TAGS, + &extras, + &groups, + &BuildOptions::default(), + &InstallOptions::default(), + ) + .expect("valid resolution") } - /// Convert a lockfile entry to a graph [`Node`]. - fn package_to_node( - &self, - package: &Package, - tags: &Tags, - build_options: &BuildOptions, - install_options: &InstallOptions, - marker_env: &ResolverMarkerEnvironment, - ) -> Result { - if install_options.include_package( - package.as_install_target(), - self.project_name(), - self.lock().members(), - ) { - self.installable_node(package, tags, marker_env, build_options) - } else { - self.non_installable_node(package, tags, marker_env) + struct OverridingInstallable<'lock> { + lock: &'lock Lock, + root_name: &'lock PackageName, + package_to_node_calls: Cell, + } + + impl<'lock> Installable<'lock> for OverridingInstallable<'lock> { + fn install_path(&self) -> &'lock Path { + Path::new(".") + } + + fn lock(&self) -> &'lock Lock { + self.lock } + + fn roots(&self) -> impl Iterator { + std::iter::once(self.root_name) + } + + fn project_name(&self) -> Option<&PackageName> { + None + } + + fn package_to_node( + &self, + _package: &Package, + _tags: &Tags, + _build_options: &BuildOptions, + _install_options: &InstallOptions, + _marker_env: &ResolverMarkerEnvironment, + ) -> Result { + self.package_to_node_calls + .set(self.package_to_node_calls.get() + 1); + Ok(Node::Root) + } + } + + fn graph_snapshot(resolution: &Resolution) -> (Vec, Vec) { + let graph = resolution.graph(); + let labels = graph + .node_weights() + .map(|node| match node { + Node::Root => "root".to_string(), + Node::Dist { + dist, + hashes, + install, + } => format!( + "{}=={} (install: {install}, hashes: {})", + dist.name(), + dist.version() + .map(ToString::to_string) + .unwrap_or_else(|| "".to_string()), + hashes.iter().map(ToString::to_string).join(", ") + ), + }) + .collect::>(); + let mut nodes = labels.clone(); + nodes.sort_unstable(); + let mut edges = graph + .edge_references() + .map(|edge| { + format!( + "{} --{:?}--> {}", + labels[edge.source().index()], + edge.weight(), + labels[edge.target().index()] + ) + }) + .collect::>(); + edges.sort_unstable(); + (nodes, edges) + } + + #[test] + fn materializes_multiple_concrete_roots_with_shared_dependencies() { + let lock = lock(); + let resolution = materialize( + &lock, + &[ + package(&lock, "root-a", "1.0.0"), + package(&lock, "root-b", "1.0.0"), + ], + &DARWIN_MARKERS, + ); + + insta::with_settings!({ + filters => [(r"sha256:[0-9a-f]{64}", "sha256:[HASH]")], + }, { + insta::assert_debug_snapshot!(graph_snapshot(&resolution), @r#" + ( + [ + "dev-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "forked==1.0.0 (install: true, hashes: sha256:[HASH])", + "optional-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-b==1.0.0 (install: true, hashes: sha256:[HASH])", + "shared==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + [ + "root --Prod--> root-a==1.0.0 (install: true, hashes: sha256:[HASH])", + "root --Prod--> root-b==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Dev(GroupName(\"dev\"))--> dev-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Optional(ExtraName(\"feature\"))--> optional-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> forked==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> shared==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-b==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> shared==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + ) + "#); + }); + } + + #[test] + fn materializes_the_selected_universal_lock_fork() { + let lock = lock(); + let root = package(&lock, "root-a", "1.0.0"); + let darwin = materialize(&lock, &[root], &DARWIN_MARKERS); + let linux = materialize(&lock, &[root], &LINUX_MARKERS); + let concrete_fork = + materialize(&lock, &[package(&lock, "forked", "1.0.0")], &DARWIN_MARKERS); + + insta::with_settings!({ + filters => [(r"sha256:[0-9a-f]{64}", "sha256:[HASH]")], + }, { + insta::assert_debug_snapshot!(graph_snapshot(&darwin), @r#" + ( + [ + "dev-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "forked==1.0.0 (install: true, hashes: sha256:[HASH])", + "optional-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH])", + "shared==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + [ + "root --Prod--> root-a==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Dev(GroupName(\"dev\"))--> dev-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Optional(ExtraName(\"feature\"))--> optional-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> forked==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> shared==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + ) + "#); + insta::assert_debug_snapshot!(graph_snapshot(&linux), @r#" + ( + [ + "dev-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "forked==2.0.0 (install: true, hashes: sha256:[HASH])", + "optional-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH])", + "shared==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + [ + "root --Prod--> root-a==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Dev(GroupName(\"dev\"))--> dev-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Optional(ExtraName(\"feature\"))--> optional-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> forked==2.0.0 (install: true, hashes: sha256:[HASH])", + "root-a==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> shared==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + ) + "#); + insta::assert_debug_snapshot!(graph_snapshot(&concrete_fork), @r#" + ( + [ + "forked==1.0.0 (install: true, hashes: sha256:[HASH])", + "root", + ], + [ + "root --Prod--> forked==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + ) + "#); + }); + } + + #[test] + fn installable_to_resolution_preserves_node_overrides() { + let mut lock = lock(); + lock.manifest.requirements.clear(); + let target = OverridingInstallable { + root_name: package(&lock, "root-a", "1.0.0").name(), + lock: &lock, + package_to_node_calls: Cell::new(0), + }; + let extras = ExtrasSpecification::from_all_extras().with_defaults(DefaultExtras::default()); + let groups = DependencyGroups::from_all_groups().with_defaults(DefaultGroups::default()); + + target + .to_resolution( + &DARWIN_MARKERS, + &TAGS, + &extras, + &groups, + &BuildOptions::default(), + &InstallOptions::default(), + ) + .expect("valid resolution"); + + assert!(target.package_to_node_calls.get() > 0); } } diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index 97a5c3c15a8..ce404918ed7 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -6611,6 +6611,12 @@ enum LockErrorKind { /// The ID of the package. name: PackageName, }, + /// An error that occurs when a concrete root package does not belong to the lock. + #[error("Could not find root package `{id}` in lock", id = id.cyan())] + RootPackageMissingFromLock { + /// The ID of the package. + id: PackageId, + }, /// An error that occurs when resolving metadata for a package. #[error("Failed to generate package metadata for `{id}`", id = id.cyan())] Resolution { diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index 72a6550c9ca..595a9229fa9 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -157,6 +157,47 @@ impl CachedEnvironment { .await?, ); + Self::from_resolution( + &resolution, + build_constraints, + &interpreter, + settings, + client_builder, + state, + install, + installer_metadata, + concurrency, + cache, + printer, + preview, + ) + .await + } + + /// Get or create a [`CachedEnvironment`] from an existing [`Resolution`]. + /// + /// Prefer [`Self::from_spec`] when starting from unresolved requirements; it selects the base + /// interpreter and resolves the requirements for that interpreter before delegating here. + /// + /// This method is intended for callers that already have a concrete [`Resolution`], and + /// performs environment reuse or creation and installation without invoking the resolver. + /// `interpreter` must be the base interpreter for which `resolution` was produced. In + /// particular, callers materializing a universal lock must derive its markers and tags from + /// the same interpreter. + pub(crate) async fn from_resolution( + resolution: &Resolution, + build_constraints: Constraints, + interpreter: &Interpreter, + settings: &ResolverInstallerSettings, + client_builder: &BaseClientBuilder<'_>, + state: &PlatformState, + install: Box, + installer_metadata: bool, + concurrency: &Concurrency, + cache: &Cache, + printer: Printer, + preview: Preview, + ) -> Result { // Hash the resolution by hashing the generated lockfile. let resolution_hash = { let mut distributions = resolution @@ -216,7 +257,7 @@ impl CachedEnvironment { let temp_dir = cache.venv_dir()?; let venv = uv_virtualenv::create_venv( temp_dir.path(), - interpreter, + interpreter.clone(), uv_virtualenv::Prompt::None, false, uv_virtualenv::OnExisting::Remove(uv_virtualenv::RemovalReason::TemporaryEnvironment), @@ -227,7 +268,7 @@ impl CachedEnvironment { sync_environment( venv, - &resolution, + resolution, Modifications::Exact, build_constraints, settings.into(), diff --git a/crates/uv/src/commands/project/install_target.rs b/crates/uv/src/commands/project/install_target.rs index 974c1b01e7e..8f7f8207b35 100644 --- a/crates/uv/src/commands/project/install_target.rs +++ b/crates/uv/src/commands/project/install_target.rs @@ -6,11 +6,17 @@ use std::str::FromStr; use itertools::Either; use rustc_hash::FxHashSet; -use uv_configuration::{Constraints, DependencyGroupsWithDefaults, ExtrasSpecification}; -use uv_distribution_types::Index; +use uv_configuration::{ + BuildOptions, Constraints, DependencyGroupsWithDefaults, ExtrasSpecification, + ExtrasSpecificationWithDefaults, InstallOptions, +}; +use uv_distribution_types::{Index, Resolution}; use uv_normalize::{ExtraName, PackageName}; -use uv_pypi_types::{DependencyGroupSpecifier, LenientRequirement, VerbatimParsedUrl}; -use uv_resolver::{Installable, Lock, Package}; +use uv_platform_tags::Tags; +use uv_pypi_types::{ + DependencyGroupSpecifier, LenientRequirement, ResolverMarkerEnvironment, VerbatimParsedUrl, +}; +use uv_resolver::{Installable, Lock, LockError, Package}; use uv_scripts::Pep723Script; use uv_workspace::Workspace; use uv_workspace::pyproject::{Source, Sources, ToolUvSources}; @@ -113,6 +119,56 @@ impl<'lock> Installable<'lock> for InstallTarget<'lock> { } impl<'lock> InstallTarget<'lock> { + /// Convert the target's locked packages to a [`Resolution`]. + pub(crate) fn to_resolution( + self, + marker_env: &ResolverMarkerEnvironment, + tags: &Tags, + extras: &ExtrasSpecificationWithDefaults, + groups: &DependencyGroupsWithDefaults, + build_options: &BuildOptions, + install_options: &InstallOptions, + ) -> Result { + // Project and workspace targets have package-backed roots, so materialize them through the + // concrete-root API. Non-project workspaces can also have package roots, but their root + // dependencies live on the lock manifest, so they need the generic path. Scripts and + // invalid or ambiguous roots also use that path. + let use_concrete_roots = match self { + Self::Project { workspace, .. } + | Self::Projects { workspace, .. } + | Self::Workspace { workspace, .. } => !workspace.is_non_project(), + Self::NonProjectWorkspace { .. } | Self::Script { .. } => false, + }; + if use_concrete_roots + && let Some(roots) = self + .roots() + .map(|root_name| self.lock().find_by_name(root_name).ok().flatten()) + .collect::>>() + { + return self.lock().to_resolution( + self.install_path(), + roots, + self.project_name(), + marker_env, + tags, + extras, + groups, + build_options, + install_options, + ); + } + + Installable::to_resolution( + &self, + marker_env, + tags, + extras, + groups, + build_options, + install_options, + ) + } + /// Return an iterator over the [`Index`] definitions in the target. pub(crate) fn indexes(self) -> impl Iterator { match self { diff --git a/crates/uv/tests/sync/sync.rs b/crates/uv/tests/sync/sync.rs index b72dfebaaa3..6095a5b9a2c 100644 --- a/crates/uv/tests/sync/sync.rs +++ b/crates/uv/tests/sync/sync.rs @@ -1353,6 +1353,17 @@ fn sync_non_project_dev_dependencies() -> Result<()> { + urllib3==2.2.1 "); + // Selecting a member still includes the non-project root's default dependency group. + uv_snapshot!(context.filters(), context.sync().arg("--package").arg("child"), @" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 11 packages in [TIME] + Checked 10 packages in [TIME] + "); + Ok(()) } diff --git a/crates/uv/tests/workspace/workspace_metadata.rs b/crates/uv/tests/workspace/workspace_metadata.rs index 2ba54aaf3de..8f933f0ed80 100644 --- a/crates/uv/tests/workspace/workspace_metadata.rs +++ b/crates/uv/tests/workspace/workspace_metadata.rs @@ -1724,6 +1724,42 @@ fn workspace_metadata_group_only() -> Result<()> { "# ); + // With `--sync`, modules provided by the non-project root's dependency group should be + // attributed to their locked package. + let assert = context + .workspace_metadata() + .arg("--sync") + .current_dir(&workspace) + .assert() + .success(); + let metadata: serde_json::Value = serde_json::from_slice(&assert.get_output().stdout)?; + let module_owners = serde_json::to_string_pretty(&metadata["module_owners"])?; + + insta::assert_snapshot!(module_owners, @r#" + { + "iniconfig": [ + { + "package_id": "iniconfig==2.0.0@registry+https://pypi.org/simple" + } + ], + "iniconfig._parse": [ + { + "package_id": "iniconfig==2.0.0@registry+https://pypi.org/simple" + } + ], + "iniconfig._version": [ + { + "package_id": "iniconfig==2.0.0@registry+https://pypi.org/simple" + } + ], + "iniconfig.exceptions": [ + { + "package_id": "iniconfig==2.0.0@registry+https://pypi.org/simple" + } + ] + } + "#); + Ok(()) } From 7944d9304c277e649a7c0451d75cae6d271b35bb Mon Sep 17 00:00:00 2001 From: Zsolt Dollenstein Date: Fri, 19 Jun 2026 17:48:29 +0000 Subject: [PATCH 2/4] address review feedback --- crates/uv-resolver/src/lock/installable.rs | 198 ++++++++++----------- 1 file changed, 97 insertions(+), 101 deletions(-) diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index f9deccc04c0..cec818c43b3 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -321,124 +321,120 @@ trait InstallableExt<'lock>: Installable<'lock> { } } - // Add any requirements that are exclusive to the workspace root (e.g., dependencies in - // PEP 723 scripts). - for dependency in self - .lock() - .requirements() - .iter() - .filter(|_| include_manifest) - { - if !dependency.marker.evaluate(marker_env, &[]) { - continue; - } + if include_manifest { + // Add any requirements that are exclusive to the workspace root (e.g., dependencies in + // PEP 723 scripts). + for dependency in self.lock().requirements() { + if !dependency.marker.evaluate(marker_env, &[]) { + continue; + } - let root_name = &dependency.name; - let dist = self - .lock() - .find_by_markers(root_name, marker_env) - .map_err(|_| LockErrorKind::MultipleRootPackages { - name: root_name.clone(), - })? - .ok_or_else(|| LockErrorKind::MissingRootPackage { - name: root_name.clone(), - })?; - - // Add the package to the graph. - let index = petgraph.add_node(if groups.prod() { - self.package_to_node(dist, tags, build_options, install_options, marker_env)? - } else { - self.non_installable_node(dist, tags, marker_env)? - }); - inverse.insert(&dist.id, index); + let root_name = &dependency.name; + let dist = self + .lock() + .find_by_markers(root_name, marker_env) + .map_err(|_| LockErrorKind::MultipleRootPackages { + name: root_name.clone(), + })? + .ok_or_else(|| LockErrorKind::MissingRootPackage { + name: root_name.clone(), + })?; - // Add the edge. - petgraph.add_edge(root, index, Edge::Prod); + // Add the package to the graph. + let index = petgraph.add_node(if groups.prod() { + self.package_to_node(dist, tags, build_options, install_options, marker_env)? + } else { + self.non_installable_node(dist, tags, marker_env)? + }); + inverse.insert(&dist.id, index); - // Push its dependencies on the queue. - if seen.insert((&dist.id, None)) { - queue.push_back((dist, None)); - } - for extra in &dependency.extras { - if seen.insert((&dist.id, Some(extra))) { - queue.push_back((dist, Some(extra))); - } - } - } + // Add the edge. + petgraph.add_edge(root, index, Edge::Prod); - // Add any dependency groups that are exclusive to the workspace root (e.g., dev - // dependencies in non-project workspace roots). - for (group, dependency) in self - .lock() - .dependency_groups() - .iter() - .filter(|_| include_manifest) - .filter_map(|(group, deps)| { - if groups.contains(group) { - Some(deps.iter().map(move |dep| (group, dep))) - } else { - None + // Push its dependencies on the queue. + if seen.insert((&dist.id, None)) { + queue.push_back((dist, None)); + } + for extra in &dependency.extras { + if seen.insert((&dist.id, Some(extra))) { + queue.push_back((dist, Some(extra))); + } } - }) - .flatten() - { - if !dependency.marker.evaluate(marker_env, &[]) { - continue; } - let root_name = &dependency.name; - let dist = self + // Add any dependency groups that are exclusive to the workspace root (e.g., dev + // dependencies in non-project workspace roots). + for (group, dependency) in self .lock() - .find_by_markers(root_name, marker_env) - .map_err(|_| LockErrorKind::MultipleRootPackages { - name: root_name.clone(), - })? - .ok_or_else(|| LockErrorKind::MissingRootPackage { - name: root_name.clone(), - })?; - - // Add the package to the graph. - let index = match inverse.entry(&dist.id) { - Entry::Vacant(entry) => { - let index = petgraph.add_node(self.package_to_node( - dist, - tags, - build_options, - install_options, - marker_env, - )?); - entry.insert(index); - index + .dependency_groups() + .iter() + .filter_map(|(group, deps)| { + if groups.contains(group) { + Some(deps.iter().map(move |dep| (group, dep))) + } else { + None + } + }) + .flatten() + { + if !dependency.marker.evaluate(marker_env, &[]) { + continue; } - 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( + + let root_name = &dependency.name; + let dist = self + .lock() + .find_by_markers(root_name, marker_env) + .map_err(|_| LockErrorKind::MultipleRootPackages { + name: root_name.clone(), + })? + .ok_or_else(|| LockErrorKind::MissingRootPackage { + name: root_name.clone(), + })?; + + // Add the package to the graph. + let index = match inverse.entry(&dist.id) { + Entry::Vacant(entry) => { + let index = petgraph.add_node(self.package_to_node( dist, tags, build_options, install_options, marker_env, - )?; + )?); + entry.insert(index); + 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( + dist, + tags, + build_options, + install_options, + marker_env, + )?; + } + index + } + }; - // Add the edge. - petgraph.add_edge(root, index, Edge::Dev(group.clone())); + // Add the edge. + petgraph.add_edge(root, index, Edge::Dev(group.clone())); - // Push its dependencies on the queue. - if seen.insert((&dist.id, None)) { - queue.push_back((dist, None)); - } - for extra in &dependency.extras { - if seen.insert((&dist.id, Some(extra))) { - queue.push_back((dist, Some(extra))); + // Push its dependencies on the queue. + if seen.insert((&dist.id, None)) { + queue.push_back((dist, None)); + } + for extra in &dependency.extras { + if seen.insert((&dist.id, Some(extra))) { + queue.push_back((dist, Some(extra))); + } } } } From c61ca7c81cbedbfd88f1eb0abe344632405fd376 Mon Sep 17 00:00:00 2001 From: Zsolt Dollenstein Date: Mon, 22 Jun 2026 16:27:09 +0000 Subject: [PATCH 3/4] Validate conflicts when materializing locked subgraphs Concrete roots do not retain the workspace path that selected them, so a dependency marker can otherwise silently use conflict items outside the selected subgraph. Resolve conflict items within the subgraph and reject dependency markers that still depend on packages outside it. Evaluate PEP 508 branches for the target environment before checking ambiguity, and use the standard Installable path for project targets with conflicts. --- crates/uv-resolver/src/lock/installable.rs | 240 +++++++++++++++++- crates/uv-resolver/src/lock/mod.rs | 12 + crates/uv-resolver/src/universal_marker.rs | 34 +++ .../uv/src/commands/project/install_target.rs | 20 +- 4 files changed, 292 insertions(+), 14 deletions(-) diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index cec818c43b3..bfe0aaef9ca 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -14,7 +14,7 @@ use uv_configuration::{BuildOptions, DependencyGroupsWithDefaults, InstallOption use uv_distribution_types::{Edge, Node, Resolution, ResolvedDist}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_platform_tags::Tags; -use uv_pypi_types::ResolverMarkerEnvironment; +use uv_pypi_types::{ConflictKind, ConflictSet, ResolverMarkerEnvironment}; use crate::lock::{Dependency, HashedDist, LockErrorKind, Package, TagPolicy}; use crate::{Lock, LockError}; @@ -180,6 +180,8 @@ trait InstallableExt<'lock>: Installable<'lock> { let mut activated_projects: Vec<&PackageName> = vec![]; let mut activated_extras: Vec<(&PackageName, &ExtraName)> = vec![]; let mut activated_groups: Vec<(&PackageName, &GroupName)> = vec![]; + let validate_conflicts = !include_manifest && !self.lock().conflicts().is_empty(); + let mut dependencies_for_conflict_validation = vec![]; let root = petgraph.add_node(Node::Root); @@ -252,6 +254,9 @@ trait InstallableExt<'lock>: Installable<'lock> { }) .flatten() { + if validate_conflicts { + dependencies_for_conflict_validation.push((dist, dep)); + } let additional_activated_extras = newly_activated_extras(dep, &activated_extras); if !dep.complexified_marker.evaluate( marker_env, @@ -575,6 +580,9 @@ trait InstallableExt<'lock>: Installable<'lock> { Either::Right(package.dependencies.iter()) }; for dep in deps { + if validate_conflicts { + dependencies_for_conflict_validation.push((package, dep)); + } if !dep.complexified_marker.evaluate( marker_env, activated_projects.iter().copied(), @@ -626,6 +634,50 @@ trait InstallableExt<'lock>: Installable<'lock> { } } + // Evaluate conflict markers from concrete roots, not from workspace members that depend on + // them. Reject markers that still depend on conflict items outside the resulting subgraph. + if validate_conflicts { + let subgraph_packages = inverse + .keys() + .map(|package_id| &package_id.name) + .collect::>(); + + for (package, dependency) in dependencies_for_conflict_validation { + let mut marker = dependency.complexified_marker; + for item in self.lock().conflicts().iter().flat_map(ConflictSet::iter) { + if !subgraph_packages.contains(item.package()) { + continue; + } + + let active = match item.kind() { + ConflictKind::Project => activated_projects.contains(&item.package()), + ConflictKind::Extra(extra) => { + activated_extras.contains(&(item.package(), extra)) + } + ConflictKind::Group(group) => { + activated_groups.contains(&(item.package(), group)) + } + }; + if active { + marker.assume_conflict_item(item); + } else { + marker.assume_not_conflict_item(item); + } + } + + let conflict = marker.conflict_for_environment(marker_env); + // All in-subgraph conflict items were resolved above, so a non-constant marker + // still depends on a package outside the subgraph. + if !conflict.is_constant() { + return Err(LockErrorKind::DependencyConflictOutsideSubgraph { + package: package.id.clone(), + dependency: dependency.package_id.clone(), + } + .into()); + } + } + } + Ok(Resolution::new(petgraph)) } } @@ -663,9 +715,15 @@ impl Lock { /// Each root must be a [`Package`] from this lock. Unlike [`Installable::to_resolution`], this /// method does not include requirements or dependency groups attached directly to the lock /// manifest. Extras and dependency groups on the concrete roots are still included according - /// to `extras` and `groups`. `project_name` identifies the project for project-specific - /// [`InstallOptions`] filters, if applicable. Callers are responsible for selecting roots that - /// apply to `marker_env`. + /// to `extras` and `groups`. + /// + /// Conflict-marker evaluation starts from `roots` and their requested `extras` and `groups`, + /// not from workspace members that depend on those roots. The method returns an error if a + /// dependency marker still depends on a conflict item outside the resulting subgraph. Use + /// [`Installable::to_resolution`] when materializing an existing lock target. + /// + /// `project_name` identifies the project for project-specific [`InstallOptions`] filters, if + /// applicable. Callers are responsible for selecting roots that apply to `marker_env`. pub fn to_resolution<'lock>( &'lock self, install_path: &'lock Path, @@ -727,6 +785,7 @@ mod tests { use uv_normalize::{DefaultExtras, DefaultGroups}; use uv_pep508::{MarkerEnvironment, MarkerEnvironmentBuilder}; use uv_platform_tags::{Arch, Os, Platform, TagsOptions}; + use uv_warnings::anstream; use super::*; @@ -856,6 +915,91 @@ sdist = { url = "https://example.com/unrelated-1.0.0.tar.gz", hash = "sha256:888 .expect("valid lock") } + fn conflict_lock() -> Lock { + toml::from_str( + r#" +version = 1 +revision = 3 +requires-python = ">=3.11" +conflicts = [ + [ + { package = "tool", extra = "cpu" }, + { package = "tool", extra = "gpu" }, + ], + [ + { package = "project", extra = "foo" }, + { package = "project", extra = "bar" }, + ], +] + +[[package]] +name = "contextual-dependency" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/contextual_dependency-1.0.0.tar.gz", hash = "sha256:1111111111111111111111111111111111111111111111111111111111111111" } + +[[package]] +name = "contextual-tool" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +dependencies = [ + { name = "contextual-dependency", marker = "sys_platform == 'linux' or (sys_platform == 'darwin' and extra == 'extra-7-project-foo')" }, +] +sdist = { url = "https://example.com/contextual_tool-1.0.0.tar.gz", hash = "sha256:2222222222222222222222222222222222222222222222222222222222222222" } + +[[package]] +name = "cpu-backend" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/cpu_backend-1.0.0.tar.gz", hash = "sha256:3333333333333333333333333333333333333333333333333333333333333333" } + +[[package]] +name = "gpu-backend" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/gpu_backend-1.0.0.tar.gz", hash = "sha256:4444444444444444444444444444444444444444444444444444444444444444" } + +[[package]] +name = "project" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +sdist = { url = "https://example.com/project-1.0.0.tar.gz", hash = "sha256:5555555555555555555555555555555555555555555555555555555555555555" } + +[package.optional-dependencies] +foo = [] +bar = [] + +[package.metadata] +provides-extras = ["foo", "bar"] + +[[package]] +name = "runtime" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +dependencies = [ + { name = "cpu-backend", marker = "extra == 'extra-4-tool-cpu'" }, + { name = "gpu-backend", marker = "extra == 'extra-4-tool-gpu'" }, +] +sdist = { url = "https://example.com/runtime-1.0.0.tar.gz", hash = "sha256:6666666666666666666666666666666666666666666666666666666666666666" } + +[[package]] +name = "tool" +version = "1.0.0" +source = { registry = "https://example.com/simple" } +dependencies = [{ name = "runtime" }] +sdist = { url = "https://example.com/tool-1.0.0.tar.gz", hash = "sha256:7777777777777777777777777777777777777777777777777777777777777777" } + +[package.optional-dependencies] +cpu = [] +gpu = [] + +[package.metadata] +provides-extras = ["cpu", "gpu"] +"#, + ) + .expect("valid lock") + } + fn package<'lock>(lock: &'lock Lock, name: &str, version: &str) -> &'lock Package { lock.packages() .iter() @@ -889,6 +1033,27 @@ sdist = { url = "https://example.com/unrelated-1.0.0.tar.gz", hash = "sha256:888 .expect("valid resolution") } + fn materialize_with_extras( + lock: &Lock, + roots: &[&Package], + marker_env: &ResolverMarkerEnvironment, + extras: &ExtrasSpecification, + ) -> Result { + let extras = extras.with_defaults(DefaultExtras::default()); + let groups = DependencyGroupsWithDefaults::none(); + lock.to_resolution( + Path::new("."), + roots.iter().copied(), + None, + marker_env, + &TAGS, + &extras, + &groups, + &BuildOptions::default(), + &InstallOptions::default(), + ) + } + struct OverridingInstallable<'lock> { lock: &'lock Lock, root_name: &'lock PackageName, @@ -1067,6 +1232,73 @@ sdist = { url = "https://example.com/unrelated-1.0.0.tar.gz", hash = "sha256:888 }); } + #[test] + fn materializes_conflicting_extras_within_the_synthetic_root() { + let lock = conflict_lock(); + let extras = + ExtrasSpecification::from_extra(vec!["cpu".parse().expect("valid extra name")]); + let resolution = materialize_with_extras( + &lock, + &[package(&lock, "tool", "1.0.0")], + &DARWIN_MARKERS, + &extras, + ) + .expect("conflict markers are resolved within the subgraph"); + + insta::with_settings!({ + filters => [(r"sha256:[0-9a-f]{64}", "sha256:[HASH]")], + }, { + insta::assert_debug_snapshot!(graph_snapshot(&resolution), @r#" + ( + [ + "cpu-backend==1.0.0 (install: true, hashes: sha256:[HASH])", + "root", + "runtime==1.0.0 (install: true, hashes: sha256:[HASH])", + "tool==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + [ + "root --Prod--> tool==1.0.0 (install: true, hashes: sha256:[HASH])", + "runtime==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> cpu-backend==1.0.0 (install: true, hashes: sha256:[HASH])", + "tool==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> runtime==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + ) + "#); + }); + } + + #[test] + fn rejects_conflicts_outside_the_synthetic_root() { + let lock = conflict_lock(); + let root = package(&lock, "contextual-tool", "1.0.0"); + let extras = ExtrasSpecification::default(); + + let error = materialize_with_extras(&lock, &[root], &DARWIN_MARKERS, &extras) + .expect_err("Darwin dependency depends on the project extra"); + let error = error.to_string(); + let error = anstream::adapter::strip_str(&error); + insta::assert_snapshot!(error, @"Cannot materialize dependency `contextual-dependency==1.0.0 @ registry+https://example.com/simple` of `contextual-tool==1.0.0 @ registry+https://example.com/simple` because its conflict marker depends on a package outside the selected subgraph"); + + let linux = materialize_with_extras(&lock, &[root], &LINUX_MARKERS, &extras) + .expect("the dependency is unconditional on Linux"); + insta::with_settings!({ + filters => [(r"sha256:[0-9a-f]{64}", "sha256:[HASH]")], + }, { + insta::assert_debug_snapshot!(graph_snapshot(&linux), @r#" + ( + [ + "contextual-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "contextual-tool==1.0.0 (install: true, hashes: sha256:[HASH])", + "root", + ], + [ + "contextual-tool==1.0.0 (install: true, hashes: sha256:[HASH]) --Prod--> contextual-dependency==1.0.0 (install: true, hashes: sha256:[HASH])", + "root --Prod--> contextual-tool==1.0.0 (install: true, hashes: sha256:[HASH])", + ], + ) + "#); + }); + } + #[test] fn installable_to_resolution_preserves_node_overrides() { let mut lock = lock(); diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index ce404918ed7..4f83499c1c6 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -6617,6 +6617,18 @@ enum LockErrorKind { /// The ID of the package. id: PackageId, }, + /// A dependency marker depends on a package outside the selected subgraph. + #[error( + "Cannot materialize dependency `{dependency}` of `{package}` because its conflict marker depends on a package outside the selected subgraph", + package = package.cyan(), + dependency = dependency.cyan() + )] + DependencyConflictOutsideSubgraph { + /// The ID of the package that declares the dependency. + package: PackageId, + /// The ID of the dependency whose inclusion is ambiguous. + dependency: PackageId, + }, /// An error that occurs when resolving metadata for a package. #[error("Failed to generate package metadata for `{id}`", id = id.cyan())] Resolution { diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs index 319a220b1c8..9748e7d74d6 100644 --- a/crates/uv-resolver/src/universal_marker.rs +++ b/crates/uv-resolver/src/universal_marker.rs @@ -379,6 +379,35 @@ impl UniversalMarker { marker: self.marker.only_extras(), } } + + /// Returns the conflict marker that remains after evaluating all PEP 508 expressions in the + /// given environment. + /// + /// Unlike [`UniversalMarker::conflict`], this preserves the relationship between PEP 508 and + /// conflict expressions. For example, given `sys_platform == 'linux' or extra == 'foo'`, the + /// conflict marker is always true on Linux but still depends on `foo` elsewhere. + pub(crate) fn conflict_for_environment(self, env: &MarkerEnvironment) -> ConflictMarker { + let mut remaining = MarkerTree::FALSE; + + 'conjunctions: for conjunction in self.marker.to_dnf() { + let mut conflict = MarkerTree::TRUE; + for expression in conjunction { + match expression { + expression @ MarkerExpression::Extra { .. } => { + conflict.and(MarkerTree::expression(expression)); + } + expression => { + if !MarkerTree::expression(expression).evaluate(env, &[]) { + continue 'conjunctions; + } + } + } + } + remaining.or(conflict); + } + + ConflictMarker { marker: remaining } + } } impl std::fmt::Debug for UniversalMarker { @@ -495,6 +524,11 @@ impl ConflictMarker { self.marker.is_true() } + /// Returns true if this conflict marker always evaluates to the same value. + pub(crate) fn is_constant(self) -> bool { + self.marker.is_true() || self.marker.is_false() + } + /// Returns inclusion and exclusion (respectively) conflict items parsed /// from this conflict marker. /// diff --git a/crates/uv/src/commands/project/install_target.rs b/crates/uv/src/commands/project/install_target.rs index 8f7f8207b35..f14913461ad 100644 --- a/crates/uv/src/commands/project/install_target.rs +++ b/crates/uv/src/commands/project/install_target.rs @@ -129,16 +129,16 @@ impl<'lock> InstallTarget<'lock> { build_options: &BuildOptions, install_options: &InstallOptions, ) -> Result { - // Project and workspace targets have package-backed roots, so materialize them through the - // concrete-root API. Non-project workspaces can also have package roots, but their root - // dependencies live on the lock manifest, so they need the generic path. Scripts and - // invalid or ambiguous roots also use that path. - let use_concrete_roots = match self { - Self::Project { workspace, .. } - | Self::Projects { workspace, .. } - | Self::Workspace { workspace, .. } => !workspace.is_non_project(), - Self::NonProjectWorkspace { .. } | Self::Script { .. } => false, - }; + // Package-backed project and workspace targets without conflicts can use concrete roots. + // Other targets need the generic path to include manifest dependencies or evaluate + // conflict markers from project roots. + let use_concrete_roots = self.lock().conflicts().is_empty() + && match self { + Self::Project { workspace, .. } + | Self::Projects { workspace, .. } + | Self::Workspace { workspace, .. } => !workspace.is_non_project(), + Self::NonProjectWorkspace { .. } | Self::Script { .. } => false, + }; if use_concrete_roots && let Some(roots) = self .roots() From c95ebd2a9c66635cca0e987bb8b79632c228ffe3 Mon Sep 17 00:00:00 2001 From: Zsolt Dollenstein Date: Tue, 23 Jun 2026 08:53:16 +0000 Subject: [PATCH 4/4] optimize validation --- crates/uv-resolver/src/lock/installable.rs | 12 +++++++++--- crates/uv-resolver/src/universal_marker.rs | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index bfe0aaef9ca..7bb8f7429ef 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -254,7 +254,7 @@ trait InstallableExt<'lock>: Installable<'lock> { }) .flatten() { - if validate_conflicts { + if validate_conflicts && dep.complexified_marker.has_conflict_marker() { dependencies_for_conflict_validation.push((dist, dep)); } let additional_activated_extras = newly_activated_extras(dep, &activated_extras); @@ -580,7 +580,7 @@ trait InstallableExt<'lock>: Installable<'lock> { Either::Right(package.dependencies.iter()) }; for dep in deps { - if validate_conflicts { + if validate_conflicts && dep.complexified_marker.has_conflict_marker() { dependencies_for_conflict_validation.push((package, dep)); } if !dep.complexified_marker.evaluate( @@ -636,13 +636,19 @@ trait InstallableExt<'lock>: Installable<'lock> { // Evaluate conflict markers from concrete roots, not from workspace members that depend on // them. Reject markers that still depend on conflict items outside the resulting subgraph. - if validate_conflicts { + if !dependencies_for_conflict_validation.is_empty() { let subgraph_packages = inverse .keys() .map(|package_id| &package_id.name) .collect::>(); + // The environment and conflict state are shared by every dependency, so repeated + // markers have the same result. + let mut validated_markers = FxHashSet::default(); for (package, dependency) in dependencies_for_conflict_validation { + if !validated_markers.insert(dependency.complexified_marker) { + continue; + } let mut marker = dependency.complexified_marker; for item in self.lock().conflicts().iter().flat_map(ConflictSet::iter) { if !subgraph_packages.contains(item.package()) { diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs index 9748e7d74d6..92af61afc08 100644 --- a/crates/uv-resolver/src/universal_marker.rs +++ b/crates/uv-resolver/src/universal_marker.rs @@ -279,6 +279,15 @@ impl UniversalMarker { self.marker.is_false() } + /// Returns true if this universal marker contains a conflict marker. + /// + /// Conflict items are encoded as `extra` expressions in `marker`, while `pep508` is the same + /// canonical marker with all `extra` expressions removed. Since [`MarkerTree`] equality is + /// semantic, the trees differ exactly when the marker depends on a conflict item. + pub(crate) fn has_conflict_marker(self) -> bool { + self.marker != self.pep508 + } + /// Returns true if this universal marker is disjoint with the one given. /// /// Two universal markers are disjoint when it is impossible for them both @@ -1053,6 +1062,14 @@ mod tests { assert_eq!(format!("{dep_conflict_marker:?}"), "true"); } + #[test] + fn has_conflict_marker() { + let pep508 = + MarkerTree::from_str("sys_platform == 'darwin'").expect("valid marker expression"); + assert!(!UniversalMarker::from_combined(pep508).has_conflict_marker()); + assert!(UniversalMarker::new(pep508, create_extra_marker("foo")).has_conflict_marker()); + } + #[test] fn resolve() { let known_conflicts = create_known_conflicts([("foo", "sys_platform == 'darwin'")]);