git: pass RefName enum to git_ref_filter callback

I think it's slightly better to compare each ref fragment than building
"refs/remotes/{remote}/{branch}" pattern to be matched.
This commit is contained in:
Yuya Nishihara 2023-06-28 19:32:49 +09:00
parent da3c03206c
commit a07574a233
2 changed files with 28 additions and 11 deletions

View file

@ -147,7 +147,7 @@ pub fn import_some_refs(
mut_repo: &mut MutableRepo,
git_repo: &git2::Repository,
git_settings: &GitSettings,
git_ref_filter: impl Fn(&str) -> bool,
git_ref_filter: impl Fn(&RefName) -> bool,
) -> Result<(), GitImportError> {
let store = mut_repo.store().clone();
let mut jj_view_git_refs = mut_repo.view().git_refs().clone();
@ -200,7 +200,7 @@ pub fn import_some_refs(
continue;
};
pinned_git_heads.insert(ref_name.clone(), vec![id.clone()]);
if !git_ref_filter(full_name) {
if !git_ref_filter(&ref_name) {
continue;
}
// TODO: Make it configurable which remotes are publishing and update public
@ -218,7 +218,7 @@ pub fn import_some_refs(
for (full_name, target) in jj_view_git_refs {
// TODO: or clean up invalid ref in case it was stored due to historical bug?
let ref_name = parse_git_ref(&full_name).expect("stored git ref should be parsable");
if git_ref_filter(&full_name) {
if git_ref_filter(&ref_name) {
mut_repo.remove_git_ref(&full_name);
changed_git_refs.insert(ref_name, (Some(target), None));
} else {
@ -520,16 +520,23 @@ pub fn fetch(
tracing::debug!("import_refs");
if let Some(globs) = branch_name_globs {
let pattern = format!(
"^refs/remotes/{remote_name}/({})$",
"^({})$",
globs.iter().map(|glob| glob.replace('*', ".*")).join("|")
);
tracing::debug!(?globs, ?pattern, "globs as regex");
let regex = regex::Regex::new(&pattern).map_err(|_| GitFetchError::InvalidGlob)?;
let branch_regex = regex::Regex::new(&pattern).map_err(|_| GitFetchError::InvalidGlob)?;
import_some_refs(
mut_repo,
git_repo,
git_settings,
move |git_ref_name: &str| -> bool { regex.is_match(git_ref_name) },
|ref_name: &RefName| -> bool {
match ref_name {
RefName::RemoteBranch { branch, remote } => {
remote == remote_name && branch_regex.is_match(branch)
}
RefName::LocalBranch(..) | RefName::Tag(..) | RefName::GitRef(..) => false,
}
},
)
} else {
import_refs(mut_repo, git_repo, git_settings)

View file

@ -30,6 +30,7 @@ use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::op_store::{BranchTarget, RefTarget};
use jujutsu_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
use jujutsu_lib::settings::{GitSettings, UserSettings};
use jujutsu_lib::view::RefName;
use maplit::{btreemap, hashset};
use tempfile::TempDir;
use testutils::{create_random_commit, load_repo_at_head, write_random_commit, TestRepo};
@ -687,10 +688,19 @@ fn test_import_some_refs() {
empty_git_commit(&git_repo, "refs/remotes/origin/feature4", &[&commit_feat3]);
let commit_ign = empty_git_commit(&git_repo, "refs/remotes/origin/ignored", &[]);
fn get_remote_branch(ref_name: &RefName) -> Option<&str> {
match ref_name {
RefName::RemoteBranch { branch, remote } if remote == "origin" => Some(branch),
_ => None,
}
}
// 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")
get_remote_branch(ref_name)
.map(|branch| branch.starts_with("feature"))
.unwrap_or(false)
})
.unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
@ -755,7 +765,7 @@ fn test_import_some_refs() {
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"
get_remote_branch(ref_name) == Some("feature2")
})
.unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
@ -771,7 +781,7 @@ fn test_import_some_refs() {
// 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"
get_remote_branch(ref_name) == Some("feature1")
})
.unwrap();
// No descendant should be rewritten.
@ -788,7 +798,7 @@ fn test_import_some_refs() {
// 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"
get_remote_branch(ref_name) == Some("feature3")
})
.unwrap();
// No descendant should be rewritten
@ -804,7 +814,7 @@ fn test_import_some_refs() {
// 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"
get_remote_branch(ref_name) == Some("feature4")
})
.unwrap();
// No descendant should be rewritten