From 82330f870ecdcc14b4cd354fe861a742f640dfdf Mon Sep 17 00:00:00 2001 From: Christian Visintin Date: Sun, 12 Feb 2023 22:27:42 +0100 Subject: [PATCH] Optimize transfers (#147) * When the file is exchanged, all times attributes are set (if supported by the protocol) * If local/remote file have the same last modification time (`mtime`), the file is not transferred --- CHANGELOG.md | 3 + Cargo.lock | 1 + Cargo.toml | 1 + src/host/mod.rs | 53 ++++++++- src/ui/activities/filetransfer/session.rs | 133 +++++++++++++++------- 5 files changed, 149 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf67493..5a27d62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,9 @@ Released on 14/02/2023 > 🦥 The lazy update +- **Transfers optimized**: + - If local/remote file have the same "last modification time" (`mtime`), the file is not transferred + - When the file is exchanged, all times attributes are set (if supported by the protocol) - **Default ssh config path**: - SSH configuration path is now `~/.ssh/config` by default - Added ARM64 Linux builds diff --git a/Cargo.lock b/Cargo.lock index a118e41..92b5538 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2670,6 +2670,7 @@ dependencies = [ "content_inspector", "dirs", "edit", + "filetime", "hostname", "keyring", "lazy-regex", diff --git a/Cargo.toml b/Cargo.toml index dd5b60d..81af93f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ chrono = "^0.4" content_inspector = "^0.2" dirs = "^4.0" edit = "^0.1" +filetime = "^0.2" hostname = "^0.3" keyring = { version = "^1.2", optional = true } lazy-regex = "^2.4" diff --git a/src/host/mod.rs b/src/host/mod.rs index 2d8ced7..7140e1f 100644 --- a/src/host/mod.rs +++ b/src/host/mod.rs @@ -3,6 +3,7 @@ //! `host` is the module which provides functionalities to host file system // ext +use filetime::{self, FileTime}; #[cfg(target_family = "unix")] use remotefs::fs::UnixPex; use remotefs::fs::{File, FileType, Metadata}; @@ -406,6 +407,27 @@ impl Localhost { Ok(File { path, metadata }) } + /// Set file stat + pub fn setstat(&self, path: &Path, metadata: &Metadata) -> Result<(), HostError> { + debug!("Setting stat for file at {}", path.display()); + if let Some(mtime) = metadata.modified { + let mtime = FileTime::from_system_time(mtime); + debug!("setting mtime {:?}", mtime); + filetime::set_file_mtime(path, mtime) + .map_err(|e| HostError::new(HostErrorType::FileNotAccessible, Some(e), path))?; + } + if let Some(atime) = metadata.accessed { + let atime = FileTime::from_system_time(atime); + filetime::set_file_atime(path, atime) + .map_err(|e| HostError::new(HostErrorType::FileNotAccessible, Some(e), path))?; + } + #[cfg(target_family = "unix")] + if let Some(mode) = metadata.mode { + self.chmod(path, mode)?; + } + Ok(()) + } + /// Execute a command on localhost pub fn exec(&self, cmd: &str) -> Result { // Make command @@ -625,8 +647,8 @@ mod tests { use super::*; #[cfg(target_family = "unix")] - use crate::utils::test_helpers::{create_sample_file, make_fsentry}; - use crate::utils::test_helpers::{make_dir_at, make_file_at}; + use crate::utils::test_helpers::make_fsentry; + use crate::utils::test_helpers::{create_sample_file, make_dir_at, make_file_at}; use pretty_assertions::assert_eq; #[cfg(target_family = "unix")] @@ -636,6 +658,8 @@ mod tests { #[cfg(target_family = "unix")] use std::os::unix::fs::{symlink, PermissionsExt}; + use std::time::SystemTime; + use std::{ops::AddAssign, time::Duration}; #[test] fn test_host_error_new() { @@ -896,6 +920,31 @@ mod tests { .is_err()); } + #[test] + fn should_setstat() { + let tmpdir: tempfile::TempDir = tempfile::TempDir::new().unwrap(); + let file: tempfile::NamedTempFile = create_sample_file(); + let host: Localhost = Localhost::new(PathBuf::from(tmpdir.path())).ok().unwrap(); + // stat + let mut filemeta = host.stat(file.path()).unwrap(); + + let mut new_atime = SystemTime::UNIX_EPOCH; + new_atime.add_assign(Duration::from_secs(1612164210)); + + let mut new_mtime = SystemTime::UNIX_EPOCH; + new_mtime.add_assign(Duration::from_secs(1613160210)); + + filemeta.metadata.accessed = Some(new_atime); + filemeta.metadata.modified = Some(new_mtime); + + // setstat + assert!(host.setstat(filemeta.path(), filemeta.metadata()).is_ok()); + let new_metadata = host.stat(file.path()).unwrap(); + + assert_eq!(new_metadata.metadata().accessed, Some(new_atime)); + assert_eq!(new_metadata.metadata().modified, Some(new_mtime)); + } + #[cfg(target_family = "unix")] #[test] fn test_host_chmod() { diff --git a/src/ui/activities/filetransfer/session.rs b/src/ui/activities/filetransfer/session.rs index 8e7b770..efedda2 100644 --- a/src/ui/activities/filetransfer/session.rs +++ b/src/ui/activities/filetransfer/session.rs @@ -9,7 +9,7 @@ use crate::utils::fmt::fmt_millis; // Ext use bytesize::ByteSize; -use remotefs::fs::{File, ReadStream, UnixPex, Welcome, WriteStream}; +use remotefs::fs::{File, Metadata, ReadStream, UnixPex, Welcome, WriteStream}; use remotefs::{RemoteError, RemoteErrorType}; use std::fs::File as StdFile; use std::io::{Read, Seek, Write}; @@ -405,6 +405,18 @@ impl FileTransferActivity { .stat(local.path.as_path()) .map_err(TransferErrorReason::HostError) .map(|x| x.metadata().clone())?; + + if !self.has_remote_file_changed(remote, &metadata) { + self.log( + LogLevel::Info, + format!( + "file {} won't be transferred since hasn't changed", + local.path().display() + ), + ); + self.transfer.full.update_progress(metadata.size as usize); + return Ok(()); + } // Upload file // Try to open local file match self.host.open_file_read(local.path.as_path()) { @@ -507,6 +519,10 @@ impl FileTransferActivity { if self.transfer.aborted() { return Err(TransferErrorReason::Abrupted); } + // set stat + if let Err(err) = self.client.setstat(remote, local.metadata().clone()) { + error!("failed to set stat for {}: {}", remote.display(), err); + } self.log( LogLevel::Info, format!( @@ -549,6 +565,10 @@ impl FileTransferActivity { if let Err(err) = self.client.create_file(remote, &metadata, Box::new(reader)) { return Err(TransferErrorReason::FileTransferError(err)); } + // set stat + if let Err(err) = self.client.setstat(remote, metadata) { + error!("failed to set stat for {}: {}", remote.display(), err); + } // Set transfer size ok self.transfer.partial.update_progress(file_size); self.transfer.full.update_progress(file_size); @@ -684,19 +704,19 @@ impl FileTransferActivity { match self.host.mkdir_ex(local_dir_path.as_path(), true) { Ok(_) => { // Apply file mode to directory - #[cfg(any(target_family = "unix", target_os = "macos", target_os = "linux"))] - if let Some(mode) = entry.metadata().mode { - if let Err(err) = self.host.chmod(local_dir_path.as_path(), mode) { - self.log( - LogLevel::Error, - format!( - "Could not apply file mode {:o} to \"{}\": {}", - u32::from(mode), - local_dir_path.display(), - err - ), - ); - } + if let Err(err) = self + .host + .setstat(local_dir_path.as_path(), entry.metadata()) + { + self.log( + LogLevel::Error, + format!( + "Could not set stat to directory {:?} to \"{}\": {}", + entry.metadata(), + local_dir_path.display(), + err + ), + ); } self.log( LogLevel::Info, @@ -812,6 +832,21 @@ impl FileTransferActivity { remote: &File, file_name: String, ) -> Result<(), TransferErrorReason> { + // check if files are equal (in case, don't transfer) + if !self.has_local_file_changed(local, remote) { + self.log( + LogLevel::Info, + format!( + "file {} won't be transferred since hasn't changed", + remote.path().display() + ), + ); + self.transfer + .full + .update_progress(remote.metadata().size as usize); + return Ok(()); + } + // Try to open local file match self.host.open_file_write(local) { Ok(local_file) => { @@ -909,19 +944,16 @@ impl FileTransferActivity { return Err(TransferErrorReason::Abrupted); } // Apply file mode to file - #[cfg(target_family = "unix")] - if let Some(mode) = remote.metadata.mode { - if let Err(err) = self.host.chmod(local, mode) { - self.log( - LogLevel::Error, - format!( - "Could not apply file mode {:o} to \"{}\": {}", - u32::from(mode), - local.display(), - err - ), - ); - } + if let Err(err) = self.host.setstat(local, remote.metadata()) { + self.log( + LogLevel::Error, + format!( + "Could not set stat to file {:?} to \"{}\": {}", + remote.metadata(), + local.display(), + err + ), + ); } // Log self.log( @@ -970,19 +1002,16 @@ impl FileTransferActivity { self.update_progress_bar(format!("Downloading \"{file_name}\"")); self.view(); // Apply file mode to file - #[cfg(target_family = "unix")] - if let Some(mode) = remote.metadata.mode { - if let Err(err) = self.host.chmod(local, mode) { - self.log( - LogLevel::Error, - format!( - "Could not apply file mode {:o} to \"{}\": {}", - u32::from(mode), - local.display(), - err - ), - ); - } + if let Err(err) = self.host.setstat(local, remote.metadata()) { + self.log( + LogLevel::Error, + format!( + "Could not set stat to file {:?} to \"{}\": {}", + remote.metadata(), + local.display(), + err + ), + ); } // Log self.log( @@ -1134,6 +1163,30 @@ impl FileTransferActivity { } } + // file changed + + /// Check whether provided file has changed on local disk, compared to remote file + fn has_local_file_changed(&self, local: &Path, remote: &File) -> bool { + // check if files are equal (in case, don't transfer) + if let Ok(local_file) = self.host.stat(local) { + local_file.metadata().modified != remote.metadata().modified + || local_file.metadata().size != remote.metadata().size + } else { + true + } + } + + /// Checks whether remote file has changed compared to local file + fn has_remote_file_changed(&mut self, remote: &Path, local_metadata: &Metadata) -> bool { + // check if files are equal (in case, don't transfer) + if let Ok(remote_file) = self.client.stat(remote) { + local_metadata.modified != remote_file.metadata().modified + || local_metadata.size != remote_file.metadata().size + } else { + true + } + } + // -- file exist pub(crate) fn local_file_exists(&mut self, p: &Path) -> bool {