cli: make non-colocated repo also preserve git_refs on op undo/restore

Now we have a separate map for "git" tracking remote, we can always preserve
the last imported/exported git_refs. The option to restore git-tracking refs
has been removed. Perhaps, --what can be reorganized as --local and --remote
<NAME>.

cli/src/commands/operation.rs

@@ -54,13 +54,8 @@ pub struct OperationRestoreArgs {
 
     /// What portions of the local state to restore (can be repeated)
     ///
-    /// Defaults to everything for non-colocated repos.
-    ///
-    /// Defaults to `repo` and `remote-tracking` for colocated repos. This
-    /// ensures that the automatic `jj git export` succeeds.
-    ///
     /// This option is EXPERIMENTAL.
-    #[arg(long)]
+    #[arg(long, value_enum, default_values_t = DEFAULT_UNDO_WHAT)]
     what: Vec<UndoWhatToRestore>,
 }
 
@@ -78,13 +73,8 @@ pub struct OperationUndoArgs {
 
     /// What portions of the local state to restore (can be repeated)
     ///
-    /// Defaults to everything for non-colocated repos.
-    ///
-    /// Defaults to `repo` and `remote-tracking` for colocated repos. This
-    /// ensures that the automatic `jj git export` succeeds.
-    ///
     /// This option is EXPERIMENTAL.
-    #[arg(long)]
+    #[arg(long, value_enum, default_values_t = DEFAULT_UNDO_WHAT)]
     what: Vec<UndoWhatToRestore>,
 }
 
@@ -95,10 +85,11 @@ enum UndoWhatToRestore {
     /// The remote-tracking branches. Do not restore these if you'd like to push
     /// after the undo
     RemoteTracking,
-    /// Remembered git repo state from the last `jj git import`
-    GitTracking,
 }
 
+const DEFAULT_UNDO_WHAT: [UndoWhatToRestore; 2] =
+    [UndoWhatToRestore::Repo, UndoWhatToRestore::RemoteTracking];
+
 fn cmd_op_log(
     ui: &mut Ui,
     command: &CommandHelper,
@@ -179,13 +170,8 @@ fn view_with_desired_portions_restored(
         current_view.clone()
     };
 
-    let git_source_view = if what.contains(&UndoWhatToRestore::GitTracking) {
+    new_view.git_refs = current_view.git_refs.clone();
-        view_being_restored
+    new_view.git_head = current_view.git_head.clone();
-    } else {
-        current_view
-    };
-    new_view.git_refs = git_source_view.git_refs.clone();
-    new_view.git_head = git_source_view.git_head.clone();
 
     if what.contains(&UndoWhatToRestore::RemoteTracking) == what.contains(&UndoWhatToRestore::Repo)
     {
@@ -229,39 +215,6 @@ fn view_with_desired_portions_restored(
     new_view
 }
 
-fn process_what_arg(what_arg: &[UndoWhatToRestore], colocated: bool) -> Vec<UndoWhatToRestore> {
-    if !what_arg.is_empty() {
-        what_arg.to_vec()
-    } else {
-        let mut default_what = vec![UndoWhatToRestore::Repo, UndoWhatToRestore::RemoteTracking];
-        if !colocated {
-            // In a colocated repo, restoring the git-tracking refs is harmful
-            // (https://github.com/martinvonz/jj/issues/922).
-            //
-            // The issue is that `jj undo` does not directly change the local
-            // git repo's branches. Keeping those up to date the job of the
-            // automatic `jj git import` and `jj git export`, and they rely on the
-            // git-tracking refs matching the git repo's branches.
-            //
-            // Consider, for example, undoing a `jj branch set` command. If the
-            // git-tracking refs were restored by `undo`, they would no longer
-            // match the actual positions of branches in the git repo. So, the
-            // automatic `jj git export` would fail and the automatic `jj git
-            // import` would create a conflict, as demonstrated by the bug
-            // linked above.
-            //
-            // So, we have `undo` *not* move the git-tracking branches. After
-            // the undo, git-tracking refs will still match the actual positions
-            // of the git repo's branches (in the normal case where they matched
-            // before the undo). The automatic `jj git export` that happens
-            // immediately after the undo will successfully export whatever
-            // changes to branches `undo` caused.
-            default_what.push(UndoWhatToRestore::GitTracking);
-        }
-        default_what
-    }
-}
-
 pub fn cmd_op_undo(
     ui: &mut Ui,
     command: &CommandHelper,
@@ -269,7 +222,6 @@ 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"));
@@ -287,7 +239,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, repo_is_colocated),
+        &args.what,
     );
     tx.mut_repo().set_view(new_view);
     tx.finish(ui)?;
@@ -302,13 +254,12 @@ 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, repo_is_colocated),
+        &args.what,
     );
     tx.mut_repo().set_view(new_view);
     tx.finish(ui)?;

cli/tests/test_git_import_export.rs

@@ -88,8 +88,14 @@ fn test_git_export_undo() {
     a: qpvuntsm 230dd059 (empty) (no description set)
     "###);
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @"");
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["log", "-ra@git"]), @r###"
+    @  qpvuntsm test.user@example.com 2001-02-03 04:05:07.000 +07:00 a 230dd059
+    │  (empty) (no description set)
+    ~
+    "###);
 
-    // "git export" can't be undone.
+    // Exported refs won't be removed by undoing the export, but the git-tracking
+    // branch is. This is the same as remote-tracking branches.
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["op", "undo"]), @r###"
     "###);
     insta::assert_debug_snapshot!(get_git_repo_refs(&git_repo), @r###"
@@ -102,10 +108,18 @@ fn test_git_export_undo() {
         ),
     ]
     "###);
+    insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "-ra@git"]), @r###"
+    Error: Revision "a@git" doesn't exist
+    Hint: Did you mean "a"?
+    "###);
 
-    // This would re-export branch "a" as the internal state has been rolled back.
+    // This would re-export branch "a" and create git-tracking branch.
-    // It might be better to preserve the state, and say "Nothing changed" here.
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "export"]), @"");
+    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["log", "-ra@git"]), @r###"
+    @  qpvuntsm test.user@example.com 2001-02-03 04:05:07.000 +07:00 a 230dd059
+    │  (empty) (no description set)
+    ~
+    "###);
 }
 
 #[test]
@@ -132,7 +146,7 @@ fn test_git_import_undo() {
     a: qpvuntsm 230dd059 (empty) (no description set)
     "###);
 
-    // "git import" can be undone by default in non-colocated repositories.
+    // "git import" can be undone by default.
     let stdout = test_env.jj_cmd_success(&repo_path, &["op", "restore", &base_operation_id]);
     insta::assert_snapshot!(stdout, @r###"
     "###);
@@ -142,27 +156,6 @@ fn test_git_import_undo() {
     insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
     a: qpvuntsm 230dd059 (empty) (no description set)
     "###);
-
-    // Even if we don't restore the git_refs, "git import" can re-import the
-    // local branch "a".
-    // TODO: This will be the default "op restore" mode.
-    let stdout = test_env.jj_cmd_success(
-        &repo_path,
-        &[
-            "op",
-            "restore",
-            &base_operation_id,
-            "--what=repo",
-            "--what=remote-tracking",
-        ],
-    );
-    insta::assert_snapshot!(stdout, @r###"
-    "###);
-    insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @"");
-    insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["git", "import"]), @"");
-    insta::assert_snapshot!(get_branch_output(&test_env, &repo_path), @r###"
-    a: qpvuntsm 230dd059 (empty) (no description set)
-    "###);
 }
 
 #[test]