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
samueltardieu committed Mar 2, 2023
1 parent 182919f commit 5ecdeed
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 9 deletions.
18 changes: 17 additions & 1 deletion lib/src/git.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,23 @@ 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) = 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)?;
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
View file
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
151 changes: 144 additions & 7 deletions tests/test_git_fetch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,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 @@ -646,11 +642,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 @@ -735,3 +728,147 @@ 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
"###);
}

#[test]
fn test_git_fetch_removed_parent_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 all branches in origin.
test_env.jj_cmd_success(&source_git_repo_path, &["branch", "forget", "--glob", "*"]);

// Fetch branches master, trunk1 and a1 from origin and check that only those
// branches have been removed and that others were not rebased because of
// abandoned commits.
let stdout = test_env.jj_cmd_success(
&target_jj_repo_path,
&[
"git", "fetch", "--branch", "master", "--branch", "trunk1", "--branch", "a1",
],
);
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 ff36dc55760e descr_for_trunk1
│ @ 230dd059e1b0
├─╯
o 000000000000
"###);
}

0 comments on commit 5ecdeed

Please sign in to comment.