From 106c61a096d27cd252b97a81e034b979fe0f0b55 Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Wed, 17 Dec 2025 10:05:47 +0100 Subject: [PATCH] context: refactor context command running --- src/context/api.rs | 36 ++++------ src/context/local.rs | 29 ++------ src/context/mod.rs | 2 +- src/context/ssh.rs | 160 ++++++++++++++++++------------------------- 4 files changed, 88 insertions(+), 139 deletions(-) diff --git a/src/context/api.rs b/src/context/api.rs index fbb2158..842e884 100644 --- a/src/context/api.rs +++ b/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; - fn output(&mut self) -> io::Result; -} - pub trait ContextDriver { fn ensure_available(&self, src: &Path, dest_root: &str) -> io::Result; fn retrieve_path(&self, src: &Path, dest: &Path) -> io::Result<()>; fn list_files(&self, path: &Path) -> io::Result>; - fn create_runner(&self, program: String) -> Box; + fn run(&self, program: &str, args: &[String]) -> io::Result; + fn run_output(&self, program: &str, args: &[String]) -> io::Result; fn prepare_work_dir(&self) -> io::Result; } @@ -44,10 +34,11 @@ pub enum Context { impl Context { pub fn command>(&self, program: S) -> ContextCommand { - let runner = self - .driver() - .create_runner(program.as_ref().to_string_lossy().to_string()); - ContextCommand { runner } + ContextCommand { + driver: self.driver(), + program: program.as_ref().to_string_lossy().to_string(), + 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, + driver: Box, + program: String, + args: Vec, } impl ContextCommand { pub fn arg>(&mut self, arg: S) -> &mut Self { - self.runner - .add_arg(arg.as_ref().to_string_lossy().to_string()); + self.args.push(arg.as_ref().to_string_lossy().to_string()); self } @@ -118,11 +110,11 @@ impl ContextCommand { } pub fn status(&mut self) -> io::Result { - self.runner.status() + self.driver.run(&self.program, &self.args) } // Capture output pub fn output(&mut self) -> io::Result { - self.runner.output() + self.driver.run_output(&self.program, &self.args) } } diff --git a/src/context/local.rs b/src/context/local.rs index 1f1c2d1..591fbb9 100644 --- a/src/context/local.rs +++ b/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 { - Box::new(LocalRunner { - program, - args: Vec::new(), - }) - } - fn prepare_work_dir(&self) -> io::Result { // 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 { - pub program: String, - pub args: Vec, -} - -impl CommandRunner for LocalRunner { - fn add_arg(&mut self, arg: String) { - self.args.push(arg); + fn run(&self, program: &str, args: &[String]) -> io::Result { + Command::new(program).args(args).status() } - fn status(&mut self) -> io::Result { - Command::new(&self.program).args(&self.args).status() - } - - fn output(&mut self) -> io::Result { - Command::new(&self.program).args(&self.args).output() + fn run_output(&self, program: &str, args: &[String]) -> io::Result { + Command::new(program).args(args).output() } } diff --git a/src/context/mod.rs b/src/context/mod.rs index 3a01503..874addd 100644 --- a/src/context/mod.rs +++ b/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 { diff --git a/src/context/ssh.rs b/src/context/ssh.rs index eda9fb0..5356e19 100644 --- a/src/context/ssh.rs +++ b/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 { - 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 { + 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 { + 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 { 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, - pub port: Option, - pub program: String, - pub args: Vec, -} - -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 { - 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 { - 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, - }) - } -}