Skip to content

Commit

Permalink
WIP: Don't detatch Git HEAD when advance-branches is enabled for a br…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
emesterhazy committed Mar 13, 2024
1 parent 18e7d0b commit bbff561
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 9 deletions.
25 changes: 22 additions & 3 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
28 changes: 26 additions & 2 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand All @@ -941,6 +964,7 @@ pub fn reset_head(
git_repo.cleanup_state()?;
mut_repo.set_git_head_target(RefTarget::absent());
}

Ok(())
}

Expand Down
8 changes: 4 additions & 4 deletions lib/tests/test_git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
Expand Down

0 comments on commit bbff561

Please sign in to comment.