git: spawn a separate git process 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 spawning
a `git` subprocess for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.

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:
```
$ cargo build
```

Clone a private repo:
```
$ path/to/jj git clone --config='git.subprocess=true' <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ path/to/jj describe -m 'test commit'
$ path/to/jj git push --config='git.subprocess=true' -b <branch>
```


Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984

Author: bsdinis

.github/workflows/build.yml

@@ -86,9 +86,21 @@ jobs:
         tool: nextest
     - name: Build
       run: cargo build --workspace --all-targets --verbose ${{ matrix.cargo_flags }}
-    - name: Test
+    - name: Test [non-windows]
+      if: ${{ matrix.os != 'windows-latest' }}
+      run: |
+        cargo nextest run --workspace --all-targets --verbose ${{ matrix.cargo_flags }}
+      env:
+        # tests clear the PATH variable, so we need to set the git executable
+        TEST_GIT_EXECUTABLE_PATH: 'git'
+        RUST_BACKTRACE: 1
+        CARGO_TERM_COLOR: always
+    - name: Test [windows]
+      if: ${{ matrix.os == 'windows-latest' }}
       run: cargo nextest run --workspace --all-targets --verbose ${{ matrix.cargo_flags }}
       env:
+        # tests clear the PATH variable, so we need to set the git executable
+        TEST_GIT_EXECUTABLE_PATH: 'C:\Program Files\Git\bin\git.exe'
         RUST_BACKTRACE: 1
         CARGO_TERM_COLOR: always
 

CHANGELOG.md

@@ -62,6 +62,11 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### New features
 
+* `jj git {push,clone,fetch}` can now spawn an external `git` subprocess, via
+   the `git.subprocess = true` config knob. This provides an alternative that,
+   when turned on, fixes SSH bugs when interacting with Git remotes due to
+   `libgit2`s limitations [#4979](https://github.com/jj-vcs/jj/issues/4979).
+
 * `jj evolog` and `jj op log` now accept `--reversed`.
 
 * `jj restore` now supports `-i`/`--interactive` selection.

cli/src/commands/git/clone.rs

@@ -200,8 +200,7 @@ fn do_git_clone(
     git_repo.remote(remote_name, source).unwrap();
     let git_settings = workspace_command.settings().git_settings()?;
     let mut fetch_tx = workspace_command.start_transaction();
-
-    let stats = with_remote_git_callbacks(ui, None, |cb| {
+    let stats = with_remote_git_callbacks(ui, None, &git_settings, |cb| {
         git::fetch(
             fetch_tx.repo_mut(),
             &git_repo,
@@ -213,11 +212,12 @@ fn do_git_clone(
         )
     })
     .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::Subprocess(err) => user_error(err),
         GitFetchError::InvalidBranchPattern => {
             unreachable!("we didn't provide any globs")
         }

cli/src/commands/git/push.rs

@@ -357,9 +357,23 @@ 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)
-    })
+
+    let git_settings = tx.settings().git_settings()?;
+    with_remote_git_callbacks(
+        ui,
+        Some(&mut sideband_progress_callback),
+        &git_settings,
+        |cb| {
+            git::push_branches(
+                tx.repo_mut(),
+                &git_repo,
+                &git_settings,
+                &remote,
+                &targets,
+                cb,
+            )
+        },
+    )
     .map_err(|err| match err {
         GitPushError::InternalGitError(err) => map_git_error(err),
         GitPushError::RefInUnexpectedLocation(refs) => user_error_with_hint(

cli/src/config-schema.json

@@ -377,6 +377,16 @@
                     "type": "boolean",
                     "description": "Whether jj should sign commits before pushing",
                     "default": "false"
+                },
+                "subprocess": {
+                    "type": "boolean",
+                    "description": "Whether jj spawns a git subprocess for network operations (push/fetch/clone)",
+                    "default": false
+                },
+                "executable-path": {
+                    "type": "string",
+                    "description": "Path to the git executable",
+                    "default": "git"
                 }
             }
         },

cli/src/git_util.rs

@@ -41,6 +41,7 @@ use jj_lib::op_store::RefTarget;
 use jj_lib::op_store::RemoteRef;
 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;
@@ -282,29 +283,40 @@ type SidebandProgressCallback<'a> = &'a mut dyn FnMut(&[u8]);
 pub fn with_remote_git_callbacks<T>(
     ui: &Ui,
     sideband_progress_callback: Option<SidebandProgressCallback<'_>>,
+    git_settings: &GitSettings,
     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 git_settings.subprocess {
+        // TODO: with git2, we are able to display progress from the data that is given
+        // With the git processes themselves, this is significantly harder, as it
+        // requires parsing the output directly
+        //
+        // In any case, this would be the place to add that funcionalty
+        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(
@@ -635,7 +647,7 @@ pub fn git_fetch(
     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, &git_settings, |cb| {
             git::fetch(
                 tx.repo_mut(),
                 git_repo,

cli/tests/common/mod.rs

@@ -80,6 +80,7 @@ impl Default for TestEnvironment {
 'format_time_range(time_range)' = 'time_range.start() ++ " - " ++ time_range.end()'
         "#,
         );
+
         env
     }
 }
@@ -280,6 +281,18 @@ impl TestEnvironment {
         self.env_vars.insert(key.into(), val.into());
     }
 
+    pub fn set_up_git_subprocessing(&self) {
+        self.add_config("git.subprocess = true");
+
+        // add a git path from env: this is only used if git.subprocess = true
+        if let Ok(git_executable_path) = std::env::var("TEST_GIT_EXECUTABLE_PATH") {
+            self.add_config(format!(
+                "git.executable-path = {}",
+                to_toml_value(git_executable_path)
+            ));
+        }
+    }
+
     pub fn current_operation_id(&self, repo_path: &Path) -> String {
         let id_and_newline =
             self.jj_cmd_success(repo_path, &["debug", "operation", "--display=id"]);