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

revsets: remove all_heads()

Browse source 
Now that we remove hidden heads whenever a transaction commits,
`non_obsolete_heads()` should always be the same as `all_heads()`,
except during a transaction. I don't think we depend on the difference
even during a transaction. Let's simplify a bit by removing the revset
function `all_heads()` and renaming `non_obsolete_heads()` to
`heads()`. This is part of issue #32.
This commit is contained in:
Martin von Zweigbergk 2021-09-25 14:36:34 -07:00
parent 09a33ec212
commit 9ec27645cf
4 changed files with 54 additions and 155 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

95
lib/src/revset.rs
View file

@ -95,10 +95,7 @@ fn resolve_commit_id(repo: RepoRef, symbol: &str) -> Result<Vec<CommitId>, Revse
Err(RevsetError::NoSuchRevision(symbol.to_owned()))
}
fn resolve_non_obsolete_change_id(
repo: RepoRef,
change_id_prefix: &str,
) -> Result<Vec<CommitId>, RevsetError> {
fn resolve_change_id(repo: RepoRef, change_id_prefix: &str) -> Result<Vec<CommitId>, RevsetError> {
if let Some(hex_prefix) = HexPrefix::new(change_id_prefix.to_owned()) {
let evolution = repo.evolution();
match evolution.resolve_change_id_prefix(&hex_prefix) {
@ -146,8 +143,8 @@ pub fn resolve_symbol(repo: RepoRef, symbol: &str) -> Result<Vec<CommitId>, Revs
return commit_id_result;
}
// Try to resolve as a change id (the non-obsolete commits in the change).
let change_id_result = resolve_non_obsolete_change_id(repo, symbol);
// Try to resolve as a change id.
let change_id_result = resolve_change_id(repo, symbol);
if !matches!(change_id_result, Err(RevsetError::NoSuchRevision(_))) {
return change_id_result;
}
@ -191,7 +188,7 @@ pub enum RevsetExpression {
roots: Rc<RevsetExpression>,
heads: Rc<RevsetExpression>,
},
AllHeads,
Heads,
PublicHeads,
Branches,
Tags,
@ -215,6 +212,10 @@ impl RevsetExpression {
Rc::new(RevsetExpression::None)
}
pub fn all() -> Rc<RevsetExpression> {
RevsetExpression::heads().ancestors()
}
pub fn symbol(value: String) -> Rc<RevsetExpression> {
Rc::new(RevsetExpression::Symbol(value))
}
@ -227,8 +228,8 @@ impl RevsetExpression {
Rc::new(RevsetExpression::Commits(commit_ids))
}
pub fn all_heads() -> Rc<RevsetExpression> {
Rc::new(RevsetExpression::AllHeads)
pub fn heads() -> Rc<RevsetExpression> {
Rc::new(RevsetExpression::Heads)
}
pub fn public_heads() -> Rc<RevsetExpression> {
@ -247,14 +248,6 @@ impl RevsetExpression {
Rc::new(RevsetExpression::GitRefs)
}
pub fn all_non_obsolete_heads() -> Rc<RevsetExpression> {
RevsetExpression::all_heads().non_obsolete_heads()
}
pub fn all_non_obsolete_commits() -> Rc<RevsetExpression> {
RevsetExpression::all_non_obsolete_heads().ancestors()
}
/// Non-obsolete heads among `self`.
pub fn non_obsolete_heads(self: &Rc<RevsetExpression>) -> Rc<RevsetExpression> {
Rc::new(RevsetExpression::NonObsoleteHeads(self.clone()))
@ -411,7 +404,7 @@ fn parse_range_expression_rule(
if let Some(next) = pairs.next() {
match next.as_rule() {
Rule::descendants_op => {
expression = expression.descendants(&RevsetExpression::all_non_obsolete_heads());
expression = expression.descendants(&RevsetExpression::heads());
}
Rule::dag_range_op => {
let heads_expression =
@ -438,7 +431,7 @@ fn parse_children_expression_rule(
for operator in pairs {
match operator.as_rule() {
Rule::children_op => {
expression = expression.children(&RevsetExpression::all_non_obsolete_heads());
expression = expression.children(&RevsetExpression::heads());
}
_ => {
panic!(
@ -524,7 +517,7 @@ fn parse_function_expression(
if arg_count == 1 {
let expression =
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?;
Ok(expression.children(&RevsetExpression::all_non_obsolete_heads()))
Ok(expression.children(&RevsetExpression::heads()))
} else {
Err(RevsetParseError::InvalidFunctionArguments {
name,
@ -546,7 +539,7 @@ fn parse_function_expression(
if arg_count == 1 {
let expression =
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?;
Ok(expression.descendants(&RevsetExpression::all_non_obsolete_heads()))
Ok(expression.descendants(&RevsetExpression::heads()))
} else {
Err(RevsetParseError::InvalidFunctionArguments {
name,
@ -554,9 +547,9 @@ fn parse_function_expression(
})
}
}
"all_heads" => {
"heads" => {
if arg_count == 0 {
Ok(RevsetExpression::all_heads())
Ok(RevsetExpression::heads())
} else {
Err(RevsetParseError::InvalidFunctionArguments {
name,
@ -564,21 +557,6 @@ fn parse_function_expression(
})
}
}
"non_obsolete_heads" => {
if arg_count == 0 {
Ok(RevsetExpression::all_non_obsolete_heads())
} else if arg_count == 1 {
Ok(
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?
.non_obsolete_heads(),
)
} else {
Err(RevsetParseError::InvalidFunctionArguments {
name,
message: "Expected 0 or 1 argument".to_string(),
})
}
}
"public_heads" => {
if arg_count == 0 {
Ok(RevsetExpression::public_heads())
@ -627,7 +605,7 @@ fn parse_function_expression(
});
}
let candidates = if arg_count == 0 {
RevsetExpression::all_non_obsolete_commits()
RevsetExpression::all()
} else {
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?
};
@ -645,7 +623,7 @@ fn parse_function_expression(
argument_pairs.next().unwrap().into_inner(),
)?;
let candidates = if arg_count == 1 {
RevsetExpression::all_non_obsolete_commits()
RevsetExpression::all()
} else {
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?
};
@ -1042,7 +1020,7 @@ pub fn evaluate_expression<'repo>(
index_entries: result,
}))
}
RevsetExpression::AllHeads => {
RevsetExpression::Heads => {
let index = repo.index();
let heads = repo.view().heads();
let mut index_entries = heads
@ -1240,10 +1218,6 @@ mod tests {
heads: checkout_symbol.clone()
})
);
assert_eq!(
foo_symbol.non_obsolete_heads(),
Rc::new(RevsetExpression::NonObsoleteHeads(foo_symbol.clone()))
);
assert_eq!(
foo_symbol.with_parent_count(3..5),
Rc::new(RevsetExpression::ParentCount {
@ -1299,14 +1273,14 @@ mod tests {
// Parse the "children" operator
assert_eq!(
parse("@:"),
Ok(checkout_symbol.children(&RevsetExpression::all_non_obsolete_heads()))
Ok(checkout_symbol.children(&RevsetExpression::heads()))
);
// Parse the "ancestors" operator
assert_eq!(parse(",,@"), Ok(checkout_symbol.ancestors()));
// Parse the "descendants" operator
assert_eq!(
parse("@,,"),
Ok(checkout_symbol.descendants(&RevsetExpression::all_non_obsolete_heads()))
Ok(checkout_symbol.descendants(&RevsetExpression::heads()))
);
// Parse the "dag range" operator
assert_eq!(parse("foo,,bar"), Ok(foo_symbol.descendants(&bar_symbol)));
@ -1326,11 +1300,11 @@ mod tests {
assert_matches!(parse("foo | :"), Err(RevsetParseError::SyntaxError(_)));
// Space is allowed around infix operators and function arguments
assert_eq!(
parse(" description( arg1 , arg2 ) - parents( arg1 ) - all_heads( ) "),
parse(" description( arg1 , arg2 ) - parents( arg1 ) - heads( ) "),
Ok(RevsetExpression::symbol("arg2".to_string())
.with_description("arg1".to_string())
.minus(&RevsetExpression::symbol("arg1".to_string()).parents())
.minus(&RevsetExpression::all_heads()))
.minus(&RevsetExpression::heads()))
);
}
@ -1346,9 +1320,9 @@ mod tests {
assert_eq!(
parse("foo:::"),
Ok(foo_symbol
.children(&RevsetExpression::all_non_obsolete_heads())
.children(&RevsetExpression::all_non_obsolete_heads())
.children(&RevsetExpression::all_non_obsolete_heads()))
.children(&RevsetExpression::heads())
.children(&RevsetExpression::heads())
.children(&RevsetExpression::heads()))
);
// Parse repeated "ancestors"/"descendants"/"dag range" operators
assert_matches!(parse(",,foo,,"), Err(RevsetParseError::SyntaxError(_)));
@ -1361,21 +1335,15 @@ mod tests {
// The former bind more strongly.
assert_eq!(
parse(":foo:"),
Ok(foo_symbol
.parents()
.children(&RevsetExpression::all_non_obsolete_heads()))
Ok(foo_symbol.parents().children(&RevsetExpression::heads()))
);
assert_eq!(
parse(":foo,,"),
Ok(foo_symbol
.parents()
.descendants(&RevsetExpression::all_non_obsolete_heads()))
Ok(foo_symbol.parents().descendants(&RevsetExpression::heads()))
);
assert_eq!(
parse(",,foo:"),
Ok(foo_symbol
.children(&RevsetExpression::all_non_obsolete_heads())
.ancestors())
Ok(foo_symbol.children(&RevsetExpression::heads()).ancestors())
);
}
@ -1402,11 +1370,10 @@ mod tests {
Ok(RevsetExpression::symbol("bar".to_string()).with_description("foo".to_string()))
);
assert_eq!(
parse("description(all_heads(),bar)"),
parse("description(heads(),bar)"),
Err(RevsetParseError::InvalidFunctionArguments {
name: "description".to_string(),
message: "Expected function argument of type string, found: all_heads()"
.to_string()
message: "Expected function argument of type string, found: heads()".to_string()
})
);
assert_eq!(

3
lib/src/rewrite.rs
View file

@ -138,8 +138,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
let new_commits_expression =
RevsetExpression::commits(rewritten.values().flatten().cloned().collect());
let to_visit_expression =
old_commits_expression.descendants(&RevsetExpression::all_heads());
let to_visit_expression = old_commits_expression.descendants(&RevsetExpression::heads());
let to_visit_revset = to_visit_expression
.evaluate(mut_repo.as_repo_ref())
.unwrap();

105
lib/tests/test_revset.rs
View file

@ -496,18 +496,13 @@ fn test_evaluate_expression_children(use_git: bool) {
.set_parents(vec![commit1.id().clone()])
.write_to_repo(mut_repo);
let commit3 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit1.id().clone()])
.set_predecessors(vec![commit2.id().clone()])
.set_change_id(commit2.change_id().clone())
.set_parents(vec![commit2.id().clone()])
.write_to_repo(mut_repo);
let commit4 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit3.id().clone()])
.write_to_repo(mut_repo);
let commit5 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit1.id().clone()])
.write_to_repo(mut_repo);
let commit6 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit4.id().clone(), commit5.id().clone()])
let commit5 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit3.id().clone(), commit4.id().clone()])
.write_to_repo(mut_repo);
// Can find children of the root commit
@ -516,23 +511,17 @@ fn test_evaluate_expression_children(use_git: bool) {
vec![commit1.id().clone(), wc_commit.id().clone()]
);
// Children do not include hidden commits (commit2)
assert_eq!(
resolve_commit_ids(mut_repo.as_repo_ref(), &format!("{}:", commit1.id().hex())),
vec![commit5.id().clone(), commit3.id().clone()]
);
// Children of all commits in input are returned, including those already in the
// input set
assert_eq!(
resolve_commit_ids(
mut_repo.as_repo_ref(),
&format!("({} | {}):", commit1.id().hex(), commit3.id().hex())
&format!("({} | {}):", commit1.id().hex(), commit2.id().hex())
),
vec![
commit5.id().clone(),
commit4.id().clone(),
commit3.id().clone()
commit3.id().clone(),
commit2.id().clone()
]
);
@ -540,9 +529,9 @@ fn test_evaluate_expression_children(use_git: bool) {
assert_eq!(
resolve_commit_ids(
mut_repo.as_repo_ref(),
&format!("({} | {}):", commit4.id().hex(), commit5.id().hex())
&format!("({} | {}):", commit3.id().hex(), commit4.id().hex())
),
vec![commit6.id().clone()]
vec![commit5.id().clone()]
);
tx.discard();
@ -740,29 +729,23 @@ fn test_evaluate_expression_descendants(use_git: bool) {
.set_parents(vec![commit1.id().clone()])
.write_to_repo(mut_repo);
let commit3 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit1.id().clone()])
.set_predecessors(vec![commit2.id().clone()])
.set_change_id(commit2.change_id().clone())
.set_parents(vec![commit2.id().clone()])
.write_to_repo(mut_repo);
let commit4 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit3.id().clone()])
.write_to_repo(mut_repo);
let commit5 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit1.id().clone()])
.write_to_repo(mut_repo);
let commit6 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit4.id().clone(), commit5.id().clone()])
let commit5 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit3.id().clone(), commit4.id().clone()])
.write_to_repo(mut_repo);
// The descendants of the root commit is all the non-hidden commits in the repo
// (commit2 is excluded)
// The descendants of the root commit are all the commits in the repo
assert_eq!(
resolve_commit_ids(mut_repo.as_repo_ref(), "root,,"),
vec![
commit6.id().clone(),
commit5.id().clone(),
commit4.id().clone(),
commit3.id().clone(),
commit2.id().clone(),
commit1.id().clone(),
wc_commit.id().clone(),
root_commit.id().clone(),
@ -771,11 +754,11 @@ fn test_evaluate_expression_descendants(use_git: bool) {
// Can find descendants of a specific commit
assert_eq!(
resolve_commit_ids(mut_repo.as_repo_ref(), &format!("{},,", commit3.id().hex())),
resolve_commit_ids(mut_repo.as_repo_ref(), &format!("{},,", commit2.id().hex())),
vec![
commit6.id().clone(),
commit4.id().clone(),
commit5.id().clone(),
commit3.id().clone(),
commit2.id().clone(),
]
);
@ -784,7 +767,7 @@ fn test_evaluate_expression_descendants(use_git: bool) {
#[test_case(false ; "local backend")]
#[test_case(true ; "git backend")]
fn test_evaluate_expression_all_heads(use_git: bool) {
fn test_evaluate_expression_heads(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
@ -796,7 +779,7 @@ fn test_evaluate_expression_all_heads(use_git: bool) {
let commit2 = graph_builder.commit_with_parents(&[&commit1]);
assert_eq!(
resolve_commit_ids(mut_repo.as_repo_ref(), "all_heads()"),
resolve_commit_ids(mut_repo.as_repo_ref(), "heads()"),
vec![commit2.id().clone(), wc_commit.id().clone()]
);
@ -907,53 +890,6 @@ fn test_evaluate_expression_git_refs(use_git: bool) {
tx.discard();
}
#[test_case(false ; "local backend")]
#[test_case(true ; "git backend")]
fn test_evaluate_expression_obsolete(use_git: bool) {
let settings = testutils::user_settings();
let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
let mut tx = repo.start_transaction("test");
let mut_repo = tx.mut_repo();
let root_commit = repo.store().root_commit();
let wc_commit = repo.working_copy_locked().current_commit();
let commit1 = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo);
let commit2 = testutils::create_random_commit(&settings, &repo)
.set_predecessors(vec![commit1.id().clone()])
.set_change_id(commit1.change_id().clone())
.write_to_repo(mut_repo);
let commit3 = testutils::create_random_commit(&settings, &repo)
.set_predecessors(vec![commit2.id().clone()])
.set_change_id(commit2.change_id().clone())
.write_to_repo(mut_repo);
let commit4 = testutils::create_random_commit(&settings, &repo)
.set_parents(vec![commit3.id().clone()])
.set_pruned(true)
.write_to_repo(mut_repo);
assert_eq!(
resolve_commit_ids(mut_repo.as_repo_ref(), "non_obsolete_heads()"),
vec![commit3.id().clone(), wc_commit.id().clone()]
);
assert_eq!(
resolve_commit_ids(
mut_repo.as_repo_ref(),
&format!("non_obsolete_heads({})", commit4.id().hex())
),
vec![commit3.id().clone()]
);
assert_eq!(
resolve_commit_ids(
mut_repo.as_repo_ref(),
&format!("non_obsolete_heads({})", commit1.id().hex())
),
vec![root_commit.id().clone()]
);
tx.discard();
}
#[test_case(false ; "local backend")]
#[test_case(true ; "git backend")]
fn test_evaluate_expression_merges(use_git: bool) {
@ -1023,10 +959,7 @@ fn test_evaluate_expression_description(use_git: bool) {
);
// Searches only among candidates if specified
assert_eq!(
resolve_commit_ids(
mut_repo.as_repo_ref(),
"description(\"commit 2\",all_heads())"
),
resolve_commit_ids(mut_repo.as_repo_ref(), "description(\"commit 2\",heads())"),
vec![]
);

6
src/commands.rs
View file

@ -691,7 +691,7 @@ With the `--from` and/or `--to` options, shows the difference from/to the given
.long("revisions")
.short("r")
.takes_value(true)
.default_value(",,non_obsolete_heads()")
.default_value(",,heads()")
.help("Which revisions to show"),
)
.arg(
@ -2543,8 +2543,8 @@ fn cmd_rebase(
// Manually rebase children because we don't want to rebase them onto the
// rewritten commit. (But we still want to record the commit as rewritten so
// branches and the working copy get updated to the rewritten commit.)
let children_expression = RevsetExpression::commit(old_commit.id().clone())
.children(&RevsetExpression::all_non_obsolete_heads());
let children_expression =
RevsetExpression::commit(old_commit.id().clone()).children(&RevsetExpression::heads());
let mut num_rebased_descendants = 0;
let store = repo_command.repo.store();
for child_index_entry in children_expression