git: simply call fetch() with one or more branch name filters

cli/src/commands/git.rs

@@ -100,7 +100,7 @@ pub struct GitFetchArgs {
     ///
     /// By default, the specified name matches exactly. Use `glob:` prefix to
     /// expand `*` as a glob. The other wildcard characters aren't supported.
-    #[arg(long, value_parser = parse_string_pattern)]
+    #[arg(long, default_value = "glob:*", value_parser = parse_string_pattern)]
     branch: Vec<StringPattern>,
     /// The remote to fetch from (only named remotes are supported, can be
     /// repeated)
@@ -319,7 +319,7 @@ fn cmd_git_fetch(
                 tx.mut_repo(),
                 &git_repo,
                 &remote,
-                (!args.branch.is_empty()).then_some(&args.branch),
+                &args.branch,
                 cb,
                 &command.settings().git_settings(),
             )
@@ -535,7 +535,7 @@ fn do_git_clone(
             fetch_tx.mut_repo(),
             &git_repo,
             remote_name,
-            None,
+            &[StringPattern::everything()],
             cb,
             &command.settings().git_settings(),
         )

lib/src/git.rs

@@ -1001,16 +1001,10 @@ pub fn fetch(
     mut_repo: &mut MutableRepo,
     git_repo: &git2::Repository,
     remote_name: &str,
-    branch_names: Option<&[StringPattern]>,
+    branch_names: &[StringPattern],
     callbacks: RemoteCallbacks<'_>,
     git_settings: &GitSettings,
 ) -> Result<GitFetchStats, GitFetchError> {
-    let branch_name_filter = |branch: &str| {
-        branch_names.map_or(true, |patterns| {
-            patterns.iter().any(|pattern| pattern.matches(branch))
-        })
-    };
-
     // Perform a `git fetch` on the local git repo, updating the remote-tracking
     // branches in the git repo.
     let mut remote = git_repo.find_remote(remote_name).map_err(|err| {
@@ -1026,27 +1020,28 @@ pub fn fetch(
     fetch_options.proxy_options(proxy_options);
     let callbacks = callbacks.into_git();
     fetch_options.remote_callbacks(callbacks);
-    let refspecs = {
-        // If no globs have been given, import all branches
-        let globs = if let Some(patterns) = branch_names {
-            patterns
-                .iter()
-                .map(|pattern| pattern.to_glob())
-                .collect::<Option<Vec<_>>>()
-                .ok_or(GitFetchError::InvalidBranchPattern)?
-        } else {
-            vec!["*".into()]
+    // At this point, we are only updating Git's remote tracking branches, not the
+    // local branches.
+    let refspecs: Vec<_> = branch_names
+        .iter()
+        .map(|pattern| {
+            pattern
+                .to_glob()
+                .filter(|glob| !glob.contains(INVALID_REFSPEC_CHARS))
+                .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}"))
+        })
+        .collect::<Option<_>>()
+        .ok_or(GitFetchError::InvalidBranchPattern)?;
+    if refspecs.is_empty() {
+        // Don't fall back to the base refspecs.
+        let stats = GitFetchStats {
+            default_branch: None,
+            import_stats: GitImportStats {
+                abandoned_commits: vec![],
+            },
         };
-        if globs.iter().any(|g| g.contains(INVALID_REFSPEC_CHARS)) {
-            return Err(GitFetchError::InvalidBranchPattern);
-        }
-        // At this point, we are only updating Git's remote tracking branches, not the
-        // local branches.
-        globs
-            .iter()
-            .map(|glob| format!("+refs/heads/{glob}:refs/remotes/{remote_name}/{glob}"))
-            .collect_vec()
-    };
+        return Ok(stats);
+    }
     tracing::debug!("remote.download");
     remote.download(&refspecs, Some(&mut fetch_options))?;
     tracing::debug!("remote.prune");
@@ -1075,7 +1070,7 @@ pub fn fetch(
     tracing::debug!("import_refs");
     let import_stats = import_some_refs(mut_repo, git_repo, git_settings, |ref_name| {
         to_remote_branch(ref_name, remote_name)
-            .map(branch_name_filter)
+            .map(|branch| branch_names.iter().any(|pattern| pattern.matches(branch)))
             .unwrap_or_else(|| matches!(ref_name, RefName::Tag(_)))
     })?;
     let stats = GitFetchStats {

lib/tests/test_git.rs

@@ -35,6 +35,7 @@ use jj_lib::op_store::{BranchTarget, RefTarget, RemoteRef, RemoteRefState};
 use jj_lib::refs::BranchPushUpdate;
 use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
 use jj_lib::settings::{GitSettings, UserSettings};
+use jj_lib::str_util::StringPattern;
 use jj_lib::workspace::Workspace;
 use maplit::{btreemap, hashset};
 use tempfile::TempDir;
@@ -1964,7 +1965,7 @@ fn test_fetch_empty_repo() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -1989,7 +1990,7 @@ fn test_fetch_initial_commit() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -2038,7 +2039,7 @@ fn test_fetch_success() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -2063,7 +2064,7 @@ fn test_fetch_success() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -2119,7 +2120,7 @@ fn test_fetch_prune_deleted_ref() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -2138,7 +2139,7 @@ fn test_fetch_prune_deleted_ref() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -2160,7 +2161,7 @@ fn test_fetch_no_default_branch() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -2183,7 +2184,7 @@ fn test_fetch_no_default_branch() {
         tx.mut_repo(),
         &test_data.git_repo,
         "origin",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     )
@@ -2192,6 +2193,37 @@ fn test_fetch_no_default_branch() {
     assert_eq!(stats.default_branch, None);
 }
 
+#[test]
+fn test_fetch_empty_refspecs() {
+    let test_data = GitRepoData::create();
+    let git_settings = GitSettings::default();
+    empty_git_commit(&test_data.origin_repo, "refs/heads/main", &[]);
+
+    // Base refspecs shouldn't be respected
+    let mut tx = test_data
+        .repo
+        .start_transaction(&test_data.settings, "test");
+    git::fetch(
+        tx.mut_repo(),
+        &test_data.git_repo,
+        "origin",
+        &[],
+        git::RemoteCallbacks::default(),
+        &git_settings,
+    )
+    .unwrap();
+    assert!(tx
+        .mut_repo()
+        .get_remote_branch("main", "origin")
+        .is_absent());
+    // No remote refs should have been fetched
+    git::import_refs(tx.mut_repo(), &test_data.git_repo, &git_settings).unwrap();
+    assert!(tx
+        .mut_repo()
+        .get_remote_branch("main", "origin")
+        .is_absent());
+}
+
 #[test]
 fn test_fetch_no_such_remote() {
     let test_data = GitRepoData::create();
@@ -2203,7 +2235,7 @@ fn test_fetch_no_such_remote() {
         tx.mut_repo(),
         &test_data.git_repo,
         "invalid-remote",
-        None,
+        &[StringPattern::everything()],
         git::RemoteCallbacks::default(),
         &git_settings,
     );