context: refactor context command running

src/context/api.rs

@@ -5,22 +5,12 @@ use std::path::{Path, PathBuf};
 
 use super::local::LocalDriver;
 use super::ssh::SshDriver;
-
-/// Internal trait defining the strategy for executing a command.
-///
-/// This allows to have different execution behaviors (Local, SSH, ...) while keeping the
-/// `ContextCommand` API consistent.
-pub trait CommandRunner {
-    fn add_arg(&mut self, arg: String);
-    fn status(&mut self) -> io::Result<std::process::ExitStatus>;
-    fn output(&mut self) -> io::Result<std::process::Output>;
-}
-
 pub trait ContextDriver {
     fn ensure_available(&self, src: &Path, dest_root: &str) -> io::Result<PathBuf>;
     fn retrieve_path(&self, src: &Path, dest: &Path) -> io::Result<()>;
     fn list_files(&self, path: &Path) -> io::Result<Vec<PathBuf>>;
-    fn create_runner(&self, program: String) -> Box<dyn CommandRunner>;
+    fn run(&self, program: &str, args: &[String]) -> io::Result<std::process::ExitStatus>;
+    fn run_output(&self, program: &str, args: &[String]) -> io::Result<std::process::Output>;
     fn prepare_work_dir(&self) -> io::Result<String>;
 }
 
