From 56c7f32023d4aea764eb2ea0fa6fc1d249ffc658 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Wed, 6 Oct 2021 21:56:22 -0700
Subject: [PATCH] revset: avoid using evolution for resolving a change id to
 its commits

This rewrites the code for resolving a change id to simply walk the
entire index. That's obviously not optimal, but it's not worse than
what we did in the evolution-based resolution. This is yet another
step towards removing support for evolution (#32).
---
 lib/src/evolution.rs | 31 -------------------------------
 lib/src/revset.rs    | 28 ++++++++++++++++++----------
 2 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/lib/src/evolution.rs b/lib/src/evolution.rs
index 1d7a3881e..495ec2558 100644
--- a/lib/src/evolution.rs
+++ b/lib/src/evolution.rs
@@ -20,7 +20,6 @@ use itertools::Itertools;
 use crate::backend::{ChangeId, CommitId};
 use crate::commit::Commit;
 use crate::dag_walk::{bfs, leaves};
-use crate::index::{HexPrefix, PrefixResolution};
 use crate::repo::{MutableRepo, ReadonlyRepo, RepoRef};
 
 // TODO: Combine some maps/sets and use a struct as value instead.
@@ -137,21 +136,6 @@ impl State {
             .map_or(false, |non_obsoletes| non_obsoletes.len() > 1)
     }
 
-    // TODO: We should probably add a change id table to the commit index and move
-    // this there
-    fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
-        let mut result = PrefixResolution::NoMatch;
-        for change_id in self.non_obsoletes_by_changeid.keys() {
-            if change_id.hex().starts_with(prefix.hex()) {
-                if result != PrefixResolution::NoMatch {
-                    return PrefixResolution::AmbiguousMatch;
-                }
-                result = PrefixResolution::SingleMatch(change_id.clone());
-            }
-        }
-        result
-    }
-
     fn non_obsoletes(&self, change_id: &ChangeId) -> HashSet<CommitId> {
         self.non_obsoletes_by_changeid
             .get(change_id)
@@ -359,13 +343,6 @@ impl EvolutionRef<'_> {
         }
     }
 
-    pub fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
-        match self {
-            EvolutionRef::Readonly(evolution) => evolution.resolve_change_id_prefix(prefix),
-            EvolutionRef::Mutable(evolution) => evolution.resolve_change_id_prefix(prefix),
-        }
-    }
-
     pub fn is_divergent(&self, change_id: &ChangeId) -> bool {
         match self {
             EvolutionRef::Readonly(evolution) => evolution.is_divergent(change_id),
@@ -414,10 +391,6 @@ impl ReadonlyEvolution {
         self.state.is_divergent(change_id)
     }
 
-    pub fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
-        self.state.resolve_change_id_prefix(prefix)
-    }
-
     pub fn non_obsoletes(&self, change_id: &ChangeId) -> HashSet<CommitId> {
         self.state.non_obsoletes(change_id)
     }
@@ -454,10 +427,6 @@ impl MutableEvolution {
         self.state.is_divergent(change_id)
     }
 
-    pub fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<ChangeId> {
-        self.state.resolve_change_id_prefix(prefix)
-    }
-
     pub fn non_obsoletes(&self, change_id: &ChangeId) -> HashSet<CommitId> {
         self.state.non_obsoletes(change_id)
     }
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index f10f7f25f..90b00c6cc 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -99,18 +99,26 @@ fn resolve_commit_id(repo: RepoRef, symbol: &str) -> Result<Vec<CommitId>, Revse
 
 fn resolve_change_id(repo: RepoRef, change_id_prefix: &str) -> Result<Vec<CommitId>, RevsetError> {
     if let Some(hex_prefix) = HexPrefix::new(change_id_prefix.to_owned()) {
-        let evolution = repo.evolution();
-        match evolution.resolve_change_id_prefix(&hex_prefix) {
-            PrefixResolution::NoMatch => {
-                Err(RevsetError::NoSuchRevision(change_id_prefix.to_owned()))
-            }
-            PrefixResolution::AmbiguousMatch => Err(RevsetError::AmbiguousChangeIdPrefix(
-                change_id_prefix.to_owned(),
-            )),
-            PrefixResolution::SingleMatch(change_id) => {
-                Ok(evolution.non_obsoletes(&change_id).into_iter().collect())
+        let mut found_change_id = None;
+        let mut commit_ids = vec![];
+        // TODO: Create a persistent lookup from change id to (visible?) commit ids.
+        for index_entry in RevsetExpression::all().evaluate(repo).unwrap().iter() {
+            let change_id = index_entry.change_id();
+            if change_id.hex().starts_with(hex_prefix.hex()) {
+                if let Some(previous_change_id) = found_change_id.replace(change_id.clone()) {
+                    if previous_change_id != change_id {
+                        return Err(RevsetError::AmbiguousChangeIdPrefix(
+                            change_id_prefix.to_owned(),
+                        ));
+                    }
+                }
+                commit_ids.push(index_entry.commit_id());
             }
         }
+        if found_change_id.is_none() {
+            return Err(RevsetError::NoSuchRevision(change_id_prefix.to_owned()));
+        }
+        Ok(commit_ids)
     } else {
         Err(RevsetError::NoSuchRevision(change_id_prefix.to_owned()))
     }