uv-resolver: add error checking for conflicting distributions

This PR adds some additional sanity checking on resolution graphs to
ensure we can never install different versions of the same package into
the same environment.

I used code similar to this to provoke bugs in the resolver before the
release, but it never made it into `main`. Here, we add the error
checking to the creation of `ResolutionGraph`, since this is where it's
most convenient to access the "full" markers of each distribution.

We only report an error when `debug_assertions` are enabled to avoid
rendering `uv` *completely* unusuable if a bug were to occur in a
production binary. For example, maybe a conflict is detected in a marker
environment that isn't actually used. While not ideal, `uv` is still
usable for any other marker environment.

Closes #5598

crates/uv-resolver/src/error.rs

@@ -19,6 +19,7 @@ use crate::pubgrub::{
     PubGrubPackage, PubGrubPackageInner, PubGrubReportFormatter, PubGrubSpecifierError,
 };
 use crate::python_requirement::PythonRequirement;
+use crate::resolution::ConflictingDistributionError;
 use crate::resolver::{IncompletePackage, ResolverMarkers, UnavailablePackage, UnavailableReason};
 
 #[derive(Debug, thiserror::Error)]
@@ -102,6 +103,9 @@ pub enum ResolveError {
     #[error("In `--require-hashes` mode, all requirements must be pinned upfront with `==`, but found: `{0}`")]
     UnhashedPackage(PackageName),
 
+    #[error("found conflicting distribution in resolution: {0}")]
+    ConflictingDistribution(ConflictingDistributionError),
+
     /// Something unexpected happened.
     #[error("{0}")]
     Failure(String),

crates/uv-resolver/src/lib.rs

@@ -13,7 +13,9 @@ pub use prerelease::PrereleaseMode;
 pub use pubgrub::{PubGrubSpecifier, PubGrubSpecifierError};
 pub use python_requirement::PythonRequirement;
 pub use requires_python::{RequiresPython, RequiresPythonError, RequiresPythonRange};
-pub use resolution::{AnnotationStyle, DisplayResolutionGraph, ResolutionGraph};
+pub use resolution::{
+    AnnotationStyle, ConflictingDistributionError, DisplayResolutionGraph, ResolutionGraph,
+};
 pub use resolution_mode::ResolutionMode;
 pub use resolver::{
     BuildId, DefaultResolverProvider, InMemoryIndex, MetadataResponse, PackageVersionsResult,

crates/uv-resolver/src/lock/mod.rs

@@ -428,7 +428,7 @@ impl Lock {
                 }
             }
         }
-        Ok(Self {
+        let lock = Self {
             version,
             fork_markers,
             supported_environments,
@@ -437,7 +437,8 @@ impl Lock {
             packages,
             by_id,
             manifest,
-        })
+        };
+        Ok(lock)
     }
 
     /// Record the requirements that were used to generate this lock.
@@ -1465,38 +1466,6 @@ impl TryFrom<LockWire> for Lock {
             fork_markers,
         )?;
 
-        /*
-        // TODO: Use the below in tests to validate we don't produce a
-        // trivially incorrect lock file.
-        let mut name_to_markers: BTreeMap<&PackageName, Vec<(&Version, &MarkerTree)>> =
-            BTreeMap::new();
-        for package in &lock.packages {
-            for dep in &package.dependencies {
-                name_to_markers
-                    .entry(&dep.package_id.name)
-                    .or_default()
-                    .push((&dep.package_id.version, &dep.marker));
-            }
-        }
-        for (name, marker_trees) in name_to_markers {
-            for (i, (version1, marker1)) in marker_trees.iter().enumerate() {
-                for (version2, marker2) in &marker_trees[i + 1..] {
-                    if version1 == version2 {
-                        continue;
-                    }
-                    if !marker1.is_disjoint(marker2) {
-                        assert!(
-                            false,
-                            "[{marker1:?}] (for version {version1}) is not disjoint with \
-                             [{marker2:?}] (for version {version2}) \
-                             for package `{name}`",
-                        );
-                    }
-                }
-            }
-        }
-        */
-
         Ok(lock)
     }
 }

crates/uv-resolver/src/resolution/graph.rs

