git: on external HEAD move, do not abandon old branch

The current behavior was introduced by 20eb9ecec1 "git: don't abandon
HEAD commit when it loses a branch." While the change made HEAD mutation
behavior more consistent with a plain ref operation, HEAD can also move on
checkout, and checkout shouldn't be considered a history rewriting operation.

I'm not saying the new behavior is always correct, but I think it's safer
than losing old HEAD branch. I also think this change will help if we want
to extract HEAD management function from git::import_refs().

Fixes #1042.
This commit is contained in:
Yuya Nishihara 2023-05-10 19:10:40 +09:00
parent 66d405fa5f
commit 92cfffd843
4 changed files with 18 additions and 9 deletions

View File

@ -105,6 +105,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* It is no longer allowed to create branches at the root commit.
* `git checkout` (without using `jj`) in colocated repo no longer abandons
the previously checked-out anonymous branch.
[#1042](https://github.com/martinvonz/jj/issues/1042).
## [0.7.0] - 2023-02-16
### Breaking changes

View File

@ -91,9 +91,6 @@ pub fn import_some_refs(
new_git_heads.extend(old_target.adds());
}
}
if let Some(old_git_head) = mut_repo.view().git_head() {
old_git_heads.extend(old_git_head.adds());
}
// 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).
@ -101,6 +98,9 @@ pub fn import_some_refs(
.head()
.and_then(|head_ref| head_ref.peel_to_commit())
{
// Add the current HEAD to `new_git_heads` to pin the branch. It's not added
// to `old_git_heads` because HEAD move doesn't automatically mean the old
// HEAD branch has been rewritten.
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());

View File

@ -336,11 +336,13 @@ fn test_import_refs_reimport_git_head_without_ref() {
// Move HEAD to commit2 (by e.g. `git checkout` command)
git_repo.set_head_detached(git_id(&commit2)).unwrap();
// Reimport HEAD, which abandons the old HEAD branch because jj thinks it
// would be rewritten by e.g. `git commit --amend` command.
// Reimport HEAD, which doesn't abandon the old HEAD branch because jj thinks it
// would be moved by `git checkout` command. This isn't always true because the
// detached HEAD commit could be rewritten by e.g. `git commit --amend` command,
// but it should be safer than abandoning old checkout branch.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(!tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit1.id()));
assert!(tx.mut_repo().view().heads().contains(commit2.id()));
}
@ -374,7 +376,7 @@ fn test_import_refs_reimport_git_head_with_moved_ref() {
.unwrap();
git_repo.set_head_detached(git_id(&commit2)).unwrap();
// Reimport HEAD and main, which abandons the old HEAD/main branch.
// Reimport HEAD and main, which abandons the old main branch.
git::import_refs(tx.mut_repo(), &git_repo, &git_settings).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
assert!(!tx.mut_repo().view().heads().contains(commit1.id()));

View File

@ -297,10 +297,13 @@ fn test_git_colocated_external_checkout() {
)
.unwrap();
// The old HEAD branch gets abandoned because jj thinks it has been rewritten.
// The old working-copy commit gets abandoned, but the whole branch should not
// be abandoned. (#1042)
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ 0521ce3b8c4e29aab79f3c750e2845dcbc4c3874
a86754f975f953fa25da4265764adc0c62e9ce6b master
66f4d1806ae41bd604f69155dece64062a0056cf
a86754f975f953fa25da4265764adc0c62e9ce6b master
0000000000000000000000000000000000000000
"###);
}