From 0e82e52c3a8c6d2f56e459c086fc559170feafa7 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Fri, 6 Oct 2023 23:06:11 +0900
Subject: [PATCH] revset: remove extra step to resolve full commit id, use
 prefix matching

Since both has_id() and resolve_prefix() do binary search, their costs are
practically the same. I think has_id() would complete with fewer ops, but such
level of optimization wouldn't be needed here. More importantly, this ensures
that unreachable commits aren't imported by GitBackend::read_commit().
---
 lib/src/revset.rs | 27 +--------------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 06dfe52ea..ade867434 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -32,7 +32,7 @@ use pest::Parser;
 use pest_derive::Parser;
 use thiserror::Error;
 
-use crate::backend::{BackendError, BackendResult, ChangeId, CommitId, ObjectId};
+use crate::backend::{BackendError, BackendResult, ChangeId, CommitId};
 use crate::commit::Commit;
 use crate::git::{self, get_local_git_tracking_branch};
 use crate::hex_util::to_forward_hex;
@@ -2056,26 +2056,6 @@ fn make_no_such_symbol_error(repo: &dyn Repo, name: impl Into<String>) -> Revset
     RevsetResolutionError::NoSuchRevision { name, candidates }
 }
 
-fn resolve_full_commit_id(
-    repo: &dyn Repo,
-    symbol: &str,
-) -> Result<Option<Vec<CommitId>>, RevsetResolutionError> {
-    if let Ok(binary_commit_id) = hex::decode(symbol) {
-        if repo.store().commit_id_length() != binary_commit_id.len() {
-            return Ok(None);
-        }
-        let commit_id = CommitId::new(binary_commit_id);
-        match repo.store().get_commit(&commit_id) {
-            // Only recognize a commit if we have indexed it
-            Ok(_) if repo.index().has_id(&commit_id) => Ok(Some(vec![commit_id])),
-            Ok(_) | Err(BackendError::ObjectNotFound { .. }) => Ok(None),
-            Err(err) => Err(RevsetResolutionError::StoreError(err)),
-        }
-    } else {
-        Ok(None)
-    }
-}
-
 pub trait SymbolResolver {
     fn resolve_symbol(&self, symbol: &str) -> Result<Vec<CommitId>, RevsetResolutionError>;
 }
@@ -2153,11 +2133,6 @@ impl SymbolResolver for DefaultSymbolResolver<'_> {
             return Ok(ids);
         }
 
-        // Try to resolve as a full commit id.
-        if let Some(ids) = resolve_full_commit_id(self.repo, symbol)? {
-            return Ok(ids);
-        }
-
         // Try to resolve as a commit id.
         if let Some(prefix) = HexPrefix::new(symbol) {
             match (self.commit_id_resolver)(self.repo, &prefix) {