git: don't abandon root commit when all refs are gone

If you remove all refs from the backing Git repo and then run `jj git
import`, we would see that all commits disappeared from the Git repo,
so we would remove them from the jj repo too. However, we do that by
doing a history walk from old heads to the new heads, which includes
the root commit when the new heads is an empty set. That means that we
mark the root commit as abandoned, which led to a crash in
`rewrite.rs` (when we try pick the root commit's first parent to use
as parent for rebased commits).
This commit is contained in:
Martin von Zweigbergk 2022-10-28 16:18:56 -07:00 committed by Martin von Zweigbergk
parent 20eb9ecec1
commit 416a36a59c
5 changed files with 40 additions and 2 deletions

View file

@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `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.
* `jj git import` no longer crashes when all Git refs are removed.
## [0.5.1] - 2022-10-17
No changes (just trying to get automated GitHub release to work).

View file

@ -151,8 +151,11 @@ pub fn import_refs(
.walk_revs(&old_git_heads, &new_git_heads)
.map(|entry| entry.commit_id())
.collect_vec();
let root_commit_id = mut_repo.store().root_commit_id().clone();
for abandoned_commit in abandoned_commits {
mut_repo.record_abandoned_commit(abandoned_commit);
if abandoned_commit != root_commit_id {
mut_repo.record_abandoned_commit(abandoned_commit);
}
}
Ok(())

View file

@ -539,6 +539,7 @@ impl MutableRepo {
/// convenient place to record it. It won't matter after the transaction
/// has been committed.
pub fn record_rewritten_commit(&mut self, old_id: CommitId, new_id: CommitId) {
assert_ne!(old_id, *self.store().root_commit_id());
self.rewritten_commits
.entry(old_id)
.or_default()
@ -556,6 +557,7 @@ impl MutableRepo {
/// convenient place to record it. It won't matter after the transaction
/// has been committed.
pub fn record_abandoned_commit(&mut self, old_id: CommitId) {
assert_ne!(old_id, *self.store().root_commit_id());
self.abandoned_commits.insert(old_id);
}

View file

@ -145,6 +145,9 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
rewritten: HashMap<CommitId, HashSet<CommitId>>,
abandoned: HashSet<CommitId>,
) -> DescendantRebaser<'settings, 'repo> {
let root_commit_id = mut_repo.store().root_commit_id();
assert!(!abandoned.contains(root_commit_id));
assert!(!rewritten.contains_key(root_commit_id));
let old_commits_expression = RevsetExpression::commits(rewritten.keys().cloned().collect())
.union(&RevsetExpression::commits(
abandoned.iter().cloned().collect(),

View file

@ -287,6 +287,34 @@ fn test_import_refs_reimport_git_head_counts() {
assert!(tx.mut_repo().view().heads().contains(&commit_id));
}
#[test]
fn test_import_refs_reimport_all_from_root_removed() {
// Test that if a chain of commits all the way from the root gets unreferenced,
// we abandon the whole stack, but not including the root commit.
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", &[]);
let mut tx = repo.start_transaction("test");
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());
// Test the setup
assert!(tx.mut_repo().view().heads().contains(&commit_id));
// Remove all git refs and re-import
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();
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();
}
@ -366,7 +394,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!(