Factor client feature check from connect to helpers

This creates two helper functions and moves the logic for the SSH
client feature check (determining the `ProgramKind`) to them.

Further refactoring is possible, especially since some of the use
of mutability might be replaceable by early return with no loss of
readability. But for now this maintains the preexisting structure
of the code covering the case where it is not necessary to pre-run
the SSH client.

Author: EliahKagan

gix-transport/src/client/blocking_io/ssh/mod.rs

@@ -1,8 +1,14 @@
-use std::process::Stdio;
+use std::{
+    ffi::OsStr,
+    process::{Command, Stdio},
+};
 
-use gix_url::ArgumentSafety::*;
+use gix_url::{ArgumentSafety::*, Url};
 
-use crate::{client::blocking_io, Protocol};
+use crate::{
+    client::{self, blocking_io::file::SpawnProcessOnDemand},
+    Protocol,
+};
 
 /// The error used in [`connect()`].
 #[derive(Debug, thiserror::Error)]
@@ -100,44 +106,18 @@ pub mod connect {
 /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate.
 #[allow(clippy::result_large_err)]
 pub fn connect(
-    url: gix_url::Url,
+    url: Url,
     desired_version: Protocol,
     options: connect::Options,
     trace: bool,
-) -> Result<blocking_io::file::SpawnProcessOnDemand, Error> {
+) -> Result<SpawnProcessOnDemand, Error> {
     if url.scheme != gix_url::Scheme::Ssh || url.host().is_none() {
         return Err(Error::UnsupportedScheme(url));
     }
     let ssh_cmd = options.ssh_command();
-    let mut kind = options.kind.unwrap_or_else(|| ProgramKind::from(ssh_cmd));
-    if options.kind.is_none() && kind == ProgramKind::Simple {
-        let mut cmd = std::process::Command::from({
-            let mut prepare = gix_command::prepare(ssh_cmd)
-                .stderr(Stdio::null())
-                .stdout(Stdio::null())
-                .stdin(Stdio::null())
-                .command_may_be_shell_script()
-                .arg("-G")
-                .arg(match url.host_as_argument() {
-                    Usable(host) => host,
-                    Dangerous(host) => Err(Error::AmbiguousHostName { host: host.into() })?,
-                    Absent => panic!("BUG: host should always be present in SSH URLs"),
-                });
-            if options.disallow_shell {
-                prepare.use_shell = false;
-            }
-            prepare
-        });
-        gix_features::trace::debug!(cmd = ?cmd, "invoking `ssh` for feature check");
-        kind = if cmd.status().ok().is_some_and(|status| status.success()) {
-            ProgramKind::Ssh
-        } else {
-            ProgramKind::Simple
-        };
-    }
-
+    let kind = determine_client_kind(options.kind, ssh_cmd, &url, options.disallow_shell)?;
     let path = gix_url::expand_path::for_shell(url.path.clone());
-    Ok(blocking_io::file::SpawnProcessOnDemand::new_ssh(
+    Ok(SpawnProcessOnDemand::new_ssh(
         url,
         ssh_cmd,
         path,
@@ -148,5 +128,42 @@ pub fn connect(
     ))
 }
 
+fn determine_client_kind(
+    known_kind: Option<ProgramKind>,
+    ssh_cmd: &OsStr,
+    url: &Url,
+    disallow_shell: bool,
+) -> Result<ProgramKind, Error> {
+    let mut kind = known_kind.unwrap_or_else(|| ProgramKind::from(ssh_cmd));
+    if known_kind.is_none() && kind == ProgramKind::Simple {
+        let mut cmd = build_client_feature_check_command(ssh_cmd, url, disallow_shell)?;
+        gix_features::trace::debug!(cmd = ?cmd, "invoking `ssh` for feature check");
+        kind = if cmd.status().ok().is_some_and(|status| status.success()) {
+            ProgramKind::Ssh
+        } else {
+            ProgramKind::Simple
+        };
+    }
+    Ok(kind)
+}
+
+fn build_client_feature_check_command(ssh_cmd: &OsStr, url: &Url, disallow_shell: bool) -> Result<Command, Error> {
+    let mut prepare = gix_command::prepare(ssh_cmd)
+        .stderr(Stdio::null())
+        .stdout(Stdio::null())
+        .stdin(Stdio::null())
+        .command_may_be_shell_script()
+        .arg("-G")
+        .arg(match url.host_as_argument() {
+            Usable(host) => host,
+            Dangerous(host) => Err(Error::AmbiguousHostName { host: host.into() })?,
+            Absent => panic!("BUG: host should always be present in SSH URLs"),
+        });
+    if disallow_shell {
+        prepare.use_shell = false;
+    }
+    Ok(prepare.into())
+}
+
 #[cfg(test)]
 mod tests;