From a14e96c4887b816fb8772990580ef82e5eb17f7e Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sun, 26 Feb 2023 15:23:04 +0100 Subject: [PATCH] git: only consider references matching globs when fetching --- lib/src/git.rs | 22 ++++++++++- src/commands/git.rs | 2 +- tests/test_git_fetch.rs | 84 +++++++++++++++++++++++++++++++++++++---- 3 files changed, 99 insertions(+), 9 deletions(-) diff --git a/lib/src/git.rs b/lib/src/git.rs index 1c180dfcf50..24668cb34f5 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -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) diff --git a/src/commands/git.rs b/src/commands/git.rs index 7ba72b1a32d..7424f495308 100644 --- a/src/commands/git.rs +++ b/src/commands/git.rs @@ -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. diff --git a/tests/test_git_fetch.rs b/tests/test_git_fetch.rs index 9b4f45f8490..059f2f8a15b 100644 --- a/tests/test_git_fetch.rs +++ b/tests/test_git_fetch.rs @@ -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(); @@ -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 ├─╯ @@ -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 + "###); +}