Add support for Command and RemoteChild owning their session (#136)

Supporting session-owning commands (and children) makes it easier to
e.g. move long-running children around, or store them around in
structures, without having to deal with self-reference issues.

Currently, support is limited to clonable smart pointers which can be
coerced to `&Session`. After some testing / discussion, `Deref` seems
like a better bound than `AsRef` as it works with references, and thus
(amongst other things) allows the existing `Session::command` and
`Session::raw_command` to go through the new associated
function. Provide an `arc_[raw_]command` helper as wrapping an
`Arc<Session>` to unbound the command (and child) from the session's
scope is the primary and likely major use case of this feature.

Does not work with a bare `Session` because `Command::spawn`,
`Command::output`, and `Command::status` take self by (mutable)
reference, so there is no way to consume the `Session` out. Generally
`Command` as a builder is set up around chaining mutable references
rather than moving ownership.

Had to shuffle names a bit to try and avoid breaking backwards
compatibility, which is not great: the internal `Command` becomes the
external `OwningCommand`, and the old `RemoteChild` is now `Child`,
with the external `RemoteChild` being an alias.

Also add an owning version of `Session::subsystem` as well, following
the same pattern, since while `subsystem` creates a `OwningCommand` it
goes through a different path and thus can't be handled by
`Session::to_command`.

src/changelog.rs

@@ -5,7 +5,20 @@ use crate::*;
 ///
 /// ## Added
 ///  - Add new fn `SessionBuilder::ssh_auth_sock`
+///  - Add new fns [`Session::arc_command`], [`Session::arc_raw_command`],
+///    [`Session::to_command`], and [`Session::to_raw_command`] to support
+///    session-owning commands
+///  - Add generic [`crate::OwningCommand`], to support session-owning
+///    commands.
+///  - Add [`crate::child::Child`] as a generic version of [`RemoteChild`]
+///    to support session-owning commands
 /// ## Changed
+///  - Change [`RemoteChild`] to be an alias to [`crate::child::Child`]
+///    owning a session references.
+///  - Change [`Command`] to be an alias to [`OwningCommand`] owning a
+///    session reference.
+///  - Change [`OverSsh::over_ssh`] to be generic and support owned
+///    sessions.
 /// ## Removed
 #[doc(hidden)]
 pub mod unreleased {}

src/child.rs

@@ -1,4 +1,4 @@
-use super::{ChildStderr, ChildStdin, ChildStdout, Error, Session};
+use super::{ChildStderr, ChildStdin, ChildStdout, Error};
 
 use std::io;
 use std::process::{ExitStatus, Output};
@@ -42,27 +42,30 @@ macro_rules! delegate {
 
 /// Representation of a running or exited remote child process.
 ///
-/// This structure is used to represent and manage remote child processes. A remote child process
-/// is created via the [`Command`](crate::Command) struct through [`Session::command`], which
-/// configures the spawning process and can itself be constructed using a builder-style interface.
+/// This structure is used to represent and manage remote child
+/// processes. A remote child process is created via the
+/// [`OwningCommand`](crate::OwningCommand) struct through
+/// [`Session::command`](crate::Session::command) or one of its
+/// variants, which configures the spawning process and can itself be
+/// constructed using a builder-style interface.
 ///
-/// Calling [`wait`](RemoteChild::wait) (or other functions that wrap around it) will make the
+/// Calling [`wait`](Child::wait) (or other functions that wrap around it) will make the
 /// parent process wait until the child has actually exited before continuing.
 ///
-/// Unlike [`std::process::Child`], `RemoteChild` *does* implement [`Drop`], and will terminate the
+/// Unlike [`std::process::Child`], `Child` *does* implement [`Drop`], and will terminate the
 /// local `ssh` process corresponding to the remote process when it goes out of scope. Note that
 /// this does _not_ terminate the remote process. If you want to do that, you will need to kill it
 /// yourself by executing a remote command like `pkill` to kill it on the remote side.
 ///
-/// As a result, `RemoteChild` cannot expose `stdin`, `stdout`, and `stderr` as fields for
+/// As a result, `Child` cannot expose `stdin`, `stdout`, and `stderr` as fields for
 /// split-borrows like [`std::process::Child`] does. Instead, it exposes
-/// [`stdin`](RemoteChild::stdin), [`stdout`](RemoteChild::stdout),
-/// and [`stderr`](RemoteChild::stderr) as methods. Callers can call `.take()` to get the same
+/// [`stdin`](Child::stdin), [`stdout`](Child::stdout),
+/// and [`stderr`](Child::stderr) as methods. Callers can call `.take()` to get the same
 /// effect as a split borrow and use multiple streams concurrently. Note that for the streams to be
 /// available,`Stdio::piped()` should be passed to the corresponding method on
-/// [`Command`](crate::Command).
+/// [`OwningCommand`](crate::OwningCommand).
 ///
-/// NOTE that once `RemoteChild` is dropped, any data written to `stdin` will not be sent to the
+/// NOTE that once `Child` is dropped, any data written to `stdin` will not be sent to the
 /// remote process and `stdout` and `stderr` will yield EOF immediately.
 ///
 /// ```rust,no_run
@@ -74,18 +77,18 @@ macro_rules! delegate {
 /// # }
 /// ```
 #[derive(Debug)]
-pub struct RemoteChild<'s> {
-    session: &'s Session,
+pub struct Child<S> {
+    session: S,
     imp: RemoteChildImp,
 
     stdin: Option<ChildStdin>,
     stdout: Option<ChildStdout>,
     stderr: Option<ChildStderr>,
 }
 
-impl<'s> RemoteChild<'s> {
+impl<S> Child<S> {
     pub(crate) fn new(
-        session: &'s Session,
+        session: S,
         (imp, stdin, stdout, stderr): (
             RemoteChildImp,
             Option<ChildStdin>,
@@ -102,11 +105,6 @@ impl<'s> RemoteChild<'s> {
         }
     }
 
-    /// Access the SSH session that this remote process was spawned from.
-    pub fn session(&self) -> &'s Session {
-        self.session
-    }
-
     /// Disconnect from this given remote child process.
     ///
     /// Note that disconnecting does _not_ kill the remote process, it merely kills the local
@@ -202,3 +200,10 @@ impl<'s> RemoteChild<'s> {
         &mut self.stderr
     }
 }
+
+impl<S: Clone> Child<S> {
+    /// Access the SSH session that this remote process was spawned from.
+    pub fn session(&self) -> S {
+        self.session.clone()
+    }
+}

src/command.rs

@@ -1,12 +1,13 @@
 use crate::escape::escape;
 
+use super::child::Child;
 use super::stdio::TryFromChildIo;
-use super::RemoteChild;
 use super::Stdio;
 use super::{Error, Session};
 
 use std::borrow::Cow;
 use std::ffi::OsStr;
+use std::ops::Deref;
 use std::process;
 
 #[derive(Debug)]
@@ -112,17 +113,17 @@ pub trait OverSsh {
     /// }
     ///
     /// ```
-    fn over_ssh<'session>(
+    fn over_ssh<S: Deref<Target = Session> + Clone>(
         &self,
-        session: &'session Session,
-    ) -> Result<crate::Command<'session>, crate::Error>;
+        session: S,
+    ) -> Result<OwningCommand<S>, crate::Error>;
 }
 
 impl OverSsh for std::process::Command {
-    fn over_ssh<'session>(
+    fn over_ssh<S: Deref<Target = Session> + Clone>(
         &self,
-        session: &'session Session,
-    ) -> Result<Command<'session>, crate::Error> {
+        session: S,
+    ) -> Result<OwningCommand<S>, crate::Error> {
         // I'd really like `!self.get_envs().is_empty()` here, but that's
         // behind a `exact_size_is_empty` feature flag.
         if self.get_envs().len() > 0 {
@@ -134,7 +135,7 @@ impl OverSsh for std::process::Command {
         }
 
         let program_escaped: Cow<'_, OsStr> = escape(self.get_program());
-        let mut command = session.raw_command(program_escaped);
+        let mut command = Session::to_raw_command(session, program_escaped);
 
         let args = self.get_args().map(escape);
         command.raw_args(args);
@@ -143,10 +144,10 @@ impl OverSsh for std::process::Command {
 }
 
 impl OverSsh for tokio::process::Command {
-    fn over_ssh<'session>(
+    fn over_ssh<S: Deref<Target = Session> + Clone>(
         &self,
-        session: &'session Session,
-    ) -> Result<Command<'session>, crate::Error> {
+        session: S,
+    ) -> Result<OwningCommand<S>, crate::Error> {
         self.as_std().over_ssh(session)
     }
 }
@@ -155,10 +156,10 @@ impl<S> OverSsh for &S
 where
     S: OverSsh,
 {
-    fn over_ssh<'session>(
+    fn over_ssh<U: Deref<Target = Session> + Clone>(
         &self,
-        session: &'session Session,
-    ) -> Result<Command<'session>, crate::Error> {
+        session: U,
+    ) -> Result<OwningCommand<U>, crate::Error> {
         <S as OverSsh>::over_ssh(self, session)
     }
 }
@@ -167,29 +168,30 @@ impl<S> OverSsh for &mut S
 where
     S: OverSsh,
 {
-    fn over_ssh<'session>(
+    fn over_ssh<U: Deref<Target = Session> + Clone>(
         &self,
-        session: &'session Session,
-    ) -> Result<Command<'session>, crate::Error> {
+        session: U,
+    ) -> Result<OwningCommand<U>, crate::Error> {
         <S as OverSsh>::over_ssh(self, session)
     }
 }
 
 /// A remote process builder, providing fine-grained control over how a new remote process should
 /// be spawned.
 ///
-/// A default configuration can be generated using [`Session::command(program)`](Session::command),
-/// where `program` gives a path to the program to be executed. Additional builder methods allow
-/// the configuration to be changed (for example, by adding arguments) prior to spawning.  The
-/// interface is almost identical to that of [`std::process::Command`].
+/// A default configuration can be generated using [`Session::command(program)`](Session::command)
+/// or [`Session::arc_command(program)`](Session::arc_command), where `program` gives a path to
+/// the program to be executed. Additional builder methods allow the configuration to be changed
+/// (for example, by adding arguments) prior to spawning. The interface is almost identical to
+/// that of [`std::process::Command`].
 ///
-/// `Command` can be reused to spawn multiple remote processes. The builder methods change the
-/// command without needing to immediately spawn the process. Similarly, you can call builder
+/// `OwningCommand` can be reused to spawn multiple remote processes. The builder methods change
+/// the command without needing to immediately spawn the process. Similarly, you can call builder
 /// methods after spawning a process and then spawn a new process with the modified settings.
 ///
 /// # Environment variables and current working directory.
 ///
-/// You'll notice that unlike its `std` counterpart, `Command` does not have any methods for
+/// You'll notice that unlike its `std` counterpart, `OwningCommand` does not have any methods for
 /// setting environment variables or the current working directory for the remote command. This is
 /// because the SSH protocol does not support this (at least not in its standard configuration).
 /// For more details on this, see the `ENVIRONMENT` section of [`ssh(1)`]. To work around this,
@@ -207,17 +209,17 @@ where
 ///   [`ssh(1)`]: https://linux.die.net/man/1/ssh
 ///   [`env(1)`]: https://linux.die.net/man/1/env
 #[derive(Debug)]
-pub struct Command<'s> {
-    session: &'s Session,
+pub struct OwningCommand<S> {
+    session: S,
     imp: CommandImp,
 
     stdin_set: bool,
     stdout_set: bool,
     stderr_set: bool,
 }
 
-impl<'s> Command<'s> {
-    pub(crate) fn new(session: &'s super::Session, imp: CommandImp) -> Self {
+impl<S> OwningCommand<S> {
+    pub(crate) fn new(session: S, imp: CommandImp) -> Self {
         Self {
             session,
             imp,
@@ -231,7 +233,8 @@ impl<'s> Command<'s> {
     /// Adds an argument to pass to the remote program.
     ///
     /// Before it is passed to the remote host, `arg` is escaped so that special characters aren't
-    /// evaluated by the remote shell. If you do not want this behavior, use [`raw_arg`](Command::raw_arg).
+    /// evaluated by the remote shell. If you do not want this behavior, use
+    /// [`raw_arg`](Self::raw_arg).
     ///
     /// Only one argument can be passed per use. So instead of:
     ///
@@ -250,20 +253,20 @@ impl<'s> Command<'s> {
     /// # ; }
     /// ```
     ///
-    /// To pass multiple arguments see [`args`](Command::args).
-    pub fn arg<S: AsRef<str>>(&mut self, arg: S) -> &mut Self {
+    /// To pass multiple arguments see [`args`](Self::args).
+    pub fn arg<A: AsRef<str>>(&mut self, arg: A) -> &mut Self {
         self.raw_arg(&*shell_escape::unix::escape(Cow::Borrowed(arg.as_ref())))
     }
 
     /// Adds an argument to pass to the remote program.
     ///
-    /// Unlike [`arg`](Command::arg), this method does not shell-escape `arg`. The argument is passed as written
+    /// Unlike [`arg`](Self::arg), this method does not shell-escape `arg`. The argument is passed as written
     /// to `ssh`, which will pass it again as an argument to the remote shell. Since the remote
     /// shell may do argument parsing, characters such as spaces and `*` may be interpreted by the
     /// remote shell.
     ///
-    /// To pass multiple unescaped arguments see [`raw_args`](Command::raw_args).
-    pub fn raw_arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
+    /// To pass multiple unescaped arguments see [`raw_args`](Self::raw_args).
+    pub fn raw_arg<A: AsRef<OsStr>>(&mut self, arg: A) -> &mut Self {
         delegate!(&mut self.imp, imp, {
             imp.raw_arg(arg.as_ref());
         });
@@ -274,13 +277,13 @@ impl<'s> Command<'s> {
     ///
     /// Before they are passed to the remote host, each argument in `args` is escaped so that
     /// special characters aren't evaluated by the remote shell. If you do not want this behavior,
-    /// use [`raw_args`](Command::raw_args).
+    /// use [`raw_args`](Self::raw_args).
     ///
-    /// To pass a single argument see [`arg`](Command::arg).
-    pub fn args<I, S>(&mut self, args: I) -> &mut Self
+    /// To pass a single argument see [`arg`](Self::arg).
+    pub fn args<I, A>(&mut self, args: I) -> &mut Self
     where
-        I: IntoIterator<Item = S>,
-        S: AsRef<str>,
+        I: IntoIterator<Item = A>,
+        A: AsRef<str>,
     {
         for arg in args {
             self.arg(arg);
@@ -290,16 +293,16 @@ impl<'s> Command<'s> {
 
     /// Adds multiple arguments to pass to the remote program.
     ///
-    /// Unlike [`args`](Command::args), this method does not shell-escape `args`. The arguments are passed as
+    /// Unlike [`args`](Self::args), this method does not shell-escape `args`. The arguments are passed as
     /// written to `ssh`, which will pass them again as arguments to the remote shell. However,
     /// since the remote shell may do argument parsing, characters such as spaces and `*` may be
     /// interpreted by the remote shell.
     ///
-    /// To pass a single argument see [`raw_arg`](Command::raw_arg).
-    pub fn raw_args<I, S>(&mut self, args: I) -> &mut Self
+    /// To pass a single argument see [`raw_arg`](Self::raw_arg).
+    pub fn raw_args<I, A>(&mut self, args: I) -> &mut Self
     where
-        I: IntoIterator<Item = S>,
-        S: AsRef<OsStr>,
+        I: IntoIterator<Item = A>,
+        A: AsRef<OsStr>,
     {
         for arg in args {
             self.raw_arg(arg);
@@ -351,10 +354,12 @@ impl<'s> Command<'s> {
         self.stderr_set = true;
         self
     }
+}
 
-    async fn spawn_impl(&mut self) -> Result<RemoteChild<'s>, Error> {
-        Ok(RemoteChild::new(
-            self.session,
+impl<S: Clone> OwningCommand<S> {
+    async fn spawn_impl(&mut self) -> Result<Child<S>, Error> {
+        Ok(Child::new(
+            self.session.clone(),
             delegate!(&mut self.imp, imp, {
                 let (imp, stdin, stdout, stderr) = imp.spawn().await?;
                 (
@@ -371,7 +376,7 @@ impl<'s> Command<'s> {
     /// instead.
     ///
     /// By default, stdin, stdout and stderr are inherited.
-    pub async fn spawn(&mut self) -> Result<RemoteChild<'s>, Error> {
+    pub async fn spawn(&mut self) -> Result<Child<S>, Error> {
         if !self.stdin_set {
             self.stdin(Stdio::inherit());
         }