feat(shell-out): allow shelling out to git for network related calls

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.

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

 - it is using a new feature flag `shell` to switch on to shelling out.
   this doesn't seem the best approach, and it would be great to get
   some feedback on what would be best. A flag on jj git + adding it to
   the jj config seems to be a good enough idea

Testing:

Run the rust tests:
```
$ cargo test # checks we didn't screw up the baseline
$ cargo test --features=shell # test the shelling out
```

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

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

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

cli/Cargo.toml

@@ -115,6 +115,7 @@ packaging = ["gix-max-performance"]
 test-fakes = ["jj-lib/testing"]
 vendored-openssl = ["git2/vendored-openssl", "jj-lib/vendored-openssl"]
 watchman = ["jj-lib/watchman"]
+shell = []
 
 [package.metadata.binstall]
 # The archive name is jj, not jj-cli. Also, `cargo binstall` gets

cli/src/commands/git/clone.rs

@@ -226,11 +226,23 @@ 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::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/git_util.rs

@@ -261,25 +261,30 @@ pub fn with_remote_git_callbacks<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 !cfg!(feature = "shell") {
+        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)
+    } else {
+        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(

cli/tests/test_git_clone.rs

@@ -96,10 +96,10 @@ fn test_git_clone() {
     let stdout = test_env.normalize_output(&get_stdout_string(&assert));
     let stderr = test_env.normalize_output(&get_stderr_string(&assert));
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     Fetching into new repo in "$TEST_ENV/failed"
-    Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6)
-    "###);
+    Error: could not find repository at '$TEST_ENV/bad'
+    "#);
     assert!(!test_env.env_root().join("failed").exists());
 
     // Failed clone shouldn't remove the existing destination directory
@@ -111,10 +111,10 @@ fn test_git_clone() {
     let stdout = test_env.normalize_output(&get_stdout_string(&assert));
     let stderr = test_env.normalize_output(&get_stderr_string(&assert));
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     Fetching into new repo in "$TEST_ENV/failed"
-    Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6)
-    "###);
+    Error: could not find repository at '$TEST_ENV/bad'
+    "#);
     assert!(test_env.env_root().join("failed").exists());
     assert!(!test_env.env_root().join("failed").join(".jj").exists());
 
@@ -284,10 +284,10 @@ fn test_git_clone_colocate() {
     let stdout = test_env.normalize_output(&get_stdout_string(&assert));
     let stderr = test_env.normalize_output(&get_stderr_string(&assert));
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     Fetching into new repo in "$TEST_ENV/failed"
-    Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6)
-    "###);
+    Error: could not find repository at '$TEST_ENV/bad'
+    "#);
     assert!(!test_env.env_root().join("failed").exists());
 
     // Failed clone shouldn't remove the existing destination directory
@@ -302,10 +302,10 @@ fn test_git_clone_colocate() {
     let stdout = test_env.normalize_output(&get_stdout_string(&assert));
     let stderr = test_env.normalize_output(&get_stderr_string(&assert));
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     Fetching into new repo in "$TEST_ENV/failed"
-    Error: could not find repository at '$TEST_ENV/bad'; class=Repository (6)
-    "###);
+    Error: could not find repository at '$TEST_ENV/bad'
+    "#);
     assert!(test_env.env_root().join("failed").exists());
     assert!(!test_env.env_root().join("failed").join(".git").exists());
     assert!(!test_env.env_root().join("failed").join(".jj").exists());
@@ -586,6 +586,7 @@ fn test_git_clone_trunk_deleted() {
     "#);
 }
 
+#[cfg(not(feature = "shell"))]
 #[test]
 fn test_git_clone_with_depth() {
     let test_env = TestEnvironment::default();
@@ -606,6 +607,43 @@ fn test_git_clone_with_depth() {
     "#);
 }
 
+#[cfg(feature = "shell")]
+#[test]
+fn test_git_clone_with_depth() {
+    let test_env = TestEnvironment::default();
+    test_env.add_config("git.auto-local-bookmark = true");
+    let git_repo_path = test_env.env_root().join("source");
+    let clone_path = test_env.env_root().join("clone");
+    let git_repo = git2::Repository::init(git_repo_path).unwrap();
+    set_up_non_empty_git_repo(&git_repo);
+
+    // local transport *does* work in normal git
+    // we check everything works
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        test_env.env_root(),
+        &["git", "clone", "--depth", "1", "source", "clone"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Fetching into new repo in "$TEST_ENV/clone"
+    bookmark: main@origin [new] tracked
+    Setting the revset alias "trunk()" to "main@origin"
+    Working copy now at: sqpuoqvx cad212e1 (empty) (no description set)
+    Parent commit      : mzyxwzks 9f01a0e0 main | message
+    Added 1 files, modified 0 files, removed 0 files
+    "#);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(&clone_path, &["log"]);
+    insta::assert_snapshot!(stdout, @r"
+    @  sqpuoqvx [email protected] 2001-02-03 08:05:07 cad212e1
+    │  (empty) (no description set)
+    ◆  mzyxwzks [email protected] 1970-01-01 11:00:00 main 9f01a0e0
+    │  message
+    ~
+    ");
+    insta::assert_snapshot!(stderr, @"");
+}
+
 #[test]
 fn test_git_clone_invalid_immutable_heads() {
     let test_env = TestEnvironment::default();

lib/Cargo.toml

@@ -107,6 +107,7 @@ gix-max-performance = [
 vendored-openssl = ["git2/vendored-openssl"]
 watchman = ["dep:tokio", "dep:watchman_client"]
 testing = ["git"]
+shell = []
 
 [lints]
 workspace = true