From a32f25fa95e3d2faee98a7168c3b2b4941660b27 Mon Sep 17 00:00:00 2001 From: Jeremy Soller Date: Wed, 13 Nov 2024 14:36:11 -0700 Subject: [PATCH] Remove fs_extra (#655) * WIP Remove fs_extra * Finish removing fs_extra --- .gitignore | 1 + Cargo.lock | 11 - Cargo.toml | 7 +- examples/copy.rs | 30 +++ scripts/copy.sh | 21 ++ src/lib.rs | 2 +- src/{operation.rs => operation/mod.rs} | 228 +++++--------------- src/operation/recursive.rs | 286 +++++++++++++++++++++++++ 8 files changed, 393 insertions(+), 193 deletions(-) create mode 100644 examples/copy.rs create mode 100755 scripts/copy.sh rename src/{operation.rs => operation/mod.rs} (83%) create mode 100644 src/operation/recursive.rs diff --git a/.gitignore b/.gitignore index 929a370..19f3d51 100644 --- a/.gitignore +++ b/.gitignore @@ -5,5 +5,6 @@ /debian/files /heaptrack.* /target/ +/test/ /vendor.tar /vendor/ diff --git a/Cargo.lock b/Cargo.lock index fbe3988..e5fcd76 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1259,7 +1259,6 @@ dependencies = [ "flate2", "fork", "freedesktop_entry_parser", - "fs_extra", "gio", "glib", "glob", @@ -2184,11 +2183,6 @@ dependencies = [ "thiserror", ] -[[package]] -name = "fs_extra" -version = "1.3.0" -source = "git+https://github.com/pop-os/fs_extra.git#7e7222eb2b7830d40b67cd02e6ebd156524ee866" - [[package]] name = "fsevent-sys" version = "4.1.0" @@ -2814,7 +2808,6 @@ dependencies = [ "bytes", "dnd", "glam", - "iced_accessibility", "log", "mime 0.1.0", "num-traits", @@ -2897,7 +2890,6 @@ source = "git+https://github.com/pop-os/libcosmic.git#e3fabf7d12e4a7d2662613ce11 dependencies = [ "bytes", "dnd", - "iced_accessibility", "iced_core", "iced_futures", "raw-window-handle", @@ -2959,7 +2951,6 @@ version = "0.14.0-dev" source = "git+https://github.com/pop-os/libcosmic.git#e3fabf7d12e4a7d2662613ce11e5f73f3191dd17" dependencies = [ "dnd", - "iced_accessibility", "iced_renderer", "iced_runtime", "num-traits", @@ -2978,7 +2969,6 @@ version = "0.14.0-dev" source = "git+https://github.com/pop-os/libcosmic.git#e3fabf7d12e4a7d2662613ce11e5f73f3191dd17" dependencies = [ "dnd", - "iced_accessibility", "iced_futures", "iced_graphics", "iced_runtime", @@ -3574,7 +3564,6 @@ dependencies = [ "freedesktop-desktop-entry", "freedesktop-icons", "iced", - "iced_accessibility", "iced_core", "iced_futures", "iced_renderer", diff --git a/Cargo.toml b/Cargo.toml index b4f305f..27e4eec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,6 @@ chrono = { version = "0.4", features = ["unstable-locales"] } dirs = "5.0.1" env_logger = "0.11" freedesktop_entry_parser = "1.3" -fs_extra = { git = "https://github.com/pop-os/fs_extra.git" } gio = { version = "0.20", optional = true } glib = { version = "0.20", optional = true } glob = "0.3" @@ -62,7 +61,8 @@ uzers = "0.12.0" [dependencies.libcosmic] git = "https://github.com/pop-os/libcosmic.git" default-features = false -features = ["a11y", "multi-window", "tokio", "winit"] +#TODO: a11y feature crashes +features = ["multi-window", "tokio", "winit"] [features] default = ["bzip2", "desktop", "gvfs", "liblzma", "notify", "wgpu"] @@ -96,9 +96,6 @@ filetime = { git = "https://github.com/jackpot51/filetime" } # [patch.'https://github.com/pop-os/cosmic-text'] # cosmic-text = { path = "../cosmic-text" } -# [patch.'https://github.com/pop-os/fs_extra'] -# fs_extra = { path = "../fs_extra" } - # [patch.'https://github.com/pop-os/libcosmic'] # libcosmic = { path = "../libcosmic" } # cosmic-config = { path = "../libcosmic/cosmic-config" } diff --git a/examples/copy.rs b/examples/copy.rs new file mode 100644 index 0000000..4deafde --- /dev/null +++ b/examples/copy.rs @@ -0,0 +1,30 @@ +use cosmic_files::operation::{recursive::Context, ReplaceResult}; +use std::{error::Error, io}; + +fn main() -> Result<(), Box> { + let mut context = Context::new() + .on_progress(|op, progress| { + println!("{:?}: {:?}", op.to, progress); + }) + .on_replace(|op| { + println!("replace {:?}? (y/N)", op.to); + let mut line = String::new(); + match io::stdin().read_line(&mut line) { + Ok(_) => { + if line == "y" { + ReplaceResult::Replace(false) + } else { + ReplaceResult::Skip(false) + } + } + Err(err) => { + eprintln!("failed to read stdin: {}", err); + ReplaceResult::Cancel + } + } + }); + + context.recursive_copy("test/a", "test/b")?; + context.recursive_move("test/b", "test/c")?; + Ok(()) +} diff --git a/scripts/copy.sh b/scripts/copy.sh new file mode 100755 index 0000000..b1d00b4 --- /dev/null +++ b/scripts/copy.sh @@ -0,0 +1,21 @@ +#!/usr/bin/env bash + +set -ex +cargo fmt +cargo build --release --example copy +rm -rf test +mkdir test +cp -a samples test/a +mkdir test/a/link +touch test/a/link/a +ln -s a test/a/link/b +mkdir test/a/perms +touch test/a/perms/400 +chmod 400 test/a/perms/400 +touch test/a/perms/600 +chmod 600 test/a/perms/600 +touch test/a/perms/700 +chmod 700 test/a/perms/700 +time target/release/examples/copy +ls -lR test +meld test/a test/c diff --git a/src/lib.rs b/src/lib.rs index 96f1f91..36defdf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -17,7 +17,7 @@ mod mime_app; pub mod mime_icon; mod mounter; mod mouse_area; -mod operation; +pub mod operation; mod spawn_detached; use tab::Location; pub mod tab; diff --git a/src/operation.rs b/src/operation/mod.rs similarity index 83% rename from src/operation.rs rename to src/operation/mod.rs index 4e03cbb..bbb91ac 100644 --- a/src/operation.rs +++ b/src/operation/mod.rs @@ -9,6 +9,7 @@ use std::{ use tokio::sync::{mpsc, Mutex}; use walkdir::WalkDir; +use self::recursive::Context; use crate::{ app::{ArchiveType, DialogPage, Message}, config::IconSizes, @@ -18,6 +19,8 @@ use crate::{ tab, }; +pub mod recursive; + fn handle_replace( msg_tx: &Arc>>, file_from: PathBuf, @@ -57,38 +60,6 @@ fn handle_replace( }) } -fn handle_progress_state( - msg_tx: &Arc>>, - progress: &fs_extra::TransitProcess, -) -> fs_extra::dir::TransitProcessResult { - log::warn!("{:?}", progress); - match progress.state { - fs_extra::dir::TransitState::Normal => fs_extra::dir::TransitProcessResult::ContinueOrAbort, - fs_extra::dir::TransitState::Exists => { - let Some(file_from) = progress.file_from.clone() else { - log::warn!("missing file_from in progress"); - return fs_extra::dir::TransitProcessResult::Abort; - }; - - let Some(file_to) = progress.file_to.clone() else { - log::warn!("missing file_to in progress"); - return fs_extra::dir::TransitProcessResult::Abort; - }; - - if file_from == file_to { - log::warn!("trying to copy {:?} to itself", file_from); - return fs_extra::dir::TransitProcessResult::Abort; - } - - handle_replace(msg_tx, file_from, file_to, true).into() - } - fs_extra::dir::TransitState::NoAccess => { - //TODO: permission error dialog - fs_extra::dir::TransitProcessResult::ContinueOrAbort - } - } -} - fn get_directory_name(file_name: &str) -> &str { const SUPPORTED_EXTENSIONS: [&str; 4] = [".tar.gz", ".tgz", ".tar", ".zip"]; @@ -108,32 +79,6 @@ pub enum ReplaceResult { Cancel, } -impl From for fs_extra::dir::TransitProcessResult { - fn from(f: ReplaceResult) -> fs_extra::dir::TransitProcessResult { - match f { - ReplaceResult::Replace(apply_to_all) => { - if apply_to_all { - fs_extra::dir::TransitProcessResult::OverwriteAll - } else { - fs_extra::dir::TransitProcessResult::Overwrite - } - } - ReplaceResult::KeepBoth => { - log::warn!("tried to keep both when replacing multiple files"); - fs_extra::dir::TransitProcessResult::Abort - } - ReplaceResult::Skip(apply_to_all) => { - if apply_to_all { - fs_extra::dir::TransitProcessResult::SkipAll - } else { - fs_extra::dir::TransitProcessResult::Skip - } - } - ReplaceResult::Cancel => fs_extra::dir::TransitProcessResult::Abort, - } - } -} - #[derive(Clone, Debug, Eq, Hash, PartialEq)] pub enum Operation { /// Compress files @@ -190,49 +135,52 @@ async fn copy_or_move( id: u64, msg_tx: &Arc>>, ) -> Result<(), String> { - // Handle duplicate file names by renaming paths - let (paths, to): (Vec<_>, Vec<_>) = tokio::task::spawn_blocking(move || { - paths - .into_iter() - .zip(std::iter::repeat(to.as_path())) - .map(|(from, to)| { - if matches!(from.parent(), Some(parent) if parent == to) && !moving { - // `from`'s parent is equal to `to` which means we're copying to the same - // directory (duplicating files) - let to = copy_unique_path(&from, &to); - (from, to) - } else if let Some(name) = (from.is_file() || moving) - .then(|| from.file_name()) - .flatten() - { - let to = to.join(name); - (from, to) - } else { - (from, to.to_owned()) - } - }) - .unzip() - }) - .await - .unwrap(); - let msg_tx = msg_tx.clone(); - tokio::task::spawn_blocking(move || -> fs_extra::error::Result<()> { + tokio::task::spawn_blocking(move || -> Result<(), String> { log::info!( "{} {:?} to {:?}", if moving { "Move" } else { "Copy" }, paths, to ); - let total_paths = paths.len(); - for (path_i, (from, mut to)) in paths.into_iter().zip(to.into_iter()).enumerate() { - let handler = |copied_bytes, total_bytes| { - let item_progress = if total_bytes == 0 { - 1.0 + + // Handle duplicate file names by renaming paths + let from_to_pairs: Vec<(PathBuf, PathBuf)> = paths + .into_iter() + .zip(std::iter::repeat(to.as_path())) + .filter_map(|(from, to)| { + if matches!(from.parent(), Some(parent) if parent == to) && !moving { + // `from`'s parent is equal to `to` which means we're copying to the same + // directory (duplicating files) + let to = copy_unique_path(&from, &to); + Some((from, to)) + } else if let Some(name) = from.file_name() { + let to = to.join(name); + Some((from, to)) } else { - copied_bytes as f32 / total_bytes as f32 + //TODO: how to handle from missing file name? + None + } + }) + .collect(); + + let mut context = Context::new(); + + { + let msg_tx = msg_tx.clone(); + context = context.on_progress(move |op, progress| { + let item_progress = match progress.total_bytes { + Some(total_bytes) => { + if total_bytes == 0 { + 1.0 + } else { + progress.current_bytes as f32 / total_bytes as f32 + } + } + None => 0.0, }; - let total_progress = (item_progress + path_i as f32) / total_paths as f32; + let total_progress = + (item_progress + progress.current_ops as f32) / progress.total_ops as f32; executor::block_on(async { let _ = msg_tx .lock() @@ -240,90 +188,18 @@ async fn copy_or_move( .send(Message::PendingProgress(id, 100.0 * total_progress)) .await; }) - }; - - if from == to { - log::info!( - "Skipping {} of {:?} to itself", - if moving { "move" } else { "copy" }, - from - ); - handler(0, 0); - continue; - } - - if from.is_dir() { - let options = fs_extra::dir::CopyOptions::default().copy_inside(true); - if moving { - fs_extra::move_items_with_progress( - &[from], - to, - &options, - |progress: fs_extra::TransitProcess| { - handler(progress.copied_bytes, progress.total_bytes); - handle_progress_state(&msg_tx, &progress) - }, - )?; - } else { - fs_extra::copy_items_with_progress( - &[from], - to, - &options, - |progress: fs_extra::TransitProcess| { - handler(progress.copied_bytes, progress.total_bytes); - handle_progress_state(&msg_tx, &progress) - }, - )?; - } - } else { - let mut options = fs_extra::file::CopyOptions::default(); - if to.exists() { - match handle_replace(&msg_tx, from.clone(), to.clone(), false) { - ReplaceResult::Replace(_) => { - options.overwrite = true; - } - ReplaceResult::KeepBoth => { - match to.parent() { - Some(to_parent) => { - to = copy_unique_path(&from, &to_parent); - } - None => { - log::warn!("failed to get parent of {:?}", to); - //TODO: error? - } - } - } - ReplaceResult::Skip(_) => { - options.skip_exist = true; - } - ReplaceResult::Cancel => { - //TODO: be silent, but collect actual changes made for undo - continue; - } - } - } - if moving { - //TODO: optimize to fs::rename when possible - fs_extra::file::move_file_with_progress( - from, - to, - &options, - |progress: fs_extra::file::TransitProcess| { - handler(progress.copied_bytes, progress.total_bytes); - }, - )?; - } else { - fs_extra::file::copy_with_progress( - from, - to, - &options, - |progress: fs_extra::file::TransitProcess| { - handler(progress.copied_bytes, progress.total_bytes); - }, - )?; - } - } + }); } + + { + let msg_tx = msg_tx.clone(); + context = context.on_replace(move |op| { + handle_replace(&msg_tx, op.from.clone(), op.to.clone(), true) + }); + } + + context.recursive_copy_or_move(from_to_pairs, moving)?; + Ok(()) }) .await @@ -578,7 +454,7 @@ impl Operation { let new_paths_it = WalkDir::new(path).into_iter(); for entry in new_paths_it.skip(1) { let entry = entry.map_err(err_str)?; - paths.push(entry.path().to_path_buf()); + paths.push(entry.into_path()); } } } diff --git a/src/operation/recursive.rs b/src/operation/recursive.rs new file mode 100644 index 0000000..2116a2e --- /dev/null +++ b/src/operation/recursive.rs @@ -0,0 +1,286 @@ +use std::{ + error::Error, + fs, + io::{Read, Write}, + ops::ControlFlow, + path::PathBuf, +}; +use walkdir::WalkDir; + +use super::{copy_unique_path, ReplaceResult}; + +pub struct Context { + buf: Vec, + on_progress: Box, + on_replace: Box ReplaceResult + 'static>, + replace_result_opt: Option, +} + +impl Context { + pub fn new() -> Self { + Self { + buf: vec![0; 4 * 1024 * 1024], + on_progress: Box::new(|_op, _progress| {}), + on_replace: Box::new(|_op| ReplaceResult::Cancel), + replace_result_opt: None, + } + } + + pub fn recursive_copy_or_move( + &mut self, + from_to_pairs: Vec<(PathBuf, PathBuf)>, + moving: bool, + ) -> Result { + let mut ops = Vec::new(); + let mut cleanup_ops = Vec::new(); + for (from_parent, to_parent) in from_to_pairs { + if from_parent == to_parent { + // Skip matching source and destination + continue; + } + + for entry in WalkDir::new(&from_parent).into_iter() { + let entry = entry.map_err(|err| { + format!("failed to walk directory {:?}: {}", from_parent, err) + })?; + let file_type = entry.file_type(); + let from = entry.into_path(); + let kind = if file_type.is_dir() { + OpKind::Mkdir + } else if file_type.is_file() { + if moving { + OpKind::Move + } else { + OpKind::Copy + } + } else if file_type.is_symlink() { + let target = fs::read_link(&from) + .map_err(|err| format!("failed to read link {:?}: {}", from, err))?; + OpKind::Symlink { target } + } else { + //TODO: present dialog and allow continue + return Err(format!("{} is not a known file type", from.display()).into()); + }; + let to = if from == from_parent { + // When copying a file, from matches from_parent, and to_parent must be used + to_parent.clone() + } else { + let relative = from.strip_prefix(&from_parent).map_err(|err| { + format!( + "failed to remove prefix {:?} from {:?}: {}", + from_parent, from, err + ) + })?; + //TODO: ensure to is inside of to_parent? + to_parent.join(relative) + }; + let op = Op { kind, from, to }; + if moving { + if let Some(cleanup_op) = op.move_cleanup_op() { + cleanup_ops.push(cleanup_op); + } + } + ops.push(op); + } + } + + // Add cleanup ops after standard ops, in reverse + for cleanup_op in cleanup_ops.into_iter().rev() { + ops.push(cleanup_op); + } + + let total_ops = ops.len(); + for (current_ops, mut op) in ops.into_iter().enumerate() { + let progress = Progress { + current_ops, + total_ops, + current_bytes: 0, + total_bytes: None, + }; + (self.on_progress)(&op, &progress); + if !op.run(self, progress).map_err(|err| { + format!( + "failed to {:?} {:?} to {:?}: {}", + op.kind, op.from, op.to, err + ) + })? { + return Ok(false); + } + } + + Ok(true) + } + + pub fn on_progress(mut self, f: F) -> Self { + self.on_progress = Box::new(f); + self + } + + pub fn on_replace ReplaceResult + 'static>(mut self, f: F) -> Self { + self.on_replace = Box::new(f); + self + } + + fn replace(&mut self, op: &Op) -> Result, Box> { + let replace_result = self + .replace_result_opt + .unwrap_or_else(|| (self.on_replace)(op)); + match replace_result { + ReplaceResult::Replace(apply_to_all) => { + if apply_to_all { + self.replace_result_opt = Some(replace_result); + } + fs::remove_file(&op.to)?; + Ok(ControlFlow::Continue(op.to.clone())) + } + ReplaceResult::KeepBoth => match op.to.parent() { + Some(to_parent) => Ok(ControlFlow::Continue(copy_unique_path( + &op.from, &to_parent, + ))), + None => Err(format!("failed to get parent of {:?}", op.to).into()), + }, + ReplaceResult::Skip(apply_to_all) => { + if apply_to_all { + self.replace_result_opt = Some(replace_result); + } + Ok(ControlFlow::Break(true)) + } + ReplaceResult::Cancel => Ok(ControlFlow::Break(false)), + } + } +} + +#[derive(Debug)] +pub struct Progress { + pub current_ops: usize, + pub total_ops: usize, + pub current_bytes: u64, + pub total_bytes: Option, +} + +#[derive(Debug)] +pub enum OpKind { + Copy, + Move, + Mkdir, + Remove, + Rmdir, + Symlink { target: PathBuf }, +} + +#[derive(Debug)] +pub struct Op { + pub kind: OpKind, + pub from: PathBuf, + pub to: PathBuf, +} + +impl Op { + fn move_cleanup_op(&self) -> Option { + let kind = match self.kind { + OpKind::Copy | OpKind::Move | OpKind::Symlink { .. } => OpKind::Remove, + OpKind::Mkdir => OpKind::Rmdir, + OpKind::Remove | OpKind::Rmdir => return None, + }; + Some(Self { + kind, + from: self.from.clone(), + //TODO: it is strange to have `to` here + to: self.to.clone(), + }) + } + + fn run(&mut self, ctx: &mut Context, mut progress: Progress) -> Result> { + match self.kind { + OpKind::Copy => { + let mut from_file = fs::OpenOptions::new().read(true).open(&self.from)?; + let metadata = from_file.metadata()?; + // Remove `to` if overwriting and it is an existing file + if self.to.is_file() { + match ctx.replace(&self)? { + ControlFlow::Continue(to) => { + self.to = to; + } + ControlFlow::Break(ret) => { + return Ok(ret); + } + } + } + progress.total_bytes = Some(metadata.len()); + (ctx.on_progress)(&self, &progress); + // This is atomic and ensures `to` is not created by any other process + let mut to_file = fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(&self.to)?; + to_file.set_permissions(metadata.permissions())?; + loop { + let count = from_file.read(&mut ctx.buf)?; + if count == 0 { + break; + } + to_file.write_all(&ctx.buf[..count])?; + progress.current_bytes += count as u64; + (ctx.on_progress)(&self, &progress); + } + to_file.sync_all()?; + } + OpKind::Move => { + // Remove `to` if overwriting and it is an existing file + if self.to.is_file() { + match ctx.replace(&self)? { + ControlFlow::Continue(to) => { + self.to = to; + } + ControlFlow::Break(ret) => { + return Ok(ret); + } + } + } + // This is atomic and ensures `to` is not created by any other process + match fs::hard_link(&self.from, &self.to) { + Ok(()) => {} + Err(err) => { + //TODO: what is the error code on Windows? + if err.raw_os_error() == Some(libc::EXDEV) { + // Try standard copy if hard link fails with cross device error + let mut copy_op = Op { + kind: OpKind::Copy, + from: self.from.clone(), + to: self.to.clone(), + }; + copy_op.run(ctx, progress)?; + } else { + return Err(err.into()); + } + } + } + } + OpKind::Mkdir => { + fs::create_dir_all(&self.to)?; + } + OpKind::Remove => { + fs::remove_file(&self.from)?; + } + OpKind::Rmdir => { + fs::remove_dir(&self.from)?; + } + OpKind::Symlink { ref target } => { + // Remove `to` if overwriting and it is an existing file + if self.to.is_file() { + match ctx.replace(&self)? { + ControlFlow::Continue(to) => { + self.to = to; + } + ControlFlow::Break(ret) => { + return Ok(ret); + } + } + } + //TODO: use OS-specific function + fs::soft_link(&target, &self.to)?; + } + } + Ok(true) + } +}