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`. It was not entirely clear whether `Deref`,
`Borrow`, or `AsRef` would be best, may need to be updated before
release it it proves sub-par.

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.
  • Loading branch information
masklinn committed Sep 25, 2023
1 parent 499c672 commit 5eb30fd
Show file tree
Hide file tree
Showing 6 changed files with 237 additions and 70 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 [`crate::command::Command`]
/// owning a session reference.
/// - Change [`OverSsh::over_ssh`] to be generic and support owned
/// sessions.
/// ## Removed
#[doc(hidden)]
pub mod unreleased {}
Expand Down
36 changes: 19 additions & 17 deletions src/child.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,26 +43,26 @@ 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
/// is created via the [`Command`](crate::command::Command) struct through [`Session::command`], 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).
/// [`Command`](crate::command::Command).
///
/// 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 +74,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 +102,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 +197,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()
}
}
86 changes: 45 additions & 41 deletions src/command.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::escape::escape;

use super::child::Child;
use super::stdio::TryFromChildIo;
use super::RemoteChild;
use super::Stdio;
use super::{Error, Session};

Expand Down Expand Up @@ -112,17 +112,17 @@ pub trait OverSsh {
/// }
///
/// ```
fn over_ssh<'session>(
fn over_ssh<S: AsRef<Session> + Clone>(
&self,
session: &'session Session,
) -> Result<crate::Command<'session>, crate::Error>;
session: S,
) -> Result<Command<S>, crate::Error>;
}

impl OverSsh for std::process::Command {
fn over_ssh<'session>(
fn over_ssh<S: AsRef<Session> + Clone> (
&self,
session: &'session Session,
) -> Result<Command<'session>, crate::Error> {
session: S,
) -> Result<Command<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 +134,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 +143,10 @@ impl OverSsh for std::process::Command {
}

impl OverSsh for tokio::process::Command {
fn over_ssh<'session>(
fn over_ssh<S: AsRef<Session> + Clone> (
&self,
session: &'session Session,
) -> Result<Command<'session>, crate::Error> {
session: S,
) -> Result<Command<S>, crate::Error> {
self.as_std().over_ssh(session)
}
}
Expand All @@ -155,10 +155,10 @@ impl<S> OverSsh for &S
where
S: OverSsh,
{
fn over_ssh<'session>(
fn over_ssh<U: AsRef<Session> + Clone>(
&self,
session: &'session Session,
) -> Result<Command<'session>, crate::Error> {
session: U,
) -> Result<Command<U>, crate::Error> {
<S as OverSsh>::over_ssh(self, session)
}
}
Expand All @@ -167,29 +167,30 @@ impl<S> OverSsh for &mut S
where
S: OverSsh,
{
fn over_ssh<'session>(
fn over_ssh<U: AsRef<Session> + Clone>(
&self,
session: &'session Session,
) -> Result<Command<'session>, crate::Error> {
session: U,
) -> Result<Command<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
/// `OwnedCommand` 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, `OwnedCommand` 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 +208,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 Command<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> Command<S> {
pub(crate) fn new(session: S, imp: CommandImp) -> Self {
Self {
session,
imp,
Expand All @@ -231,7 +232,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`](Command::raw_arg).
///
/// Only one argument can be passed per use. So instead of:
///
Expand All @@ -251,7 +253,7 @@ impl<'s> Command<'s> {
/// ```
///
/// To pass multiple arguments see [`args`](Command::args).
pub fn arg<S: AsRef<str>>(&mut self, arg: S) -> &mut Self {
pub fn arg<A: AsRef<str>>(&mut self, arg: A) -> &mut Self {
self.raw_arg(&*shell_escape::unix::escape(Cow::Borrowed(arg.as_ref())))
}

Expand All @@ -263,7 +265,7 @@ impl<'s> Command<'s> {
/// 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 {
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 @@ -277,10 +279,10 @@ impl<'s> Command<'s> {
/// use [`raw_args`](Command::raw_args).
///
/// To pass a single argument see [`arg`](Command::arg).
pub fn args<I, S>(&mut self, args: I) -> &mut Self
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 @@ -296,10 +298,10 @@ impl<'s> Command<'s> {
/// 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
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 +353,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> Command<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 +375,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
12 changes: 8 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@
//! a remote command. You can [spawn](Command::spawn) the remote command, which just gives you a
//! handle to the running process, you can run the command and wait for its
//! [output](Command::output), or you can run it and just extract its [exit
//! status](Command::status). Unlike its `std` counterpart though, these methods on [`Command`] can
//! status](Command::status). Unlike its `std` counterpart though, these methods on [`Command`](OwningCommand) can
//! fail even if the remote command executed successfully, since there is a fallible network
//! separating you from it.
//!
//! Also unlike its `std` counterpart, [`spawn`](Command::spawn) gives you a [`RemoteChild`] rather
//! Also unlike its `std` counterpart, [`spawn`](OwningCommand::spawn) gives you a [`Child`] rather
//! than a [`std::process::Child`]. Behind the scenes, a remote child is really just a process
//! handle to the _local_ `ssh` instance corresponding to the spawned remote command. The behavior
//! of the methods of [`RemoteChild`] therefore match the behavior of `ssh`, rather than that of
Expand Down Expand Up @@ -167,12 +167,16 @@ mod builder;
pub use builder::{KnownHosts, SessionBuilder};

mod command;
pub use command::{Command, OverSsh};
pub use command::{Command as OwningCommand, OverSsh};
/// Convenience [`OwningCommand`] alias when working with a session reference.
pub type Command<'s> = OwningCommand<&'s Session>;

mod escape;

mod child;
pub use child::RemoteChild;
pub use child::Child;
/// Convenience [`Child`] alias when working with a session reference.
pub type RemoteChild<'a> = Child<&'a Session>;

mod error;
pub use error::Error;
Expand Down
Loading

0 comments on commit 5eb30fd

Please sign in to comment.