uv-resolver: add error checking for conflicting distributions

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,
 }