From 9de3c945f61534fd654b552435e5cb810ff7ce7b Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Wed, 14 Aug 2024 10:06:18 -0400
Subject: [PATCH] Remove `same-graph` merging in resolver (#6077)

## Summary

This was added in https://github.com/astral-sh/uv/pull/5405 but is now
the cause of an instability in `github_wikidata_bot`. Specifically, on
the initial run, we fork in `pydantic==2.8.2`, via:

```
Requires-Dist: typing-extensions>=4.12.2; python_version >= '3.13'
Requires-Dist: typing-extensions>=4.6.1; python_version < '3.13'
```

In the end, we resolve a single version of `typing-extensions`
(`4.12.2`)... But we don't recognize the two resolutions as the "same
graph", because we propagate the fork markers, and so the "edges" have
different markers on them...

In the second run through, when we have the forks in advance, we don't
split on Pydantic... We just try to solve from the root with the current
forks. This is fundamentally different and I fear it will be the cause
of many instabilities. But removing this graph check fixes the proximate
issue.

I don't really understand why this was added since there was no test
coverage in the PR.
---
 crates/uv-resolver/src/resolver/mod.rs | 38 --------------------------
 crates/uv/tests/ecosystem.rs           |  4 +--
 crates/uv/tests/lock.rs                | 24 ++++++++--------
 3 files changed, 12 insertions(+), 54 deletions(-)

diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs
index 70e248aef..1ec7658ec 100644
--- a/crates/uv-resolver/src/resolver/mod.rs
+++ b/crates/uv-resolver/src/resolver/mod.rs
@@ -400,29 +400,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                         );
                     }
 
-                    // If another fork had the same resolution, merge into that fork instead.
-                    if let Some(existing_resolution) = resolutions
-                        .iter_mut()
-                        .find(|existing_resolution| resolution.same_graph(existing_resolution))
-                    {
-                        let ResolverMarkers::Fork(existing_markers) = &existing_resolution.markers
-                        else {
-                            panic!("A non-forking resolution exists in forking mode")
-                        };
-                        let mut new_markers = existing_markers.clone();
-                        new_markers.or(resolution
-                            .markers
-                            .fork_markers()
-                            .expect("A non-forking resolution exists in forking mode")
-                            .clone());
-                        existing_resolution.markers = if new_markers.is_true() {
-                            ResolverMarkers::universal(vec![])
-                        } else {
-                            ResolverMarkers::Fork(new_markers)
-                        };
-                        continue 'FORK;
-                    }
-
                     Self::trace_resolution(&resolution);
                     resolutions.push(resolution);
                     continue 'FORK;
@@ -2516,21 +2493,6 @@ pub(crate) struct ResolutionDependencyEdge {
     pub(crate) marker: MarkerTree,
 }
 
-impl Resolution {
-    /// Whether we got two identical resolutions in two separate forks.
-    ///
-    /// Ignores pins since the which distribution we prioritized for each version doesn't matter.
-    fn same_graph(&self, other: &Self) -> bool {
-        // TODO(konsti): The edges being equal is not a requirement for the graph being equal. While
-        // an exact solution is too much here, we should ignore different in edges that point to
-        // nodes that are always installed. Example: root requires foo, root requires bar. bar
-        // forks, and one for the branches has bar -> foo while the other doesn't. The resolution
-        // is still the same graph since the presence or absence of the bar -> foo edge cannot
-        // change which packages and versions are installed.
-        self.nodes == other.nodes && self.edges == other.edges
-    }
-}
-
 impl ResolutionPackage {
     pub(crate) fn is_base(&self) -> bool {
         self.extra.is_none() && self.dev.is_none()
diff --git a/crates/uv/tests/ecosystem.rs b/crates/uv/tests/ecosystem.rs
index c301d2169..4d2429a76 100644
--- a/crates/uv/tests/ecosystem.rs
+++ b/crates/uv/tests/ecosystem.rs
@@ -33,9 +33,7 @@ fn packse() -> Result<()> {
 // Source: https://github.com/konstin/github-wikidata-bot/blob/8218d20985eb480cb8633026f9dabc9e5ec4b5e3/pyproject.toml
 #[test]
 fn github_wikidata_bot() -> Result<()> {
-    // TODO(charlie): This test became non-deterministic in https://github.com/astral-sh/uv/pull/6065.
-    // However, that fix is _correct_, and the non-determinism itself is an existing bug.
-    lock_ecosystem_package_non_deterministic("3.12", "github-wikidata-bot")
+    lock_ecosystem_package("3.12", "github-wikidata-bot")
 }
 
 // Source: https://github.com/psf/black/blob/9ff047a9575f105f659043f28573e1941e9cdfb3/pyproject.toml
diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs
index 2cf2ee1b3..fe91eb45e 100644
--- a/crates/uv/tests/lock.rs
+++ b/crates/uv/tests/lock.rs
@@ -3626,19 +3626,17 @@ fn lock_python_version_marker_complement() -> Result<()> {
         );
     });
 
-    // TODO(charlie): This test became non-deterministic in https://github.com/astral-sh/uv/pull/6065.
-    // But that fix is correct, and the non-determinism itself is a bug.
-    // // Re-run with `--locked`.
-    // uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###"
-    // success: false
-    // exit_code: 2
-    // ----- stdout -----
-    //
-    // ----- stderr -----
-    // warning: `uv lock` is experimental and may change without warning
-    // Resolved 4 packages in [TIME]
-    // error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided. To update the lockfile, run `uv lock`.
-    // "###);
+    // Re-run with `--locked`.
+    uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###"
+    success: false
+    exit_code: 2
+    ----- stdout -----
+
+    ----- stderr -----
+    warning: `uv lock` is experimental and may change without warning
+    Resolved 4 packages in [TIME]
+    error: The lockfile at `uv.lock` needs to be updated, but `--locked` was provided. To update the lockfile, run `uv lock`.
+    "###);
 
     Ok(())
 }