Skip to content

Commit

Permalink
git: only consider references matching globs when fetching
Browse files Browse the repository at this point in the history
  • Loading branch information
samueltardieu committed Mar 1, 2023
1 parent 47c41ee commit a14e96c
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 9 deletions.
22 changes: 21 additions & 1 deletion lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,27 @@ pub fn fetch(
tracing::debug!("remote.disconnect");
remote.disconnect()?;
tracing::debug!("import_refs");
import_refs(mut_repo, git_repo, git_settings).map_err(|err| match err {
if let Some(globs) = globs {
let mut pattern = format!("^refs/remotes/{remote_name}/(");
for (i, glob) in globs.iter().enumerate() {
if i != 0 {
pattern.push('|');
}
pattern.push_str(&glob.replace('*', ".*"));
}
pattern.push_str(")$");
tracing::debug!(?globs, ?pattern, "globs as regex");
let 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) },
)
} else {
import_refs(mut_repo, git_repo, git_settings)
}
.map_err(|err| match err {
GitImportError::InternalGitError(source) => GitFetchError::InternalGitError(source),
})?;
Ok(default_branch)
Expand Down
2 changes: 1 addition & 1 deletion src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub struct GitRemoteListArgs {}
/// Fetch from a Git remote
#[derive(clap::Args, Clone, Debug)]
pub struct GitFetchArgs {
/// Fetch only some of the branches (caution: known bugs)
/// Fetch only some of the branches
///
/// Any `*` in the argument is expanded as a glob. So, one `--branch` can
/// match several branches.
Expand Down
84 changes: 77 additions & 7 deletions tests/test_git_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,10 +598,6 @@ fn test_git_fetch_some_of_many_branches() {
"###);
}

// TODO: Fix the bug this test demonstrates. (https://github.com/martinvonz/jj/issues/1300)
// The issue likely stems from the fact that `jj undo` does not undo the fetch
// inside the git repo backing the `target` repo. It is unclear whether it
// should.
#[test]
fn test_git_fetch_undo() {
let test_env = TestEnvironment::default();
Expand Down Expand Up @@ -654,11 +650,8 @@ fn test_git_fetch_undo() {
// Now try to fetch just one branch
let stdout = test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch", "--branch", "b"]);
insta::assert_snapshot!(stdout, @"");
// BUG: Both branches got fetched.
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
o c7d4bdcbc215 descr_for_b b
│ o 359a9a02457d descr_for_a1 a1
├─╯
o ff36dc55760e descr_for_trunk1
│ @ 230dd059e1b0
├─╯
Expand Down Expand Up @@ -743,3 +736,80 @@ fn test_git_fetch_rename_fetch() {
Nothing changed.
"###);
}

#[test]
fn test_git_fetch_removed_branch() {
let test_env = TestEnvironment::default();
let source_git_repo_path = test_env.env_root().join("source");
let _git_repo = git2::Repository::init(source_git_repo_path.clone()).unwrap();

// Clone an empty repo. The target repo is a normal `jj` repo, *not* colocated
let stdout =
test_env.jj_cmd_success(test_env.env_root(), &["git", "clone", "source", "target"]);
insta::assert_snapshot!(stdout, @r###"
Fetching into new repo in "$TEST_ENV/target"
Nothing changed.
"###);
let target_jj_repo_path = test_env.env_root().join("target");

let source_log =
create_colocated_repo_and_branches_from_trunk1(&test_env, &source_git_repo_path);
insta::assert_snapshot!(source_log, @r###"
===== Source git repo contents =====
@ c7d4bdcbc215 descr_for_b b
│ o decaa3966c83 descr_for_a2 a2
├─╯
│ o 359a9a02457d descr_for_a1 a1
├─╯
o ff36dc55760e descr_for_trunk1 master trunk1
o 000000000000
"###);

// Fetch all branches
let stdout = test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
o c7d4bdcbc215 descr_for_b b
│ o decaa3966c83 descr_for_a2 a2
├─╯
│ o 359a9a02457d descr_for_a1 a1
├─╯
o ff36dc55760e descr_for_trunk1 master trunk1
│ @ 230dd059e1b0
├─╯
o 000000000000
"###);

// Remove a2 branch in origin
test_env.jj_cmd_success(&source_git_repo_path, &["branch", "forget", "a2"]);

// Fetch branch a1 from origin and check that a2 is still there
let stdout = test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch", "--branch", "a1"]);
insta::assert_snapshot!(stdout, @r###"
Nothing changed.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
o c7d4bdcbc215 descr_for_b b
│ o decaa3966c83 descr_for_a2 a2
├─╯
│ o 359a9a02457d descr_for_a1 a1
├─╯
o ff36dc55760e descr_for_trunk1 master trunk1
│ @ 230dd059e1b0
├─╯
o 000000000000
"###);

// Fetch branches a2 from origin, and check that it has been removed locally
let stdout = test_env.jj_cmd_success(&target_jj_repo_path, &["git", "fetch", "--branch", "a2"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
o c7d4bdcbc215 descr_for_b b
│ o 359a9a02457d descr_for_a1 a1
├─╯
o ff36dc55760e descr_for_trunk1 master trunk1
│ @ 230dd059e1b0
├─╯
o 000000000000
"###);
}

0 comments on commit a14e96c

Please sign in to comment.