forked from mirrors/jj
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.
This commit is contained in:
parent
cf5b86b9b7
commit
20eb9ecec1
5 changed files with 58 additions and 24 deletions
|
@ -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).
|
||||
|
|
|
@ -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(())
|
||||
}
|
||||
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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!(
|
||||
|
|
|
@ -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
|
||||
"###);
|
||||
|
|
Loading…
Reference in a new issue