From bafb3572097961bda64a7e249dea2c48fcde88d3 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 24 Jul 2024 18:50:10 +0900
Subject: [PATCH] git: on abandoning unreachable commits, don't count HEAD ref

This basically reverts 20eb9ecec10f "git: don't abandon HEAD commit when it
loses a branch." I think the new behavior is more consistent because the Git
HEAD is equivalent to @- in jj, so it shouldn't be considered a named ref.

Note that we've made old HEAD branch not considered at 92cfffd84377 "git: on
external HEAD move, do not abandon old branch."

#4108
---
 CHANGELOG.md                    |  4 ++++
 cli/tests/test_git_colocated.rs | 12 +++++++-----
 lib/src/git.rs                  |  7 +++----
 lib/tests/test_git.rs           |  8 ++++----
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 20aa15dc0c..186522b664 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 * [The "fileset" language](docs/filesets.md) is now enabled by default. It can
   still be disable by setting `ui.allow-filesets=false`.
 
+* On `jj git fetch`/`import`, commits referred to by `HEAD@git` are no longer
+  preserved. If a checked-out named branch gets deleted locally or remotely, the
+  corresponding commits will be abandoned.
+
 ### Deprecations
 
 ### New features
diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs
index b98575a543..fa523fdba7 100644
--- a/cli/tests/test_git_colocated.rs
+++ b/cli/tests/test_git_colocated.rs
@@ -281,8 +281,8 @@ fn test_git_colocated_rebase_on_import() {
     // refs/heads/master we just exported
     test_env.jj_cmd_ok(&workspace_root, &["st"]);
 
-    // Move `master` and HEAD backwards, which should result in commit2 getting
-    // hidden, and a new working-copy commit at the new position.
+    // Move `master` backwards, which should result in commit2 getting hidden,
+    // and the working-copy commit rebased.
     let commit2_oid = git_repo
         .find_branch("master", git2::BranchType::Local)
         .unwrap()
@@ -292,16 +292,18 @@ fn test_git_colocated_rebase_on_import() {
     let commit2 = git_repo.find_commit(commit2_oid).unwrap();
     let commit1 = commit2.parents().next().unwrap();
     git_repo.branch("master", &commit1, true).unwrap();
-    git_repo.set_head("refs/heads/master").unwrap();
     let (stdout, stderr) = get_log_output_with_stderr(&test_env, &workspace_root);
     insta::assert_snapshot!(stdout, @r###"
-    @  5539e55eb3690b85a7ebd4a37a5e3b57f469ee94
+    @  15b1d70c5e33b5d2b18383292b85324d5153ffed
     ○  47fe984daf66f7bf3ebf31b9cb3513c995afb857 master HEAD@git add a file
     ◆  0000000000000000000000000000000000000000
     "###);
     insta::assert_snapshot!(stderr, @r###"
-    Reset the working copy parent to the new Git HEAD.
     Abandoned 1 commits that are no longer reachable.
+    Rebased 1 descendant commits off of commits rewritten from git
+    Working copy now at: zsuskuln 15b1d70c (empty) (no description set)
+    Parent commit      : qpvuntsm 47fe984d master | add a file
+    Added 0 files, modified 1 files, removed 0 files
     Done importing changes from the underlying Git repo.
     "###);
 }
diff --git a/lib/src/git.rs b/lib/src/git.rs
index 666ea23194..d7066a22f7 100644
--- a/lib/src/git.rs
+++ b/lib/src/git.rs
@@ -19,7 +19,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
 use std::default::Default;
 use std::io::Read;
 use std::path::PathBuf;
-use std::{fmt, iter, str};
+use std::{fmt, str};
 
 use git2::Oid;
 use itertools::Itertools;
@@ -496,16 +496,15 @@ fn default_remote_ref_state_for(ref_name: &RefName, git_settings: &GitSettings)
     }
 }
 
-/// Commits referenced by local branches, tags, or HEAD@git.
+/// Commits referenced by local branches or tags.
 ///
 /// On `import_refs()`, this is similar to collecting commits referenced by
 /// `view.git_refs()`. Main difference is that local branches can be moved by
 /// tracking remotes, and such mutation isn't applied to `view.git_refs()` yet.
 fn pinned_commit_ids(view: &View) -> Vec<CommitId> {
-    itertools::chain!(
+    itertools::chain(
         view.local_branches().map(|(_, target)| target),
         view.tags().values(),
-        iter::once(view.git_head()),
     )
     .flat_map(|target| target.added_ids())
     .cloned()
diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs
index 5ae15842f0..35fe45650e 100644
--- a/lib/tests/test_git.rs
+++ b/lib/tests/test_git.rs
@@ -352,9 +352,9 @@ fn test_import_refs_reimport_head_removed() {
 }
 
 #[test]
-fn test_import_refs_reimport_git_head_counts() {
-    // Test that if a branch is removed but the Git HEAD points to the commit (or a
-    // descendant of it), we still keep it alive.
+fn test_import_refs_reimport_git_head_does_not_count() {
+    // Test that if a branch is removed, the corresponding commit is abandoned
+    // no matter if the Git HEAD points to the commit (or a descendant of it.)
     let settings = testutils::user_settings();
     let git_settings = GitSettings::default();
     let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
@@ -379,7 +379,7 @@ fn test_import_refs_reimport_git_head_counts() {
     git::import_head(tx.mut_repo()).unwrap();
     git::import_refs(tx.mut_repo(), &git_settings).unwrap();
     tx.mut_repo().rebase_descendants(&settings).unwrap();
-    assert!(tx.mut_repo().view().heads().contains(&jj_id(&commit)));
+    assert!(!tx.mut_repo().view().heads().contains(&jj_id(&commit)));
 }
 
 #[test]