From 0fe25575ba4f1bc40dc5c13515d313fb53a1a52d Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 2 Oct 2023 20:56:51 +0900 Subject: [PATCH] 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. --- cli/src/cli_util.rs | 9 +-------- cli/tests/test_commit_template.rs | 3 +-- cli/tests/test_git_colocated.rs | 29 ++++++++++++--------------- cli/tests/test_git_fetch.rs | 33 ++++++++++++++----------------- cli/tests/test_git_submodule.rs | 1 - 5 files changed, 30 insertions(+), 45 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index b4c41859c6..7d3ac7847b 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -849,20 +849,13 @@ impl WorkspaceCommandHelper { .downcast_ref::() .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 index 9871c67b6a..7a93e6269f 100644 --- 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. @ rlvkpnrz test.user@example.com 2001-02-03 04:05:09.000 +07:00 50aaf475 │ initial - ◉ qpvuntsm test.user@example.com 2001-02-03 04:05:07.000 +07:00 master HEAD@git 230dd059 + ◉ qpvuntsm test.user@example.com 2001-02-03 04:05:07.000 +07:00 HEAD@git 230dd059 │ (empty) (no description set) ◉ zzzzzzzz root() 00000000 "###); diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs index 57125ce0bd..a737e415ec 100644 --- 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,13 +232,12 @@ 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"]); @@ -246,9 +245,7 @@ fn test_git_colocated_branch_forget() { // 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,16 +365,15 @@ 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"]); @@ -384,7 +381,7 @@ fn test_git_colocated_squash_undo() { // (#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 index e984ccb587..c21e40df24 100644 --- 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 index 7a4b834e79..14f8adcb0d 100644 --- 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