diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 9d75dd0b83..ea99dc2281 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1174,11 +1174,30 @@ 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 parent_branch = self + .get_advanceable_branch(tx.repo(), wc_commit.parent_ids()) + .ok() + .flatten() + .map(|b| b.name); + 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)?; diff --git a/lib/src/git.rs b/lib/src/git.rs index e05a6987c5..6656f38dea 100644 --- a/lib/src/git.rs +++ b/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(()) } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 5bdf2c0970..c6b33b3acc 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -1942,7 +1942,7 @@ 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(), @@ -1950,7 +1950,7 @@ fn test_reset_head_to_root() { ); // 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()); @@ -1958,7 +1958,7 @@ fn test_reset_head_to_root() { 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