Skip to content

Commit

Permalink
Add support for Command and RemoteChild owning their session
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
masklinn committed Oct 2, 2023
1 parent 499c672 commit 87f8dec
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 103 deletions.
13 changes: 13 additions & 0 deletions src/changelog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down
45 changes: 25 additions & 20 deletions src/child.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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
Expand All @@ -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>,
Expand All @@ -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
Expand Down Expand Up @@ -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()
}
}
101 changes: 53 additions & 48 deletions src/command.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand All @@ -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)
}
}
Expand All @@ -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)
}
}
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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:
///
Expand All @@ -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());
});
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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?;
(
Expand All @@ -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());
}
Expand Down
Loading

0 comments on commit 87f8dec

Please sign in to comment.