WIP: Don't detatch Git HEAD when advance-branches is enabled for a br…

…anch

When setting the working copy commit, if a single branch points to a parent of
the working copy and advance-branches is enabled for that branch, set Git HEAD
to the branch instead of detatching at the parent commit.

cli/src/cli_util.rs

@@ -1142,11 +1142,31 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
             .transpose()?;
         if self.working_copy_shared_with_git {
             let git_repo = self.git_backend().unwrap().open_git_repo()?;
-            if let Some(wc_commit) = &maybe_new_wc_commit {
-                git::reset_head(tx.mut_repo(), &git_repo, wc_commit)?;
-            }
+            // TODO(emesterhazy): Is it problematic that we're exporting these
+            //   refs before resetting head? If the ref export fails, the head
+            //   won't be reset. We could defer returning the error until after
+            //   HEAD is reset, but we need to try to export the refs first so
+            //   that we can set HEAD to an advanceable branch if one exists.
             let failed_branches = git::export_refs(tx.mut_repo())?;
             print_failed_git_export(ui, &failed_branches)?;
+            if let Some(wc_commit) = &maybe_new_wc_commit {
+                // If there's a single branch pointing to one of the working
+                // copy's parents and advance-branches is enabled for it,
+                // then set the Git HEAD to that branch instead of detaching at
+                // the commit.
+                let advanceable = self.get_advanceable_branches(tx.repo(), wc_commit.parent_ids());
+                let parent_branch = if advanceable.len() == 1 {
+                    Some(advanceable[0].name.clone())
+                } else {
+                    None
+                };
+                git::reset_head(
+                    tx.mut_repo(),
+                    &git_repo,
+                    wc_commit,
+                    parent_branch.as_deref(),
+                )?;
+            }
         }
         self.user_repo = ReadonlyUserRepo::new(tx.commit(description));
         self.report_repo_changes(ui, &old_repo)?;

lib/src/git.rs

@@ -904,20 +904,43 @@ fn update_git_ref(
 }
 
 /// Sets `HEAD@git` to the parent of the given working-copy commit and resets
-/// the Git index.
+/// the Git index. If `try_branch` points to the parent of the given
+/// working-copy commit, then the Git HEAD is set to the branch instead of being
+/// detached.
 pub fn reset_head(
     mut_repo: &mut MutableRepo,
     git_repo: &git2::Repository,
     wc_commit: &Commit,
+    try_branch: Option<&str>,
 ) -> Result<(), git2::Error> {
     let first_parent_id = &wc_commit.parent_ids()[0];
+    // Try to look up the branch reference if `try_branch` is provided, but
+    // don't return an error and proceed to detach HEAD it the lookup fails.
+    // Setting HEAD to a branch instead of a commit provides a better Git
+    // interop experience for CLI users that enable the "advance-branches"
+    // feature.
+    let branch_ref = if let Some(branch) = try_branch {
+        git_repo.resolve_reference_from_short_name(branch).ok()
+    } else {
+        None
+    };
+
     if first_parent_id != mut_repo.store().root_commit_id() {
         let first_parent = RefTarget::normal(first_parent_id.clone());
         let git_head = mut_repo.view().git_head();
         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)?;
         if git_head != &first_parent {
-            git_repo.set_head_detached(new_git_commit_id)?;
+            if let Some(branch_ref) = branch_ref {
+                let branch_commit = branch_ref.peel_to_commit()?.id();
+                if branch_commit == new_git_commit_id {
+                    // Set Git HEAD to the branch pointing to the parent Git
+                    // commit instead of detaching.
+                    git_repo.set_head_bytes(branch_ref.name_bytes())?;
+                }
+            } else {
+                git_repo.set_head_detached(new_git_commit_id)?;
+            }
             mut_repo.set_git_head_target(first_parent);
         }
         git_repo.reset(new_git_commit.as_object(), git2::ResetType::Mixed, None)?;
@@ -941,6 +964,7 @@ pub fn reset_head(
         git_repo.cleanup_state()?;
         mut_repo.set_git_head_target(RefTarget::absent());
     }
+
     Ok(())
 }
 

lib/tests/test_git.rs

@@ -1942,23 +1942,23 @@ fn test_reset_head_to_root() {
         .unwrap();
 
     // Set Git HEAD to commit2's parent (i.e. commit1)
-    git::reset_head(tx.mut_repo(), &git_repo, &commit2).unwrap();
+    git::reset_head(tx.mut_repo(), &git_repo, &commit2, None).unwrap();
     assert!(git_repo.head().is_ok());
     assert_eq!(
         tx.mut_repo().git_head(),
         RefTarget::normal(commit1.id().clone())
     );
 
     // Set Git HEAD back to root
-    git::reset_head(tx.mut_repo(), &git_repo, &commit1).unwrap();
+    git::reset_head(tx.mut_repo(), &git_repo, &commit1, None).unwrap();
     assert!(git_repo.head().is_err());
     assert!(tx.mut_repo().git_head().is_absent());
 
     // Move placeholder ref as if new commit were created by git
     git_repo
         .reference("refs/jj/root", git_id(&commit1), false, "")
         .unwrap();
-    git::reset_head(tx.mut_repo(), &git_repo, &commit2).unwrap();
+    git::reset_head(tx.mut_repo(), &git_repo, &commit2, None).unwrap();
     assert!(git_repo.head().is_ok());
     assert_eq!(
         tx.mut_repo().git_head(),
@@ -1967,7 +1967,7 @@ fn test_reset_head_to_root() {
     assert!(git_repo.find_reference("refs/jj/root").is_ok());
 
     // Set Git HEAD back to root
-    git::reset_head(tx.mut_repo(), &git_repo, &commit1).unwrap();
+    git::reset_head(tx.mut_repo(), &git_repo, &commit1, None).unwrap();
     assert!(git_repo.head().is_err());
     assert!(tx.mut_repo().git_head().is_absent());
     // The placeholder ref should be deleted