cli: relocate git fetch code to cli/src/commands/git/fetch.rs

This code was living in cli/src/git_utils.rs but no one else was using it
jj-vcs · Jan 24, 2025 · c915582 · c915582
1 parent 496e6f9
commit c915582
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 82 deletions.
diff --git a/cli/src/commands/git/fetch.rs b/cli/src/commands/git/fetch.rs
@@ -15,16 +15,23 @@
 use clap_complete::ArgValueCandidates;
 use itertools::Itertools;
 use jj_lib::config::ConfigGetResultExt as _;
+use jj_lib::git::GitFetch;
+use jj_lib::git::GitFetchError;
 use jj_lib::repo::Repo;
 use jj_lib::settings::UserSettings;
 use jj_lib::str_util::StringPattern;
 
 use crate::cli_util::CommandHelper;
+use crate::cli_util::WorkspaceCommandTransaction;
+use crate::command_error::user_error;
+use crate::command_error::user_error_with_hint;
 use crate::command_error::CommandError;
 use crate::commands::git::get_single_remote;
 use crate::complete;
 use crate::git_util::get_git_repo;
-use crate::git_util::git_fetch;
+use crate::git_util::map_git_error;
+use crate::git_util::print_git_import_stats;
+use crate::git_util::with_remote_git_callbacks;
 use crate::ui::Ui;
 
 /// Fetch from a Git remote
@@ -78,7 +85,7 @@ pub fn cmd_git_fetch(
         args.remotes.clone()
     };
     let mut tx = workspace_command.start_transaction();
-    git_fetch(ui, &mut tx, &git_repo, &remotes, &args.branch)?;
+    do_git_fetch(ui, &mut tx, &git_repo, &remotes, &args.branch)?;
     tx.finish(
         ui,
         format!("fetch from git remote(s) {}", remotes.iter().join(",")),
@@ -119,3 +126,77 @@ fn get_all_remotes(git_repo: &git2::Repository) -> Result<Vec<String>, CommandEr
         .filter_map(|x| x.map(ToOwned::to_owned))
         .collect())
 }
+
+fn do_git_fetch(
+    ui: &mut Ui,
+    tx: &mut WorkspaceCommandTransaction,
+    git_repo: &git2::Repository,
+    remotes: &[String],
+    branch_names: &[StringPattern],
+) -> Result<(), CommandError> {
+    let git_settings = tx.settings().git_settings()?;
+    let mut git_fetch = GitFetch::new(tx.repo_mut(), git_repo, &git_settings);
+
+    for remote_name in remotes {
+        with_remote_git_callbacks(ui, None, &git_settings, |callbacks| {
+            git_fetch
+                .fetch(remote_name, branch_names, callbacks, None)
+                .map_err(|err| match err {
+                    GitFetchError::InvalidBranchPattern => {
+                        if branch_names
+                            .iter()
+                            .any(|pattern| pattern.as_exact().is_some_and(|s| s.contains('*')))
+                        {
+                            user_error_with_hint(
+                                "Branch names may not include `*`.",
+                                "Prefix the pattern with `glob:` to expand `*` as a glob",
+                            )
+                        } else {
+                            user_error(err)
+                        }
+                    }
+                    GitFetchError::InternalGitError(err) => map_git_error(err),
+                    _ => user_error(err),
+                })
+        })?;
+    }
+    let import_stats = git_fetch.import_refs()?;
+    print_git_import_stats(ui, tx.repo(), &import_stats, true)?;
+    warn_if_branches_not_found(
+        ui,
+        tx,
+        branch_names,
+        &remotes.iter().map(StringPattern::exact).collect_vec(),
+    )
+}
+
+fn warn_if_branches_not_found(
+    ui: &mut Ui,
+    tx: &WorkspaceCommandTransaction,
+    branches: &[StringPattern],
+    remotes: &[StringPattern],
+) -> Result<(), CommandError> {
+    for branch in branches {
+        let matches = remotes.iter().any(|remote| {
+            tx.repo()
+                .view()
+                .remote_bookmarks_matching(branch, remote)
+                .next()
+                .is_some()
+                || tx
+                    .base_repo()
+                    .view()
+                    .remote_bookmarks_matching(branch, remote)
+                    .next()
+                    .is_some()
+        });
+        if !matches {
+            writeln!(
+                ui.warning_default(),
+                "No branch matching `{branch}` found on any specified/configured remote",
+            )?;
+        }
+    }
+
+    Ok(())
+}
diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs
@@ -33,8 +33,6 @@ use jj_lib::fmt_util::binary_prefix;
 use jj_lib::git;
 use jj_lib::git::FailedRefExport;
 use jj_lib::git::FailedRefExportReason;
