From da2b6097e9b13f73b02c9156ab509529259ac9b7 Mon Sep 17 00:00:00 2001 From: Steppy Date: Mon, 3 Feb 2025 00:29:23 +0100 Subject: [PATCH 1/7] Add relative local path ankers, when parsing servers --- src/main.rs | 4 ++-- src/server.rs | 41 +++++++++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 14 deletions(-) diff --git a/src/main.rs b/src/main.rs index e9831ba..16095a5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,7 @@ use crate::command::{ExecutionError, LogRunnable, SpecificExecutionError}; use crate::file::{FileMatcher, FileNameInfo}; use crate::logger::{LogLevel, Logger}; use crate::os_string_builder::ReplaceWithOsStr; -use crate::server::ServerAddress; +use crate::server::{RelativeLocalPathAnker, ServerAddress}; use clap::{Parser, Subcommand, ValueEnum}; use lazy_regex::{lazy_regex, Lazy, Regex}; use server::{Server, ServerReference}; @@ -614,7 +614,7 @@ fn parse_server_configuration(config_str: &str) -> Result, String> { config_str .split(',') .map(|server_entry| { - Server::from_str(server_entry) + Server::from_str(server_entry, RelativeLocalPathAnker::Home) .map_err(|e| format!("Invalid server entry '{server_entry}': {e}")) }) .collect() diff --git a/src/server.rs b/src/server.rs index b6a8a43..2d1ebdc 100644 --- a/src/server.rs +++ b/src/server.rs @@ -2,6 +2,7 @@ use crate::get_home_directory; use std::cell::LazyCell; use std::error::Error; use std::fmt::{Display, Formatter}; +use std::fs; use std::hash::{Hash, Hasher}; use std::ops::Deref; use std::path::PathBuf; @@ -71,7 +72,7 @@ impl FromStr for ServerReference { type Err = ServerReferenceParseError; fn from_str(s: &str) -> Result { - Server::from_str(s) + Server::from_str(s, RelativeLocalPathAnker::CurrentDirectory) .map(Self::Resolved) .or_else(|_| Ok(Self::Identifier(s.to_string()))) } @@ -124,22 +125,28 @@ impl Server { ServerAddress::Localhost => "this computer", } } -} -impl FromStr for Server { - type Err = ServerParseError; - - fn from_str(s: &str) -> Result { + pub fn from_str( + s: &str, + relative_local_path_anker: RelativeLocalPathAnker, + ) -> Result { s.split_once(':') .ok_or(ServerParseError::MissingServerDirectory) .and_then(|(identifier, server_directory)| { let address = ServerAddress::from_str(identifier); - let mut server_directory_path = PathBuf::from(server_directory); - if let ServerAddress::Localhost = &address { - let home_directory = get_home_directory() - .map_err(|e| ServerParseError::HomeDirectoryRequired { detail_message: e })?; - server_directory_path = home_directory.join(&server_directory_path); - } + let server_directory_path = match &address { + ServerAddress::Ssh { .. } => PathBuf::from(server_directory), + ServerAddress::Localhost => fs::canonicalize(match relative_local_path_anker { + RelativeLocalPathAnker::Home => { + let home_directory = get_home_directory() + .map_err(|e| ServerParseError::HomeDirectoryRequired { detail_message: e })?; + home_directory.join(server_directory) + } + RelativeLocalPathAnker::CurrentDirectory => PathBuf::from(server_directory), + }) + .map_err(|_| ServerParseError::ServerDirectoryNonExistent)?, + }; + Ok(Self { address, server_directory_path, @@ -162,6 +169,12 @@ impl Display for Server { } } +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub enum RelativeLocalPathAnker { + Home, + CurrentDirectory, +} + #[derive(Debug, Clone, Eq, PartialEq, Hash)] pub enum ServerAddress { Ssh { ssh_address: String }, @@ -201,6 +214,7 @@ impl ServerAddress { #[derive(Debug)] pub enum ServerParseError { MissingServerDirectory, + ServerDirectoryNonExistent, HomeDirectoryRequired { detail_message: String }, } @@ -218,6 +232,9 @@ impl Display for ServerParseError { f, "localhost requires home directory, but: {detail_message}" ), + ServerParseError::ServerDirectoryNonExistent => { + write!(f, "The specified server directory doesn't exist") + } } } } From 329ba8f38190d7bc88ec6f5a683208482b57fcda Mon Sep 17 00:00:00 2001 From: Steppy Date: Mon, 3 Feb 2025 00:51:56 +0100 Subject: [PATCH 2/7] Canonicalize upload directory when uploading to local --- src/main.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 16095a5..72de303 100644 --- a/src/main.rs +++ b/src/main.rs @@ -181,7 +181,7 @@ fn main() -> Result<(), String> { files, file_server, old_version_policy, - upload_directory, + mut upload_directory, no_confirm, file_name, } => { @@ -234,6 +234,15 @@ fn main() -> Result<(), String> { let actions = servers .iter() .map(|server| { + //on local server canonicalize upload_directory + if let ServerAddress::Localhost = &server.address { + //create upload directory if it doesn't exist + fs::create_dir_all(&upload_directory) + .map_err(|e| format!("Failed to create upload-directory: {e}"))?; + + upload_directory = fs::canonicalize(&upload_directory) + .map_err(|e| format!("failed to resolve upload-directory: {e}"))?; + } let working_directory = server.server_directory_path.join(&upload_directory); Ok(ServerActions { server, @@ -658,6 +667,7 @@ mod test { assert_eq!(PathBuf::from("/home"), joined); } + /// When renaming a file in a folder, the folder is relevant in the new name #[test] fn rename_experiment() { fs::rename("test-ressources/files/test", "test-ressources/files/test1") @@ -665,4 +675,10 @@ mod test { fs::rename("test-ressources/files/test1", "test-ressources/files/test") .expect("failed to rename test1 file back to test"); } + + #[test] + fn mkdir_experiment() { + fs::create_dir_all("./test-ressources/files/../python") + .expect("failed to create directory with relative path"); + } } From 6e4a23254a737d0b7620ffaf39def02985691684 Mon Sep 17 00:00:00 2001 From: Steppy Date: Mon, 3 Feb 2025 01:47:36 +0100 Subject: [PATCH 3/7] Canonicalize files to upload if they are on remote server --- src/main.rs | 39 +++++++++++++++++++++++++++++++++------ src/os_string_builder.rs | 7 +++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/main.rs b/src/main.rs index 72de303..aa6ec71 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,9 +15,11 @@ use clap::{Parser, Subcommand, ValueEnum}; use lazy_regex::{lazy_regex, Lazy, Regex}; use server::{Server, ServerReference}; use std::cell::LazyCell; +use std::ffi::{OsStr, OsString}; use std::hash::Hash; use std::io::Write; use std::iter::once; +use std::os::unix::ffi::OsStrExt; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::{env, fs, io}; @@ -178,7 +180,7 @@ fn main() -> Result<(), String> { match args.command { Command::Upload { - files, + mut files, file_server, old_version_policy, mut upload_directory, @@ -186,6 +188,7 @@ fn main() -> Result<(), String> { file_name, } => { require_non_empty_servers(&servers)?; + require_non_empty(&files, "files to upload")?; start_ssh_agent(&logger)?; //resolve file server @@ -204,6 +207,25 @@ fn main() -> Result<(), String> { match &file_server { Some(file_server) => match &file_server.address { ServerAddress::Ssh { ssh_address } => { + //canonicalize remote files + let joined_files = osf!("{{") + + files + .iter() + .map(|file| file.as_os_str()) + .collect::>() + .join(&OsString::from(",")) + + "}"; + let realpath_output = ShellCmd::new("ssh") + .arg(ssh_address) + .arg(osf!("realpath ") + file_server.server_directory_path.join(joined_files)) + .collect_output() + .map_err(|e| format!("failed to canonicalize files to upload: {e}"))?; + files = realpath_output + .stdout + .split(|&b| b == b'\n') //split at line breaks + .map(|bytes| PathBuf::from(OsStr::from_bytes(bytes))) + .collect(); + for file in &files { check_file_exists_on_server(file, ssh_address, &file_server.server_directory_path)?; } @@ -580,12 +602,17 @@ fn get_home_directory() -> Result { .and_then(|home_dir| home_dir.ok_or("Failed to find home directory".to_string())) } -fn require_non_empty_servers(servers: &[Server]) -> Result<(), String> { - if servers.is_empty() { - Err("You did not provide any servers for this operation. Please see --help".to_string()) - } else { - Ok(()) +fn require_non_empty_servers(servers: &[T]) -> Result<(), String> { + require_non_empty(servers, "servers for this operation") +} + +fn require_non_empty(slice: &[T], slice_name: &str) -> Result<(), String> { + if slice.is_empty() { + Err(format!( + "You did not provide any {slice_name}. Please see --help" + ))? } + Ok(()) } fn start_ssh_agent(logger: &Logger) -> Result<(), String> { diff --git a/src/os_string_builder.rs b/src/os_string_builder.rs index 0879e2c..f03e373 100644 --- a/src/os_string_builder.rs +++ b/src/os_string_builder.rs @@ -2,6 +2,7 @@ use std::ffi::{OsStr, OsString}; use std::fmt::{Debug, Formatter}; use std::hash::{Hash, Hasher}; use std::ops::{Add, AddAssign}; +use std::path::Path; pub trait ReplaceWithOsStr<'a, Pattern = &'a str> { #[must_use] @@ -56,6 +57,12 @@ impl AsRef for OsStringBuilder { } } +impl AsRef for OsStringBuilder { + fn as_ref(&self) -> &Path { + self.result.as_ref() + } +} + impl

Add

for OsStringBuilder where P: AsRef, From 930e02d80229b2a2d5e794cd85065de33e6fa1e1 Mon Sep 17 00:00:00 2001 From: Steppy Date: Mon, 3 Feb 2025 02:19:01 +0100 Subject: [PATCH 4/7] Add remaining TODOs --- src/command.rs | 1 + src/main.rs | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/command.rs b/src/command.rs index b301a1f..6474703 100644 --- a/src/command.rs +++ b/src/command.rs @@ -9,6 +9,7 @@ use std::process::{Command, ExitStatus, Output}; pub trait LogRunnable { fn run(&mut self, logger: &Logger) -> Result<(), SpecificExecutionError>; fn collect_output(&mut self) -> Result; + //TODO collect_full_output } impl LogRunnable for Command { diff --git a/src/main.rs b/src/main.rs index aa6ec71..10c6fdf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -208,6 +208,7 @@ fn main() -> Result<(), String> { Some(file_server) => match &file_server.address { ServerAddress::Ssh { ssh_address } => { //canonicalize remote files + //TODO only join when there are multiple paths let joined_files = osf!("{{") + files .iter() @@ -215,6 +216,8 @@ fn main() -> Result<(), String> { .collect::>() .join(&OsString::from(",")) + "}"; + + //TODO handle bad exit status let realpath_output = ShellCmd::new("ssh") .arg(ssh_address) .arg(osf!("realpath ") + file_server.server_directory_path.join(joined_files)) @@ -225,6 +228,7 @@ fn main() -> Result<(), String> { .split(|&b| b == b'\n') //split at line breaks .map(|bytes| PathBuf::from(OsStr::from_bytes(bytes))) .collect(); + log!(logger, debug, "canonical files: {files:?}"); for file in &files { check_file_exists_on_server(file, ssh_address, &file_server.server_directory_path)?; @@ -587,7 +591,7 @@ where error: ExecutionError::BadExitStatus(_), //test failed .. }) => Err(format!( - "File '{}' doesn't exist on file server", + "File '{}' doesn't exist on file-server", full_path.to_string_lossy() )), Err(e) => Err(format!( From ffd8f71f8b531418306e24dcac948d948bcba7c0 Mon Sep 17 00:00:00 2001 From: Steppy Date: Mon, 3 Feb 2025 12:33:15 +0100 Subject: [PATCH 5/7] Add collect_full_output method to commands --- src/command.rs | 92 ++++++++++++++++++++++++++++++++++++-------------- src/main.rs | 21 +++++------- 2 files changed, 75 insertions(+), 38 deletions(-) diff --git a/src/command.rs b/src/command.rs index 6474703..5636d99 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,27 +1,34 @@ use crate::log; use crate::logger::{LogLevel, Logger}; use std::error::Error; -use std::fmt::{Display, Formatter}; +use std::fmt::{Debug, Display, Formatter}; use std::io; use std::iter::once; use std::process::{Command, ExitStatus, Output}; pub trait LogRunnable { - fn run(&mut self, logger: &Logger) -> Result<(), SpecificExecutionError>; - fn collect_output(&mut self) -> Result; - //TODO collect_full_output + fn run(&mut self, logger: &Logger) -> Result<(), CommandSpecificError>; + fn collect_output(&mut self) -> Result>; + fn collect_full_output(&mut self) -> Result>; } impl LogRunnable for Command { - fn run(&mut self, logger: &Logger) -> Result<(), SpecificExecutionError> { - run(self, logger).map_err(|error| SpecificExecutionError { + fn run(&mut self, logger: &Logger) -> Result<(), CommandSpecificError> { + run(self, logger).map_err(|error| CommandSpecificError { command: self, error, }) } - fn collect_output(&mut self) -> Result { - collect_output(self, None).map_err(|error| SpecificExecutionError { + fn collect_output(&mut self) -> Result> { + collect_output(self, None).map_err(|error| CommandSpecificError { + command: self, + error, + }) + } + + fn collect_full_output(&mut self) -> Result> { + collect_full_output(self).map_err(|error| CommandSpecificError { command: self, error, }) @@ -47,7 +54,7 @@ fn collect_output( command: &mut Command, logger: Option<&Logger>, ) -> Result { - let output = command.output()?; //pipes stdout and stderr automatically + let output = collect_full_output(command)?; if !output.status.success() { if let Some(logger) = logger { log!(logger, error, "{}", String::from_utf8_lossy(&output.stdout)); @@ -58,13 +65,20 @@ fn collect_output( Ok(output) } -#[derive(Debug)] -pub struct SpecificExecutionError<'a> { - pub command: &'a Command, - pub error: ExecutionError, +fn collect_full_output(command: &mut Command) -> Result { + Ok(command.output()?) } -impl Display for SpecificExecutionError<'_> { +#[derive(Debug)] +pub struct CommandSpecificError<'a, E> { + pub command: &'a Command, + pub error: E, +} + +impl Display for CommandSpecificError<'_, E> +where + E: Display, +{ fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!( f, @@ -76,22 +90,52 @@ impl Display for SpecificExecutionError<'_> { } fn command_to_string(command: &Command) -> String { - once(command.get_program().to_string_lossy()) - .chain(command.get_args().map(|arg| arg.to_string_lossy())) + once(command.get_program().to_string_lossy().to_string()) + .chain(command.get_args().map(|arg| { + let arg_str = arg.to_string_lossy(); + if arg_str.contains(' ') { + format!("\"{arg_str}\"") + } else { + arg_str.to_string() + } + })) .collect::>() .join(" ") } -impl Error for SpecificExecutionError<'_> {} +impl Error for CommandSpecificError<'_, E> where E: Debug + Display {} + +#[derive(Debug)] +pub struct StartError(io::Error); + +impl From for StartError { + fn from(value: io::Error) -> Self { + StartError(value) + } +} + +impl Display for StartError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "Failed to start command: {}", self.0) + } +} + +impl Error for StartError {} #[derive(Debug)] pub enum ExecutionError { - StartError(io::Error), + StartError(StartError), BadExitStatus(ExitStatus), } impl From for ExecutionError { fn from(value: io::Error) -> Self { + Self::StartError(StartError(value)) + } +} + +impl From for ExecutionError { + fn from(value: StartError) -> Self { Self::StartError(value) } } @@ -105,10 +149,8 @@ impl From for ExecutionError { impl Display for ExecutionError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - ExecutionError::StartError(e) => write!(f, "Failed to start command: {}", e), - ExecutionError::BadExitStatus(status) => { - write!(f, "Command failed with {}", status) - } + ExecutionError::StartError(e) => Display::fmt(e, f), + ExecutionError::BadExitStatus(status) => write!(f, "Command failed with {}", status), } } } @@ -117,7 +159,7 @@ impl Error for ExecutionError {} #[cfg(test)] mod test { - use crate::command::{ExecutionError, LogRunnable, SpecificExecutionError}; + use crate::command::{CommandSpecificError, ExecutionError, LogRunnable}; use crate::logger::Logger; use std::path::PathBuf; use std::process::Command; @@ -126,7 +168,7 @@ mod test { fn test_unknown_command() { let mut command = Command::new("python7"); let Err( - e @ SpecificExecutionError { + e @ CommandSpecificError { error: ExecutionError::StartError(_), .. }, @@ -143,7 +185,7 @@ mod test { fn test_error() { let mut command = Command::new("python3"); let Err( - e @ SpecificExecutionError { + e @ CommandSpecificError { error: ExecutionError::BadExitStatus(_), .. }, diff --git a/src/main.rs b/src/main.rs index 10c6fdf..bc62143 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,7 @@ mod os_string_builder; mod server; use crate::action::{Action, FileAction, ServerActions}; -use crate::command::{ExecutionError, LogRunnable, SpecificExecutionError}; +use crate::command::{CommandSpecificError, ExecutionError, LogRunnable}; use crate::file::{FileMatcher, FileNameInfo}; use crate::logger::{LogLevel, Logger}; use crate::os_string_builder::ReplaceWithOsStr; @@ -216,7 +216,7 @@ fn main() -> Result<(), String> { .collect::>() .join(&OsString::from(",")) + "}"; - + //TODO handle bad exit status let realpath_output = ShellCmd::new("ssh") .arg(ssh_address) @@ -234,17 +234,12 @@ fn main() -> Result<(), String> { check_file_exists_on_server(file, ssh_address, &file_server.server_directory_path)?; } } - ServerAddress::Localhost => { - for file in &files { - check_local_file_exists(file_server.server_directory_path.join(file))?; - } - } + ServerAddress::Localhost => files + .iter() + .map(|file| file_server.server_directory_path.join(file)) + .try_for_each(check_local_file_exists)?, }, - None => { - for file in &files { - check_local_file_exists(file)?; - } - } + None => files.iter().try_for_each(check_local_file_exists)?, } let file_details = files @@ -587,7 +582,7 @@ where .collect_output() { Ok(_) => Ok(()), //file exists on file server - Err(SpecificExecutionError { + Err(CommandSpecificError { error: ExecutionError::BadExitStatus(_), //test failed .. }) => Err(format!( From 2eb37aa07a3723c4c844d9ee081575641755fe03 Mon Sep 17 00:00:00 2001 From: Steppy Date: Mon, 3 Feb 2025 13:05:47 +0100 Subject: [PATCH 6/7] Fix canonicalization of remote files --- src/main.rs | 54 ++++++++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/src/main.rs b/src/main.rs index bc62143..30dde00 100644 --- a/src/main.rs +++ b/src/main.rs @@ -15,7 +15,7 @@ use clap::{Parser, Subcommand, ValueEnum}; use lazy_regex::{lazy_regex, Lazy, Regex}; use server::{Server, ServerReference}; use std::cell::LazyCell; -use std::ffi::{OsStr, OsString}; +use std::ffi::OsStr; use std::hash::Hash; use std::io::Write; use std::iter::once; @@ -207,32 +207,35 @@ fn main() -> Result<(), String> { match &file_server { Some(file_server) => match &file_server.address { ServerAddress::Ssh { ssh_address } => { - //canonicalize remote files - //TODO only join when there are multiple paths - let joined_files = osf!("{{") - + files - .iter() - .map(|file| file.as_os_str()) - .collect::>() - .join(&OsString::from(",")) - + "}"; + //canonicalize remote files -> also makes sure they exist + files = files + .iter() + .map(|file| { + let output = ShellCmd::new("ssh") + .arg(ssh_address) + .arg(osf!("realpath -e ") + file_server.server_directory_path.join(file)) + .collect_full_output() + .map_err(|e| format!("Failed to canonicalize files: {e}"))?; - //TODO handle bad exit status - let realpath_output = ShellCmd::new("ssh") - .arg(ssh_address) - .arg(osf!("realpath ") + file_server.server_directory_path.join(joined_files)) - .collect_output() - .map_err(|e| format!("failed to canonicalize files to upload: {e}"))?; - files = realpath_output - .stdout - .split(|&b| b == b'\n') //split at line breaks - .map(|bytes| PathBuf::from(OsStr::from_bytes(bytes))) + if !output.status.success() { + Err(format!( + "Path doesn't match any files on file-server: {}", + file.to_string_lossy() + ))?; + } + + let denoted_files = output + .stdout + .split(|&b| b == b'\n') //split at line breaks + .map(|bytes| PathBuf::from(OsStr::from_bytes(bytes))) + .collect::>(); + + Ok(denoted_files) + }) + .collect::, String>>()? + .into_iter() + .flatten() .collect(); - log!(logger, debug, "canonical files: {files:?}"); - - for file in &files { - check_file_exists_on_server(file, ssh_address, &file_server.server_directory_path)?; - } } ServerAddress::Localhost => files .iter() @@ -565,6 +568,7 @@ where Ok(()) } +#[allow(dead_code)] fn check_file_exists_on_server( path: P, ssh_address: S, From 6d8b16b011fc37c5a455c649010fe17da5214811 Mon Sep 17 00:00:00 2001 From: Steppy Date: Mon, 3 Feb 2025 13:14:40 +0100 Subject: [PATCH 7/7] Fix canonicalization adding files with empty name --- src/main.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main.rs b/src/main.rs index 30dde00..6c66e3d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -227,6 +227,7 @@ fn main() -> Result<(), String> { let denoted_files = output .stdout .split(|&b| b == b'\n') //split at line breaks + .filter(|bytes| !bytes.is_empty()) //needed since realpath sometimes gives us empty lines .map(|bytes| PathBuf::from(OsStr::from_bytes(bytes))) .collect::>(); @@ -236,6 +237,7 @@ fn main() -> Result<(), String> { .into_iter() .flatten() .collect(); + log!(logger, debug, "canonical files: {files:?}"); } ServerAddress::Localhost => files .iter()