diff --git a/cli/src/commands/operation/abandon.rs b/cli/src/commands/operation/abandon.rs index 5334835cbc..082c3e2abf 100644 --- a/cli/src/commands/operation/abandon.rs +++ b/cli/src/commands/operation/abandon.rs @@ -13,15 +13,14 @@ // limitations under the License. use std::io::Write as _; -use std::slice; +use std::{iter, slice}; use itertools::Itertools as _; -use jj_lib::op_store::OperationId; use jj_lib::op_walk; use jj_lib::operation::Operation; use crate::cli_util::{short_operation_hash, CommandHelper}; -use crate::command_error::{cli_error, user_error, user_error_with_hint, CommandError}; +use crate::command_error::{cli_error, user_error, CommandError}; use crate::ui::Ui; /// Abandon operation history @@ -51,15 +50,15 @@ pub fn cmd_op_abandon( let mut workspace = command.load_workspace()?; let repo_loader = workspace.repo_loader(); let op_store = repo_loader.op_store(); + let op_heads_store = repo_loader.op_heads_store(); // It doesn't make sense to create concurrent operations that will be merged // with the current head. if command.global_args().at_operation.is_some() { return Err(cli_error("--at-op is not respected")); } - let current_head_op = op_walk::resolve_op_for_load(repo_loader, "@")?; - let resolve_op = - |op_str| op_walk::resolve_op_at(op_store, slice::from_ref(¤t_head_op), op_str); - let (abandon_root_op, abandon_head_op) = + let current_head_ops = op_walk::get_current_head_ops(op_store, op_heads_store.as_ref())?; + let resolve_op = |op_str| op_walk::resolve_op_at(op_store, ¤t_head_ops, op_str); + let (abandon_root_op, abandon_head_ops) = if let Some((root_op_str, head_op_str)) = args.operation.split_once("..") { let root_op = if root_op_str.is_empty() { let id = op_store.root_operation_id(); @@ -68,12 +67,12 @@ pub fn cmd_op_abandon( } else { resolve_op(root_op_str)? }; - let head_op = if head_op_str.is_empty() { - current_head_op.clone() + let head_ops = if head_op_str.is_empty() { + current_head_ops.clone() } else { - resolve_op(head_op_str)? + vec![resolve_op(head_op_str)?] }; - (root_op, head_op) + (root_op, head_ops) } else { let op = resolve_op(&args.operation)?; let parent_ops: Vec<_> = op.parents().try_collect()?; @@ -82,25 +81,37 @@ pub fn cmd_op_abandon( 1 => parent_ops.into_iter().next().unwrap(), _ => return Err(user_error("Cannot abandon a merge operation")), }; - (parent_op, op) + (parent_op, vec![op]) }; - if abandon_head_op == current_head_op { - return Err(user_error_with_hint( - "Cannot abandon the current operation", - "Run `jj undo` to revert the current operation, then use `jj op abandon`", + if let Some(op) = abandon_head_ops + .iter() + .find(|op| current_head_ops.contains(op)) + { + let mut err = user_error(format!( + "Cannot abandon the current operation {}", + short_operation_hash(op.id()) )); + if current_head_ops.len() == 1 { + err.add_hint("Run `jj undo` to revert the current operation, then use `jj op abandon`"); + } + return Err(err); } // Reparent descendants, count the number of abandoned operations. let stats = op_walk::reparent_range( op_store.as_ref(), - slice::from_ref(&abandon_head_op), - slice::from_ref(¤t_head_op), + &abandon_head_ops, + ¤t_head_ops, &abandon_root_op, )?; - let [new_head_id]: [OperationId; 1] = stats.new_head_ids.try_into().unwrap(); - if current_head_op.id() == &new_head_id { + assert_eq!( + current_head_ops.len(), + stats.new_head_ids.len(), + "all current_head_ops should be reparented as they aren't included in abandon_head_ops" + ); + let reparented_head_ops = || iter::zip(¤t_head_ops, &stats.new_head_ids); + if reparented_head_ops().all(|(old, new_id)| old.id() == new_id) { writeln!(ui.status(), "Nothing changed.")?; return Ok(()); } @@ -110,23 +121,26 @@ pub fn cmd_op_abandon( stats.unreachable_count, stats.rewritten_count, )?; - repo_loader - .op_heads_store() - .update_op_heads(slice::from_ref(current_head_op.id()), &new_head_id); + for (old, new_id) in reparented_head_ops().filter(|&(old, new_id)| old.id() != new_id) { + op_heads_store.update_op_heads(slice::from_ref(old.id()), new_id); + } // Remap the operation id of the current workspace. If there were any // concurrent operations, user will need to re-abandon their ancestors. if !command.global_args().ignore_working_copy { let mut locked_ws = workspace.start_working_copy_mutation()?; let old_op_id = locked_ws.locked_wc().old_operation_id(); - if old_op_id != current_head_op.id() { + if let Some((_, new_id)) = reparented_head_ops().find(|(old, _)| old.id() == old_op_id) { + locked_ws.finish(new_id.clone())? + } else { writeln!( ui.warning_default(), "The working copy operation {} is not updated because it differs from the repo {}.", short_operation_hash(old_op_id), - short_operation_hash(current_head_op.id()), + current_head_ops + .iter() + .map(|op| short_operation_hash(op.id())) + .join(", "), )?; - } else { - locked_ws.finish(new_head_id)? } } Ok(()) diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index 9d2d6e732c..9d2b08c047 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -414,7 +414,7 @@ fn test_op_abandon_ancestors() { // Can't abandon the current operation. let stderr = test_env.jj_cmd_failure(&repo_path, &["op", "abandon", "..@"]); insta::assert_snapshot!(stderr, @r###" - Error: Cannot abandon the current operation + Error: Cannot abandon the current operation d92d0753399f Hint: Run `jj undo` to revert the current operation, then use `jj op abandon` "###); @@ -508,6 +508,82 @@ fn test_op_abandon_without_updating_working_copy() { "###); } +#[test] +fn test_op_abandon_multiple_heads() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + // Create 1 base operation + 2 operations to be diverged. + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "commit 1"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "commit 2"]); + test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "commit 3"]); + let stdout = test_env.jj_cmd_success( + &repo_path, + &["op", "log", "--no-graph", r#"-Tid.short() ++ "\n""#], + ); + let (head_op_id, prev_op_id) = stdout.lines().next_tuple().unwrap(); + insta::assert_snapshot!(head_op_id, @"cd2b4690faf2"); + insta::assert_snapshot!(prev_op_id, @"c2878c428b1c"); + + // Create 1 other concurrent operation. + test_env.jj_cmd_ok(&repo_path, &["commit", "--at-op=@--", "-m", "commit 4"]); + + // Can't resolve operation relative to @. + let stderr = test_env.jj_cmd_failure(&repo_path, &["op", "abandon", "@-"]); + insta::assert_snapshot!(stderr, @r###" + Error: The "@" expression resolved to more than one operation + Hint: Try specifying one of the operations by ID: cd2b4690faf2, 603773cdd351 + "###); + let (_, other_head_op_id) = stderr.trim_end().rsplit_once(", ").unwrap(); + insta::assert_snapshot!(other_head_op_id, @"603773cdd351"); + assert_ne!(head_op_id, other_head_op_id); + + // Can't abandon one of the head operations. + let stderr = test_env.jj_cmd_failure(&repo_path, &["op", "abandon", head_op_id]); + insta::assert_snapshot!(stderr, @r###" + Error: Cannot abandon the current operation cd2b4690faf2 + "###); + + // Can't abandon the other head operation. + let stderr = test_env.jj_cmd_failure(&repo_path, &["op", "abandon", other_head_op_id]); + insta::assert_snapshot!(stderr, @r###" + Error: Cannot abandon the current operation 603773cdd351 + "###); + + // Can abandon the operation which is not an ancestor of the other head. + // This would crash if we attempted to remap the unchanged op in the op + // heads store. + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "abandon", prev_op_id]); + insta::assert_snapshot!(stderr, @r###" + Abandoned 1 operations and reparented 2 descendant operations. + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "log"]); + insta::assert_snapshot!(stdout, @r###" + @ a232d055d331 test-username@host.example.com 2001-02-03 04:05:17.000 +07:00 - 2001-02-03 04:05:17.000 +07:00 + ├─╮ resolve concurrent operations + │ │ args: jj op log + ○ │ 467d42715f00 test-username@host.example.com 2001-02-03 04:05:10.000 +07:00 - 2001-02-03 04:05:10.000 +07:00 + │ │ commit 220cb0b1b5d1c03cc0d351139d824598bb3c1967 + │ │ args: jj commit -m 'commit 3' + │ ○ 603773cdd351 test-username@host.example.com 2001-02-03 04:05:12.000 +07:00 - 2001-02-03 04:05:12.000 +07:00 + ├─╯ commit 81a4ef3dd421f3184289df1c58bd3a16ea1e3d8e + │ args: jj commit '--at-op=@--' -m 'commit 4' + ○ 5d0ab09ab0fa test-username@host.example.com 2001-02-03 04:05:08.000 +07:00 - 2001-02-03 04:05:08.000 +07:00 + │ commit 230dd059e1b059aefc0da06a2e5a7dbf22362f22 + │ args: jj commit -m 'commit 1' + ○ b51416386f26 test-username@host.example.com 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 + │ add workspace 'default' + ○ 9a7d829846af test-username@host.example.com 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 + │ initialize repo + ○ 000000000000 root() + "###); + insta::assert_snapshot!(stderr, @r###" + Concurrent modification detected, resolving automatically. + "###); +} + #[test] fn test_op_recover_from_bad_gc() { let test_env = TestEnvironment::default(); @@ -551,8 +627,7 @@ fn test_op_recover_from_bad_gc() { // Do concurrent modification to make the situation even worse. At this // point, the index can be loaded, so this command succeeds. - // TODO: test_env.jj_cmd_ok(&repo_path, &["--at-op=@-", "describe", "-m4.1"]); - // TODO: "op abandon" doesn't work if there are multiple op heads. + test_env.jj_cmd_ok(&repo_path, &["--at-op=@-", "describe", "-m4.1"]); let stderr = test_env.jj_cmd_internal_error(&repo_path, &["--at-op", head_op_id, "debug", "reindex"]); @@ -563,7 +638,10 @@ fn test_op_recover_from_bad_gc() { "###); // "op log" should still be usable. - let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "log", "--ignore-working-copy"]); + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["op", "log", "--ignore-working-copy", "--at-op", head_op_id], + ); insta::assert_snapshot!(stdout, @r###" @ 43d51d9b0c0c test-username@host.example.com 2001-02-03 04:05:12.000 +07:00 - 2001-02-03 04:05:12.000 +07:00 │ describe commit 37bb762e5dc08073ec4323bdffc023a0f0cc901e @@ -592,19 +670,23 @@ fn test_op_recover_from_bad_gc() { let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "abandon", &format!("..{bad_op_id}")]); insta::assert_snapshot!(stderr, @r###" - Abandoned 4 operations and reparented 3 descendant operations. + Abandoned 4 operations and reparented 4 descendant operations. "###); // The repo should no longer be corrupt. let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log"]); insta::assert_snapshot!(stdout, @r###" - @ mzvwutvl test.user@example.com 2001-02-03 08:05:12 6d868f04 - │ (empty) 4 + ○ mzvwutvl?? test.user@example.com 2001-02-03 08:05:15 dc2c6d52 + │ (empty) 4.1 + │ @ mzvwutvl?? test.user@example.com 2001-02-03 08:05:12 6d868f04 + ├─╯ (empty) 4 ○ zsuskuln test.user@example.com 2001-02-03 08:05:10 HEAD@git f652c321 │ (empty) (no description set) ◆ zzzzzzzz root() 00000000 "###); - insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(stderr, @r###" + Concurrent modification detected, resolving automatically. + "###); } #[test] diff --git a/lib/src/op_walk.rs b/lib/src/op_walk.rs index a0cf1a65ea..baeb57ccd6 100644 --- a/lib/src/op_walk.rs +++ b/lib/src/op_walk.rs @@ -250,7 +250,7 @@ pub fn walk_ancestors(head_ops: &[Operation]) -> impl Iterator, /// The number of rewritten operations. pub rewritten_count: usize,