Merge pull request #5152 from tertsdiepraam/cp-prepare-for-nu

`cp`: make more types public and add more documentation (for nushell)

src/uu/cp/src/copydir.rs

@@ -25,7 +25,7 @@ use walkdir::{DirEntry, WalkDir};
 
 use crate::{
     aligned_ancestors, context_for, copy_attributes, copy_file, copy_link, preserve_hardlinks,
-    CopyResult, Error, Options, TargetSlice,
+    CopyResult, Error, Options,
 };
 
 /// Ensure a Windows path starts with a `\\?`.
@@ -307,7 +307,7 @@ fn copy_direntry(
 pub(crate) fn copy_directory(
     progress_bar: &Option<ProgressBar>,
     root: &Path,
-    target: &TargetSlice,
+    target: &Path,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,
     source_in_command_line: bool,

src/uu/cp/src/cp.rs

@@ -9,10 +9,11 @@
 // For the full copyright and license information, please view the LICENSE file
 // that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) copydir ficlone fiemap ftruncate linkgs lstat nlink nlinks pathbuf pwrite reflink strs xattrs symlinked deduplicated advcpmv
+// spell-checker:ignore (ToDO) copydir ficlone fiemap ftruncate linkgs lstat nlink nlinks pathbuf pwrite reflink strs xattrs symlinked deduplicated advcpmv nushell
 
 use quick_error::quick_error;
 use std::borrow::Cow;
+use std::cmp::Ordering;
 use std::collections::HashSet;
 use std::env;
 #[cfg(not(windows))]
@@ -51,6 +52,7 @@ use crate::copydir::copy_directory;
 
 mod copydir;
 mod platform;
+
 quick_error! {
     #[derive(Debug)]
     pub enum Error {
@@ -108,12 +110,8 @@ impl UError for Error {
 }
 
 pub type CopyResult<T> = Result<T, Error>;
-pub type Source = PathBuf;
-pub type SourceSlice = Path;
-pub type Target = PathBuf;
-pub type TargetSlice = Path;
 
-/// Specifies whether when overwrite files
+/// Specifies how to overwrite files.
 #[derive(Clone, Copy, Eq, PartialEq)]
 pub enum ClobberMode {
     Force,
@@ -121,7 +119,7 @@ pub enum ClobberMode {
     Standard,
 }
 
-/// Specifies whether when overwrite files
+/// Specifies whether files should be overwritten.
 #[derive(Clone, Copy, Eq, PartialEq)]
 pub enum OverwriteMode {
     /// [Default] Always overwrite existing files
@@ -148,12 +146,13 @@ pub enum SparseMode {
     Never,
 }
 
-/// Specifies the expected file type of copy target
+/// The expected file type of copy target
 pub enum TargetType {
     Directory,
     File,
 }
 
+/// Copy action to perform
 pub enum CopyMode {
     Link,
     SymLink,
@@ -162,77 +161,128 @@ pub enum CopyMode {
     AttrOnly,
 }
 
+/// Preservation settings for various attributes
+///
+/// It should be derived from options as follows:
+///
+///  - if there is a list of attributes to preserve (i.e. `--preserve=ATTR_LIST`) parse that list with [`Attributes::parse_iter`],
+///  - if `-p` or `--preserve` is given without arguments, use [`Attributes::DEFAULT`],
+///  - if `-a`/`--archive` is passed, use [`Attributes::ALL`],
+///  - if `-d` is passed use [`Attributes::LINKS`],
+///  - otherwise, use [`Attributes::NONE`].
+///
+/// For full compatibility with GNU, these options should also combine. We
+/// currently only do a best effort imitation of that behavior, because it is
+/// difficult to achieve in clap, especially with `--no-preserve`.
 #[derive(Debug)]
 pub struct Attributes {
     #[cfg(unix)]
-    ownership: Preserve,
+    pub ownership: Preserve,
-    mode: Preserve,
+    pub mode: Preserve,
-    timestamps: Preserve,
+    pub timestamps: Preserve,
-    context: Preserve,
+    pub context: Preserve,
-    links: Preserve,
+    pub links: Preserve,
-    xattr: Preserve,
+    pub xattr: Preserve,
 }
 
-impl Attributes {
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-    pub(crate) fn max(&mut self, other: Self) {
-        #[cfg(unix)]
-        {
-            self.ownership = self.ownership.max(other.ownership);
-        }
-        self.mode = self.mode.max(other.mode);
-        self.timestamps = self.timestamps.max(other.timestamps);
-        self.context = self.context.max(other.context);
-        self.links = self.links.max(other.links);
-        self.xattr = self.xattr.max(other.xattr);
-    }
-}
-
-#[derive(Debug, PartialEq, Eq)]
 pub enum Preserve {
     No,
     Yes { required: bool },
 }
 
-impl Preserve {
+impl PartialOrd for Preserve {
-    /// Preservation level should only increase, with no preservation being the lowest option,
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-    /// preserve but don't require - middle, and preserve and require - top.
+        Some(self.cmp(other))
-    pub(crate) fn max(&self, other: Self) -> Self {
+    }
+}
+
+impl Ord for Preserve {
+    fn cmp(&self, other: &Self) -> Ordering {
         match (self, other) {
-            (Self::Yes { required: true }, _) | (_, Self::Yes { required: true }) => {
+            (Self::No, Self::No) => Ordering::Equal,
-                Self::Yes { required: true }
+            (Self::Yes { .. }, Self::No) => Ordering::Greater,
-            }
+            (Self::No, Self::Yes { .. }) => Ordering::Less,
-            (Self::Yes { required: false }, _) | (_, Self::Yes { required: false }) => {
+            (
-                Self::Yes { required: false }
+                Self::Yes { required: req_self },
-            }
+                Self::Yes {
-            _ => Self::No,
+                    required: req_other,
+                },
+            ) => req_self.cmp(req_other),
         }
     }
 }
 
-/// Re-usable, extensible copy options
+/// Options for the `cp` command
+///
+/// All options are public so that the options can be programmatically
+/// constructed by other crates, such as nushell. That means that this struct
+/// is part of our public API. It should therefore not be changed without good
+/// reason.
+///
+/// The fields are documented with the arguments that determine their value.
 #[allow(dead_code)]
 pub struct Options {
-    attributes_only: bool,
+    /// `--attributes-only`
-    backup: BackupMode,
+    pub attributes_only: bool,
-    copy_contents: bool,
+    /// `--backup[=CONTROL]`, `-b`
-    cli_dereference: bool,
+    pub backup: BackupMode,
-    copy_mode: CopyMode,
+    /// `--copy-contents`
-    dereference: bool,
+    pub copy_contents: bool,
-    no_target_dir: bool,
+    /// `-H`
-    one_file_system: bool,
+    pub cli_dereference: bool,
-    overwrite: OverwriteMode,
+    /// Determines the type of copying that should be done
-    parents: bool,
+    ///
-    sparse_mode: SparseMode,
+    /// Set by the following arguments:
-    strip_trailing_slashes: bool,
+    ///  - `-l`, `--link`: [`CopyMode::Link`]
-    reflink_mode: ReflinkMode,
+    ///  - `-s`, `--symbolic-link`: [`CopyMode::SymLink`]
-    attributes: Attributes,
+    ///  - `-u`, `--update[=WHEN]`: [`CopyMode::Update`]
-    recursive: bool,
+    ///  - `--attributes-only`: [`CopyMode::AttrOnly`]
-    backup_suffix: String,
+    ///  - otherwise: [`CopyMode::Copy`]
-    target_dir: Option<PathBuf>,
+    pub copy_mode: CopyMode,
-    update: UpdateMode,
+    /// `-L`, `--dereference`
-    debug: bool,
+    pub dereference: bool,
-    verbose: bool,
+    /// `-T`, `--no-target-dir`
-    progress_bar: bool,
+    pub no_target_dir: bool,
+    /// `-x`, `--one-file-system`
+    pub one_file_system: bool,
+    /// Specifies what to do with an existing destination
+    ///
+    /// Set by the following arguments:
+    ///  - `-i`, `--interactive`: [`OverwriteMode::Interactive`]
+    ///  - `-n`, `--no-clobber`: [`OverwriteMode::NoClobber`]
+    ///  - otherwise: [`OverwriteMode::Clobber`]
+    ///
+    /// The `Interactive` and `Clobber` variants have a [`ClobberMode`] argument,
+    /// set by the following arguments:
+    ///  - `-f`, `--force`: [`ClobberMode::Force`]
+    ///  - `--remove-destination`: [`ClobberMode::RemoveDestination`]
+    ///  - otherwise: [`ClobberMode::Standard`]
+    pub overwrite: OverwriteMode,
+    /// `--parents`
+    pub parents: bool,
+    /// `--sparse[=WHEN]`
+    pub sparse_mode: SparseMode,
+    /// `--strip-trailing-slashes`
+    pub strip_trailing_slashes: bool,
+    /// `--reflink[=WHEN]`
+    pub reflink_mode: ReflinkMode,
+    /// `--preserve=[=ATTRIBUTE_LIST]` and `--no-preserve=ATTRIBUTE_LIST`
+    pub attributes: Attributes,
+    /// `-R`, `-r`, `--recursive`
+    pub recursive: bool,
+    /// `-S`, `--suffix`
+    pub backup_suffix: String,
+    /// `-t`, `--target-directory`
+    pub target_dir: Option<PathBuf>,
+    /// `--update[=UPDATE]`
+    pub update: UpdateMode,
+    /// `--debug`
+    pub debug: bool,
+    /// `-v`, `--verbose`
+    pub verbose: bool,
+    /// `-g`, `--progress`
+    pub progress_bar: bool,
 }
 
 /// Enum representing various debug states of the offload and reflink actions.
@@ -741,67 +791,90 @@ impl CopyMode {
 }
 
 impl Attributes {
+    pub const ALL: Self = Self {
+        #[cfg(unix)]
+        ownership: Preserve::Yes { required: true },
+        mode: Preserve::Yes { required: true },
+        timestamps: Preserve::Yes { required: true },
+        context: {
+            #[cfg(feature = "feat_selinux")]
+            {
+                Preserve::Yes { required: false }
+            }
+            #[cfg(not(feature = "feat_selinux"))]
+            {
+                Preserve::No
+            }
+        },
+        links: Preserve::Yes { required: true },
+        xattr: Preserve::Yes { required: false },
+    };
+
+    pub const NONE: Self = Self {
+        #[cfg(unix)]
+        ownership: Preserve::No,
+        mode: Preserve::No,
+        timestamps: Preserve::No,
+        context: Preserve::No,
+        links: Preserve::No,
+        xattr: Preserve::No,
+    };
+
     // TODO: ownership is required if the user is root, for non-root users it's not required.
-    // See: https://github.com/coreutils/coreutils/blob/master/src/copy.c#L3181
+    pub const DEFAULT: Self = Self {
+        #[cfg(unix)]
+        ownership: Preserve::Yes { required: true },
+        mode: Preserve::Yes { required: true },
+        timestamps: Preserve::Yes { required: true },
+        ..Self::NONE
+    };
 
-    fn all() -> Self {
+    pub const LINKS: Self = Self {
+        links: Preserve::Yes { required: true },
+        ..Self::NONE
+    };
+
+    pub fn union(self, other: &Self) -> Self {
         Self {
             #[cfg(unix)]
-            ownership: Preserve::Yes { required: true },
+            ownership: self.ownership.max(other.ownership),
-            mode: Preserve::Yes { required: true },
+            context: self.context.max(other.context),
-            timestamps: Preserve::Yes { required: true },
+            timestamps: self.timestamps.max(other.timestamps),
-            context: {
+            mode: self.mode.max(other.mode),
-                #[cfg(feature = "feat_selinux")]
+            links: self.links.max(other.links),
-                {
+            xattr: self.xattr.max(other.xattr),
-                    Preserve::Yes { required: false }
-                }
-                #[cfg(not(feature = "feat_selinux"))]
-                {
-                    Preserve::No
-                }
-            },
-            links: Preserve::Yes { required: true },
-            xattr: Preserve::Yes { required: false },
         }
     }
 
-    fn default() -> Self {
+    pub fn parse_iter<T>(values: impl Iterator<Item = T>) -> Result<Self, Error>
-        Self {
+    where
-            #[cfg(unix)]
+        T: AsRef<str>,
-            ownership: Preserve::Yes { required: true },
+    {
-            mode: Preserve::Yes { required: true },
+        let mut new = Self::NONE;
-            timestamps: Preserve::Yes { required: true },
+        for value in values {
-            context: Preserve::No,
+            new = new.union(&Self::parse_single_string(value.as_ref())?);
-            links: Preserve::No,
-            xattr: Preserve::No,
-        }
-    }
-
-    fn none() -> Self {
-        Self {
-            #[cfg(unix)]
-            ownership: Preserve::No,
-            mode: Preserve::No,
-            timestamps: Preserve::No,
-            context: Preserve::No,
-            links: Preserve::No,
-            xattr: Preserve::No,
         }
+        Ok(new)
     }
 
     /// Tries to match string containing a parameter to preserve with the corresponding entry in the
     /// Attributes struct.
-    fn try_set_from_string(&mut self, value: &str) -> Result<(), Error> {
+    fn parse_single_string(value: &str) -> Result<Self, Error> {
-        let preserve_yes_required = Preserve::Yes { required: true };
+        let value = value.to_lowercase();
 
-        match &*value.to_lowercase() {
+        if value == "all" {
-            "mode" => self.mode = preserve_yes_required,
+            return Ok(Self::ALL);
+        }
+
+        let mut new = Self::NONE;
+        let attribute = match value.as_ref() {
+            "mode" => &mut new.mode,
             #[cfg(unix)]
-            "ownership" => self.ownership = preserve_yes_required,
+            "ownership" => &mut new.ownership,
-            "timestamps" => self.timestamps = preserve_yes_required,
+            "timestamps" => &mut new.timestamps,
-            "context" => self.context = preserve_yes_required,
+            "context" => &mut new.context,
-            "link" | "links" => self.links = preserve_yes_required,
+            "link" | "links" => &mut new.links,
-            "xattr" => self.xattr = preserve_yes_required,
+            "xattr" => &mut new.xattr,
             _ => {
                 return Err(Error::InvalidArgument(format!(
                     "invalid attribute {}",
@@ -809,7 +882,10 @@ impl Attributes {
                 )));
             }
         };
-        Ok(())
+
+        *attribute = Preserve::Yes { required: true };
+
+        Ok(new)
     }
 }
 
@@ -858,39 +934,22 @@ impl Options {
         };
 
         // Parse attributes to preserve
-        let attributes: Attributes = if matches.contains_id(options::PRESERVE) {
+        let attributes = if let Some(attribute_strs) = matches.get_many::<String>(options::PRESERVE)
-            match matches.get_many::<String>(options::PRESERVE) {
+        {
-                None => Attributes::default(),
+            if attribute_strs.len() == 0 {
-                Some(attribute_strs) => {
+                Attributes::DEFAULT
-                    let mut attributes: Attributes = Attributes::none();
+            } else {
-                    let mut attributes_empty = true;
+                Attributes::parse_iter(attribute_strs)?
-                    for attribute_str in attribute_strs {
-                        attributes_empty = false;
-                        if attribute_str == "all" {
-                            attributes.max(Attributes::all());
-                        } else {
-                            attributes.try_set_from_string(attribute_str)?;
-                        }
-                    }
-                    // `--preserve` case, use the defaults
-                    if attributes_empty {
-                        Attributes::default()
-                    } else {
-                        attributes
-                    }
-                }
             }
         } else if matches.get_flag(options::ARCHIVE) {
             // --archive is used. Same as --preserve=all
-            Attributes::all()
+            Attributes::ALL
         } else if matches.get_flag(options::NO_DEREFERENCE_PRESERVE_LINKS) {
-            let mut attributes = Attributes::none();
+            Attributes::LINKS
-            attributes.links = Preserve::Yes { required: true };
-            attributes
         } else if matches.get_flag(options::PRESERVE_DEFAULT_ATTRIBUTES) {
-            Attributes::default()
+            Attributes::DEFAULT
         } else {
-            Attributes::none()
+            Attributes::NONE
         };
 
         #[cfg(not(feature = "feat_selinux"))]
@@ -1000,7 +1059,7 @@ impl TargetType {
     ///
     /// Treat target as a dir if we have multiple sources or the target
     /// exists and already is a directory
-    fn determine(sources: &[Source], target: &TargetSlice) -> Self {
+    fn determine(sources: &[PathBuf], target: &Path) -> Self {
         if sources.len() > 1 || target.is_dir() {
             Self::Directory
         } else {
@@ -1010,7 +1069,10 @@ impl TargetType {
 }
 
 /// Returns tuple of (Source paths, Target)
-fn parse_path_args(mut paths: Vec<Source>, options: &Options) -> CopyResult<(Vec<Source>, Target)> {
+fn parse_path_args(
+    mut paths: Vec<PathBuf>,
+    options: &Options,
+) -> CopyResult<(Vec<PathBuf>, PathBuf)> {
     if paths.is_empty() {
         // No files specified
         return Err("missing file operand".into());
@@ -1122,14 +1184,13 @@ fn show_error_if_needed(error: &Error) {
     }
 }
 
-/// Copy all `sources` to `target`.  Returns an
+/// Copy all `sources` to `target`.
-/// `Err(Error::NotAllFilesCopied)` if at least one non-fatal error was
-/// encountered.
 ///
-/// Behavior depends on path`options`, see [`Options`] for details.
+/// Returns an `Err(Error::NotAllFilesCopied)` if at least one non-fatal error
+/// was encountered.
 ///
-/// [`Options`]: ./struct.Options.html
+/// Behavior is determined by the `options` parameter, see [`Options`] for details.
-fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResult<()> {
+pub fn copy(sources: &[PathBuf], target: &Path, options: &Options) -> CopyResult<()> {
     let target_type = TargetType::determine(sources, target);
     verify_target_type(target, &target_type)?;
 
@@ -1192,7 +1253,7 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu
 
 fn construct_dest_path(
     source_path: &Path,
-    target: &TargetSlice,
+    target: &Path,
     target_type: &TargetType,
     options: &Options,
 ) -> CopyResult<PathBuf> {
@@ -1223,8 +1284,8 @@ fn construct_dest_path(
 
 fn copy_source(
     progress_bar: &Option<ProgressBar>,
-    source: &SourceSlice,
+    source: &Path,
-    target: &TargetSlice,
+    target: &Path,
     target_type: &TargetType,
     options: &Options,
     symlinked_files: &mut HashSet<FileInformation>,