cli: move some git utilities to new crate::git_utils

Gerrit also needs to be able to push low-level refs into upstream remotes, just
like `jj git push` does. Doing so requires providing callbacks e.g. for various
password entry mechanisms, which was private to the `git` command module.

Pull these out to a new module `git_utils` so we can reuse it across the two
call sites.

This also moves a few other strictly Git-related functions into `git_utils`
as well, just for the sake of consistency.

Signed-off-by: Austin Seipp <aseipp@pobox.com>

cli/src/cli_util.rs

@@ -32,10 +32,7 @@ use indexmap::{IndexMap, IndexSet};
 use itertools::Itertools;
 use jj_lib::backend::{BackendError, ChangeId, CommitId, MergedTreeId};
 use jj_lib::commit::Commit;
-use jj_lib::git::{
+use jj_lib::git::{GitConfigParseError, GitExportError, GitImportError, GitRemoteManagementError};
-    FailedRefExport, FailedRefExportReason, GitConfigParseError, GitExportError, GitImportError,
-    GitImportStats, GitRemoteManagementError,
-};
 use jj_lib::git_backend::GitBackend;
 use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::hex_util::to_reverse_hex;
@@ -82,6 +79,7 @@ use crate::config::{
     new_config_path, AnnotatedValue, CommandNameAndArgs, ConfigSource, LayeredConfigs,
 };
 use crate::formatter::{FormatRecorder, Formatter, PlainTextFormatter};
+use crate::git_util::{print_failed_git_export, print_git_import_stats};
 use crate::merge_tools::{ConflictResolveError, DiffEditError, DiffGenerateError};
 use crate::template_parser::{TemplateAliasesMap, TemplateParseError};
 use crate::templater::Template;
@@ -946,7 +944,9 @@ impl WorkspaceCommandHelper {
             // TODO: maybe use path_by_key() and interpolate(), which can process non-utf-8
             // path on Unix.
             if let Some(value) = config.string_by_key("core.excludesFile") {
-                str::from_utf8(&value).ok().map(expand_git_path)
+                str::from_utf8(&value)
+                    .ok()
+                    .map(crate::git_util::expand_git_path)
             } else {
                 xdg_config_home().ok().map(|x| x.join("git").join("ignore"))
             }
@@ -1943,55 +1943,6 @@ Discard the conflicting changes with `jj restore --from {}`.",
     Ok(())
 }
 
