repo: migrate revset::resolve_change_id() to use IdIndex for ReadonlyRepo

The MutableRepo implementation is the same as before.

lib/src/repo.rs

@@ -92,6 +92,16 @@ impl<'a> RepoRef<'a> {
         }
     }
 
+    pub fn resolve_change_id_prefix(
+        &self,
+        prefix: &HexPrefix,
+    ) -> PrefixResolution<Vec<IndexEntry<'a>>> {
+        match self {
+            RepoRef::Readonly(repo) => repo.resolve_change_id_prefix(prefix),
+            RepoRef::Mutable(repo) => repo.resolve_change_id_prefix(prefix),
+        }
+    }
+
     pub fn shortest_unique_id_prefix_len(&self, target_id_bytes: &[u8]) -> usize {
         match self {
             RepoRef::Readonly(repo) => repo.shortest_unique_id_prefix_len(target_id_bytes),
@@ -660,6 +670,31 @@ impl MutableRepo {
         (self.index, self.view.into_inner())
     }
 
+    pub fn resolve_change_id_prefix<'a>(
+        &'a self,
+        prefix: &HexPrefix,
+    ) -> PrefixResolution<Vec<IndexEntry<'a>>> {
+        // TODO: Create a persistent lookup from change id to (visible?) commit ids.
+        let mut found_change_id = None;
+        let mut found_entries = vec![];
+        let heads = self.view().heads().iter().cloned().collect_vec();
+        for entry in self.index().walk_revs(&heads, &[]) {
+            let change_id = entry.change_id();
+            if prefix.matches(&change_id) {
+                if let Some(previous_change_id) = found_change_id.replace(change_id.clone()) {
+                    if previous_change_id != change_id {
+                        return PrefixResolution::AmbiguousMatch;
+                    }
+                }
+                found_entries.push(entry);
+            }
+        }
+        if found_change_id.is_none() {
+            return PrefixResolution::NoMatch;
+        }
+        PrefixResolution::SingleMatch(found_entries)
+    }
+
     pub fn new_commit(
         &mut self,
         settings: &UserSettings,

lib/src/revset.rs

@@ -118,29 +118,17 @@ fn resolve_short_commit_id(
     }
 }
 
-fn resolve_change_id(
-    repo: RepoRef,
-    change_id_prefix: &str,
-) -> Result<Option<Vec<CommitId>>, RevsetError> {
-    if let Some(hex_prefix) = HexPrefix::new(change_id_prefix) {
-        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, None).unwrap().iter() {
-            let change_id = index_entry.change_id();
-            if hex_prefix.matches(&change_id) {
-                if let Some(previous_change_id) = found_change_id.replace(change_id.clone()) {
-                    if previous_change_id != change_id {
-                        return Err(RevsetError::AmbiguousIdPrefix(change_id_prefix.to_owned()));
-                    }
-                }
-                commit_ids.push(index_entry.commit_id());
+fn resolve_change_id(repo: RepoRef, symbol: &str) -> Result<Option<Vec<CommitId>>, RevsetError> {
+    if let Some(prefix) = HexPrefix::new(symbol) {
+        match repo.resolve_change_id_prefix(&prefix) {
+            PrefixResolution::NoMatch => Ok(None),
+            PrefixResolution::AmbiguousMatch => {
+                Err(RevsetError::AmbiguousIdPrefix(symbol.to_owned()))
+            }
+            PrefixResolution::SingleMatch(entries) => {
+                Ok(Some(entries.iter().map(|e| e.commit_id()).collect()))
             }
         }
-        if found_change_id.is_none() {
-            return Ok(None);
-        }
-        Ok(Some(commit_ids))
     } else {
         Ok(None)
     }

lib/tests/test_revset.rs

@@ -156,8 +156,9 @@ fn test_resolve_symbol_commit_id() {
     );
 }
 
-#[test]
-fn test_resolve_symbol_change_id() {
+#[test_case(false ; "mutable")]
+#[test_case(true ; "readonly")]
+fn test_resolve_symbol_change_id(readonly: bool) {
     let settings = testutils::user_settings();
     // Test only with git so we can get predictable change ids
     let test_repo = TestRepo::init(true);
@@ -196,7 +197,6 @@ fn test_resolve_symbol_change_id() {
 
     let mut tx = repo.start_transaction(&settings, "test");
     git::import_refs(tx.mut_repo(), &git_repo).unwrap();
-    let repo = tx.commit();
 
     // Test the test setup
     assert_eq!(
@@ -220,8 +220,15 @@ fn test_resolve_symbol_change_id() {
         "040031cb4ad0cbc3287914f1d205dabf4a7eb889"
     );
 
+    let repo;
+    let repo_ref = if readonly {
+        repo = tx.commit();
+        repo.as_repo_ref()
+    } else {
+        tx.repo().as_repo_ref()
+    };
+
     // Test lookup by full change id
-    let repo_ref = repo.as_repo_ref();
     assert_eq!(
         resolve_symbol(repo_ref, "04e12a5467bba790efb88a9870894ec2", None).unwrap(),
         vec![CommitId::from_hex(