From 774c9198e1eb08bd3d6794b8b023d0941f44ee46 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Wed, 31 May 2023 20:55:46 -0700 Subject: [PATCH] undo: preserve git-tracking refs in colocated repos by default --- CHANGELOG.md | 4 ++++ src/commands/operation.rs | 22 ++++++++++++++-------- tests/test_git_colocated.rs | 6 ++---- tests/test_undo.rs | 14 ++++++++------ 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f50e37975..a333755dcc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -150,6 +150,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * It is now possible to `jj branch forget` deleted branches. [#1537](https://github.com/martinvonz/jj/issues/1537) +* In colocated repos, a bug causing conflicts when undoing branch moves (#922) + has been fixed. Some surprising behaviors related to undoing `jj git push` or + `jj git fetch` remain. + ## [0.7.0] - 2023-02-16 ### Breaking changes diff --git a/src/commands/operation.rs b/src/commands/operation.rs index 254cf8491e..1aaa89ad2c 100644 --- a/src/commands/operation.rs +++ b/src/commands/operation.rs @@ -206,15 +206,19 @@ fn view_with_desired_portions_restored( new_view } -fn process_what_arg(what_arg: &[UndoWhatToRestore]) -> BTreeSet { +fn process_what_arg( + what_arg: &[UndoWhatToRestore], + colocated: bool, +) -> BTreeSet { if !what_arg.is_empty() { what_arg.iter().cloned().collect() } else { - btreeset!( - UndoWhatToRestore::Repo, - UndoWhatToRestore::RemoteTracking, - UndoWhatToRestore::GitTracking - ) + let mut default_what = + btreeset!(UndoWhatToRestore::Repo, UndoWhatToRestore::RemoteTracking); + if !colocated { + default_what.insert(UndoWhatToRestore::GitTracking); + } + default_what } } @@ -225,6 +229,7 @@ pub fn cmd_op_undo( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let bad_op = workspace_command.resolve_single_op(&args.operation)?; + let repo_is_colocated = workspace_command.working_copy_shared_with_git(); let parent_ops = bad_op.parents(); if parent_ops.len() > 1 { return Err(user_error("Cannot undo a merge operation")); @@ -242,7 +247,7 @@ pub fn cmd_op_undo( let new_view = view_with_desired_portions_restored( tx.repo().view().store_view(), tx.base_repo().view().store_view(), - process_what_arg(&args.what), + process_what_arg(&args.what, repo_is_colocated), ); tx.mut_repo().set_view(new_view); tx.finish(ui)?; @@ -257,12 +262,13 @@ fn cmd_op_restore( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let target_op = workspace_command.resolve_single_op(&args.operation)?; + let repo_is_colocated = workspace_command.working_copy_shared_with_git(); let mut tx = workspace_command .start_transaction(&format!("restore to operation {}", target_op.id().hex())); let new_view = view_with_desired_portions_restored( target_op.view().store_view(), tx.base_repo().view().store_view(), - process_what_arg(&args.what), + process_what_arg(&args.what, repo_is_colocated), ); tx.mut_repo().set_view(new_view); tx.finish(ui)?; diff --git a/tests/test_git_colocated.rs b/tests/test_git_colocated.rs index c481dc6f8c..16405ff990 100644 --- a/tests/test_git_colocated.rs +++ b/tests/test_git_colocated.rs @@ -374,10 +374,8 @@ fn test_git_colocated_squash_undo() { // TODO: There should be no divergence here; 2f376ea1478c should be hidden // (#922) insta::assert_snapshot!(get_log_output_divergence(&test_env, &repo_path), @r###" - ◉ qpvuntsmwlqt 2f376ea1478c A master !divergence! - │ @ rlvkpnrzqnoo 8f71e3b6a3be - │ ◉ qpvuntsmwlqt a86754f975f9 A HEAD@git !divergence! - ├─╯ + @ rlvkpnrzqnoo 8f71e3b6a3be + ◉ qpvuntsmwlqt a86754f975f9 A master HEAD@git ◉ zzzzzzzzzzzz 000000000000 "###); } diff --git a/tests/test_undo.rs b/tests/test_undo.rs index 027cfa2e2e..eda4b2f155 100644 --- a/tests/test_undo.rs +++ b/tests/test_undo.rs @@ -199,27 +199,29 @@ fn test_git_push_undo_colocated() { // Undo the push test_env.jj_cmd_success(&repo_path, &["op", "restore", &pre_push_opid]); - // === Before auto-import ==== + // === Before auto-export ==== // | jj refs | jj's | git // | | git | repo // | |tracking| // ------------------------------------------ // local `main` | BB | BB | BB - // remote-tracking | AA | AA | BB - // === After automatic `jj git import` ==== + // remote-tracking | AA | BB | BB + // === After automatic `jj git export` ==== // | jj refs | jj's | git // | | git | repo // | |tracking| // ------------------------------------------ // local `main` | BB | BB | BB - // remote-tracking | BB | BB | BB + // remote-tracking | AA | AA | AA test_env.advance_test_rng_seed_to_multiple_of(100_000); test_env.jj_cmd_success(&repo_path, &["describe", "-m", "CC"]); test_env.jj_cmd_success(&repo_path, &["git", "fetch"]); - // This currently gives an identical result to `test_git_push_undo_import` + // This seems like an OK behavior: `jj` forgot that the remote-tracking branch + // was undone, but keeps track of the latest local version. TODO: Would this + // be improved if `jj git export` exported remote-tracking branches? insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###" main: 0a3e99f08a48 CC - @origin (ahead by 1 commits, behind by 1 commits): 8c05de152218 BB + @origin (ahead by 1 commits, behind by 1 commits): 0cffb6146141 AA "###); }