diff --git a/CHANGELOG.md b/CHANGELOG.md index d210bbbc237..73b8921aabe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the previously checked-out anonymous branch. [#1042](https://github.com/martinvonz/jj/issues/1042). +* `jj git fetch` in a colocated repo now abandones branches deleted on the + remote, just like in a non-colocated repo. + [#864](https://github.com/martinvonz/jj/issues/864) + ## [0.7.0] - 2023-02-16 ### Breaking changes diff --git a/docs/git-compatibility.md b/docs/git-compatibility.md index bfe04a300a6..e0e8bbcce7c 100644 --- a/docs/git-compatibility.md +++ b/docs/git-compatibility.md @@ -98,7 +98,7 @@ commands and readonly Git commands. It's also useful when tools (e.g. build tools) expect a Git repo to be present. There are some bugs and surprising behavior related to `jj undo` in this mode, -such as #864 and #922. +such as #922. ## Creating a repo by cloning a Git repo diff --git a/lib/src/git.rs b/lib/src/git.rs index 3df6080c00d..95627316270 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -12,12 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::default::Default; use std::path::PathBuf; use git2::Oid; use itertools::Itertools; +use maplit::hashset; use thiserror::Error; use crate::backend::{CommitId, ObjectId}; @@ -83,12 +84,15 @@ pub fn import_some_refs( let store = mut_repo.store().clone(); let mut existing_git_refs = mut_repo.view().git_refs().clone(); let mut old_git_heads = vec![]; - let mut new_git_heads = HashSet::new(); + let mut new_git_heads = HashMap::new(); for (ref_name, old_target) in &existing_git_refs { if git_ref_filter(ref_name) { old_git_heads.extend(old_target.adds()); } else { - new_git_heads.extend(old_target.adds()); + new_git_heads.insert( + ref_name.to_string(), + old_target.adds().iter().cloned().collect(), + ); } } @@ -103,7 +107,7 @@ pub fn import_some_refs( // 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()); + new_git_heads.insert("HEAD".to_string(), hashset![head_commit_id.clone()]); prevent_gc(git_repo, &head_commit_id); mut_repo.add_head(&head_commit); mut_repo.set_git_head(RefTarget::Normal(head_commit_id)); @@ -136,7 +140,7 @@ pub fn import_some_refs( } }; let id = CommitId::from_bytes(git_commit.id().as_bytes()); - new_git_heads.insert(id.clone()); + new_git_heads.insert(full_name.to_string(), hashset![id.clone()]); if !git_ref_filter(&full_name) { continue; } @@ -168,11 +172,20 @@ pub fn import_some_refs( continue; } if let RefName::RemoteBranch { branch, remote: _ } = ref_name { + // TODO: Make `merge_single_ref` return a value mut_repo.merge_single_ref( - &RefName::LocalBranch(branch), + &RefName::LocalBranch(branch.clone()), old_git_target.as_ref(), new_git_target.as_ref(), ); + if mut_repo.get_local_branch(&branch).is_none() { + // We are deleting this local branch to make it match the change to the + // remote-tracking branches. Note that this only happens if + // the branch was *not* changed locally, otherwise mut_repo. + // get_local_branch would return a conflict at this point. + new_git_heads.remove(&format!("refs/heads/{branch}")); + // TODO: Nice functions + } } } } @@ -180,17 +193,18 @@ pub fn import_some_refs( // Find commits that are no longer referenced in the git repo and abandon them // in jj as well. We must remove non-existing commits from new_git_heads, as // they could have come from branches which were never fetched. - let new_git_heads = new_git_heads - .into_iter() - .filter(|id| mut_repo.index().has_id(id)) - .collect_vec(); + let mut new_git_heads_set = HashSet::new(); + for set in new_git_heads.into_values() { + new_git_heads_set.extend(set.into_iter()); + } + new_git_heads_set.retain(|id| mut_repo.index().has_id(id)); // We could use mut_repo.record_rewrites() here but we know we only need to care // about abandoned commits for now. We may want to change this if we ever // add a way of preserving change IDs across rewrites by `git` (e.g. by // putting them in the commit message). let abandoned_commits = mut_repo .index() - .walk_revs(&old_git_heads, &new_git_heads) + .walk_revs(&old_git_heads, &new_git_heads_set.into_iter().collect_vec()) .map(|entry| entry.commit_id()) .collect_vec(); let root_commit_id = mut_repo.store().root_commit_id().clone(); diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index a57f7c590c4..ec8ac50c04f 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -231,9 +231,7 @@ fn test_import_refs_reimport() { let view = repo.view(); let expected_heads = hashset! { - // BUG: commit5 is still listed as a head (#864) even though the - // feature-remote branch was deleted as we'll see below. - jj_id(&commit5), + // feature-remote, which was deleted on the remote, is no longer a head (#864) jj_id(&commit6), commit7.id().clone(), }; diff --git a/tests/test_git_colocated.rs b/tests/test_git_colocated.rs index 4f46969a34f..53801187cae 100644 --- a/tests/test_git_colocated.rs +++ b/tests/test_git_colocated.rs @@ -260,11 +260,9 @@ fn test_git_colocated_fetch_deleted_branch() { test_env.jj_cmd_success(&origin_path, &["branch", "delete", "B"]); let stdout = test_env.jj_cmd_success(&clone_path, &["git", "fetch"]); insta::assert_snapshot!(stdout, @""); - // TODO: e1f4 should have been abandoned (#864) + // e1f4 is abandoned, as it was deleted on the remote (#864) insta::assert_snapshot!(get_log_output(&test_env, &clone_path), @r###" @ 1fa8b2e27c8c3da9764bda953dd81a06fb292d1a - │ ◉ e1f4268fabd2c84e880c5eb5bd87e076180fc8e3 - ├─╯ ◉ a86754f975f953fa25da4265764adc0c62e9ce6b A master HEAD@git ◉ 0000000000000000000000000000000000000000 "###);