From f73e43304eb7142ba8c1f4be07cd1ce3d9d7c6fd Mon Sep 17 00:00:00 2001 From: ChristianVisintin Date: Sat, 12 Dec 2020 16:26:03 +0100 Subject: [PATCH] FsEntry::*::symlink is now a Option>; this improved symlinks, which gave errors some times --- CHANGELOG.md | 1 + src/filetransfer/ftp_transfer.rs | 6 +- src/filetransfer/scp_transfer.rs | 14 +++- src/filetransfer/sftp_transfer.rs | 9 ++- src/fs/mod.rs | 42 +++++++++- src/host/mod.rs | 26 +++++-- .../activities/filetransfer_activity/input.rs | 76 ++----------------- .../filetransfer_activity/layout.rs | 22 +++++- 8 files changed, 108 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b63c54..1e8cc67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Released on ?? - General performance and code improvements +- Improved symlinks management ## 0.1.1 diff --git a/src/filetransfer/ftp_transfer.rs b/src/filetransfer/ftp_transfer.rs index 78e3991..6141110 100644 --- a/src/filetransfer/ftp_transfer.rs +++ b/src/filetransfer/ftp_transfer.rs @@ -613,7 +613,7 @@ mod tests { assert_eq!(file.abs_path, PathBuf::from("/tmp/omar.txt")); assert_eq!(file.name, String::from("omar.txt")); assert_eq!(file.size, 8192); - assert_eq!(file.symlink, None); + assert!(file.symlink.is_none()); assert_eq!(file.user, None); assert_eq!(file.group, None); assert_eq!(file.unix_pex.unwrap(), (6, 6, 4)); @@ -653,7 +653,7 @@ mod tests { assert_eq!(file.abs_path, PathBuf::from("/tmp/omar.txt")); assert_eq!(file.name, String::from("omar.txt")); assert_eq!(file.size, 4096); - assert_eq!(file.symlink, None); + assert!(file.symlink.is_none()); assert_eq!(file.user, Some(0)); assert_eq!(file.group, Some(9)); assert_eq!(file.unix_pex.unwrap(), (7, 5, 5)); @@ -692,7 +692,7 @@ mod tests { if let FsEntry::Directory(dir) = fs_entry { assert_eq!(dir.abs_path, PathBuf::from("/tmp/docs")); assert_eq!(dir.name, String::from("docs")); - assert_eq!(dir.symlink, None); + assert!(dir.symlink.is_none()); assert_eq!(dir.user, Some(0)); assert_eq!(dir.group, Some(9)); assert_eq!(dir.unix_pex.unwrap(), (7, 7, 5)); diff --git a/src/filetransfer/scp_transfer.rs b/src/filetransfer/scp_transfer.rs index 5da59a8..ee4028e 100644 --- a/src/filetransfer/scp_transfer.rs +++ b/src/filetransfer/scp_transfer.rs @@ -68,7 +68,7 @@ impl ScpFileTransfer { /// ### parse_ls_output /// /// Parse a line of `ls -l` output and tokenize the output into a `FsEntry` - fn parse_ls_output(&self, path: &Path, line: &str) -> Result { + fn parse_ls_output(&mut self, path: &Path, line: &str) -> Result { // Prepare list regex // NOTE: about this damn regex lazy_static! { @@ -179,6 +179,14 @@ impl ScpFileTransfer { true => self.get_name_and_link(metadata.get(8).unwrap().as_str()), false => (String::from(metadata.get(8).unwrap().as_str()), None), }; + // Get symlink + let symlink: Option> = match symlink_path { + None => None, + Some(p) => match self.stat(p.as_path()) { + Ok(e) => Some(Box::new(e)), + Err(_) => None, // Ignore errors + } + }; // Check if file_name is '.' or '..' if file_name.as_str() == "." || file_name.as_str() == ".." { return Err(()); @@ -199,7 +207,7 @@ impl ScpFileTransfer { last_access_time: mtime, creation_time: mtime, readonly: false, - symlink: symlink_path, + symlink, user: uid, group: gid, unix_pex: Some(unix_pex), @@ -213,7 +221,7 @@ impl ScpFileTransfer { size: filesize, ftype: extension, readonly: false, - symlink: symlink_path, + symlink, user: uid, group: gid, unix_pex: Some(unix_pex), diff --git a/src/filetransfer/sftp_transfer.rs b/src/filetransfer/sftp_transfer.rs index 35611ab..1e9c806 100644 --- a/src/filetransfer/sftp_transfer.rs +++ b/src/filetransfer/sftp_transfer.rs @@ -121,7 +121,7 @@ impl SftpFileTransfer { /// ### make_fsentry /// /// Make fsentry from path and metadata - fn make_fsentry(&self, path: &Path, metadata: &FileStat) -> FsEntry { + fn make_fsentry(&mut self, path: &Path, metadata: &FileStat) -> FsEntry { // Get common parameters let file_name: String = String::from(path.file_name().unwrap().to_str().unwrap_or("")); let file_type: Option = match path.extension() { @@ -149,11 +149,14 @@ impl SftpFileTransfer { .unwrap_or(SystemTime::UNIX_EPOCH); // Check if symlink let is_symlink: bool = metadata.file_type().is_symlink(); - let symlink: Option = match is_symlink { + let symlink: Option> = match is_symlink { true => { // Read symlink match self.sftp.as_ref().unwrap().readlink(path) { - Ok(p) => Some(p), + Ok(p) => match self.stat(p.as_path()) { + Ok(entry) => Some(Box::new(entry)), + Err(_) => None, // Ignore errors + }, Err(_) => None, } } diff --git a/src/fs/mod.rs b/src/fs/mod.rs index 5b20f84..b4e3e8c 100644 --- a/src/fs/mod.rs +++ b/src/fs/mod.rs @@ -57,7 +57,7 @@ pub struct FsDirectory { pub last_access_time: SystemTime, pub creation_time: SystemTime, pub readonly: bool, - pub symlink: Option, // UNIX only + pub symlink: Option>, // UNIX only pub user: Option, // UNIX only pub group: Option, // UNIX only pub unix_pex: Option<(u8, u8, u8)>, // UNIX only @@ -77,12 +77,48 @@ pub struct FsFile { pub size: usize, pub ftype: Option, // File type pub readonly: bool, - pub symlink: Option, // UNIX only + pub symlink: Option>, // UNIX only pub user: Option, // UNIX only pub group: Option, // UNIX only pub unix_pex: Option<(u8, u8, u8)>, // UNIX only } +impl FsEntry { + /// ### get_abs_path + /// + /// Get absolute path from `FsEntry` + pub fn get_abs_path(&self) -> PathBuf { + match self { + FsEntry::Directory(dir) => dir.abs_path.clone(), + FsEntry::File(file) => file.abs_path.clone(), + } + } + + /// ### get_realfile + /// + /// Return the real file pointed by a `FsEntry` + pub fn get_realfile(&self) -> Option { + match self { + FsEntry::Directory(dir) => match &dir.symlink { + Some(symlink) => match symlink.get_realfile() { + // Recursive call + Some(e) => e.get_realfile(), // Recursive call + None => Some(*symlink.clone()), + }, + None => None, + }, + FsEntry::File(file) => match &file.symlink { + Some(symlink) => match symlink.get_realfile() { + // Recursive call + Some(e) => e.get_realfile(), // Recursive call + None => Some(*symlink.clone()), + }, + None => None, + }, + } + } +} + impl std::fmt::Display for FsEntry { /// ### fmt_ls /// @@ -279,3 +315,5 @@ impl std::fmt::Display for FsEntry { } } } + +// TODO: tests diff --git a/src/host/mod.rs b/src/host/mod.rs index 03391ca..15ffad0 100644 --- a/src/host/mod.rs +++ b/src/host/mod.rs @@ -281,7 +281,10 @@ impl Localhost { creation_time: attr.created().unwrap_or(SystemTime::UNIX_EPOCH), readonly: attr.permissions().readonly(), symlink: match fs::read_link(path) { - Ok(p) => Some(p), + Ok(p) => match self.stat(p.as_path()) { + Ok(entry) => Some(Box::new(entry)), + Err(_) => None, + }, Err(_) => None, }, user: Some(attr.uid()), @@ -304,8 +307,11 @@ impl Localhost { size: attr.len() as usize, ftype: extension, symlink: match fs::read_link(path) { - Ok(p) => Some(p), - Err(_) => None, + Ok(p) => match self.stat(p.as_path()) { + Ok(entry) => Some(Box::new(entry)), + Err(_) => None, + }, + Err(_) => None, // Ignore errors }, user: Some(attr.uid()), group: Some(attr.gid()), @@ -336,7 +342,10 @@ impl Localhost { creation_time: attr.created().unwrap_or(SystemTime::UNIX_EPOCH), readonly: attr.permissions().readonly(), symlink: match fs::read_link(path) { - Ok(p) => Some(p), + Ok(p) => match self.stat(p.as_path()) { + Ok(entry) => Some(Box::new(entry)), + Err(_) => None, // Ignore errors + }, Err(_) => None, }, user: None, @@ -359,7 +368,10 @@ impl Localhost { size: attr.len() as usize, ftype: extension, symlink: match fs::read_link(path) { - Ok(p) => Some(p), + Ok(p) => match self.stat(p.as_path()) { + Ok(entry) => Some(Box::new(entry)), + Err(_) => None, + }, Err(_) => None, }, user: None, @@ -618,7 +630,7 @@ mod tests { assert!(file_0.symlink.is_none()); } else { assert_eq!( - *file_0.symlink.as_ref().unwrap(), + *file_0.symlink.as_ref().unwrap().get_abs_path(), PathBuf::from(format!("{}/foo.txt", tmpdir.path().display())) ); } @@ -631,7 +643,7 @@ mod tests { FsEntry::File(file_1) => { if file_1.name == String::from("bar.txt") { assert_eq!( - *file_1.symlink.as_ref().unwrap(), + *file_1.symlink.as_ref().unwrap().get_abs_path(), PathBuf::from(format!("{}/foo.txt", tmpdir.path().display())) ); } else { diff --git a/src/ui/activities/filetransfer_activity/input.rs b/src/ui/activities/filetransfer_activity/input.rs index e574ebe..4bdd994 100644 --- a/src/ui/activities/filetransfer_activity/input.rs +++ b/src/ui/activities/filetransfer_activity/input.rs @@ -118,43 +118,10 @@ impl FileTransferActivity { } FsEntry::File(file) => { // Check if symlink - if let Some(realpath) = &file.symlink { - // Stat realpath - match self - .context - .as_ref() - .unwrap() - .local - .stat(realpath.as_path()) - { - Ok(real_file) => { - // If real file is a directory, enter directory - if let FsEntry::Directory(real_dir) = real_file { - self.local_changedir( - real_dir.abs_path.as_path(), - true, - ) - } - } - Err(err) => { - self.log( - LogLevel::Error, - format!( - "Failed to stat file \"{}\": {}", - realpath.display(), - err - ) - .as_ref(), - ); - self.input_mode = InputMode::Popup(PopupType::Alert( - Color::Red, - format!( - "Failed to stat file \"{}\": {}", - realpath.display(), - err - ), - )); - } + if let Some(symlink_entry) = &file.symlink { + // If symlink entry is a directory, go to directory + if let FsEntry::Directory(dir) = &**symlink_entry { + self.local_changedir(dir.abs_path.as_path(), true) } } } @@ -319,37 +286,10 @@ impl FileTransferActivity { } FsEntry::File(file) => { // Check if symlink - if let Some(realpath) = &file.symlink { - // Stat realpath - match self.client.stat(realpath.as_path()) { - Ok(real_file) => { - // If real file is a directory, enter directory - if let FsEntry::Directory(real_dir) = real_file { - self.remote_changedir( - real_dir.abs_path.as_path(), - true, - ) - } - } - Err(err) => { - self.log( - LogLevel::Error, - format!( - "Failed to stat file \"{}\": {}", - realpath.display(), - err - ) - .as_ref(), - ); - self.input_mode = InputMode::Popup(PopupType::Alert( - Color::Red, - format!( - "Failed to stat file \"{}\": {}", - realpath.display(), - err - ), - )); - } + if let Some(symlink_entry) = &file.symlink { + // If symlink entry is a directory, go to directory + if let FsEntry::Directory(dir) = &**symlink_entry { + self.remote_changedir(dir.abs_path.as_path(), true) } } } diff --git a/src/ui/activities/filetransfer_activity/layout.rs b/src/ui/activities/filetransfer_activity/layout.rs index 84b4594..397ca93 100644 --- a/src/ui/activities/filetransfer_activity/layout.rs +++ b/src/ui/activities/filetransfer_activity/layout.rs @@ -473,7 +473,16 @@ impl FileTransferActivity { Span::styled( match &dir.symlink { Some(symlink) => { - format!("{} => {}", dir.abs_path.display(), symlink.display()) + // Get symlink path + let symlink_path: &Path = match &**symlink { + FsEntry::Directory(s_dir) => s_dir.abs_path.as_path(), + FsEntry::File(s_file) => s_file.abs_path.as_path(), + }; + format!( + "{} => {}", + dir.abs_path.display(), + symlink_path.display() + ) } None => dir.abs_path.to_string_lossy().to_string(), }, @@ -560,7 +569,16 @@ impl FileTransferActivity { Span::styled( match &file.symlink { Some(symlink) => { - format!("{} => {}", file.abs_path.display(), symlink.display()) + // Get symlink path + let symlink_path: &Path = match &**symlink { + FsEntry::Directory(s_dir) => s_dir.abs_path.as_path(), + FsEntry::File(s_file) => s_file.abs_path.as_path(), + }; + format!( + "{} => {}", + file.abs_path.display(), + symlink_path.display() + ) } None => file.abs_path.to_string_lossy().to_string(), },