-use jj_lib::git::GitFetch;
-use jj_lib::git::GitFetchError;
 use jj_lib::git::GitImportStats;
 use jj_lib::git::RefName;
 use jj_lib::git_backend::GitBackend;
@@ -44,12 +42,10 @@ use jj_lib::repo::ReadonlyRepo;
 use jj_lib::repo::Repo;
 use jj_lib::settings::GitSettings;
 use jj_lib::store::Store;
-use jj_lib::str_util::StringPattern;
 use jj_lib::workspace::Workspace;
 use unicode_width::UnicodeWidthStr;
 
 use crate::cleanup_guard::CleanupGuard;
-use crate::cli_util::WorkspaceCommandTransaction;
 use crate::command_error::cli_error;
 use crate::command_error::user_error;
 use crate::command_error::user_error_with_hint;
@@ -642,82 +638,6 @@ export or their "parent" bookmarks."#,
     Ok(())
 }
 
-// TODO: move to cli/src/commands/git/fetch
-// No other aprt of the code is using this
-pub fn git_fetch(
-    ui: &mut Ui,
-    tx: &mut WorkspaceCommandTransaction,
-    git_repo: &git2::Repository,
-    remotes: &[String],
-    branch_names: &[StringPattern],
-) -> Result<(), CommandError> {
-    let git_settings = tx.settings().git_settings()?;
-    let mut git_fetch = GitFetch::new(tx.repo_mut(), git_repo, &git_settings);
-
-    for remote_name in remotes {
-        with_remote_git_callbacks(ui, None, &git_settings, |cb| {
-            git_fetch
-                .fetch(remote_name, branch_names, cb, None)
-                .map_err(|err| match err {
-                    GitFetchError::InvalidBranchPattern => {
-                        if branch_names
-                            .iter()
-                            .any(|pattern| pattern.as_exact().is_some_and(|s| s.contains('*')))
-                        {
-                            user_error_with_hint(
-                                "Branch names may not include `*`.",
-                                "Prefix the pattern with `glob:` to expand `*` as a glob",
-                            )
-                        } else {
-                            user_error(err)
-                        }
-                    }
-                    GitFetchError::InternalGitError(err) => map_git_error(err),
-                    _ => user_error(err),
-                })
-        })?;
-    }
-    let import_stats = git_fetch.import_refs()?;
-    print_git_import_stats(ui, tx.repo(), &import_stats, true)?;
-    warn_if_branches_not_found(
-        ui,
-        tx,
-        branch_names,
-        &remotes.iter().map(StringPattern::exact).collect_vec(),
-    )
-}
-
-fn warn_if_branches_not_found(
-    ui: &mut Ui,
-    tx: &WorkspaceCommandTransaction,
-    branches: &[StringPattern],
-    remotes: &[StringPattern],
-) -> Result<(), CommandError> {
-    for branch in branches {
-        let matches = remotes.iter().any(|remote| {
-            tx.repo()
-                .view()
-                .remote_bookmarks_matching(branch, remote)
-                .next()
-                .is_some()
-                || tx
-                    .base_repo()
-                    .view()
-                    .remote_bookmarks_matching(branch, remote)
-                    .next()
-                    .is_some()
-        });
-        if !matches {
-            writeln!(
-                ui.warning_default(),
-                "No branch matching `{branch}` found on any specified/configured remote",
-            )?;
-        }
-    }
-
-    Ok(())
-}
-
 #[cfg(test)]
 mod tests {
     use std::path::MAIN_SEPARATOR;