ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

revset: add "resolve" method to RevsetExpression, always call it

Browse source 
I'll make the resolution stage mandatory, and have it return a "resolved"
type. RevsetExpression::evaluate() will be moved to the "resolved" type.
This commit is contained in:
Yuya Nishihara 2023-04-07 19:31:32 +09:00
parent 6d9b836d10
commit 6c2525cb93
6 changed files with 68 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
lib/src/repo.rs
View file

@ -225,7 +225,11 @@ impl ReadonlyRepo {
let change_id_index: &'a (dyn ChangeIdIndex + 'a) = self
.change_id_index
.get_or_init(|| {
let revset: Box<dyn Revset<'a>> = RevsetExpression::all().evaluate(self).unwrap();
let revset: Box<dyn Revset<'a>> = RevsetExpression::all()
.resolve(self)
.unwrap()
.evaluate(self)
.unwrap();
let change_id_index: Box<dyn ChangeIdIndex + 'a> = revset.change_id_index();
// evaluate() above only borrows the index, not the whole repo
let change_id_index: Box<dyn ChangeIdIndex> =
@ -1158,13 +1162,21 @@ impl Repo for MutableRepo {
}
fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> {
let revset = RevsetExpression::all().evaluate(self).unwrap();
let revset = RevsetExpression::all()
.resolve(self)
.unwrap()
.evaluate(self)
.unwrap();
let change_id_index = revset.change_id_index();
change_id_index.resolve_prefix(prefix)
}
fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
let revset = RevsetExpression::all().evaluate(self).unwrap();
let revset = RevsetExpression::all()
.resolve(self)
.unwrap()
.evaluate(self)
.unwrap();
let change_id_index = revset.change_id_index();
change_id_index.shortest_unique_prefix_len(target_id)
}

17
lib/src/revset.rs
View file

@ -421,6 +421,21 @@ impl RevsetExpression {
Rc::new(RevsetExpression::Difference(self.clone(), other.clone()))
}
pub fn resolve(
self: Rc<Self>,
repo: &dyn Repo,
) -> Result<Rc<RevsetExpression>, RevsetResolutionError> {
resolve_symbols(repo, self, None)
}
pub fn resolve_in_workspace(
self: Rc<Self>,
repo: &dyn Repo,
workspace_ctx: &RevsetWorkspaceContext,
) -> Result<Rc<RevsetExpression>, RevsetResolutionError> {
resolve_symbols(repo, self, Some(workspace_ctx))
}
pub fn evaluate<'index>(
&self,
repo: &'index dyn Repo,
@ -1687,7 +1702,7 @@ fn resolve_commit_ref(
// TODO: Maybe return a new type (RevsetParameters?) instead of
// RevsetExpression. Then pass that to evaluate(), so it's clear which variants
// are allowed.
pub fn resolve_symbols(
fn resolve_symbols(
repo: &dyn Repo,
expression: Rc<RevsetExpression>,
workspace_ctx: Option<&RevsetWorkspaceContext>,

8
lib/src/rewrite.rs
View file

@ -167,13 +167,19 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
.parents()
.minus(&old_commits_expression);
let heads_to_add = heads_to_add_expression
.resolve(mut_repo)
.unwrap()
.evaluate(mut_repo)
.unwrap()
.iter()
.collect();
let to_visit_expression = old_commits_expression.descendants();
let to_visit_revset = to_visit_expression.evaluate(mut_repo).unwrap();
let to_visit_revset = to_visit_expression
.resolve(mut_repo)
.unwrap()
.evaluate(mut_repo)
.unwrap();
let to_visit: Vec<_> = to_visit_revset.iter().commits(store).try_collect().unwrap();
drop(to_visit_revset);
let to_visit_set: HashSet<CommitId> =

22
lib/tests/test_revset.rs
View file

@ -24,9 +24,9 @@ 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_symbol, resolve_symbols, ReverseRevsetGraphIterator, Revset,
RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge,
RevsetResolutionError, RevsetWorkspaceContext,
optimize, parse, resolve_symbol, ReverseRevsetGraphIterator, Revset, RevsetAliasesMap,
RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge, RevsetResolutionError,
RevsetWorkspaceContext,
};
use jujutsu_lib::settings::GitSettings;
use jujutsu_lib::tree::merge_trees;
@ -41,6 +41,8 @@ fn revset_for_commits<'index>(
commits: &[&Commit],
) -> Box<dyn Revset<'index> + 'index> {
RevsetExpression::commits(commits.iter().map(|commit| commit.id().clone()).collect())
.resolve(repo)
.unwrap()
.evaluate(repo)
.unwrap()
}
@ -172,7 +174,7 @@ fn test_resolve_symbol_commit_id() {
// Test present() suppresses only NoSuchRevision error
assert_eq!(resolve_commit_ids(repo.as_ref(), "present(foo)"), []);
assert_matches!(
resolve_symbols(repo.as_ref(), optimize(parse("present(04)", &RevsetAliasesMap::new(), None).unwrap()), None),
optimize(parse("present(04)", &RevsetAliasesMap::new(), None).unwrap()).resolve(repo.as_ref()),
Err(RevsetResolutionError::AmbiguousIdPrefix(s)) if s == "04"
);
assert_eq!(
@ -502,7 +504,7 @@ fn test_resolve_symbol_git_refs() {
fn resolve_commit_ids(repo: &dyn Repo, revset_str: &str) -> Vec<CommitId> {
let expression = optimize(parse(revset_str, &RevsetAliasesMap::new(), None).unwrap());
let expression = resolve_symbols(repo, expression, None).unwrap();
let expression = expression.resolve(repo).unwrap();
expression.evaluate(repo).unwrap().iter().collect()
}
@ -519,7 +521,9 @@ fn resolve_commit_ids_in_workspace(
};
let expression =
optimize(parse(revset_str, &RevsetAliasesMap::new(), Some(&workspace_ctx)).unwrap());
let expression = resolve_symbols(repo, expression, Some(&workspace_ctx)).unwrap();
let expression = expression
.resolve_in_workspace(repo, &workspace_ctx)
.unwrap();
expression.evaluate(repo).unwrap().iter().collect()
}
@ -2125,7 +2129,11 @@ fn test_evaluate_expression_file(use_git: bool) {
let mut_repo = &*mut_repo;
let expression =
RevsetExpression::filter(RevsetFilterPredicate::File(Some(vec![file_path.clone()])));
let revset = expression.evaluate(mut_repo).unwrap();
let revset = expression
.resolve(mut_repo)
.unwrap()
.evaluate(mut_repo)
.unwrap();
revset.iter().collect()
};

12
src/cli_util.rs
View file

@ -44,9 +44,8 @@ use jujutsu_lib::repo::{
};
use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};
use jujutsu_lib::revset::{
resolve_symbols, Revset, RevsetAliasesMap, RevsetEvaluationError, RevsetExpression,
RevsetIteratorExt, RevsetParseError, RevsetParseErrorKind, RevsetResolutionError,
RevsetWorkspaceContext,
Revset, RevsetAliasesMap, RevsetEvaluationError, RevsetExpression, RevsetIteratorExt,
RevsetParseError, RevsetParseErrorKind, RevsetResolutionError, RevsetWorkspaceContext,
};
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::transaction::Transaction;
@ -873,11 +872,8 @@ impl WorkspaceCommandHelper {
&'repo self,
revset_expression: Rc<RevsetExpression>,
) -> Result<Box<dyn Revset<'repo> + 'repo>, CommandError> {
let revset_expression = resolve_symbols(
self.repo.as_ref(),
revset_expression,
Some(&self.revset_context()),
)?;
let revset_expression =
revset_expression.resolve_in_workspace(self.repo.as_ref(), &self.revset_context())?;
Ok(revset_expression.evaluate(self.repo.as_ref())?)
}

12
src/commands/mod.rs
View file

@ -2057,6 +2057,7 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
let new_parents = new_children.parents();
if let Some(commit_id) = new_children
.dag_range_to(&new_parents)
.resolve(tx.repo())?
.evaluate(tx.repo())?
.iter()
.next()
@ -2068,6 +2069,7 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
)));
}
let mut new_parents_commits: Vec<Commit> = new_parents
.resolve(tx.repo())?
.evaluate(tx.repo())?
.iter()
.commits(tx.repo().store())
@ -2115,6 +2117,7 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
// Exclude children that are ancestors of the new commit
let to_rebase = old_parents.children().minus(&old_parents.ancestors());
let commits_to_rebase: Vec<Commit> = to_rebase
.resolve(tx.base_repo().as_ref())?
.evaluate(tx.base_repo().as_ref())?
.iter()
.commits(tx.base_repo().store())
@ -2125,6 +2128,7 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
RevsetExpression::commits(child_commit.parent_ids().to_owned());
let new_parents = commit_parents.minus(&old_parents);
let mut new_parent_commits: Vec<Commit> = new_parents
.resolve(tx.base_repo().as_ref())?
.evaluate(tx.base_repo().as_ref())?
.iter()
.commits(tx.base_repo().store())
@ -2885,6 +2889,8 @@ fn rebase_branch(
.range(&RevsetExpression::commits(branch_commit_ids))
.roots();
let root_commits: IndexSet<_> = roots_expression
.resolve(workspace_command.repo().as_ref())
.unwrap()
.evaluate(workspace_command.repo().as_ref())
.unwrap()
.iter()
@ -2936,6 +2942,8 @@ fn rebase_revision(
check_rebase_destinations(workspace_command.repo(), new_parents, &old_commit)?;
let children_expression = RevsetExpression::commit(old_commit.id().clone()).children();
let child_commits: Vec<_> = children_expression
.resolve(workspace_command.repo().as_ref())
.unwrap()
.evaluate(workspace_command.repo().as_ref())
.unwrap()
.iter()
@ -2975,6 +2983,8 @@ fn rebase_revision(
.ancestors(),
);
let new_child_parents: Vec<Commit> = new_child_parents_expression
.resolve(tx.base_repo().as_ref())
.unwrap()
.evaluate(tx.base_repo().as_ref())
.unwrap()
.iter()
@ -3198,7 +3208,7 @@ fn cmd_debug_revset(
writeln!(ui, "{expression:#?}")?;
writeln!(ui)?;
let expression = revset::resolve_symbols(repo, expression, Some(&workspace_ctx))?;
let expression = expression.resolve_in_workspace(repo, &workspace_ctx)?;
writeln!(ui, "-- Resolved:")?;
writeln!(ui, "{expression:#?}")?;
writeln!(ui)?;