From faa9b8d77fca59c3144f7bf19d96948b9376764f Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 10 Jan 2024 17:20:06 +0900 Subject: [PATCH] cli: run "op abandon" without loading repo, reject --at-op If indexing failed due to missing commit objects, the repo won't be loadable without --ignore-working-copy (at least in colocated environment.) In that case, we can use "op abandon" to recover, but we had to work around the failed index loading by --ignore-working-copy. Since "op abandon" isn't a repo-level command, it's better to bypass working-copy snapshot and import of git refs at all. --at-op is rejected because it's useless and we'll need extra care for "@" expression resolution and working-copy updates. --- cli/src/commands/operation.rs | 52 +++++++++++++++++++++-------- cli/tests/test_operations.rs | 63 +++++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/cli/src/commands/operation.rs b/cli/src/commands/operation.rs index 72da509a7..b70e144a6 100644 --- a/cli/src/commands/operation.rs +++ b/cli/src/commands/operation.rs @@ -23,7 +23,8 @@ use jj_lib::op_walk; use jj_lib::repo::Repo; use crate::cli_util::{ - user_error, user_error_with_hint, CommandError, CommandHelper, LogContentFormat, + short_operation_hash, user_error, user_error_with_hint, CommandError, CommandHelper, + LogContentFormat, }; use crate::graphlog::{get_graphlog, Edge}; use crate::operation_templater; @@ -279,27 +280,37 @@ fn cmd_op_abandon( command: &CommandHelper, args: &OperationAbandonArgs, ) -> Result<(), CommandError> { - let mut workspace_command = command.workspace_helper(ui)?; - let repo = workspace_command.repo(); - let current_head_op = repo.operation(); + // Don't load the repo so that this command can be used to recover from + // corrupted repo state. + let mut workspace = command.load_workspace()?; + let repo_loader = workspace.repo_loader(); + let op_store = repo_loader.op_store(); + // It doesn't make sense to create concurrent operations that will be merged + // with the current head. + let head_op_str = &command.global_args().at_operation; + if head_op_str != "@" { + return Err(user_error("--at-op is not respected")); + } + let current_head_op = op_walk::resolve_op_for_load(repo_loader, head_op_str)?; + let resolve_op = |op_str| op_walk::resolve_op_at(op_store, ¤t_head_op, op_str); let (abandon_root_op, abandon_head_op) = if let Some((root_op_str, head_op_str)) = args.operation.split_once("..") { let root_op = if root_op_str.is_empty() { // TODO: Introduce a virtual root operation and use it instead. - op_walk::walk_ancestors(slice::from_ref(current_head_op)) + op_walk::walk_ancestors(slice::from_ref(¤t_head_op)) .last() .unwrap()? } else { - workspace_command.resolve_single_op(root_op_str)? + resolve_op(root_op_str)? }; let head_op = if head_op_str.is_empty() { current_head_op.clone() } else { - workspace_command.resolve_single_op(head_op_str)? + resolve_op(head_op_str)? }; (root_op, head_op) } else { - let op = workspace_command.resolve_single_op(&args.operation)?; + let op = resolve_op(&args.operation)?; let parent_ops: Vec<_> = op.parents().try_collect()?; let parent_op = match parent_ops.len() { 0 => return Err(user_error("Cannot abandon the root operation")), @@ -309,7 +320,7 @@ fn cmd_op_abandon( (parent_op, op) }; - if abandon_head_op == *current_head_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`", @@ -318,9 +329,9 @@ fn cmd_op_abandon( // Reparent descendants, count the number of abandoned operations. let stats = op_walk::reparent_range( - repo.op_store().as_ref(), + op_store.as_ref(), slice::from_ref(&abandon_head_op), - slice::from_ref(current_head_op), + slice::from_ref(¤t_head_op), &abandon_root_op, )?; let [new_head_id]: [OperationId; 1] = stats.new_head_ids.try_into().unwrap(); @@ -334,12 +345,25 @@ fn cmd_op_abandon( stats.unreachable_count, stats.rewritten_count, )?; - repo.op_heads_store() + repo_loader + .op_heads_store() .update_op_heads(slice::from_ref(current_head_op.id()), &new_head_id); // Remap the operation id of the current workspace. If there were any // concurrent operations, user will need to re-abandon their ancestors. - let (locked_ws, _) = workspace_command.start_working_copy_mutation()?; - locked_ws.finish(new_head_id)?; + 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() { + writeln!( + ui.warning(), + "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()), + )?; + } else { + locked_ws.finish(new_head_id)? + } + } Ok(()) } diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index ce7f90251..3994e3bcf 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -344,6 +344,12 @@ fn test_op_abandon_ancestors() { Hint: Run `jj undo` to revert the current operation, then use `jj op abandon` "###); + // Can't create concurrent abandoned operations explicitly. + let stderr = test_env.jj_cmd_failure(&repo_path, &["op", "abandon", "--at-op=@-", "@"]); + insta::assert_snapshot!(stderr, @r###" + Error: --at-op is not respected + "###); + // Abandon the current operation by undoing it first. test_env.jj_cmd_ok(&repo_path, &["undo"]); let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "abandon", "@-"]); @@ -352,11 +358,11 @@ fn test_op_abandon_ancestors() { "###); insta::assert_snapshot!( test_env.jj_cmd_success(&repo_path, &["debug", "workingcopy", "--ignore-working-copy"]), @r###" - Current operation: OperationId("05aebafee59813d56c0ea1576520b3074f5ba3e128f2b31df7370284cee593bed5043475dc2cdd30a6f22662c1dfb6aba92b83806147e77c17ad14356c07079d") + Current operation: OperationId("571221174898ef510c580f96bfb54f720bcfe0cd457e2ac5531d511fd10762b883f89b06c4ce5a8924db74406ddb59adbc2cbe0204540c7749ca24ded3fce94b") Current tree: Legacy(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")) "###); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["op", "log"]), @r###" - @ 05aebafee598 test-username@host.example.com 2001-02-03 04:05:20.000 +07:00 - 2001-02-03 04:05:20.000 +07:00 + @ 571221174898 test-username@host.example.com 2001-02-03 04:05:21.000 +07:00 - 2001-02-03 04:05:21.000 +07:00 │ undo operation ee40c9ad806a7d42f351beab5aa81a8ac38d926d02711c059229bf6a7388b7b4a7c04c004067ee6c5b6253e8398fa82bc74d0d621f8bc2c8c11f33d445f90b77 │ args: jj undo ◉ fb5252a68411 test-username@host.example.com 2001-02-03 04:05:09.000 +07:00 - 2001-02-03 04:05:09.000 +07:00 @@ -372,12 +378,63 @@ fn test_op_abandon_ancestors() { Nothing changed. "###); insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["op", "log", "-l1"]), @r###" - @ 05aebafee598 test-username@host.example.com 2001-02-03 04:05:20.000 +07:00 - 2001-02-03 04:05:20.000 +07:00 + @ 571221174898 test-username@host.example.com 2001-02-03 04:05:21.000 +07:00 - 2001-02-03 04:05:21.000 +07:00 │ undo operation ee40c9ad806a7d42f351beab5aa81a8ac38d926d02711c059229bf6a7388b7b4a7c04c004067ee6c5b6253e8398fa82bc74d0d621f8bc2c8c11f33d445f90b77 │ args: jj undo "###); } +#[test] +fn test_op_abandon_without_updating_working_copy() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + 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"]); + + // Abandon without updating the working copy. + let (_stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["op", "abandon", "@-", "--ignore-working-copy"], + ); + insta::assert_snapshot!(stderr, @r###" + Abandoned 1 operations and reparented 1 descendant operations. + "###); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["debug", "workingcopy", "--ignore-working-copy"]), @r###" + Current operation: OperationId("a6a87becb46cb138b33b8bc238ff066e1141da3e988574260ab54db676a68a070a592cacfd37e06177f604882f7de01189e2efb75148bc00ee5f9f55513feb26") + Current tree: Legacy(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")) + "###); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["op", "log", "-l1", "--ignore-working-copy"]), @r###" + @ 5dca1e30e810 test-username@host.example.com 2001-02-03 04:05:10.000 +07:00 - 2001-02-03 04:05:10.000 +07:00 + │ commit 268f5f16139313ff25bef31280b2ec2e675200f3 + │ args: jj commit -m 'commit 3' + "###); + + // The working-copy operation id isn't updated if it differs from the repo. + // It could be updated if the tree matches, but there's no extra logic for + // that. + let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "abandon", "@-"]); + insta::assert_snapshot!(stderr, @r###" + Abandoned 1 operations and reparented 1 descendant operations. + The working copy operation a6a87becb46c is not updated because it differs from the repo 5dca1e30e810. + "###); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["debug", "workingcopy", "--ignore-working-copy"]), @r###" + Current operation: OperationId("a6a87becb46cb138b33b8bc238ff066e1141da3e988574260ab54db676a68a070a592cacfd37e06177f604882f7de01189e2efb75148bc00ee5f9f55513feb26") + Current tree: Legacy(TreeId("4b825dc642cb6eb9a060e54bf8d69288fbee4904")) + "###); + insta::assert_snapshot!( + test_env.jj_cmd_success(&repo_path, &["op", "log", "-l1", "--ignore-working-copy"]), @r###" + @ e3b64811d26b test-username@host.example.com 2001-02-03 04:05:10.000 +07:00 - 2001-02-03 04:05:10.000 +07:00 + │ commit 268f5f16139313ff25bef31280b2ec2e675200f3 + │ args: jj commit -m 'commit 3' + "###); +} + fn get_log_output(test_env: &TestEnvironment, repo_path: &Path, op_id: &str) -> String { test_env.jj_cmd_success( repo_path,