cli: shell out to git for network operations

Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```

cli/src/commands/git/clone.rs

@@ -34,6 +34,7 @@ use crate::command_error::user_error;
 use crate::command_error::user_error_with_message;
 use crate::command_error::CommandError;
 use crate::commands::git::maybe_add_gitignore;
+use crate::git_util::get_config_git_shell;
 use crate::git_util::get_git_repo;
 use crate::git_util::map_git_error;
 use crate::git_util::print_git_import_stats;
@@ -129,6 +130,7 @@ pub fn cmd_git_clone(
     // `/some/path/.`
     let canonical_wc_path = dunce::canonicalize(&wc_path)
         .map_err(|err| user_error_with_message(format!("Failed to create {wc_path_str}"), err))?;
+    let shell = get_config_git_shell(command)?;
     let clone_result = do_git_clone(
         ui,
         command,
@@ -137,6 +139,7 @@ pub fn cmd_git_clone(
         remote_name,
         &source,
         &canonical_wc_path,
+        shell,
     );
     if clone_result.is_err() {
         let clean_up_dirs = || -> io::Result<()> {
@@ -196,6 +199,7 @@ fn do_git_clone(
     remote_name: &str,
     source: &str,
     wc_path: &Path,
+    shell: bool,
 ) -> Result<(WorkspaceCommandHelper, GitFetchStats), CommandError> {
     let (workspace, repo) = if colocate {
         Workspace::init_colocated_git(command.settings(), wc_path)?
@@ -214,7 +218,7 @@ fn do_git_clone(
     let mut fetch_tx = workspace_command.start_transaction();
     let git_settings = command.settings().git_settings()?;
 
-    let stats = with_remote_git_callbacks(ui, None, |cb| {
+    let stats = with_remote_git_callbacks(ui, None, shell, |cb| {
         git::fetch(
             fetch_tx.repo_mut(),
             &git_repo,
@@ -223,14 +227,27 @@ fn do_git_clone(
             cb,
             &git_settings,
             depth,
+            shell,
         )
     })
     .map_err(|err| match err {
-        GitFetchError::NoSuchRemote(_) => {
-            panic!("shouldn't happen as we just created the git remote")
+        GitFetchError::NoSuchRemote(repo_name) => {
+            user_error(format!("Could not find repository at '{repo_name}'"))
         }
         GitFetchError::GitImportError(err) => CommandError::from(err),
         GitFetchError::InternalGitError(err) => map_git_error(err),
+        GitFetchError::GitForkError(err) => CommandError::with_message(
+            crate::command_error::CommandErrorKind::Internal,
+            "External git process failed",
+            err,
+        ),
+        GitFetchError::ExternalGitError(err) => {
+            CommandError::new(crate::command_error::CommandErrorKind::Internal, err)
+        }
+        GitFetchError::PathConversionError(path) => CommandError::new(
+            crate::command_error::CommandErrorKind::Internal,
+            format!("Failed to convert path {} to string", path.display()),
+        ),
         GitFetchError::InvalidBranchPattern => {
             unreachable!("we didn't provide any globs")
         }

cli/src/commands/git/fetch.rs

@@ -23,6 +23,7 @@ use crate::cli_util::CommandHelper;
 use crate::command_error::CommandError;
 use crate::commands::git::get_single_remote;
 use crate::complete;
+use crate::git_util::get_config_git_shell;
 use crate::git_util::get_git_repo;
 use crate::git_util::git_fetch;
 use crate::ui::Ui;
@@ -69,6 +70,7 @@ pub fn cmd_git_fetch(
     args: &GitFetchArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
+    let shell_flag = get_config_git_shell(command)?;
     let git_repo = get_git_repo(workspace_command.repo().store())?;
     let remotes = if args.all_remotes {
         get_all_remotes(&git_repo)?
@@ -78,7 +80,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)?;
+    git_fetch(ui, &mut tx, &git_repo, &remotes, &args.branch, shell_flag)?;
     tx.finish(
         ui,
         format!("fetch from git remote(s) {}", remotes.iter().join(",")),

cli/src/commands/git/push.rs

@@ -49,6 +49,7 @@ use crate::command_error::CommandError;
 use crate::commands::git::get_single_remote;
 use crate::complete;
 use crate::formatter::Formatter;
+use crate::git_util::get_config_git_shell;
 use crate::git_util::get_git_repo;
 use crate::git_util::map_git_error;
 use crate::git_util::with_remote_git_callbacks;
@@ -166,6 +167,7 @@ pub fn cmd_git_push(
     args: &GitPushArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
+    let shell_flag = get_config_git_shell(command)?;
     let git_repo = get_git_repo(workspace_command.repo().store())?;
 
     let remote = if let Some(name) = &args.remote {
@@ -313,9 +315,13 @@ pub fn cmd_git_push(
     let mut sideband_progress_callback = |progress_message: &[u8]| {
         _ = writer.write(ui, progress_message);
     };
-    with_remote_git_callbacks(ui, Some(&mut sideband_progress_callback), |cb| {
-        git::push_branches(tx.repo_mut(), &git_repo, &remote, &targets, cb)
-    })
+
+    with_remote_git_callbacks(
+        ui,
+        Some(&mut sideband_progress_callback),
+        shell_flag,
+        |cb| git::push_branches(tx.repo_mut(), &git_repo, &remote, &targets, cb, shell_flag),
+    )
     .map_err(|err| match err {
         GitPushError::InternalGitError(err) => map_git_error(err),
         GitPushError::RefInUnexpectedLocation(refs) => user_error_with_hint(

cli/src/config-schema.json

@@ -362,6 +362,11 @@
                     "type": "string",
                     "description": "The remote to which commits are pushed",
                     "default": "origin"
+                },
+                "shell": {
+                    "type": "boolean",
+                    "description": "Whether jj shells out to git for network operations (push/fetch/clone)",
+                    "default": false
                 }
             }
         },

cli/src/git_util.rs

@@ -24,6 +24,7 @@ use std::process::Stdio;
 use std::time::Instant;
 
 use itertools::Itertools;
+use jj_lib::config::ConfigGetError;
 use jj_lib::git;
 use jj_lib::git::FailedRefExport;
 use jj_lib::git::FailedRefExportReason;
@@ -40,6 +41,7 @@ use jj_lib::str_util::StringPattern;
 use jj_lib::workspace::Workspace;
 use unicode_width::UnicodeWidthStr;
 
+use crate::cli_util::CommandHelper;
 use crate::cli_util::WorkspaceCommandTransaction;
 use crate::command_error::user_error;
 use crate::command_error::user_error_with_hint;
@@ -257,29 +259,35 @@ type SidebandProgressCallback<'a> = &'a mut dyn FnMut(&[u8]);
 pub fn with_remote_git_callbacks<T>(
     ui: &Ui,
     sideband_progress_callback: Option<SidebandProgressCallback<'_>>,
+    shell: bool,
     f: impl FnOnce(git::RemoteCallbacks<'_>) -> T,
 ) -> T {
-    let mut callbacks = git::RemoteCallbacks::default();
-    let mut progress_callback = None;
-    if let Some(mut output) = ui.progress_output() {
-        let mut progress = Progress::new(Instant::now());
-        progress_callback = Some(move |x: &git::Progress| {
-            _ = progress.update(Instant::now(), x, &mut output);
-        });
+    if shell {
+        f(git::RemoteCallbacks::default())
+    } else {
+        let mut callbacks = git::RemoteCallbacks::default();
+        let mut progress_callback = None;
+        if let Some(mut output) = ui.progress_output() {
+            let mut progress = Progress::new(Instant::now());
+            progress_callback = Some(move |x: &git::Progress| {
+                _ = progress.update(Instant::now(), x, &mut output);
+            });
+        }
+        callbacks.progress = progress_callback
+            .as_mut()
+            .map(|x| x as &mut dyn FnMut(&git::Progress));
+        callbacks.sideband_progress =
+            sideband_progress_callback.map(|x| x as &mut dyn FnMut(&[u8]));
+        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, url));
+        callbacks.get_password = Some(&mut get_pw);
+        let mut get_user_pw =
+            |url: &str| Some((terminal_get_username(ui, url)?, terminal_get_pw(ui, url)?));
+        callbacks.get_username_password = Some(&mut get_user_pw);
+        f(callbacks)
     }
-    callbacks.progress = progress_callback
-        .as_mut()
-        .map(|x| x as &mut dyn FnMut(&git::Progress));
-    callbacks.sideband_progress = sideband_progress_callback.map(|x| x as &mut dyn FnMut(&[u8]));
-    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, url));
-    callbacks.get_password = Some(&mut get_pw);
-    let mut get_user_pw =
-        |url: &str| 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(
@@ -462,11 +470,12 @@ pub fn git_fetch(
     git_repo: &git2::Repository,
     remotes: &[String],
     branch: &[StringPattern],
+    shell: bool,
 ) -> Result<(), CommandError> {
     let git_settings = tx.settings().git_settings()?;
 
     for remote in remotes {
-        let stats = with_remote_git_callbacks(ui, None, |cb| {
+        let stats = with_remote_git_callbacks(ui, None, shell, |cb| {
             git::fetch(
                 tx.repo_mut(),
                 git_repo,
@@ -475,6 +484,7 @@ pub fn git_fetch(
                 cb,
                 &git_settings,
                 None,
+                shell,
             )
         })
         .map_err(|err| match err {
@@ -535,3 +545,13 @@ fn warn_if_branches_not_found(
 
     Ok(())
 }
+
+pub(crate) fn get_config_git_shell(command: &CommandHelper) -> Result<bool, ConfigGetError> {
+    command.settings().get_bool("git.shell").or_else(|e| {
+        if matches!(e, ConfigGetError::NotFound { .. }) {
+            Ok(false)
+        } else {
+            Err(e)
+        }
+    })
+}

cli/tests/cli-reference@.md.snap

@@ -1,6 +1,7 @@
 ---
 source: cli/tests/test_generate_md_cli_help.rs
 description: "AUTO-GENERATED FILE, DO NOT EDIT. This cli reference is generated by a test as an `insta` snapshot. MkDocs includes this snapshot from docs/cli-reference.md."
+snapshot_kind: text
 ---
 <!-- BEGIN MARKDOWN-->