-pub fn print_git_import_stats(ui: &mut Ui, stats: &GitImportStats) -> Result<(), CommandError> {
-    if !stats.abandoned_commits.is_empty() {
-        writeln!(
-            ui.stderr(),
-            "Abandoned {} commits that are no longer reachable.",
-            stats.abandoned_commits.len()
-        )?;
-    }
-    Ok(())
-}
-
-pub fn print_failed_git_export(
-    ui: &Ui,
-    failed_branches: &[FailedRefExport],
-) -> Result<(), std::io::Error> {
-    if !failed_branches.is_empty() {
-        writeln!(ui.warning(), "Failed to export some branches:")?;
-        let mut formatter = ui.stderr_formatter();
-        for FailedRefExport { name, reason } in failed_branches {
-            formatter.write_str("  ")?;
-            write!(formatter.labeled("branch"), "{name}")?;
-            writeln!(formatter, ": {reason}")?;
-        }
-        drop(formatter);
-        if failed_branches
-            .iter()
-            .any(|failed| matches!(failed.reason, FailedRefExportReason::FailedToSet(_)))
-        {
-            writeln!(
-                ui.hint(),
-                r#"Hint: Git doesn't allow a branch name that looks like a parent directory of
-another (e.g. `foo` and `foo/bar`). Try to rename the branches that failed to
-export or their "parent" branches."#,
-            )?;
-        }
-    }
-    Ok(())
-}
-
-/// Expands "~/" to "$HOME/" as Git seems to do for e.g. core.excludesFile.
-fn expand_git_path(path_str: &str) -> PathBuf {
-    if let Some(remainder) = path_str.strip_prefix("~/") {
-        if let Ok(home_dir_str) = std::env::var("HOME") {
-            return PathBuf::from(home_dir_str).join(remainder);
-        }
-    }
-    PathBuf::from(path_str)
-}
-
 pub fn parse_string_pattern(src: &str) -> Result<StringPattern, StringPatternParseError> {
     if let Some((kind, pat)) = src.split_once(':') {
         StringPattern::from_str_kind(pat, kind)

cli/src/commands/git.rs

@@ -13,12 +13,9 @@
 // limitations under the License.
 
 use std::collections::HashSet;
-use std::io::{Read, Write};
+use std::io::Write;
 use std::ops::Deref;
 use std::path::{Path, PathBuf};
-use std::process::{Command, Stdio};
-use std::sync::Mutex;
-use std::time::Instant;
 use std::{fmt, fs, io};
 
 use clap::{ArgGroup, Subcommand};
@@ -27,7 +24,6 @@ use jj_lib::backend::TreeValue;
 use jj_lib::git::{
     self, parse_gitmodules, GitBranchPushTargets, GitFetchError, GitFetchStats, GitPushError,
 };
-use jj_lib::git_backend::GitBackend;
 use jj_lib::object_id::ObjectId;
 use jj_lib::op_store::RefTarget;
 use jj_lib::refs::{
@@ -37,18 +33,19 @@ use jj_lib::repo::Repo;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::revset::{self, RevsetExpression, RevsetIteratorExt as _};
 use jj_lib::settings::{ConfigResultExt as _, UserSettings};
-use jj_lib::store::Store;
 use jj_lib::str_util::StringPattern;
 use jj_lib::view::View;
 use jj_lib::workspace::Workspace;
 use maplit::hashset;
 
 use crate::cli_util::{
-    parse_string_pattern, print_failed_git_export, print_git_import_stats,
+    parse_string_pattern, resolve_multiple_nonempty_revsets, short_change_hash, short_commit_hash,
-    resolve_multiple_nonempty_revsets, short_change_hash, short_commit_hash, user_error,
+    user_error, user_error_with_hint, CommandError, CommandHelper, RevisionArg,
-    user_error_with_hint, CommandError, CommandHelper, RevisionArg, WorkspaceCommandHelper,
+    WorkspaceCommandHelper,
+};
+use crate::git_util::{
+    get_git_repo, print_failed_git_export, print_git_import_stats, with_remote_git_callbacks,
 };
-use crate::progress::Progress;
 use crate::ui::Ui;
 
 /// Commands for working with the underlying Git repo
@@ -212,13 +209,6 @@ fn make_branch_term(branch_names: &[impl fmt::Display]) -> String {
     }
 }
 
-fn get_git_repo(store: &Store) -> Result<git2::Repository, CommandError> {
-    match store.backend_impl().downcast_ref::<GitBackend>() {
-        None => Err(user_error("The repo is not backed by a git repo")),
-        Some(git_backend) => Ok(git_backend.open_git_repo()?),
-    }
-}
-
 fn map_git_error(err: git2::Error) -> CommandError {
     if err.class() == git2::ErrorClass::Ssh {
         let hint =
@@ -338,7 +328,7 @@ fn cmd_git_fetch(
     };
     let mut tx = workspace_command.start_transaction();
     for remote in &remotes {
-        let stats = with_remote_callbacks(ui, |cb| {
+        let stats = with_remote_git_callbacks(ui, |cb| {
             git::fetch(
                 tx.mut_repo(),
                 &git_repo,
@@ -556,7 +546,7 @@ fn do_git_clone(
     git_repo.remote(remote_name, source).unwrap();
     let mut fetch_tx = workspace_command.start_transaction();
 
-    let stats = with_remote_callbacks(ui, |cb| {
+    let stats = with_remote_git_callbacks(ui, |cb| {
         git::fetch(
             fetch_tx.mut_repo(),
             &git_repo,
@@ -581,119 +571,6 @@ fn do_git_clone(
     Ok((workspace_command, stats))
 }
 
-fn with_remote_callbacks<T>(ui: &mut Ui, f: impl FnOnce(git::RemoteCallbacks<'_>) -> T) -> T {
-    let mut ui = Mutex::new(ui);
-    let mut callback = None;
-    if let Some(mut output) = ui.get_mut().unwrap().progress_output() {
-        let mut progress = Progress::new(Instant::now());
-        callback = Some(move |x: &git::Progress| {
-            _ = progress.update(Instant::now(), x, &mut output);
-        });
-    }
-    let mut callbacks = git::RemoteCallbacks::default();
-    callbacks.progress = callback
-        .as_mut()
-        .map(|x| x as &mut dyn FnMut(&git::Progress));
-    let mut get_ssh_keys = get_ssh_keys; // Coerce to unit fn type
-    callbacks.get_ssh_keys = Some(&mut get_ssh_keys);
-    let mut get_pw = |url: &str, _username: &str| {
-        pinentry_get_pw(url).or_else(|| terminal_get_pw(*ui.lock().unwrap(), url))
-    };
-    callbacks.get_password = Some(&mut get_pw);
-    let mut get_user_pw = |url: &str| {
-        let ui = &mut *ui.lock().unwrap();
-        Some((terminal_get_username(ui, url)?, terminal_get_pw(ui, url)?))
-    };
-    callbacks.get_username_password = Some(&mut get_user_pw);
-    f(callbacks)
-}
-
-fn terminal_get_username(ui: &mut Ui, url: &str) -> Option<String> {
-    ui.prompt(&format!("Username for {url}")).ok()
-}
-
-fn terminal_get_pw(ui: &mut Ui, url: &str) -> Option<String> {
-    ui.prompt_password(&format!("Passphrase for {url}: ")).ok()
-}
-
-fn pinentry_get_pw(url: &str) -> Option<String> {
-    let mut pinentry = Command::new("pinentry")
-        .stdin(Stdio::piped())
-        .stdout(Stdio::piped())
-        .spawn()
-        .ok()?;
-    #[rustfmt::skip]
-    pinentry
-        .stdin
-        .take()
-        .unwrap()
-        .write_all(
-            format!(
-                "SETTITLE jj passphrase\n\
-                 SETDESC Enter passphrase for {url}\n\
-                 SETPROMPT Passphrase:\n\
-                 GETPIN\n"
-            )
-            .as_bytes(),
-        )
-        .ok()?;
-    let mut out = String::new();
-    pinentry
-        .stdout
-        .take()
-        .unwrap()
-        .read_to_string(&mut out)
-        .ok()?;
-    _ = pinentry.wait();
-    for line in out.split('\n') {
-        if !line.starts_with("D ") {
-            continue;
-        }
-        let (_, encoded) = line.split_at(2);
-        return decode_assuan_data(encoded);
-    }
-    None
-}
-
-// https://www.gnupg.org/documentation/manuals/assuan/Server-responses.html#Server-responses
-fn decode_assuan_data(encoded: &str) -> Option<String> {
-    let encoded = encoded.as_bytes();
-    let mut decoded = Vec::with_capacity(encoded.len());
-    let mut i = 0;
-    while i < encoded.len() {
-        if encoded[i] != b'%' {
-            decoded.push(encoded[i]);
-            i += 1;
-            continue;
-        }
-        i += 1;
-        let byte =
-            u8::from_str_radix(std::str::from_utf8(encoded.get(i..i + 2)?).ok()?, 16).ok()?;
-        decoded.push(byte);
-        i += 2;
-    }
-    String::from_utf8(decoded).ok()
-}
-
-#[tracing::instrument]
-fn get_ssh_keys(_username: &str) -> Vec<PathBuf> {
-    let mut paths = vec![];
-    if let Some(home_dir) = dirs::home_dir() {
-        let ssh_dir = Path::new(&home_dir).join(".ssh");
-        for filename in ["id_ed25519_sk", "id_ed25519", "id_rsa"] {
-            let key_path = ssh_dir.join(filename);
-            if key_path.is_file() {
-                tracing::info!(path = ?key_path, "found ssh key");
-                paths.push(key_path);
-            }
-        }
-    }
-    if paths.is_empty() {
-        tracing::info!("no ssh key found");
-    }
-    paths
-}
-
 fn cmd_git_push(
     ui: &mut Ui,
     command: &CommandHelper,
@@ -978,7 +855,7 @@ fn cmd_git_push(
         branch_updates,
         force_pushed_branches,
     };
-    with_remote_callbacks(ui, |cb| {
+    with_remote_git_callbacks(ui, |cb| {
         git::push_branches(tx.mut_repo(), &git_repo, &remote, &targets, cb)
     })
     .map_err(|err| match err {

cli/src/commands/init.rs

@@ -24,9 +24,8 @@ use jj_lib::workspace::Workspace;
 use tracing::instrument;
 
 use super::git;
-use crate::cli_util::{
+use crate::cli_util::{user_error, user_error_with_hint, CommandError, CommandHelper};
-    print_git_import_stats, user_error, user_error_with_hint, CommandError, CommandHelper,
+use crate::git_util::print_git_import_stats;
-};
 use crate::ui::Ui;
 
 /// Create a new repo in the given directory

cli/src/git_util.rs

@@ -0,0 +1,201 @@
+// Copyright 2024 The Jujutsu Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Git utilities shared by various commands.
+
+use std::io::{Read, Write};
+use std::path::{Path, PathBuf};
+use std::process::Stdio;
+use std::sync::Mutex;
+use std::time::Instant;
+
+use jj_lib::git::{self, FailedRefExport, FailedRefExportReason, GitImportStats};
+use jj_lib::git_backend::GitBackend;
+use jj_lib::store::Store;
+
+use crate::cli_util::{user_error, CommandError};
+use crate::progress::Progress;
+use crate::ui::Ui;
+
+pub fn get_git_repo(store: &Store) -> Result<git2::Repository, CommandError> {
+    match store.backend_impl().downcast_ref::<GitBackend>() {
+        None => Err(user_error("The repo is not backed by a git repo")),
+        Some(git_backend) => Ok(git_backend.open_git_repo()?),
+    }
+}
+
+fn terminal_get_username(ui: &mut Ui, url: &str) -> Option<String> {
+    ui.prompt(&format!("Username for {url}")).ok()
+}
+
+fn terminal_get_pw(ui: &mut Ui, url: &str) -> Option<String> {
+    ui.prompt_password(&format!("Passphrase for {url}: ")).ok()
+}
+
+fn pinentry_get_pw(url: &str) -> Option<String> {
+    // https://www.gnupg.org/documentation/manuals/assuan/Server-responses.html#Server-responses
+    fn decode_assuan_data(encoded: &str) -> Option<String> {
+        let encoded = encoded.as_bytes();
+        let mut decoded = Vec::with_capacity(encoded.len());
+        let mut i = 0;
+        while i < encoded.len() {
+            if encoded[i] != b'%' {
+                decoded.push(encoded[i]);
+                i += 1;
+                continue;
+            }
+            i += 1;
+            let byte =
+                u8::from_str_radix(std::str::from_utf8(encoded.get(i..i + 2)?).ok()?, 16).ok()?;
+            decoded.push(byte);
+            i += 2;
+        }
+        String::from_utf8(decoded).ok()
+    }
+
+    let mut pinentry = std::process::Command::new("pinentry")
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .spawn()
+        .ok()?;
+    #[rustfmt::skip]
+    pinentry
+        .stdin
+        .take()
+        .unwrap()
+        .write_all(
+            format!(
+                "SETTITLE jj passphrase\n\
+                 SETDESC Enter passphrase for {url}\n\
+                 SETPROMPT Passphrase:\n\
+                 GETPIN\n"
+            )
+            .as_bytes(),
+        )
+        .ok()?;
+    let mut out = String::new();
+    pinentry
+        .stdout
+        .take()
+        .unwrap()
+        .read_to_string(&mut out)
+        .ok()?;
+    _ = pinentry.wait();
+    for line in out.split('\n') {
+        if !line.starts_with("D ") {
+            continue;
+        }
+        let (_, encoded) = line.split_at(2);
+        return decode_assuan_data(encoded);
+    }
+    None
+}
+
+#[tracing::instrument]
+fn get_ssh_keys(_username: &str) -> Vec<PathBuf> {
+    let mut paths = vec![];
+    if let Some(home_dir) = dirs::home_dir() {
+        let ssh_dir = Path::new(&home_dir).join(".ssh");
+        for filename in ["id_ed25519_sk", "id_ed25519", "id_rsa"] {
+            let key_path = ssh_dir.join(filename);
+            if key_path.is_file() {
+                tracing::info!(path = ?key_path, "found ssh key");
+                paths.push(key_path);
+            }
+        }
+    }
+    if paths.is_empty() {
+        tracing::info!("no ssh key found");
+    }
+    paths
+}
+
+pub fn with_remote_git_callbacks<T>(
+    ui: &mut Ui,
+    f: impl FnOnce(git::RemoteCallbacks<'_>) -> T,
+) -> T {
+    let mut ui = Mutex::new(ui);
+    let mut callback = None;
+    if let Some(mut output) = ui.get_mut().unwrap().progress_output() {
+        let mut progress = Progress::new(Instant::now());
+        callback = Some(move |x: &git::Progress| {
+            _ = progress.update(Instant::now(), x, &mut output);
+        });
+    }
+    let mut callbacks = git::RemoteCallbacks::default();
+    callbacks.progress = callback
+        .as_mut()
+        .map(|x| x as &mut dyn FnMut(&git::Progress));
+    let mut get_ssh_keys = get_ssh_keys; // Coerce to unit fn type
+    callbacks.get_ssh_keys = Some(&mut get_ssh_keys);
+    let mut get_pw = |url: &str, _username: &str| {
+        pinentry_get_pw(url).or_else(|| terminal_get_pw(*ui.lock().unwrap(), url))
+    };
+    callbacks.get_password = Some(&mut get_pw);
+    let mut get_user_pw = |url: &str| {
+        let ui = &mut *ui.lock().unwrap();
+        Some((terminal_get_username(ui, url)?, terminal_get_pw(ui, url)?))
+    };
+    callbacks.get_username_password = Some(&mut get_user_pw);
+    f(callbacks)
+}
+
+pub fn print_git_import_stats(ui: &mut Ui, stats: &GitImportStats) -> Result<(), CommandError> {
+    if !stats.abandoned_commits.is_empty() {
+        writeln!(
+            ui.stderr(),
+            "Abandoned {} commits that are no longer reachable.",
+            stats.abandoned_commits.len()
+        )?;
+    }
+    Ok(())
+}
+
+pub fn print_failed_git_export(
+    ui: &Ui,
+    failed_branches: &[FailedRefExport],
+) -> Result<(), std::io::Error> {
+    if !failed_branches.is_empty() {
+        writeln!(ui.warning(), "Failed to export some branches:")?;
+        let mut formatter = ui.stderr_formatter();
+        for FailedRefExport { name, reason } in failed_branches {
+            formatter.write_str("  ")?;
+            write!(formatter.labeled("branch"), "{name}")?;
+            writeln!(formatter, ": {reason}")?;
+        }
+        drop(formatter);
+        if failed_branches
+            .iter()
+            .any(|failed| matches!(failed.reason, FailedRefExportReason::FailedToSet(_)))
+        {
+            writeln!(
+                ui.hint(),
+                r#"Hint: Git doesn't allow a branch name that looks like a parent directory of
+another (e.g. `foo` and `foo/bar`). Try to rename the branches that failed to
+export or their "parent" branches."#,
+            )?;
+        }
+    }
+    Ok(())
+}
+
+/// Expands "~/" to "$HOME/" as Git seems to do for e.g. core.excludesFile.
+pub fn expand_git_path(path_str: &str) -> PathBuf {
+    if let Some(remainder) = path_str.strip_prefix("~/") {
+        if let Ok(home_dir_str) = std::env::var("HOME") {
+            return PathBuf::from(home_dir_str).join(remainder);
+        }
+    }
+    PathBuf::from(path_str)
+}

cli/src/lib.rs

@@ -22,6 +22,7 @@ pub mod config;
 pub mod description_util;
 pub mod diff_util;
 pub mod formatter;
+pub mod git_util;
 pub mod graphlog;
 pub mod merge_tools;
 pub mod operation_templater;