feat(pull,push): match ares-cli path rules

ares-pull/README.md

@@ -17,13 +17,42 @@ Arguments:
 
 Options:
   -d, --device <DEVICE>  Specify DEVICE to use [env: ARES_DEVICE=]
-  -i, --ignore           Continue on errors instead of stopping at the first failure
+  -i, --ignore           Hide the detailed copy messages
+  -k, --keep-going       Continue on errors instead of stopping at the first failure
   -h, --help             Print help
 ```
 
+## Where files land
+
+The layout comes from what SOURCE is on the device and what DESTINATION already
+is on your computer, the same way `@webosose/ares-cli` decides it. A trailing
+`/` changes nothing.
+
+| SOURCE on the device | DESTINATION on the host | Result                |
+|----------------------|-------------------------|-----------------------|
+| a directory          | anything but a file     | `DESTINATION/<name>`  |
+| a directory          | a file                  | error                 |
+| a file               | a directory             | `DESTINATION/<name>`  |
+| a file               | missing, or a file      | `DESTINATION`         |
+
+Missing parent directories are made along the way.
+
+A directory keeps its own name, so `ares-pull /var/log ./out` puts it at
+`./out/log`.
+
 ## Examples
 
 ```sh
 ares-pull -d tv /var/log/messages
 ares-pull -d tv /media/developer/apps ./backup
 ```
+
+## Differences from @webosose/ares-cli
+
+- `-k, --keep-going` skips a file that fails and goes on. The original always
+  stops at the first failure. The exit code is still non-zero.
+- Pulling a file to a path whose parent does not exist works. The original
+  fails with `ENOENT`.
+- Symlinks are followed, as in the original. A broken symlink is skipped with a
+  message, and nesting past 64 levels stops with an error instead of looping.
+- The copy runs over SFTP, so the device needs no `find` binary.

ares-pull/src/main.rs

@@ -7,7 +7,7 @@ use ares_connection_lib::session::NewSession;
 use ares_device_lib::DeviceManager;
 use ares_device_lib::cli::unwrap_or_exit;
 use clap::Parser;
-use libssh_rs::{FileType, OpenFlags, Sftp};
+use libssh_rs::{Error as SshError, FileType, OpenFlags, Sftp};
 
 #[derive(Parser, Debug)]
 #[command(about)]
@@ -20,12 +20,14 @@ struct Cli {
         help = "Specify DEVICE to use"
     )]
     device: Option<String>,
+    #[arg(short, long, help = "Hide the detailed copy messages")]
+    ignore: bool,
     #[arg(
         short,
         long,
         help = "Continue on errors instead of stopping at the first failure"
     )]
-    ignore: bool,
+    keep_going: bool,
     #[arg(
         value_name = "SOURCE",
         help = "Path on the DEVICE, where files exist",
@@ -40,6 +42,10 @@ struct Cli {
     destination: String,
 }
 
+/// Directory nesting we refuse to go past. Symlinks are followed, so a link
+/// that points at a parent would otherwise never end.
+const MAX_DEPTH: usize = 64;
+
 fn main() {
     let cli = Cli::parse();
     let manager = DeviceManager::default();
@@ -51,102 +57,249 @@ fn main() {
     let session = unwrap_or_exit(device.new_session(), &format!("connect to {}", device.name));
     let sftp = unwrap_or_exit(session.sftp(), "start SFTP");
 
-    let target = resolve_target(&cli.source, &cli.destination);
-    if let Err(e) = pull(&sftp, &cli.source, &target, cli.ignore) {
+    let mut pull = Pull {
+        sftp: &sftp,
+        quiet: cli.ignore,
+        keep_going: cli.keep_going,
+        failed: false,
+    };
+    if let Err(e) = pull.run(&cli.source, &cli.destination) {
         eprintln!("Failed to pull: {e}");
         exit(1);
     }
+    if pull.failed {
+        exit(1);
+    }
 }
 
-/// Maps the remote `source` onto a local destination path. Mirrors ares-push:
-/// a trailing "/" on the destination keeps the source's last path component,
-/// otherwise the source maps directly onto the destination.
-fn resolve_target(source: &str, destination: &str) -> PathBuf {
-    let source = Path::new(source);
-    let dest_base = Path::new(destination);
-    let source_prefix = if destination.ends_with('/') {
-        source.parent().unwrap_or(source)
-    } else {
-        source
-    };
-    match source.strip_prefix(source_prefix) {
-        Ok(relative) if !relative.as_os_str().is_empty() => dest_base.join(relative),
-        _ => dest_base.to_path_buf(),
-    }
+struct Pull<'a> {
+    sftp: &'a Sftp,
+    quiet: bool,
+    keep_going: bool,
+    /// Set when --keep-going swallowed a failure, so the exit code still says so.
+    failed: bool,
 }
 
-/// Recursively pulls `remote` into the local path `local`.
-fn pull(sftp: &Sftp, remote: &str, local: &Path, ignore: bool) -> Result<(), Error> {
-    let metadata = sftp.symlink_metadata(remote).map_err(to_io)?;
-    match metadata.file_type() {
-        Some(FileType::Symlink) => {
-            eprintln!("Skipping symlink {remote}");
-            Ok(())
+impl Pull<'_> {
+    fn run(&mut self, source: &str, destination: &str) -> Result<(), Error> {
+        let source_is_dir = is_dir(self.sftp, source)
+            .map_err(|e| Error::new(e.kind(), format!("SOURCE {source}: {e}")))?;
+        let target = resolve_target(
+            source,
+            destination,
+            source_is_dir,
+            Path::new(destination).is_dir(),
+        );
+        if source_is_dir && target.exists() && !target.is_dir() {
+            return Err(Error::new(
+                ErrorKind::AlreadyExists,
+                format!("{} is not a directory", target.display()),
+            ));
         }
-        Some(FileType::Directory) => pull_dir(sftp, remote, local, ignore),
-        _ => pull_file(sftp, remote, local),
+        self.copy(source, &target, 0)
     }
-}
 
-fn pull_dir(sftp: &Sftp, remote: &str, local: &Path, ignore: bool) -> Result<(), Error> {
-    create_dir_all(local)?;
-    println!("{remote} => {}", local.display());
-    for entry in sftp.read_dir(remote).map_err(to_io)? {
-        let Some(name) = entry.name() else { continue };
-        if name == "." || name == ".." {
-            continue;
+    fn copy(&mut self, remote: &str, local: &Path, depth: usize) -> Result<(), Error> {
+        let file_type = match self.sftp.metadata(remote).map(|m| m.file_type()) {
+            Ok(file_type) => file_type,
+            Err(e) => {
+                // ares-cli walks with `find -follow`, which lists a broken
+                // symlink as neither a file nor a directory and skips it.
+                if matches!(
+                    self.sftp.symlink_metadata(remote).map(|m| m.file_type()),
+                    Ok(Some(FileType::Symlink))
+                ) {
+                    eprintln!("Skipping {remote}: it is a broken symlink");
+                    return Ok(());
+                }
+                return Err(sftp_error(remote, &e));
+            }
+        };
+        if file_type == Some(FileType::Directory) {
+            if depth >= MAX_DEPTH {
+                return Err(Error::new(
+                    ErrorKind::InvalidData,
+                    format!("{remote} is nested too deep, which usually means a symlink loop"),
+                ));
+            }
+            self.copy_dir(remote, local, depth)
+        } else {
+            self.copy_file(remote, local)
         }
-        let child_remote = format!("{}/{name}", remote.trim_end_matches('/'));
-        let child_local = local.join(name);
-        if let Err(e) = pull(sftp, &child_remote, &child_local, ignore) {
-            if ignore {
-                eprintln!("Skipping {child_remote}: {e}");
-            } else {
-                return Err(e);
+    }
+
+    fn copy_dir(&mut self, remote: &str, local: &Path, depth: usize) -> Result<(), Error> {
+        let sftp = self.sftp;
+        create_dir_all(local)?;
+        self.report(remote, local);
+        for entry in sftp.read_dir(remote).map_err(|e| sftp_error(remote, &e))? {
+            let Some(name) = entry.name() else { continue };
+            if name == "." || name == ".." {
+                continue;
+            }
+            let child_remote = format!("{}/{name}", remote.trim_end_matches('/'));
+            let child_local = local.join(name);
+            if let Err(e) = self.copy(&child_remote, &child_local, depth + 1) {
+                self.item_failed(&child_remote, e)?;
             }
         }
+        Ok(())
+    }
+
+    fn copy_file(&mut self, remote: &str, local: &Path) -> Result<(), Error> {
+        if let Some(parent) = local.parent() {
+            create_dir_all(parent)?;
+        }
+        self.report(remote, local);
+        let mut remote_file = self
+            .sftp
+            .open(remote, OpenFlags::READ_ONLY, 0)
+            .map_err(|e| sftp_error(remote, &e))?;
+        let mut local_file = File::create(local)?;
+        std::io::copy(&mut remote_file, &mut local_file)?;
+        Ok(())
+    }
+
+    fn report(&self, remote: &str, local: &Path) {
+        if !self.quiet {
+            println!("{remote} => {}", local.display());
+        }
+    }
+
+    /// Handle a failure on one item. Returns the error to stop the whole copy,
+    /// or Ok to go on when --keep-going is set.
+    fn item_failed(&mut self, what: &str, e: Error) -> Result<(), Error> {
+        if !self.keep_going {
+            return Err(e);
+        }
+        eprintln!("Skipping {what}: {e}");
+        self.failed = true;
+        Ok(())
     }
-    Ok(())
 }
 
-fn pull_file(sftp: &Sftp, remote: &str, local: &Path) -> Result<(), Error> {
-    if let Some(parent) = local.parent() {
-        create_dir_all(parent)?;
+/// Where SOURCE lands on the host.
+///
+/// This follows ares-cli: a directory always keeps its own name under
+/// DESTINATION, and a file keeps its name only when DESTINATION already is a
+/// directory. A trailing "/" changes nothing.
+fn resolve_target(
+    source: &str,
+    destination: &str,
+    source_is_dir: bool,
+    dest_is_dir: bool,
+) -> PathBuf {
+    let dest = Path::new(destination);
+    if !source_is_dir && !dest_is_dir {
+        return dest.to_path_buf();
+    }
+    match remote_name(source) {
+        Some(name) => dest.join(name),
+        None => dest.to_path_buf(),
+    }
+}
+
+/// Last component of a device path, which always uses "/". Returns None for a
+/// path with no name of its own ("/", "." and ".."), where the copy goes
+/// straight into DESTINATION.
+fn remote_name(path: &str) -> Option<&str> {
+    let name = path.trim_end_matches('/').rsplit('/').next()?;
+    if name.is_empty() || name == "." || name == ".." {
+        return None;
+    }
+    Some(name)
+}
+
+/// True when `path` is a directory on the device. ares-cli tests with `[ -f ]`
+/// and `[ -d ]`, which both follow symlinks, so follow them here too.
+fn is_dir(sftp: &Sftp, path: &str) -> Result<bool, Error> {
+    let metadata = sftp.metadata(path).map_err(|e| sftp_error(path, &e))?;
+    Ok(metadata.file_type() == Some(FileType::Directory))
+}
+
+/// Recover the numeric SFTP status code from a libssh error. `SftpError`'s code
+/// field is private, so parse it out of the Display text ("Sftp error code N").
+/// Returns `None` for non-SFTP errors.
+fn sftp_status(e: &SshError) -> Option<u32> {
+    if !matches!(e, SshError::Sftp(_)) {
+        return None;
+    }
+    e.to_string().rsplit(' ').next()?.parse().ok()
+}
+
+/// Human-readable reason for an SFTP status code (subset of `SSH_FX_*` codes).
+fn sftp_reason(code: u32) -> &'static str {
+    match code {
+        2 => "no such file or directory",
+        3 => "permission denied",
+        4 => "failure",
+        8 => "operation not supported",
+        _ => "SFTP error",
     }
-    println!("{remote} => {}", local.display());
-    let mut remote_file = sftp.open(remote, OpenFlags::READ_ONLY, 0).map_err(to_io)?;
-    let mut local_file = File::create(local)?;
-    std::io::copy(&mut remote_file, &mut local_file)?;
-    Ok(())
 }
 
-fn to_io(error: libssh_rs::Error) -> Error {
-    Error::new(ErrorKind::Other, error.to_string())
+fn sftp_error(path: &str, e: &SshError) -> Error {
+    match sftp_status(e) {
+        Some(2) => Error::new(
+            ErrorKind::NotFound,
+            format!("{path} does not exist on the device"),
+        ),
+        Some(3) => Error::new(
+            ErrorKind::PermissionDenied,
+            format!("{path}: permission denied"),
+        ),
+        Some(code) => Error::other(format!("{path}: {} (SFTP code {code})", sftp_reason(code))),
+        None => Error::other(format!("{path}: {e}")),
+    }
 }
 
 #[cfg(test)]
 mod tests {
-    use super::resolve_target;
+    use super::{remote_name, resolve_target};
 
-    fn target(source: &str, destination: &str) -> String {
-        resolve_target(source, destination)
+    fn target(source: &str, destination: &str, source_is_dir: bool, dest_is_dir: bool) -> String {
+        resolve_target(source, destination, source_is_dir, dest_is_dir)
             .to_string_lossy()
             .replace('\\', "/")
     }
 
     #[test]
-    fn file_maps_onto_destination() {
-        assert_eq!(target("/remote/f.txt", "out.txt"), "out.txt");
+    fn a_file_takes_the_destination_name() {
+        assert_eq!(target("/remote/f.txt", "out.txt", false, false), "out.txt");
+    }
+
+    #[test]
+    fn a_file_keeps_its_name_under_a_directory() {
+        assert_eq!(target("/remote/f.txt", "out", false, true), "out/f.txt");
+    }
+
+    #[test]
+    fn a_directory_always_keeps_its_own_name() {
+        // The point of the ares-cli rule: "dir" lands as out/dir, whether or not
+        // "out" already exists.
+        assert_eq!(target("/remote/dir", "out", true, true), "out/dir");
+        assert_eq!(target("/remote/dir", "out", true, false), "out/dir");
+    }
+
+    #[test]
+    fn a_trailing_slash_changes_nothing() {
+        assert_eq!(target("/remote/dir/", "out", true, true), "out/dir");
+        assert_eq!(target("/remote/dir", "out/", true, true), "out/dir");
+        assert_eq!(target("/remote/f.txt", "out/", false, true), "out/f.txt");
     }
 
     #[test]
-    fn trailing_slash_keeps_last_component() {
-        assert_eq!(target("/remote/f.txt", "dir/"), "dir/f.txt");
-        assert_eq!(target("/remote/dir", "out/"), "out/dir");
+    fn a_source_with_no_name_copies_its_contents() {
+        assert_eq!(remote_name("/"), None);
+        assert_eq!(remote_name("."), None);
+        assert_eq!(remote_name(".."), None);
+        assert_eq!(target("/", "out", true, true), "out");
     }
 
     #[test]
-    fn directory_contents_map_into_destination() {
-        assert_eq!(target("/remote/dir", "out"), "out");
+    fn names_come_from_the_last_component() {
+        assert_eq!(remote_name("/var/log/messages"), Some("messages"));
+        assert_eq!(remote_name("/var/log/"), Some("log"));
+        assert_eq!(remote_name("messages"), Some("messages"));
     }
 }