@@ -44,10 +34,11 @@ pub enum Context {
 
 impl Context {
     pub fn command<S: AsRef<OsStr>>(&self, program: S) -> ContextCommand {
-        let runner = self
+        ContextCommand {
-            .driver()
+            driver: self.driver(),
-            .create_runner(program.as_ref().to_string_lossy().to_string());
+            program: program.as_ref().to_string_lossy().to_string(),
-        ContextCommand { runner }
+            args: Vec::new(),
+        }
     }
 
     /// Ensures that the source file/directory exists at the destination context.
@@ -93,15 +84,16 @@ impl Context {
 /// details of *how* the command is actually executed, allowing the user to simple chain arguments
 /// and call `status()` or `output()`.
 ///
-/// It delegates the actual work to a `CommandRunner`.
+/// It delegates the actual work to a `ContextDriver`.
 pub struct ContextCommand {
-    runner: Box<dyn CommandRunner>,
+    driver: Box<dyn ContextDriver>,
+    program: String,
+    args: Vec<String>,
 }
 
 impl ContextCommand {
     pub fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
-        self.runner
+        self.args.push(arg.as_ref().to_string_lossy().to_string());
-            .add_arg(arg.as_ref().to_string_lossy().to_string());
         self
     }
 
@@ -118,11 +110,11 @@ impl ContextCommand {
     }
 
     pub fn status(&mut self) -> io::Result<std::process::ExitStatus> {
-        self.runner.status()
+        self.driver.run(&self.program, &self.args)
     }
 
     // Capture output
     pub fn output(&mut self) -> io::Result<std::process::Output> {
-        self.runner.output()
+        self.driver.run_output(&self.program, &self.args)
     }
 }

src/context/local.rs

@@ -1,7 +1,6 @@
-use super::api::{CommandRunner, ContextDriver};
 /// Local context: execute commands locally
 /// Context driver: Does nothing
-/// Command runner: Wrapper around 'std::process::Command'
+use super::api::ContextDriver;
 use std::io;
 use std::path::{Path, PathBuf};
 use std::process::Command;
@@ -13,13 +12,6 @@ impl ContextDriver for LocalDriver {
         src.canonicalize()
     }
 
-    fn create_runner(&self, program: String) -> Box<dyn CommandRunner> {
-        Box::new(LocalRunner {
-            program,
-            args: Vec::new(),
-        })
-    }
-
     fn prepare_work_dir(&self) -> io::Result<String> {
         // TODO: Fix that, we should not always use '..' as work directory locally
         Ok("..".to_string())
@@ -37,23 +29,12 @@ impl ContextDriver for LocalDriver {
         }
         Ok(entries)
     }
-}
 
-pub struct LocalRunner {
+    fn run(&self, program: &str, args: &[String]) -> io::Result<std::process::ExitStatus> {
-    pub program: String,
+        Command::new(program).args(args).status()
-    pub args: Vec<String>,
-}
-
-impl CommandRunner for LocalRunner {
-    fn add_arg(&mut self, arg: String) {
-        self.args.push(arg);
     }
 
-    fn status(&mut self) -> io::Result<std::process::ExitStatus> {
+    fn run_output(&self, program: &str, args: &[String]) -> io::Result<std::process::Output> {
-        Command::new(&self.program).args(&self.args).status()
+        Command::new(program).args(args).output()
-    }
-
-    fn output(&mut self) -> io::Result<std::process::Output> {
-        Command::new(&self.program).args(&self.args).output()
     }
 }

src/context/mod.rs

@@ -3,7 +3,7 @@ mod local;
 mod manager;
 mod ssh;
 
-pub use api::{CommandRunner, Context, ContextCommand};
+pub use api::{Context, ContextCommand};
 pub use manager::ContextManager;
 
 pub fn current_context() -> Context {

src/context/ssh.rs

@@ -1,9 +1,8 @@
-use super::api::{CommandRunner, ContextDriver};
+/// SSH context: execute commands over an SSH connection
+/// Context driver: Copies over SFTP with ssh2, executes commands over ssh2 channels
+use super::api::ContextDriver;
 use log::debug;
 use std::fs;
-/// SSH context: execute commands over an SSH connection
-/// Context driver: Copies over SFTP with ssh2
-/// Command runner: Executes commands over an ssh2 channel (with PTY)
 use std::io::{self, Read};
 use std::net::TcpStream;
 #[cfg(unix)]
@@ -67,16 +66,6 @@ impl ContextDriver for SshDriver {
         Ok(remote_dest)
     }
 
-    fn create_runner(&self, program: String) -> Box<dyn CommandRunner> {
-        Box::new(SshRunner {
-            host: self.host.clone(),
-            user: self.user.clone(),
-            port: self.port,
-            program,
-            args: Vec::new(),
-        })
-    }
-
     fn retrieve_path(&self, src: &Path, dest: &Path) -> io::Result<()> {
         let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
         let sftp = sess.sftp().map_err(io::Error::other)?;
@@ -101,6 +90,71 @@ impl ContextDriver for SshDriver {
         Ok(files)
     }
 
+    fn run(&self, program: &str, args: &[String]) -> io::Result<std::process::ExitStatus> {
+        let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
+        let mut channel = sess.channel_session().map_err(io::Error::other)?;
+
+        // Construct command line
+        let mut cmd_line = program.to_string();
+        for arg in args {
+            cmd_line.push(' ');
+            cmd_line.push_str(arg); // TODO: escape
+        }
+
+        debug!("Executing SSH command: {}", cmd_line);
+
+        channel
+            .request_pty("xterm", None, None)
+            .map_err(|e| io::Error::other(format!("Failed to request PTY: {}", e)))?;
+
+        channel.exec(&cmd_line).map_err(io::Error::other)?;
+
+        let mut stdout_stream = channel.stream(0);
+        let mut stdout = io::stdout();
+        io::copy(&mut stdout_stream, &mut stdout)?;
+
+        channel.wait_close().map_err(io::Error::other)?;
+
+        let code = channel.exit_status().unwrap_or(-1);
+        Ok(ExitStatus::from_raw(code))
+    }
+
+    fn run_output(&self, program: &str, args: &[String]) -> io::Result<std::process::Output> {
+        let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
+        let mut channel = sess.channel_session().map_err(io::Error::other)?;
+
+        // Construct command line
+        let mut cmd_line = program.to_string();
+        for arg in args {
+            cmd_line.push(' ');
+            cmd_line.push_str(arg); // TODO: escape
+        }
+
+        channel.exec(&cmd_line).map_err(io::Error::other)?;
+
+        let mut stdout = Vec::new();
+        channel.read_to_end(&mut stdout)?;
+
+        let mut stderr = Vec::new();
+        channel.stderr().read_to_end(&mut stderr)?;
+
+        channel.wait_close().map_err(io::Error::other)?;
+
+        #[cfg(unix)]
+        let status = std::process::ExitStatus::from_raw(channel.exit_status().unwrap_or(-1));
+
+        #[cfg(not(unix))]
+        let status = {
+            panic!("SSH output capture only supported on Unix");
+        };
+
+        Ok(std::process::Output {
+            status,
+            stdout,
+            stderr,
+        })
+    }
+
     fn prepare_work_dir(&self) -> io::Result<String> {
         let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
         let mut channel = sess.channel_session().map_err(io::Error::other)?;
@@ -167,81 +221,3 @@ impl SshDriver {
         Ok(())
     }
 }
-
-pub struct SshRunner {
-    pub host: String,
-    pub user: Option<String>,
-    pub port: Option<u16>,
-    pub program: String,
-    pub args: Vec<String>,
-}
-
-impl SshRunner {
-    fn prepare_channel(&self) -> io::Result<(ssh2::Channel, String)> {
-        let sess = connect_ssh(&self.host, self.user.as_deref(), self.port)?;
-        let channel = sess.channel_session().map_err(io::Error::other)?;
-
-        // Construct command line
-        let mut cmd_line = self.program.clone();
-        for arg in &self.args {
-            cmd_line.push(' ');
-            cmd_line.push_str(arg); // TODO: escape
-        }
-        Ok((channel, cmd_line))
-    }
-}
-
-impl CommandRunner for SshRunner {
-    fn add_arg(&mut self, arg: String) {
-        self.args.push(arg);
-    }
-
-    fn status(&mut self) -> io::Result<ExitStatus> {
-        let (mut channel, cmd_line) = self.prepare_channel()?;
-
-        debug!("Executing SSH command: {}", cmd_line);
-
-        channel
-            .request_pty("xterm", None, None)
-            .map_err(|e| io::Error::other(format!("Failed to request PTY: {}", e)))?;
-
-        channel.exec(&cmd_line).map_err(io::Error::other)?;
-
-        let mut stdout_stream = channel.stream(0);
-        let mut stdout = io::stdout();
-        io::copy(&mut stdout_stream, &mut stdout)?;
-
-        channel.wait_close().map_err(io::Error::other)?;
-
-        let code = channel.exit_status().unwrap_or(-1);
-        Ok(ExitStatus::from_raw(code))
-    }
-
-    fn output(&mut self) -> io::Result<std::process::Output> {
-        let (mut channel, cmd_line) = self.prepare_channel()?;
-
-        channel.exec(&cmd_line).map_err(io::Error::other)?;
-
-        let mut stdout = Vec::new();
-        channel.read_to_end(&mut stdout)?;
-
-        let mut stderr = Vec::new();
-        channel.stderr().read_to_end(&mut stderr)?;
-
-        channel.wait_close().map_err(io::Error::other)?;
-
-        #[cfg(unix)]
-        let status = std::process::ExitStatus::from_raw(channel.exit_status().unwrap_or(-1));
-
-        #[cfg(not(unix))]
-        let status = {
-            panic!("SSH output capture only supported on Unix");
-        };
-
-        Ok(std::process::Output {
-            status,
-            stdout,
-            stderr,
-        })
-    }
-}