cli: ensure first new HEAD is detached

The problem is that the first non-working-copy commit moves the unborn current branch to that commit, but jj doesn't "export" the moved branch. Therefore, the next jj invocation notices the "external" ref change, which was actually made by jj. I'm not sure why we play nice by setting the "current" HEAD, but I *think* it's okay to set the "new" HEAD and reset to the same commit to clear Git index.
yuja · Oct 2, 2023 · 0fe2557 · 0fe2557
1 parent b91139a
commit 0fe2557
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 45 deletions.
diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
@@ -849,20 +849,13 @@ impl WorkspaceCommandHelper {
             .downcast_ref::<GitBackend>()
             .unwrap()
             .git_repo_clone();
-        let current_git_head_ref = git_repo.find_reference("HEAD").unwrap();
-        let current_git_commit_id = current_git_head_ref
-            .peel_to_commit()
-            .ok()
-            .map(|commit| commit.id());
         if let Some(wc_commit_id) = mut_repo.view().get_wc_commit_id(self.workspace_id()) {
             let wc_commit = mut_repo.store().get_commit(wc_commit_id)?;
             let first_parent_id = wc_commit.parent_ids()[0].clone();
             if first_parent_id != *mut_repo.store().root_commit_id() {
-                if let Some(current_git_commit_id) = current_git_commit_id {
-                    git_repo.set_head_detached(current_git_commit_id)?;
-                }
                 let new_git_commit_id = Oid::from_bytes(first_parent_id.as_bytes()).unwrap();
                 let new_git_commit = git_repo.find_commit(new_git_commit_id)?;
+                git_repo.set_head_detached(new_git_commit_id)?;
                 git_repo.reset(new_git_commit.as_object(), git2::ResetType::Mixed, None)?;
                 mut_repo.set_git_head_target(RefTarget::normal(first_parent_id));
             }

diff --git a/cli/tests/test_commit_template.rs b/cli/tests/test_commit_template.rs
@@ -371,10 +371,9 @@ fn test_log_git_head() {
     std::fs::write(repo_path.join("file"), "foo\n").unwrap();
     let stdout = test_env.jj_cmd_success(&repo_path, &["log", "--color=always"]);
     insta::assert_snapshot!(stdout, @r###"
-    Done importing changes from the underlying Git repo.
     @  [1m[38;5;13mr[38;5;8mlvkpnrz[39m [38;5;[email protected][39m [38;5;14m2001-02-03 04:05:09.000 +07:00[39m [38;5;12m5[38;5;8m0aaf475[39m[0m
     │  [1minitial[0m
-    ◉  [1m[38;5;5mq[0m[38;5;8mpvuntsm[39m [38;5;[email protected][39m [38;5;6m2001-02-03 04:05:07.000 +07:00[39m [38;5;5mmaster[39m [38;5;2mHEAD@git[39m [1m[38;5;4m2[0m[38;5;8m30dd059[39m
+    ◉  [1m[38;5;5mq[0m[38;5;8mpvuntsm[39m [38;5;[email protected][39m [38;5;6m2001-02-03 04:05:07.000 +07:00[39m [38;5;2mHEAD@git[39m [1m[38;5;4m2[0m[38;5;8m30dd059[39m
     │  [38;5;2m(empty)[39m [38;5;2m(no description set)[39m
     ◉  [1m[38;5;5mz[0m[38;5;8mzzzzzzz[39m [38;5;2mroot()[39m [1m[38;5;4m0[0m[38;5;8m0000000[39m
     "###);

diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs
@@ -179,7 +179,7 @@ fn test_git_colocated_branches() {
     @  3560559274ab431feea00b7b7e0b9250ecce951f bar
     │ ◉  1e6f0b403ed2ff9713b5d6b1dc601e4804250cda foo
     ├─╯
-    ◉  230dd059e1b059aefc0da06a2e5a7dbf22362f22 master HEAD@git
+    ◉  230dd059e1b059aefc0da06a2e5a7dbf22362f22 HEAD@git
     ◉  0000000000000000000000000000000000000000
     "###);
 
@@ -232,23 +232,20 @@ fn test_git_colocated_branch_forget() {
     test_env.jj_cmd_success(&workspace_root, &["branch", "set", "foo"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###"
     @  65b6b74e08973b88d38404430f119c8c79465250 foo
-    ◉  230dd059e1b059aefc0da06a2e5a7dbf22362f22 master HEAD@git
+    ◉  230dd059e1b059aefc0da06a2e5a7dbf22362f22 HEAD@git
     ◉  0000000000000000000000000000000000000000
     "###);
     let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
     insta::assert_snapshot!(stdout, @r###"
     foo: rlvkpnrz 65b6b74e (empty) (no description set)
-    master: qpvuntsm 230dd059 (empty) (no description set)
     "###);
 
     let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "forget", "foo"]);
     insta::assert_snapshot!(stdout, @"");
     // A forgotten branch is deleted in the git repo. For a detailed demo explaining
     // this, see `test_branch_forget_export` in `test_branch_command.rs`.
     let stdout = test_env.jj_cmd_success(&workspace_root, &["branch", "list"]);
-    insta::assert_snapshot!(stdout, @r###"
-    master: qpvuntsm 230dd059 (empty) (no description set)
-    "###);
+    insta::assert_snapshot!(stdout, @"");
 }
 
 #[test]
@@ -292,7 +289,7 @@ fn test_git_colocated_fetch_deleted_or_moved_branch() {
     ├─╯
     │ ◉  929e298ae9edf969b405a304c75c10457c47d52c B_to_delete B_to_delete
     ├─╯
-    ◉  a86754f975f953fa25da4265764adc0c62e9ce6b A master HEAD@git A
+    ◉  a86754f975f953fa25da4265764adc0c62e9ce6b A HEAD@git A
     ◉  0000000000000000000000000000000000000000
     "###);
 
@@ -311,7 +308,7 @@ fn test_git_colocated_fetch_deleted_or_moved_branch() {
     ◉  04fd29df05638156b20044b3b6136b42abcb09ab C_to_move moved C
     │ @  0335878796213c3a701f1c9c34dcae242bee4131
     ├─╯
-    ◉  a86754f975f953fa25da4265764adc0c62e9ce6b A master HEAD@git A
+    ◉  a86754f975f953fa25da4265764adc0c62e9ce6b A HEAD@git A
     ◉  0000000000000000000000000000000000000000
     "###);
 }
@@ -323,13 +320,14 @@ fn test_git_colocated_external_checkout() {
     let git_repo = git2::Repository::init(&repo_path).unwrap();
     test_env.jj_cmd_success(&repo_path, &["init", "--git-repo=."]);
     test_env.jj_cmd_success(&repo_path, &["ci", "-m=A"]);
+    test_env.jj_cmd_success(&repo_path, &["branch", "set", "-r@-", "master"]);
     test_env.jj_cmd_success(&repo_path, &["new", "-m=B", "root()"]);
     test_env.jj_cmd_success(&repo_path, &["new"]);
 
     // Checked out anonymous branch
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  53637cd508ff02427dd78eca98f5b2450a6370ce
-    ◉  66f4d1806ae41bd604f69155dece64062a0056cf HEAD@git B
+    @  f8a23336e41840ed1757ef323402a770427dc89a
+    ◉  eccedddfa5152d99fc8ddd1081b375387a8a382a HEAD@git B
     │ ◉  a86754f975f953fa25da4265764adc0c62e9ce6b master A
     ├─╯
     ◉  0000000000000000000000000000000000000000
@@ -350,9 +348,9 @@ fn test_git_colocated_external_checkout() {
     // be abandoned. (#1042)
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     Done importing changes from the underlying Git repo.
-    @  0521ce3b8c4e29aab79f3c750e2845dcbc4c3874
+    @  adadbd65a794e2294962b3c3da9aada09fe1b472
     ◉  a86754f975f953fa25da4265764adc0c62e9ce6b master HEAD@git A
-    │ ◉  66f4d1806ae41bd604f69155dece64062a0056cf B
+    │ ◉  eccedddfa5152d99fc8ddd1081b375387a8a382a B
     ├─╯
     ◉  0000000000000000000000000000000000000000
     "###);
@@ -367,24 +365,23 @@ fn test_git_colocated_squash_undo() {
     test_env.jj_cmd_success(&repo_path, &["ci", "-m=A"]);
     // Test the setup
     insta::assert_snapshot!(get_log_output_divergence(&test_env, &repo_path), @r###"
-    Done importing changes from the underlying Git repo.
     @  rlvkpnrzqnoo 8f71e3b6a3be
-    ◉  qpvuntsmwlqt a86754f975f9 A master HEAD@git
+    ◉  qpvuntsmwlqt a86754f975f9 A HEAD@git
     ◉  zzzzzzzzzzzz 000000000000
     "###);
 
     test_env.jj_cmd_success(&repo_path, &["squash"]);
     insta::assert_snapshot!(get_log_output_divergence(&test_env, &repo_path), @r###"
     @  zsuskulnrvyr f0c12b0396d9
-    ◉  qpvuntsmwlqt 2f376ea1478c A master HEAD@git
+    ◉  qpvuntsmwlqt 2f376ea1478c A HEAD@git
     ◉  zzzzzzzzzzzz 000000000000
     "###);
     test_env.jj_cmd_success(&repo_path, &["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###"
     @  rlvkpnrzqnoo 8f71e3b6a3be
-    ◉  qpvuntsmwlqt a86754f975f9 A master HEAD@git
+    ◉  qpvuntsmwlqt a86754f975f9 A HEAD@git
     ◉  zzzzzzzzzzzz 000000000000
     "###);
 }

diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs
@@ -405,7 +405,7 @@ fn test_git_fetch_all() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
 
@@ -420,7 +420,6 @@ fn test_git_fetch_all() {
     a1: nknoxmzm 359a9a02 descr_for_a1
     a2: qkvnknrk decaa396 descr_for_a2
     b: vpupmnsl c7d4bdcb descr_for_b
-    master: zowqyktl ff36dc55 descr_for_trunk1
     trunk1: zowqyktl ff36dc55 descr_for_trunk1
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
@@ -429,7 +428,7 @@ fn test_git_fetch_all() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     │ @  230dd059e1b0
     ├─╯
     ◉  000000000000
@@ -446,7 +445,7 @@ fn test_git_fetch_all() {
     │ ◉  0424f6dfc1ff descr_for_a1 a1
     ├─╯
     @  8f1f14fbbf42 descr_for_trunk2 trunk2
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
     // Change a branch in the source repo as well, so that it becomes conflicted.
@@ -462,7 +461,7 @@ fn test_git_fetch_all() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     │ @  230dd059e1b0
     ├─╯
     ◉  000000000000
@@ -472,7 +471,6 @@ fn test_git_fetch_all() {
     a2: qkvnknrk decaa396 descr_for_a2
     b: vpupmnsl 061eddbb new_descr_for_b_to_create_conflict
       @origin (ahead by 1 commits, behind by 1 commits): vpupmnsl c7d4bdcb descr_for_b
-    master: zowqyktl ff36dc55 descr_for_trunk1
     trunk1: zowqyktl ff36dc55 descr_for_trunk1
     "###);
     insta::assert_snapshot!(test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch"]), @r###"
@@ -488,7 +486,6 @@ fn test_git_fetch_all() {
       + vpupmnsl 061eddbb new_descr_for_b_to_create_conflict
       + vktnwlsu babc4922 descr_for_b
       @origin (behind by 1 commits): vktnwlsu babc4922 descr_for_b
-    master: zowqyktl ff36dc55 descr_for_trunk1
     trunk1: zowqyktl ff36dc55 descr_for_trunk1
     trunk2: umznmzko 8f1f14fb descr_for_trunk2
     "###);
@@ -501,7 +498,7 @@ fn test_git_fetch_all() {
     ◉  8f1f14fbbf42 descr_for_trunk2 trunk2
     │ ◉  061eddbb43ab new_descr_for_b_to_create_conflict b??
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     │ @  230dd059e1b0
     ├─╯
     ◉  000000000000
@@ -533,7 +530,7 @@ fn test_git_fetch_some_of_many_branches() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
 
@@ -605,7 +602,7 @@ fn test_git_fetch_some_of_many_branches() {
     │ ◉  6f4e1c4dfe29 descr_for_a1 a1
     ├─╯
     @  09430ba04a82 descr_for_trunk2 trunk2
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
     // Change a branch in the source repo as well, so that it becomes conflicted.
@@ -720,7 +717,7 @@ fn test_git_fetch_undo() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
 
@@ -783,7 +780,7 @@ fn test_fetch_undo_what() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
 
@@ -949,7 +946,7 @@ fn test_git_fetch_removed_branch() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
 
@@ -962,7 +959,7 @@ fn test_git_fetch_removed_branch() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     │ @  230dd059e1b0
     ├─╯
     ◉  000000000000
@@ -982,7 +979,7 @@ fn test_git_fetch_removed_branch() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     │ @  230dd059e1b0
     ├─╯
     ◉  000000000000
@@ -998,7 +995,7 @@ fn test_git_fetch_removed_branch() {
     ◉  c7d4bdcbc215 descr_for_b b
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     │ @  230dd059e1b0
     ├─╯
     ◉  000000000000
@@ -1029,7 +1026,7 @@ fn test_git_fetch_removed_parent_branch() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     ◉  000000000000
     "###);
 
@@ -1042,7 +1039,7 @@ fn test_git_fetch_removed_parent_branch() {
     ├─╯
     │ ◉  359a9a02457d descr_for_a1 a1
     ├─╯
-    ◉  ff36dc55760e descr_for_trunk1 master trunk1
+    ◉  ff36dc55760e descr_for_trunk1 trunk1
     │ @  230dd059e1b0
     ├─╯
     ◉  000000000000

diff --git a/cli/tests/test_git_submodule.rs b/cli/tests/test_git_submodule.rs
@@ -50,7 +50,6 @@ fn test_gitsubmodule_print_gitmodules() {
         &["git", "submodule", "print-gitmodules", "-r", "@-"],
     );
     insta::assert_snapshot!(stdout, @r###"
-    Done importing changes from the underlying Git repo.
     name:old
     url:https://github.com/old/old.git
     path:old