git: don't abandon HEAD commit when it loses a branch

I was trying to create a reproduction script for #412, but the script
ran into another bug first. The script removed all the local and
remote branches from the backing Git repo. I noticed that we would
then try to abandon all commits. We should still count Git HEAD's
target as visible and not try to abandon it. This patch fixes that.

CHANGELOG.md

@@ -27,6 +27,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `jj edit root` now fails gracefully.
 
+* `jj git import` used to abandon a commit if Git branches and tags referring
+  to it were removed. We now keep it if a detached HEAD refers to it.
+
 ## [0.5.1] - 2022-10-17
 
 No changes (just trying to get automated GitHub release to work).

lib/src/git.rs

@@ -58,11 +58,30 @@ pub fn import_refs(
 ) -> Result<(), GitImportError> {
     let store = mut_repo.store().clone();
     let mut existing_git_refs = mut_repo.view().git_refs().clone();
-    let old_git_heads = existing_git_refs
+    let mut old_git_heads = existing_git_refs
         .values()
         .flat_map(|old_target| old_target.adds())
         .collect_vec();
+    if let Some(old_git_head) = mut_repo.view().git_head() {
+        old_git_heads.push(old_git_head);
+    }
+
     let mut new_git_heads = HashSet::new();
+    // TODO: Should this be a separate function? We may not always want to import
+    // the Git HEAD (and add it to our set of heads).
+    if let Ok(head_git_commit) = git_repo
+        .head()
+        .and_then(|head_ref| head_ref.peel_to_commit())
+    {
+        let head_commit_id = CommitId::from_bytes(head_git_commit.id().as_bytes());
+        let head_commit = store.get_commit(&head_commit_id).unwrap();
+        new_git_heads.insert(head_commit_id.clone());
+        mut_repo.add_head(&head_commit);
+        mut_repo.set_git_head(head_commit_id);
+    } else {
+        mut_repo.clear_git_head();
+    }
+
     let mut changed_git_refs = BTreeMap::new();
     let git_refs = git_repo.references()?;
     for git_ref in git_refs {
@@ -136,19 +155,6 @@ pub fn import_refs(
         mut_repo.record_abandoned_commit(abandoned_commit);
     }
 
-    // TODO: Should this be a separate function? We may not always want to import
-    // the Git HEAD (and add it to our set of heads).
-    if let Ok(head_git_commit) = git_repo
-        .head()
-        .and_then(|head_ref| head_ref.peel_to_commit())
-    {
-        let head_commit_id = CommitId::from_bytes(head_git_commit.id().as_bytes());
-        let head_commit = store.get_commit(&head_commit_id).unwrap();
-        mut_repo.add_head(&head_commit);
-        mut_repo.set_git_head(head_commit_id);
-    } else {
-        mut_repo.clear_git_head();
-    }
     Ok(())
 }
 

lib/src/workspace.rs

@@ -104,7 +104,7 @@ impl Workspace {
         working_copy: WorkingCopy,
         repo_loader: RepoLoader,
     ) -> Result<Workspace, PathError> {
-        let workspace_root = workspace_root.canonicalize().context(&workspace_root)?;
+        let workspace_root = workspace_root.canonicalize().context(workspace_root)?;
         Ok(Workspace {
             workspace_root,
             repo_loader,

lib/tests/test_git.rs

@@ -258,6 +258,35 @@ fn test_import_refs_reimport_head_removed() {
     assert!(!tx.mut_repo().view().heads().contains(&commit_id));
 }
 
+#[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.
+    let settings = testutils::user_settings();
+    let test_repo = TestRepo::init(true);
+    let repo = &test_repo.repo;
+    let git_repo = repo.store().git_repo().unwrap();
+
+    let commit = empty_git_commit(&git_repo, "refs/heads/main", &[]);
+    git_repo.set_head_detached(commit.id()).unwrap();
+
+    let mut tx = repo.start_transaction("test");
+    git::import_refs(tx.mut_repo(), &git_repo).unwrap();
+    tx.mut_repo().rebase_descendants(&settings).unwrap();
+
+    // Delete the branch and re-import. The commit should still be there since HEAD
+    // points to it
+    git_repo
+        .find_reference("refs/heads/main")
+        .unwrap()
+        .delete()
+        .unwrap();
+    git::import_refs(tx.mut_repo(), &git_repo).unwrap();
+    tx.mut_repo().rebase_descendants(&settings).unwrap();
+    let commit_id = CommitId::from_bytes(commit.id().as_bytes());
+    assert!(tx.mut_repo().view().heads().contains(&commit_id));
+}
+
 fn git_ref(git_repo: &git2::Repository, name: &str, target: Oid) {
     git_repo.reference(name, target, true, "").unwrap();
 }
@@ -337,9 +366,7 @@ fn test_import_refs_detached_head() {
         .unwrap();
     let repo = tx.commit();
 
-    let expected_heads = hashset! {
-            commit_id(&commit1),
-    };
+    let expected_heads = hashset!{ commit_id(&commit1) };
     assert_eq!(*repo.view().heads(), expected_heads);
     assert_eq!(repo.view().git_refs().len(), 0);
     assert_eq!(

tests/test_git_colocated.rs

@@ -57,8 +57,8 @@ fn test_git_colocated_rebase_on_import() {
     // refs/heads/master we just exported
     test_env.jj_cmd_success(&workspace_root, &["st"]);
 
-    // Move `master` backwards, which should cause the working copy to be rebased
-    // off of the old position.
+    // Move `master` and HEAD backwards, which should result in commit2 getting
+    // hidden, and a new working-copy commit at the new position.
     let commit2_oid = git_repo
         .find_branch("master", git2::BranchType::Local)
         .unwrap()
@@ -68,13 +68,11 @@ 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 =
         test_env.jj_cmd_success(&workspace_root, &["log", "-T", "commit_id \" \" branches"]);
     insta::assert_snapshot!(stdout, @r###"
-    Rebased 1 descendant commits off of commits rewritten from git
-    Working copy now at: a64f325e0516 (no description set)
-    Added 0 files, modified 1 files, removed 0 files
-    @ a64f325e05167129f3488f85a570f22a8940634f 
+    @ 840303b127545e55dfa5858a97555acf54a80513 
     o f0f3ab56bfa927e3a65c2ac9a513693d438e271b master
     o 0000000000000000000000000000000000000000 
     "###);