From d971148e4e306b722037dd8ab2d4582807a57fab Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Thu, 16 Mar 2023 23:10:09 -0700 Subject: [PATCH] revset: move resolve_symbol() back to revset module The only caller is now in `revset.rs`. --- lib/src/default_revset_engine.rs | 148 +------------------------------ lib/src/revset.rs | 146 +++++++++++++++++++++++++++++- lib/tests/test_revset.rs | 6 +- 3 files changed, 149 insertions(+), 151 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 4026ab074..7bb4abed1 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -18,14 +18,11 @@ use std::iter::Peekable; use itertools::Itertools; -use crate::backend::{BackendError, CommitId, ObjectId}; +use crate::backend::CommitId; use crate::commit::Commit; use crate::default_index_store::IndexEntry; use crate::default_revset_graph_iterator::RevsetGraphIterator; -use crate::hex_util::to_forward_hex; -use crate::index::{HexPrefix, PrefixResolution}; use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher}; -use crate::op_store::WorkspaceId; use crate::repo::Repo; use crate::revset::{ Revset, RevsetError, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge, @@ -33,147 +30,6 @@ use crate::revset::{ }; use crate::rewrite; -fn resolve_git_ref(repo: &dyn Repo, symbol: &str) -> Option> { - let view = repo.view(); - for git_ref_prefix in &["", "refs/", "refs/heads/", "refs/tags/", "refs/remotes/"] { - if let Some(ref_target) = view.git_refs().get(&(git_ref_prefix.to_string() + symbol)) { - return Some(ref_target.adds()); - } - } - None -} - -fn resolve_branch(repo: &dyn Repo, symbol: &str) -> Option> { - if let Some(branch_target) = repo.view().branches().get(symbol) { - return Some( - branch_target - .local_target - .as_ref() - .map(|target| target.adds()) - .unwrap_or_default(), - ); - } - if let Some((name, remote_name)) = symbol.split_once('@') { - if let Some(branch_target) = repo.view().branches().get(name) { - if let Some(target) = branch_target.remote_targets.get(remote_name) { - return Some(target.adds()); - } - } - } - None -} - -fn resolve_full_commit_id( - repo: &dyn Repo, - symbol: &str, -) -> Result>, RevsetError> { - 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().entry_by_id(&commit_id).is_some() => Ok(Some(vec![commit_id])), - Ok(_) | Err(BackendError::ObjectNotFound { .. }) => Ok(None), - Err(err) => Err(RevsetError::StoreError(err)), - } - } else { - Ok(None) - } -} - -fn resolve_short_commit_id( - repo: &dyn Repo, - symbol: &str, -) -> Result>, RevsetError> { - if let Some(prefix) = HexPrefix::new(symbol) { - match repo.index().resolve_prefix(&prefix) { - PrefixResolution::NoMatch => Ok(None), - PrefixResolution::AmbiguousMatch => { - Err(RevsetError::AmbiguousIdPrefix(symbol.to_owned())) - } - PrefixResolution::SingleMatch(commit_id) => Ok(Some(vec![commit_id])), - } - } else { - Ok(None) - } -} - -fn resolve_change_id(repo: &dyn Repo, symbol: &str) -> Result>, RevsetError> { - if let Some(prefix) = to_forward_hex(symbol).as_deref().and_then(HexPrefix::new) { - 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())) - } - } - } else { - Ok(None) - } -} - -pub fn resolve_symbol( - repo: &dyn Repo, - symbol: &str, - workspace_id: Option<&WorkspaceId>, -) -> Result, RevsetError> { - if symbol.ends_with('@') { - let target_workspace = if symbol == "@" { - if let Some(workspace_id) = workspace_id { - workspace_id.clone() - } else { - return Err(RevsetError::NoSuchRevision(symbol.to_owned())); - } - } else { - WorkspaceId::new(symbol.strip_suffix('@').unwrap().to_string()) - }; - if let Some(commit_id) = repo.view().get_wc_commit_id(&target_workspace) { - Ok(vec![commit_id.clone()]) - } else { - Err(RevsetError::NoSuchRevision(symbol.to_owned())) - } - } else if symbol == "root" { - Ok(vec![repo.store().root_commit_id().clone()]) - } else { - // Try to resolve as a tag - if let Some(target) = repo.view().tags().get(symbol) { - return Ok(target.adds()); - } - - // Try to resolve as a branch - if let Some(ids) = resolve_branch(repo, symbol) { - return Ok(ids); - } - - // Try to resolve as a git ref - if let Some(ids) = resolve_git_ref(repo, symbol) { - return Ok(ids); - } - - // Try to resolve as a full commit id. We assume a full commit id is unambiguous - // even if it's shorter than change id. - if let Some(ids) = resolve_full_commit_id(repo, symbol)? { - return Ok(ids); - } - - // Try to resolve as a commit id. - if let Some(ids) = resolve_short_commit_id(repo, symbol)? { - return Ok(ids); - } - - // Try to resolve as a change id. - if let Some(ids) = resolve_change_id(repo, symbol)? { - return Ok(ids); - } - - Err(RevsetError::NoSuchRevision(symbol.to_owned())) - } -} - trait ToPredicateFn<'index> { /// Creates function that tests if the given entry is included in the set. /// @@ -852,7 +708,7 @@ fn has_diff_from_parent(repo: &dyn Repo, entry: &IndexEntry<'_>, matcher: &dyn M #[cfg(test)] mod tests { use super::*; - use crate::backend::{ChangeId, CommitId}; + use crate::backend::{ChangeId, CommitId, ObjectId}; use crate::default_index_store::MutableIndexImpl; use crate::index::Index; diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 18ddc3410..0973fbb98 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -27,10 +27,11 @@ use pest::Parser; use pest_derive::Parser; use thiserror::Error; -use crate::backend::{BackendError, BackendResult, CommitId}; +use crate::backend::{BackendError, BackendResult, CommitId, ObjectId}; use crate::commit::Commit; use crate::default_index_store::{IndexEntry, IndexPosition}; -use crate::default_revset_engine::resolve_symbol; +use crate::hex_util::to_forward_hex; +use crate::index::{HexPrefix, PrefixResolution}; use crate::op_store::WorkspaceId; use crate::repo::Repo; use crate::repo_path::{FsPathParseError, RepoPath}; @@ -1391,6 +1392,147 @@ pub fn optimize(expression: Rc) -> Rc { fold_difference(&expression).unwrap_or(expression) } +fn resolve_git_ref(repo: &dyn Repo, symbol: &str) -> Option> { + let view = repo.view(); + for git_ref_prefix in &["", "refs/", "refs/heads/", "refs/tags/", "refs/remotes/"] { + if let Some(ref_target) = view.git_refs().get(&(git_ref_prefix.to_string() + symbol)) { + return Some(ref_target.adds()); + } + } + None +} + +fn resolve_branch(repo: &dyn Repo, symbol: &str) -> Option> { + if let Some(branch_target) = repo.view().branches().get(symbol) { + return Some( + branch_target + .local_target + .as_ref() + .map(|target| target.adds()) + .unwrap_or_default(), + ); + } + if let Some((name, remote_name)) = symbol.split_once('@') { + if let Some(branch_target) = repo.view().branches().get(name) { + if let Some(target) = branch_target.remote_targets.get(remote_name) { + return Some(target.adds()); + } + } + } + None +} + +fn resolve_full_commit_id( + repo: &dyn Repo, + symbol: &str, +) -> Result>, RevsetError> { + 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().entry_by_id(&commit_id).is_some() => Ok(Some(vec![commit_id])), + Ok(_) | Err(BackendError::ObjectNotFound { .. }) => Ok(None), + Err(err) => Err(RevsetError::StoreError(err)), + } + } else { + Ok(None) + } +} + +fn resolve_short_commit_id( + repo: &dyn Repo, + symbol: &str, +) -> Result>, RevsetError> { + if let Some(prefix) = HexPrefix::new(symbol) { + match repo.index().resolve_prefix(&prefix) { + PrefixResolution::NoMatch => Ok(None), + PrefixResolution::AmbiguousMatch => { + Err(RevsetError::AmbiguousIdPrefix(symbol.to_owned())) + } + PrefixResolution::SingleMatch(commit_id) => Ok(Some(vec![commit_id])), + } + } else { + Ok(None) + } +} + +fn resolve_change_id(repo: &dyn Repo, symbol: &str) -> Result>, RevsetError> { + if let Some(prefix) = to_forward_hex(symbol).as_deref().and_then(HexPrefix::new) { + 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())) + } + } + } else { + Ok(None) + } +} + +pub fn resolve_symbol( + repo: &dyn Repo, + symbol: &str, + workspace_id: Option<&WorkspaceId>, +) -> Result, RevsetError> { + if symbol.ends_with('@') { + let target_workspace = if symbol == "@" { + if let Some(workspace_id) = workspace_id { + workspace_id.clone() + } else { + return Err(RevsetError::NoSuchRevision(symbol.to_owned())); + } + } else { + WorkspaceId::new(symbol.strip_suffix('@').unwrap().to_string()) + }; + if let Some(commit_id) = repo.view().get_wc_commit_id(&target_workspace) { + Ok(vec![commit_id.clone()]) + } else { + Err(RevsetError::NoSuchRevision(symbol.to_owned())) + } + } else if symbol == "root" { + Ok(vec![repo.store().root_commit_id().clone()]) + } else { + // Try to resolve as a tag + if let Some(target) = repo.view().tags().get(symbol) { + return Ok(target.adds()); + } + + // Try to resolve as a branch + if let Some(ids) = resolve_branch(repo, symbol) { + return Ok(ids); + } + + // Try to resolve as a git ref + if let Some(ids) = resolve_git_ref(repo, symbol) { + return Ok(ids); + } + + // Try to resolve as a full commit id. We assume a full commit id is unambiguous + // even if it's shorter than change id. + if let Some(ids) = resolve_full_commit_id(repo, symbol)? { + return Ok(ids); + } + + // Try to resolve as a commit id. + if let Some(ids) = resolve_short_commit_id(repo, symbol)? { + return Ok(ids); + } + + // Try to resolve as a change id. + if let Some(ids) = resolve_change_id(repo, symbol)? { + return Ok(ids); + } + + Err(RevsetError::NoSuchRevision(symbol.to_owned())) + } +} + // TODO: Maybe return a new type (RevsetParameters?) instead of // RevsetExpression. Then pass that to evaluate(), so it's clear which variants // are allowed. diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index b2f5a6766..ea4089d87 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -17,14 +17,14 @@ use std::path::Path; use assert_matches::assert_matches; use itertools::Itertools; use jujutsu_lib::backend::{CommitId, MillisSinceEpoch, ObjectId, Signature, Timestamp}; -use jujutsu_lib::default_revset_engine::{resolve_symbol, revset_for_commits}; +use jujutsu_lib::default_revset_engine::revset_for_commits; use jujutsu_lib::git; use jujutsu_lib::op_store::{RefTarget, WorkspaceId}; use jujutsu_lib::repo::Repo; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::revset::{ - optimize, parse, resolve_symbols, ReverseRevsetGraphIterator, RevsetAliasesMap, RevsetError, - RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge, RevsetIteratorExt, + optimize, parse, resolve_symbol, resolve_symbols, ReverseRevsetGraphIterator, RevsetAliasesMap, + RevsetError, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge, RevsetIteratorExt, RevsetWorkspaceContext, }; use jujutsu_lib::settings::GitSettings;