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

op_walk: don't resolve heads beyond @ operation

Browse source 
Since `jj undo --at-op=OP @` resolves @ to OP, I think OP should be the head
in that context, and the descendants of OP shouldn't be accessible by @+.
This commit is contained in:
Yuya Nishihara 2024-01-10 19:35:08 +09:00
parent ba42b37a67
commit 83ede241e3
2 changed files with 37 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
lib/src/op_walk.rs
View file

@ -78,7 +78,8 @@ pub fn resolve_op_for_load(
Err(OpsetResolutionError::MultipleOperations("@".to_owned()).into())
})
};
resolve_single_op(op_store, op_heads_store, get_current_op, op_str)
let get_head_ops = || get_current_head_ops(op_store, op_heads_store);
resolve_single_op(op_store, get_current_op, get_head_ops, op_str)
}
/// Resolves operation set expression against the loaded repo.
@ -89,25 +90,22 @@ pub fn resolve_op_with_repo(
op_str: &str,
) -> Result<Operation, OpsetEvaluationError> {
let op_store = repo.op_store();
let op_heads_store = repo.op_heads_store().as_ref();
let get_current_op = || Ok(repo.operation().clone());
resolve_single_op(op_store, op_heads_store, get_current_op, op_str)
let get_head_ops = || Ok(vec![repo.operation().clone()]);
resolve_single_op(op_store, get_current_op, get_head_ops, op_str)
}
/// Resolves operation set expression with the given "@" symbol resolution
/// callback.
/// callbacks.
fn resolve_single_op(
op_store: &Arc<dyn OpStore>,
op_heads_store: &dyn OpHeadsStore,
get_current_op: impl FnOnce() -> Result<Operation, OpsetEvaluationError>,
get_head_ops: impl FnOnce() -> OpStoreResult<Vec<Operation>>,
op_str: &str,
) -> Result<Operation, OpsetEvaluationError> {
let op_symbol = op_str.trim_end_matches(['-', '+']);
let op_postfix = &op_str[op_symbol.len()..];
let head_ops = op_postfix
.contains('+')
.then(|| get_current_head_ops(op_store, op_heads_store))
.transpose()?;
let head_ops = op_postfix.contains('+').then(get_head_ops).transpose()?;
let mut operation = match op_symbol {
"@" => get_current_op(),
s => resolve_single_op_from_store(op_store, s),

47
lib/tests/test_operations.rs
View file

@ -501,28 +501,29 @@ fn test_resolve_op_parents_children() {
// Use monotonic timestamp to stabilize merge order of transactions
let settings = testutils::user_settings();
let test_repo = TestRepo::init_with_settings(&settings);
let mut repo = test_repo.repo;
let mut repo = &test_repo.repo;
let mut operations = Vec::new();
let mut repos = Vec::new();
for _ in 0..3 {
let tx = repo.start_transaction(&settings);
repo = tx.commit("test");
operations.push(repo.operation().clone());
repos.push(tx.commit("test"));
repo = repos.last().unwrap();
}
let operations = repos.iter().map(|repo| repo.operation()).collect_vec();
// Parent
let op2_id_hex = operations[2].id().hex();
assert_eq!(
op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}-")).unwrap(),
operations[1]
op_walk::resolve_op_with_repo(repo, &format!("{op2_id_hex}-")).unwrap(),
*operations[1]
);
assert_eq!(
op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}--")).unwrap(),
operations[0]
op_walk::resolve_op_with_repo(repo, &format!("{op2_id_hex}--")).unwrap(),
*operations[0]
);
// "{op2_id_hex}---" is the operation to initialize the repo.
assert_matches!(
op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}----")),
op_walk::resolve_op_with_repo(repo, &format!("{op2_id_hex}----")),
Err(OpsetEvaluationError::OpsetResolution(
OpsetResolutionError::EmptyOperations(_)
))
@ -531,15 +532,15 @@ fn test_resolve_op_parents_children() {
// Child
let op0_id_hex = operations[0].id().hex();
assert_eq!(
op_walk::resolve_op_with_repo(&repo, &format!("{op0_id_hex}+")).unwrap(),
operations[1]
op_walk::resolve_op_with_repo(repo, &format!("{op0_id_hex}+")).unwrap(),
*operations[1]
);
assert_eq!(
op_walk::resolve_op_with_repo(&repo, &format!("{op0_id_hex}++")).unwrap(),
operations[2]
op_walk::resolve_op_with_repo(repo, &format!("{op0_id_hex}++")).unwrap(),
*operations[2]
);
assert_matches!(
op_walk::resolve_op_with_repo(&repo, &format!("{op0_id_hex}+++")),
op_walk::resolve_op_with_repo(repo, &format!("{op0_id_hex}+++")),
Err(OpsetEvaluationError::OpsetResolution(
OpsetResolutionError::EmptyOperations(_)
))
@ -547,14 +548,26 @@ fn test_resolve_op_parents_children() {
// Child of parent
assert_eq!(
op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}--+")).unwrap(),
operations[1]
op_walk::resolve_op_with_repo(repo, &format!("{op2_id_hex}--+")).unwrap(),
*operations[1]
);
// Child at old repo: new operations shouldn't be visible
assert_eq!(
op_walk::resolve_op_with_repo(&repos[1], &format!("{op0_id_hex}+")).unwrap(),
*operations[1]
);
assert_matches!(
op_walk::resolve_op_with_repo(&repos[0], &format!("{op0_id_hex}+")),
Err(OpsetEvaluationError::OpsetResolution(
OpsetResolutionError::EmptyOperations(_)
))
);
// Merge and fork
let tx1 = repo.start_transaction(&settings);
let tx2 = repo.start_transaction(&settings);
repo = testutils::commit_transactions(&settings, vec![tx1, tx2]);
let repo = testutils::commit_transactions(&settings, vec![tx1, tx2]);
let op5_id_hex = repo.operation().id().hex();
assert_matches!(
op_walk::resolve_op_with_repo(&repo, &format!("{op5_id_hex}-")),