Add --shell support to CLI (#218)

CHANGELOG.md

@@ -7,10 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Support for `--shell` with optional path to an explicit shell as an option
+  when executing `distant spawn` in order to run the command within a shell
+  rather than directly
+
+### Changed
+
+- `distant_protocol::PROTOCOL_VERSION` now uses the crate's major, minor, and
+  patch version at compile-time (parsed via `const-str` crate) to streamline
+  version handling between crate and protocol
+
 ### Fixed
 
 - CLI commands like `distant manager select` will now output errors in a JSON
   format when configured to communicate using JSON
+- `distant-ssh2` no longer caches the remote family globally, but instead
+  caches it per `Ssh` instance
+
+### Removed
+
+- `Cmd::program` and `Cmd::arguments` functions as they were misleading (didn't
+  do what `distant-local` or `distant-ssh2` do)
 
 ## [0.20.0-alpha.12]
 

Cargo.toml

@@ -59,6 +59,7 @@ tokio = { version = "1.28.2", features = ["full"] }
 toml_edit = { version = "0.19.10", features = ["serde"] }
 terminal_size = "0.2.6"
 termwiz = "0.20.0"
+typed-path = "0.3.2"
 which = "4.4.0"
 winsplit = "0.1.0"
 whoami = "1.4.0"

distant-local/src/api/state/process/instance.rs

@@ -85,6 +85,7 @@ impl ProcessInstance {
         let args = cmd_and_args.split_off(1);
         let cmd = cmd_and_args.into_iter().next().unwrap();
 
+        debug!("Spawning process: {cmd} {args:?}");
         let mut child: Box<dyn Process> = match pty {
             Some(size) => Box::new(PtyProcess::spawn(
                 cmd.clone(),

distant-protocol/Cargo.toml

@@ -17,6 +17,7 @@ tests = []
 
 [dependencies]
 bitflags = "2.3.1"
+const-str = "0.5.6"
 derive_more = { version = "0.99.17", default-features = false, features = ["deref", "deref_mut", "display", "from", "error", "into", "into_iterator", "is_variant"] }
 regex = "1.8.3"
 serde = { version = "1.0.163", features = ["derive"] }

distant-protocol/src/common/cmd.rs

@@ -12,22 +12,6 @@ impl Cmd {
     pub fn new(cmd: impl Into<String>) -> Self {
         Self(cmd.into())
     }
-
-    /// Returns reference to the program portion of the command
-    pub fn program(&self) -> &str {
-        match self.0.split_once(' ') {
-            Some((program, _)) => program.trim(),
-            None => self.0.trim(),
-        }
-    }
-
-    /// Returns reference to the arguments portion of the command
-    pub fn arguments(&self) -> &str {
-        match self.0.split_once(' ') {
-            Some((_, arguments)) => arguments.trim(),
-            None => "",
-        }
-    }
 }
 
 impl Deref for Cmd {

distant-protocol/src/lib.rs

@@ -17,7 +17,11 @@ pub use response::*;
 
 /// Protocol version indicated by the tuple of (major, minor, patch).
 ///
-/// This is different from the crate version, which matches that of the complete suite of distant
-/// crates. Rather, this verison is used to provide stability indicators when the protocol itself
-/// changes across crate versions.
-pub const PROTOCOL_VERSION: SemVer = (0, 1, 0);
+/// This should match the version of this crate such that any significant change to the crate
+/// version will also be reflected in this constant that can be used to verify compatibility across
+/// the wire.
+pub const PROTOCOL_VERSION: SemVer = (
+    const_str::parse!(env!("CARGO_PKG_VERSION_MAJOR"), u8),
+    const_str::parse!(env!("CARGO_PKG_VERSION_MINOR"), u8),
+    const_str::parse!(env!("CARGO_PKG_VERSION_PATCH"), u8),
+);

distant-ssh2/src/lib.rs

@@ -16,7 +16,6 @@ use std::str::FromStr;
 use std::time::Duration;
 
 use async_compat::CompatExt;
-use async_once_cell::OnceCell;
 use async_trait::async_trait;
 use distant_core::net::auth::{AuthHandlerMap, DummyAuthHandler, Verifier};
 use distant_core::net::client::{Client, ClientConfig};
@@ -25,6 +24,7 @@ use distant_core::net::server::{Server, ServerRef};
 use distant_core::{DistantApiServerHandler, DistantClient, DistantSingleKeyCredentials};
 use log::*;
 use smol::channel::Receiver as SmolReceiver;
+use tokio::sync::Mutex;
 use wezterm_ssh::{
     ChildKiller, Config as WezConfig, MasterPty, PtySize, Session as WezSession,
     SessionEvent as WezSessionEvent,
@@ -325,17 +325,20 @@ impl SshAuthHandler for LocalSshAuthHandler {
     }
 }
 
-/// Represents an ssh2 client
+/// Represents an ssh2 client.
 pub struct Ssh {
     session: WezSession,
     events: SmolReceiver<WezSessionEvent>,
     host: String,
     port: u16,
     authenticated: bool,
+
+    /// Cached copy of the family representing the remote machine.
+    cached_family: Mutex<Option<SshFamily>>,
 }
 
 impl Ssh {
-    /// Connect to a remote TCP server using SSH
+    /// Connect to a remote TCP server using SSH.
     pub fn connect(host: impl AsRef<str>, opts: SshOpts) -> io::Result<Self> {
         debug!(
             "Establishing ssh connection to {} using {:?}",
@@ -416,15 +419,16 @@ impl Ssh {
             host: host.as_ref().to_string(),
             port,
             authenticated: false,
+            cached_family: Mutex::new(None),
         })
     }
 
-    /// Host this client is connected to
+    /// Host this client is connected to.
     pub fn host(&self) -> &str {
         &self.host
     }
 
-    /// Port this client is connected to on remote host
+    /// Port this client is connected to on remote host.
     pub fn port(&self) -> u16 {
         self.port
     }
@@ -434,7 +438,7 @@ impl Ssh {
         self.authenticated
     }
 
-    /// Authenticates the [`Ssh`] if not already authenticated
+    /// Authenticates the [`Ssh`] if not already authenticated.
     pub async fn authenticate(&mut self, handler: impl SshAuthHandler) -> io::Result<()> {
         // If already authenticated, exit
         if self.authenticated {
@@ -499,10 +503,10 @@ impl Ssh {
         Ok(())
     }
 
-    /// Detects the family of operating system on the remote machine
+    /// Detects the family of operating system on the remote machine.
+    ///
+    /// Caches the result such that subsequent checks will return the same family.
     pub async fn detect_family(&self) -> io::Result<SshFamily> {
-        static INSTANCE: OnceCell<SshFamily> = OnceCell::new();
-
         // Exit early if not authenticated as this is a requirement
         if !self.authenticated {
             return Err(io::Error::new(
@@ -511,18 +515,23 @@ impl Ssh {
             ));
         }
 
-        INSTANCE
-            .get_or_try_init(async move {
-                let is_windows = utils::is_windows(&self.session).await?;
+        let mut family = self.cached_family.lock().await;
 
-                Ok(if is_windows {
-                    SshFamily::Windows
-                } else {
-                    SshFamily::Unix
-                })
-            })
-            .await
-            .copied()
+        // Family value is not present, so we retrieve it now and populate our cache
+        if family.is_none() {
+            // Check if we are windows, otherwise assume unix, returning an error if encountered,
+            // which will also drop our lock on the cache
+            let is_windows = utils::is_windows(&self.session).await?;
+
+            *family = Some(if is_windows {
+                SshFamily::Windows
+            } else {
+                SshFamily::Unix
+            });
+        }
+
+        // Cache should always be Some(...) by this point
+        Ok(family.unwrap())
     }
 
     /// Consume [`Ssh`] and produce a [`DistantClient`] that is connected to a remote

src/cli/commands/client.rs

@@ -25,7 +25,10 @@ use crate::cli::common::{
     Cache, Client, JsonAuthHandler, MsgReceiver, MsgSender, PromptAuthHandler,
 };
 use crate::constants::MAX_PIPE_CHUNK_SIZE;
-use crate::options::{ClientFileSystemSubcommand, ClientSubcommand, Format, NetworkSettings};
+use crate::options::{
+    ClientFileSystemSubcommand, ClientSubcommand, Format, NetworkSettings, ParseShellError,
+    Shell as ShellOption,
+};
 use crate::{CliError, CliResult};
 
 mod lsp;
@@ -369,6 +372,7 @@ async fn async_run(cmd: ClientSubcommand) -> CliResult {
             environment,
             lsp,
             pty,
+            shell,
             network,
         } => {
             debug!("Connecting to manager");
@@ -383,28 +387,63 @@ async fn async_run(cmd: ClientSubcommand) -> CliResult {
                 use_or_lookup_connection_id(&mut cache, connection, &mut client).await?;
 
             debug!("Opening channel to connection {}", connection_id);
-            let channel = client
+            let mut channel: DistantChannel = client
                 .open_raw_channel(connection_id)
                 .await
-                .with_context(|| format!("Failed to open channel to connection {connection_id}"))?;
+                .with_context(|| format!("Failed to open channel to connection {connection_id}"))?
+                .into_client()
+                .into_channel();
 
             // Convert cmd into string
             let cmd = cmd_str.unwrap_or_else(|| cmd.join(" "));
 
+            // Check if we should attempt to run the command in a shell
+            let cmd = match shell {
+                None => cmd,
+
+                // Use default shell, which we need to figure out
+                Some(None) => {
+                    let system_info = channel
+                        .system_info()
+                        .await
+                        .context("Failed to detect remote operating system")?;
+
+                    // If system reports a default shell, use it, otherwise pick a default based on the
+                    // operating system being windows or non-windows
+                    let shell: ShellOption = if !system_info.shell.is_empty() {
+                        system_info.shell.parse()
+                    } else if system_info.family.eq_ignore_ascii_case("windows") {
+                        "cmd.exe".parse()
+                    } else {
+                        "/bin/sh".parse()
+                    }
+                    .map_err(|x: ParseShellError| anyhow::anyhow!(x))?;
+
+                    shell
+                        .make_cmd_string(&cmd)
+                        .map_err(|x| anyhow::anyhow!(x))?
+                }
+
+                // Use explicit shell
+                Some(Some(shell)) => shell
+                    .make_cmd_string(&cmd)
+                    .map_err(|x| anyhow::anyhow!(x))?,
+            };
+
             if let Some(scheme) = lsp {
                 debug!(
                     "Spawning LSP server (pty = {}, cwd = {:?}): {}",
                     pty, current_dir, cmd
                 );
-                Lsp::new(channel.into_client().into_channel())
+                Lsp::new(channel)
                     .spawn(cmd, current_dir, scheme, pty, MAX_PIPE_CHUNK_SIZE)
                     .await?;
             } else if pty {
                 debug!(
                     "Spawning pty process (environment = {:?}, cwd = {:?}): {}",
                     environment, current_dir, cmd
                 );
-                Shell::new(channel.into_client().into_channel())
+                Shell::new(channel)
                     .spawn(
                         cmd,
                         environment.into_map(),
@@ -421,7 +460,7 @@ async fn async_run(cmd: ClientSubcommand) -> CliResult {
                     .environment(environment.into_map())
                     .current_dir(current_dir)
                     .pty(None)
-                    .spawn(channel.into_client().into_channel(), &cmd)
+                    .spawn(channel, &cmd)
                     .await
                     .with_context(|| format!("Failed to spawn {cmd}"))?;
 

src/options.rs

@@ -463,6 +463,11 @@ pub enum ClientSubcommand {
         #[clap(long)]
         pty: bool,
 
+        /// If specified, will spawn the process in the specified shell, defaulting to the
+        /// user-configured shell.
+        #[clap(long, name = "SHELL")]
+        shell: Option<Option<Shell>>,
+
         /// Alternative current directory for the remote process
         #[clap(long)]
         current_dir: Option<PathBuf>,
@@ -1938,6 +1943,7 @@ mod tests {
                 current_dir: None,
                 environment: map!(),
                 lsp: Some(None),
+                shell: Some(None),
                 pty: true,
                 cmd_str: None,
                 cmd: vec![String::from("cmd")],
@@ -1977,6 +1983,7 @@ mod tests {
                     current_dir: None,
                     environment: map!(),
                     lsp: Some(None),
+                    shell: Some(None),
                     pty: true,
                     cmd_str: None,
                     cmd: vec![String::from("cmd")],
@@ -2003,6 +2010,7 @@ mod tests {
                 current_dir: None,
                 environment: map!(),
                 lsp: Some(None),
+                shell: Some(None),
                 pty: true,
                 cmd_str: None,
                 cmd: vec![String::from("cmd")],
@@ -2042,6 +2050,7 @@ mod tests {
                     current_dir: None,
                     environment: map!(),
                     lsp: Some(None),
+                    shell: Some(None),
                     pty: true,
                     cmd_str: None,
                     cmd: vec![String::from("cmd")],

src/options/common.rs

@@ -3,6 +3,7 @@ mod cmd;
 mod logging;
 mod network;
 mod search;
+mod shell;
 mod time;
 mod value;
 
@@ -11,5 +12,6 @@ pub use cmd::*;
 pub use logging::*;
 pub use network::*;
 pub use search::*;
+pub use shell::*;
 pub use time::*;
 pub use value::*;