@@ -1,3 +1,6 @@
+use std::collections::BTreeMap;
+use std::fmt::{Display, Formatter};
+
 use distribution_types::{
     Dist, DistributionMetadata, Name, ResolutionDiagnostic, ResolvedDist, VersionId,
     VersionOrUrlRef,
@@ -11,7 +14,6 @@ use petgraph::{
 };
 use pypi_types::{HashDigest, ParsedUrlError, Requirement, VerbatimParsedUrl, Yanked};
 use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
-use std::fmt::{Display, Formatter};
 use uv_configuration::{Constraints, Overrides};
 use uv_distribution::Metadata;
 use uv_git::GitResolver;
@@ -233,7 +235,7 @@ impl ResolutionGraph {
             report_missing_lower_bounds(&petgraph, &mut diagnostics);
         }
 
-        Ok(Self {
+        let graph = Self {
             petgraph,
             requires_python,
             package_markers,
@@ -243,7 +245,29 @@ impl ResolutionGraph {
             overrides: overrides.clone(),
             options,
             fork_markers,
-        })
+        };
+        let mut conflicting = graph.find_conflicting_distributions();
+        if !conflicting.is_empty() {
+            tracing::warn!(
+                "found {} conflicting distributions in resolution, \
+                 please report this as a bug at \
+                 https://github.com/astral-sh/uv/issues/new",
+                conflicting.len()
+            );
+        }
+        // When testing, we materialize any conflicting distributions as an
+        // error to ensure any relevant tests fail. Otherwise, we just leave
+        // it at the warning message above. The reason for not returning an
+        // error "in production" is that an incorrect resolution may only be
+        // incorrect in certain marker environments, but fine in most others.
+        // Returning an error in that case would make `uv` unusable whenever
+        // the bug occurs, but letting it through means `uv` *could* still be
+        // usable.
+        #[cfg(debug_assertions)]
+        if let Some(err) = conflicting.pop() {
+            return Err(ResolveError::ConflictingDistribution(err));
+        }
+        Ok(graph)
     }
 
     fn add_edge(
@@ -681,6 +705,84 @@ impl ResolutionGraph {
             Some(&package_markers[&(version.clone(), url.cloned())])
         }
     }
+
+    /// Returns a sequence of conflicting distribution errors from this
+    /// resolution.
+    ///
+    /// Correct resolutions always return an empty sequence. A non-empty
+    /// sequence implies there is a package with two distinct versions in the
+    /// same marker environment in this resolution. This in turn implies that
+    /// an installation in that marker environment could wind up trying to
+    /// install different versions of the same package, which is not allowed.
+    fn find_conflicting_distributions(&self) -> Vec<ConflictingDistributionError> {
+        let mut name_to_markers: BTreeMap<&PackageName, Vec<(&Version, &MarkerTree)>> =
+            BTreeMap::new();
+        for node in self.petgraph.node_weights() {
+            let annotated_dist = match node {
+                ResolutionGraphNode::Root => continue,
+                ResolutionGraphNode::Dist(ref annotated_dist) => annotated_dist,
+            };
+            name_to_markers
+                .entry(&annotated_dist.name)
+                .or_default()
+                .push((&annotated_dist.version, &annotated_dist.marker));
+        }
+        let mut dupes = vec![];
+        for (name, marker_trees) in name_to_markers {
+            for (i, (version1, marker1)) in marker_trees.iter().enumerate() {
+                for (version2, marker2) in &marker_trees[i + 1..] {
+                    if version1 == version2 {
+                        continue;
+                    }
+                    if !marker1.is_disjoint(marker2) {
+                        dupes.push(ConflictingDistributionError {
+                            name: name.clone(),
+                            version1: (*version1).clone(),
+                            version2: (*version2).clone(),
+                            marker1: (*marker1).clone(),
+                            marker2: (*marker2).clone(),
+                        });
+                    }
+                }
+            }
+        }
+        dupes
+    }
+}
+
+/// An error that occurs for conflicting versions of the same package.
+///
+/// Specifically, this occurs when two distributions with the same package
+/// name are found with distinct versions in at least one possible marker
+/// environment. This error reflects an error that could occur when installing
+/// the corresponding resolution into that marker environment.
+#[derive(Debug)]
+pub struct ConflictingDistributionError {
+    name: PackageName,
+    version1: Version,
+    version2: Version,
+    marker1: MarkerTree,
+    marker2: MarkerTree,
+}
+
+impl std::error::Error for ConflictingDistributionError {}
+
+impl std::fmt::Display for ConflictingDistributionError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        let ConflictingDistributionError {
+            ref name,
+            ref version1,
+            ref version2,
+            ref marker1,
+            ref marker2,
+        } = *self;
+        write!(
+            f,
+            "found conflicting versions for package `{name}`:
+             `{marker1:?}` (for version `{version1}`) is not disjoint with \
+             `{marker2:?}` (for version `{version2}`)",
+        )
+    }
 }
 
 impl From<ResolutionGraph> for distribution_types::Resolution {

crates/uv-resolver/src/resolution/mod.rs

@@ -11,8 +11,8 @@ use uv_distribution::Metadata;
 use uv_normalize::{ExtraName, GroupName, PackageName};
 
 pub use crate::resolution::display::{AnnotationStyle, DisplayResolutionGraph};
-pub use crate::resolution::graph::ResolutionGraph;
 pub(crate) use crate::resolution::graph::ResolutionGraphNode;
+pub use crate::resolution::graph::{ConflictingDistributionError, ResolutionGraph};
 pub(crate) use crate::resolution::requirements_txt::RequirementsTxtDist;
 
 mod display;
@@ -31,6 +31,11 @@ pub(crate) struct AnnotatedDist {
     pub(crate) dev: Option<GroupName>,
     pub(crate) hashes: Vec<HashDigest>,
     pub(crate) metadata: Option<Metadata>,
+    /// The "full" marker for this distribution. It precisely describes all
+    /// marker environments for which this distribution _can_ be installed.
+    /// That is, when doing a traversal over all of the distributions in a
+    /// resolution, this marker corresponds to the disjunction of all paths to
+    /// this distribution in the resolution graph.
     pub(crate) marker: MarkerTree,
 }