Remove fs_extra (#655)

* WIP Remove fs_extra

* Finish removing fs_extra

.gitignore

@@ -5,5 +5,6 @@
 /debian/files
 /heaptrack.*
 /target/
+/test/
 /vendor.tar
 /vendor/

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",

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" }

examples/copy.rs

@@ -0,0 +1,30 @@
+use cosmic_files::operation::{recursive::Context, ReplaceResult};
+use std::{error::Error, io};
+
+fn main() -> Result<(), Box<dyn Error>> {
+    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(())
+}

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

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;

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<Mutex<Sender<Message>>>,
     file_from: PathBuf,
@@ -57,38 +60,6 @@ fn handle_replace(
     })
 }
 
-fn handle_progress_state(
-    msg_tx: &Arc<Mutex<Sender<Message>>>,
-    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<ReplaceResult> 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<Mutex<Sender<Message>>>,
 ) -> 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() {
+        // Handle duplicate file names by renaming paths
-            let handler = |copied_bytes, total_bytes| {
+        let from_to_pairs: Vec<(PathBuf, PathBuf)> = paths
-                let item_progress = if total_bytes == 0 {
+            .into_iter()
-                    1.0
+            .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());
                             }
                         }
                     }

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<u8>,
+    on_progress: Box<dyn Fn(&Op, &Progress) + 'static>,
+    on_replace: Box<dyn Fn(&Op) -> ReplaceResult + 'static>,
+    replace_result_opt: Option<ReplaceResult>,
+}
+
+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<bool, String> {
+        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<F: Fn(&Op, &Progress) + 'static>(mut self, f: F) -> Self {
+        self.on_progress = Box::new(f);
+        self
+    }
+
+    pub fn on_replace<F: Fn(&Op) -> ReplaceResult + 'static>(mut self, f: F) -> Self {
+        self.on_replace = Box::new(f);
+        self
+    }
+
+    fn replace(&mut self, op: &Op) -> Result<ControlFlow<bool, PathBuf>, Box<dyn Error>> {
+        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<u64>,
+}
+
+#[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<Self> {
+        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<bool, Box<dyn Error>> {
+        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)
+    }
+}