diff --git a/CHANGELOG.md b/CHANGELOG.md index a266abc8e..a8460143b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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). diff --git a/lib/src/git.rs b/lib/src/git.rs index 3136f6ced..b6c2b8b05 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -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(()) } diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index 9bd0853c1..0246405ad 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -104,7 +104,7 @@ impl Workspace { working_copy: WorkingCopy, repo_loader: RepoLoader, ) -> Result { - let workspace_root = workspace_root.canonicalize().context(&workspace_root)?; + let workspace_root = workspace_root.canonicalize().context(workspace_root)?; Ok(Workspace { workspace_root, repo_loader, diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 61d6f79bd..58c64c43e 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -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!( diff --git a/tests/test_git_colocated.rs b/tests/test_git_colocated.rs index 06710efe5..3545a9816 100644 --- a/tests/test_git_colocated.rs +++ b/tests/test_git_colocated.rs @@ -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 "###);