From 182919ff6f2c0e0f486d8f4f060dee6380c56ea6 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sun, 26 Feb 2023 15:23:04 +0100 Subject: [PATCH] git: add function to import a selection of the git refs --- lib/src/git.rs | 43 +++++++++--- lib/tests/test_git.rs | 151 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 9 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 050e1e63c..fb01ed978 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -67,18 +67,34 @@ pub fn import_refs( mut_repo: &mut MutableRepo, git_repo: &git2::Repository, git_settings: &GitSettings, +) -> Result<(), GitImportError> { + import_some_refs(mut_repo, git_repo, git_settings, |_| true) +} + +/// Reflect changes made in the underlying Git repo in the Jujutsu repo. +/// Only branches whose git full reference name pass the filter will be +/// considered for addition, update, or deletion. +pub fn import_some_refs( + mut_repo: &mut MutableRepo, + git_repo: &git2::Repository, + git_settings: &GitSettings, + git_ref_filter: impl Fn(&str) -> bool, ) -> Result<(), GitImportError> { let store = mut_repo.store().clone(); let mut existing_git_refs = mut_repo.view().git_refs().clone(); - let mut old_git_heads = existing_git_refs - .values() - .flat_map(|old_target| old_target.adds()) - .collect_vec(); + let mut old_git_heads = vec![]; + let mut new_git_heads = HashSet::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()); + } + } if let Some(old_git_head) = mut_repo.view().git_head() { old_git_heads.extend(old_git_head.adds()); } - 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 @@ -121,6 +137,9 @@ pub fn import_refs( }; let id = CommitId::from_bytes(git_commit.id().as_bytes()); new_git_heads.insert(id.clone()); + if !git_ref_filter(&full_name) { + continue; + } // TODO: Make it configurable which remotes are publishing and update public // heads here. let old_target = existing_git_refs.remove(&full_name); @@ -134,8 +153,10 @@ pub fn import_refs( } } for (full_name, target) in existing_git_refs { - mut_repo.remove_git_ref(&full_name); - changed_git_refs.insert(full_name, (Some(target), None)); + if git_ref_filter(&full_name) { + mut_repo.remove_git_ref(&full_name); + changed_git_refs.insert(full_name, (Some(target), None)); + } } for (full_name, (old_git_target, new_git_target)) in changed_git_refs { if let Some(ref_name) = parse_git_ref(&full_name) { @@ -157,8 +178,12 @@ pub fn import_refs( } // Find commits that are no longer referenced in the git repo and abandon them - // in jj as well. - let new_git_heads = new_git_heads.into_iter().collect_vec(); + // 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(); // 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 diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index e689499a5..ced3aa40d 100644 --- a/lib/tests/test_git.rs +++ b/lib/tests/test_git.rs @@ -340,6 +340,157 @@ fn test_import_refs_reimport_all_from_root_removed() { assert!(!tx.mut_repo().view().heads().contains(&jj_id(&commit))); } +#[test] +fn test_import_some_refs() { + let settings = testutils::user_settings(); + let git_settings = GitSettings::default(); + let test_workspace = TestRepo::init(true); + let repo = &test_workspace.repo; + let git_repo = repo.store().git_repo().unwrap(); + + let commit_main = empty_git_commit(&git_repo, "refs/remotes/origin/main", &[]); + let commit_feat1 = empty_git_commit(&git_repo, "refs/remotes/origin/feature1", &[&commit_main]); + let commit_feat2 = + empty_git_commit(&git_repo, "refs/remotes/origin/feature2", &[&commit_feat1]); + let commit_feat3 = + empty_git_commit(&git_repo, "refs/remotes/origin/feature3", &[&commit_feat1]); + let commit_feat4 = + empty_git_commit(&git_repo, "refs/remotes/origin/feature4", &[&commit_feat3]); + let commit_ign = empty_git_commit(&git_repo, "refs/remotes/origin/ignored", &[]); + + // Import branches feature1, feature2, and feature3. + let mut tx = repo.start_transaction(&settings, "test"); + git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + ref_name.starts_with("refs/remotes/origin/feature") + }) + .unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + let repo = tx.commit(); + + // There are two heads, feature2 and feature4. + let view = repo.view(); + let expected_heads = hashset! { + jj_id(&commit_feat2), + jj_id(&commit_feat4), + }; + assert_eq!(*view.heads(), expected_heads); + + // Check that branches feature[1-4] have been locally imported and are known to + // be present on origin as well. + assert_eq!(view.branches().len(), 4); + let commit_feat1_target = RefTarget::Normal(jj_id(&commit_feat1)); + let commit_feat2_target = RefTarget::Normal(jj_id(&commit_feat2)); + let commit_feat3_target = RefTarget::Normal(jj_id(&commit_feat3)); + let commit_feat4_target = RefTarget::Normal(jj_id(&commit_feat4)); + let expected_feature1_branch = BranchTarget { + local_target: Some(RefTarget::Normal(jj_id(&commit_feat1))), + remote_targets: btreemap! { "origin".to_string() => commit_feat1_target }, + }; + assert_eq!( + view.branches().get("feature1"), + Some(expected_feature1_branch).as_ref() + ); + let expected_feature2_branch = BranchTarget { + local_target: Some(RefTarget::Normal(jj_id(&commit_feat2))), + remote_targets: btreemap! { "origin".to_string() => commit_feat2_target }, + }; + assert_eq!( + view.branches().get("feature2"), + Some(expected_feature2_branch).as_ref() + ); + let expected_feature3_branch = BranchTarget { + local_target: Some(RefTarget::Normal(jj_id(&commit_feat3))), + remote_targets: btreemap! { "origin".to_string() => commit_feat3_target }, + }; + assert_eq!( + view.branches().get("feature3"), + Some(expected_feature3_branch).as_ref() + ); + let expected_feature4_branch = BranchTarget { + local_target: Some(RefTarget::Normal(jj_id(&commit_feat4))), + remote_targets: btreemap! { "origin".to_string() => commit_feat4_target }, + }; + assert_eq!( + view.branches().get("feature4"), + Some(expected_feature4_branch).as_ref() + ); + assert_eq!(view.branches().get("main"), None,); + assert!(!view.heads().contains(&jj_id(&commit_main))); + assert_eq!(view.branches().get("ignored"), None,); + assert!(!view.heads().contains(&jj_id(&commit_ign))); + + // Delete branch feature1, feature3 and feature4 in git repository and import + // branch feature2 only. That should have no impact on the jj repository. + delete_git_ref(&git_repo, "refs/remotes/origin/feature1"); + delete_git_ref(&git_repo, "refs/remotes/origin/feature3"); + delete_git_ref(&git_repo, "refs/remotes/origin/feature4"); + let mut tx = repo.start_transaction(&settings, "test"); + git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + ref_name == "refs/remotes/origin/feature2" + }) + .unwrap(); + tx.mut_repo().rebase_descendants(&settings).unwrap(); + let repo = tx.commit(); + + // feature2 and feature4 will still be heads, and all four branches should be + // present. + let view = repo.view(); + assert_eq!(view.branches().len(), 4); + assert_eq!(*view.heads(), expected_heads); + + // Import feature1: this should cause the branch to be deleted, but the + // corresponding commit should stay because it is reachable from feature2. + let mut tx = repo.start_transaction(&settings, "test"); + git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + ref_name == "refs/remotes/origin/feature1" + }) + .unwrap(); + // No descendant should be rewritten. + assert_eq!(tx.mut_repo().rebase_descendants(&settings).unwrap(), 0); + let repo = tx.commit(); + + // feature2 and feature4 should still be the heads, and all three branches + // feature2, feature3, and feature3 should exist. + let view = repo.view(); + assert_eq!(view.branches().len(), 3); + assert_eq!(*view.heads(), expected_heads); + + // Import feature3: this should cause the branch to be deleted, but + // feature4 should be left alone even though it is no longer in git. + let mut tx = repo.start_transaction(&settings, "test"); + git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + ref_name == "refs/remotes/origin/feature3" + }) + .unwrap(); + // No descendant should be rewritten + assert_eq!(tx.mut_repo().rebase_descendants(&settings).unwrap(), 0); + let repo = tx.commit(); + + // feature2 and feature4 should still be the heads, and both branches + // should exist. + let view = repo.view(); + assert_eq!(view.branches().len(), 2); + assert_eq!(*view.heads(), expected_heads); + + // Import feature4: both the head and the branch will disappear. + let mut tx = repo.start_transaction(&settings, "test"); + git::import_some_refs(tx.mut_repo(), &git_repo, &git_settings, |ref_name| { + ref_name == "refs/remotes/origin/feature4" + }) + .unwrap(); + // No descendant should be rewritten + assert_eq!(tx.mut_repo().rebase_descendants(&settings).unwrap(), 0); + let repo = tx.commit(); + + // feature2 should now be the only head and only branch. + let view = repo.view(); + assert_eq!(view.branches().len(), 1); + let expected_heads = hashset! { + jj_id(&commit_feat2), + }; + assert_eq!(*view.heads(), expected_heads); +} + fn git_ref(git_repo: &git2::Repository, name: &str, target: Oid) { git_repo.reference(name, target, true, "").unwrap(); }