Skip to content

Commit

Permalink
revset: remove extra step to resolve full commit id, use prefix matching
Browse files Browse the repository at this point in the history
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().
  • Loading branch information
yuja committed Oct 6, 2023
1 parent f0ad1f5 commit 0e82e52
Showing 1 changed file with 1 addition and 26 deletions.
27 changes: 1 addition & 26 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>;
}
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 0e82e52

Please sign in to comment.