git_utils: Move git_fetch back to cli/src/commands/git/fetch.rs

With the new GitFetch api, `git_fetch` will not be shared with `jj git sync`.
martinvonz · Dec 1, 2024 · 2e4074b · 2e4074b
1 parent 5c30bbd
commit 2e4074b
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 55 deletions.
diff --git a/cli/src/commands/git/fetch.rs b/cli/src/commands/git/fetch.rs
@@ -14,17 +14,27 @@
 
 use clap_complete::ArgValueCandidates;
 use itertools::Itertools;
+use jj_lib::git;
+use jj_lib::git::GitFetch;
+use jj_lib::git::GitFetchError;
+use jj_lib::git::GitImportStats;
 use jj_lib::repo::Repo;
 use jj_lib::settings::ConfigResultExt as _;
 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::warn_if_branches_not_found;
+use crate::git_util::with_remote_git_callbacks;
 use crate::ui::Ui;
 
 /// Fetch from a Git remote
@@ -119,3 +129,51 @@ fn get_all_remotes(git_repo: &git2::Repository) -> Result<Vec<String>, CommandEr
         .filter_map(|x| x.map(ToOwned::to_owned))
         .collect())
 }
+
+pub fn git_fetch(
+    ui: &mut Ui,
+    tx: &mut WorkspaceCommandTransaction,
+    git_repo: &git2::Repository,
+    remotes: &[String],
+    branch: &[StringPattern],
+) -> Result<(), CommandError> {
+    let import_stats =
+        with_remote_git_callbacks(ui, None, |cb| -> Result<GitImportStats, CommandError> {
+            let git_settings = tx.settings().git_settings();
+            let mut git_fetch = GitFetch::new(
+                tx.repo_mut(),
+                git_repo,
+                &git_settings,
+                git::fetch_options(cb, None),
+            );
+
+            for remote in remotes {
+                git_fetch.fetch(branch, remote).map_err(|err| match err {
+                    GitFetchError::InvalidBranchPattern => {
+                        if branch
+                            .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::GitImportError(err) => err.into(),
+                    GitFetchError::InternalGitError(err) => map_git_error(err),
+                    _ => user_error(err),
+                })?;
+            }
+            Ok(git_fetch.import_refs()?)
+        })?;
+    print_git_import_stats(ui, tx.repo(), &import_stats, true)?;
+    warn_if_branches_not_found(
+        ui,
+        tx,
+        branch,
+        &remotes.iter().map(StringPattern::exact).collect_vec(),
+    )
+}
diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs
@@ -27,8 +27,6 @@ use itertools::Itertools;
 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;
@@ -457,58 +455,7 @@ export or their "parent" bookmarks."#,
     Ok(())
 }
 
-// TODO: Move this back to cli/src/commands/git/fetch.rs
-// With the new `GitFetch` api, this function is too specialized
-// to the `jj git fetch` command and should not be reused.
-pub fn git_fetch(
-    ui: &mut Ui,
-    tx: &mut WorkspaceCommandTransaction,
-    git_repo: &git2::Repository,
-    remotes: &[String],
-    branch: &[StringPattern],
-) -> Result<(), CommandError> {
-    let import_stats =
-        with_remote_git_callbacks(ui, None, |cb| -> Result<GitImportStats, CommandError> {
-            let git_settings = tx.settings().git_settings();
-            let mut git_fetch = GitFetch::new(
-                tx.repo_mut(),
-                git_repo,
-                &git_settings,
-                git::fetch_options(cb, None),
-            );
-
-            for remote in remotes {
-                git_fetch.fetch(branch, remote).map_err(|err| match err {
-                    GitFetchError::InvalidBranchPattern => {
-                        if branch
-                            .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::GitImportError(err) => err.into(),
-                    GitFetchError::InternalGitError(err) => map_git_error(err),
-                    _ => user_error(err),
-                })?;
-            }
-            Ok(git_fetch.import_refs()?)
-        })?;
-    print_git_import_stats(ui, tx.repo(), &import_stats, true)?;
-    warn_if_branches_not_found(
-        ui,
-        tx,
-        branch,
-        &remotes.iter().map(StringPattern::exact).collect_vec(),
-    )
-}
-
-fn warn_if_branches_not_found(
+pub fn warn_if_branches_not_found(
     ui: &mut Ui,
     tx: &WorkspaceCommandTransaction,
     branches: &[StringPattern],