cp: require preserve only certain attributes (#4099)

* cp: require preserve only certain attributes

# Conflicts:
#	src/uu/cp/src/copydir.rs
#	src/uu/cp/src/cp.rs

* tests/cp: preserve all and preserve xattr tests with todos

* tests/cp: rename preserve tests

* tests/cp: add android fail test for preserve=xattr

On Android, this cp with explicit preserve of xattr must fail, because of the limitations of the filesystem setup used on Android.

* cp: verify some metadata in cp preserve tests

# Conflicts:
#	tests/by-util/test_cp.rs

* cp: run test_cp_preserve_all in all OS's but only check metadata on linux

* test/cp: don't expect the mode to change in explicit cp preserve

* cp: attributes struct instead of enum for unified required tracking

* cp: refactor preserver and handle_preserve

# Conflicts:
#	src/uu/cp/src/cp.rs

* cp: update preserve attr to max

* test/cp: fix the preserve xattr test

Access timestamps appear to be modified only in this test. Running the command directly does not alter the access timestamp.

* cp/test: preserve all and context case

* cp: fix preserve args value source

* test/cp: don't check mtime on freebsd

* test/cp: don't check mtime on macos

* test/cp: fix freebsd deps

* test/cp: support freebsd tests

* cp: simplify try_set_from_string

* cp: parse context attr in preserve in any case to show warning later

* cp: print warnings for attribute errors if not required

* cp: show SELinux warning only once

* cp: show SELinux warning without error

* Revert "cp: show SELinux warning without error"

This reverts commit d130cf0d8c8e28ac2c903413992613241decf879.

* cp: add documentation for preserve components

* cp: simplify try_set_from_string

* cp: EN_US "behavior" spelling for cspell

src/uu/cp/src/copydir.rs

@@ -405,7 +405,7 @@ pub(crate) fn copy_directory(
         }
     }
     // Copy the attributes from the root directory to the target directory.
-    copy_attributes(root, target, &options.preserve_attributes)?;
+    copy_attributes(root, target, &options.attributes)?;
     Ok(())
 }
 

src/uu/cp/src/cp.rs

@@ -24,7 +24,6 @@ use std::os::unix::ffi::OsStrExt;
 #[cfg(unix)]
 use std::os::unix::fs::{FileTypeExt, PermissionsExt};
 use std::path::{Path, PathBuf, StripPrefixError};
-use std::str::FromStr;
 use std::string::ToString;
 
 use clap::{crate_version, Arg, ArgAction, ArgMatches, Command};
@@ -33,6 +32,8 @@ use indicatif::{ProgressBar, ProgressStyle};
 #[cfg(unix)]
 use libc::mkfifo;
 use quick_error::ResultExt;
+
+use platform::copy_on_write;
 use uucore::backup_control::{self, BackupMode};
 use uucore::display::Quotable;
 use uucore::error::{set_exit_code, UClapError, UError, UResult, UUsageError};
@@ -41,12 +42,10 @@ use uucore::fs::{
 };
 use uucore::{crash, format_usage, prompt_yes, show_error, show_warning};
 
-mod copydir;
 use crate::copydir::copy_directory;
 
+mod copydir;
 mod platform;
-use platform::copy_on_write;
-
 quick_error! {
     #[derive(Debug)]
     pub enum Error {
@@ -157,18 +156,51 @@ pub enum CopyMode {
     AttrOnly,
 }
 
-// The ordering here determines the order in which attributes are (re-)applied.
+#[derive(Debug)]
-// In particular, Ownership must be changed first to avoid interfering with mode change.
+pub struct Attributes {
-#[derive(Clone, Eq, PartialEq, Debug, PartialOrd, Ord)]
-pub enum Attribute {
     #[cfg(unix)]
-    Ownership,
+    ownership: Preserve,
-    Mode,
+    mode: Preserve,
-    Timestamps,
+    timestamps: Preserve,
-    #[cfg(feature = "feat_selinux")]
+    context: Preserve,
-    Context,
+    links: Preserve,
-    Links,
+    xattr: Preserve,
-    Xattr,
+}
+
+impl Attributes {
+    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 {
+    /// Preservation level should only increase, with no preservation being the lowest option,
+    /// preserve but don't require - middle, and preserve and require - top.
+    pub(crate) fn max(&self, other: Self) -> Self {
+        match (self, other) {
+            (Self::Yes { required: true }, _) | (_, Self::Yes { required: true }) => {
+                Self::Yes { required: true }
+            }
+            (Self::Yes { required: false }, _) | (_, Self::Yes { required: false }) => {
+                Self::Yes { required: false }
+            }
+            _ => Self::No,
+        }
+    }
 }
 
 /// Re-usable, extensible copy options
@@ -187,7 +219,7 @@ pub struct Options {
     sparse_mode: SparseMode,
     strip_trailing_slashes: bool,
     reflink_mode: ReflinkMode,
-    preserve_attributes: Vec<Attribute>,
+    attributes: Attributes,
     recursive: bool,
     backup_suffix: String,
     target_dir: Option<String>,
@@ -243,7 +275,6 @@ static PRESERVABLE_ATTRIBUTES: &[&str] = &[
     "mode",
     "ownership",
     "timestamps",
-    #[cfg(feature = "feat_selinux")]
     "context",
     "links",
     "xattr",
@@ -254,13 +285,6 @@ static PRESERVABLE_ATTRIBUTES: &[&str] = &[
 static PRESERVABLE_ATTRIBUTES: &[&str] =
     &["mode", "timestamps", "context", "links", "xattr", "all"];
 
-static DEFAULT_ATTRIBUTES: &[Attribute] = &[
-    Attribute::Mode,
-    #[cfg(unix)]
-    Attribute::Ownership,
-    Attribute::Timestamps,
-];
-
 pub fn uu_app() -> Command {
     const MODE_ARGS: &[&str] = &[
         options::LINK,
@@ -627,46 +651,79 @@ impl CopyMode {
     }
 }
 
-impl FromStr for Attribute {
+impl Attributes {
-    type Err = Error;
+    // 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
 
-    fn from_str(value: &str) -> CopyResult<Self> {
+    fn all() -> Self {
-        Ok(match &*value.to_lowercase() {
+        Self {
-            "mode" => Self::Mode,
             #[cfg(unix)]
-            "ownership" => Self::Ownership,
+            ownership: Preserve::Yes { required: true },
-            "timestamps" => Self::Timestamps,
+            mode: Preserve::Yes { required: true },
-            #[cfg(feature = "feat_selinux")]
+            timestamps: Preserve::Yes { required: true },
-            "context" => Self::Context,
+            context: {
-            "links" => Self::Links,
+                #[cfg(feature = "feat_selinux")]
-            "xattr" => Self::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 {
+        Self {
+            #[cfg(unix)]
+            ownership: Preserve::Yes { required: true },
+            mode: Preserve::Yes { required: true },
+            timestamps: Preserve::Yes { required: true },
+            context: Preserve::No,
+            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,
+        }
+    }
+
+    /// 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> {
+        let preserve_yes_required = Preserve::Yes { required: true };
+
+        match &*value.to_lowercase() {
+            "mode" => self.mode = preserve_yes_required,
+            #[cfg(unix)]
+            "ownership" => self.ownership = preserve_yes_required,
+            "timestamps" => self.timestamps = preserve_yes_required,
+            "context" => self.context = preserve_yes_required,
+            "links" => self.links = preserve_yes_required,
+            "xattr" => self.xattr = preserve_yes_required,
             _ => {
                 return Err(Error::InvalidArgument(format!(
                     "invalid attribute {}",
                     value.quote()
                 )));
             }
-        })
+        };
+        Ok(())
     }
 }
 
-fn add_all_attributes() -> Vec<Attribute> {
-    use Attribute::*;
-
-    let attr = vec![
-        #[cfg(unix)]
-        Ownership,
-        Mode,
-        Timestamps,
-        #[cfg(feature = "feat_selinux")]
-        Context,
-        Links,
-        Xattr,
-    ];
-
-    attr
-}
-
 impl Options {
     fn from_matches(matches: &ArgMatches) -> CopyResult<Self> {
         let not_implemented_opts = vec![
@@ -710,22 +767,23 @@ impl Options {
         };
 
         // Parse attributes to preserve
-        let mut preserve_attributes: Vec<Attribute> = if matches.contains_id(options::PRESERVE) {
+        let attributes: Attributes = if matches.contains_id(options::PRESERVE) {
             match matches.get_many::<String>(options::PRESERVE) {
-                None => DEFAULT_ATTRIBUTES.to_vec(),
+                None => Attributes::default(),
                 Some(attribute_strs) => {
-                    let mut attributes = Vec::new();
+                    let mut attributes: Attributes = Attributes::none();
+                    let mut attributes_empty = true;
                     for attribute_str in attribute_strs {
+                        attributes_empty = false;
                         if attribute_str == "all" {
-                            attributes = add_all_attributes();
+                            attributes.max(Attributes::all());
-                            break;
                         } else {
-                            attributes.push(Attribute::from_str(attribute_str)?);
+                            attributes.try_set_from_string(attribute_str)?;
                         }
                     }
                     // `--preserve` case, use the defaults
-                    if attributes.is_empty() {
+                    if attributes_empty {
-                        DEFAULT_ATTRIBUTES.to_vec()
+                        Attributes::default()
                     } else {
                         attributes
                     }
@@ -733,19 +791,27 @@ impl Options {
             }
         } else if matches.get_flag(options::ARCHIVE) {
             // --archive is used. Same as --preserve=all
-            add_all_attributes()
+            Attributes::all()
         } else if matches.get_flag(options::NO_DEREFERENCE_PRESERVE_LINKS) {
-            vec![Attribute::Links]
+            let mut attributes = Attributes::none();
+            attributes.links = Preserve::Yes { required: true };
+            attributes
         } else if matches.get_flag(options::PRESERVE_DEFAULT_ATTRIBUTES) {
-            DEFAULT_ATTRIBUTES.to_vec()
+            Attributes::default()
         } else {
-            vec![]
+            Attributes::none()
         };
 
-        // Make sure ownership is changed before other attributes,
+        #[cfg(not(feature = "feat_selinux"))]
-        // as chown clears some of the permission and therefore could undo previous changes
+        if let Preserve::Yes { required } = attributes.context {
-        // if not executed first.
+            let selinux_disabled_error =
-        preserve_attributes.sort_unstable();
+                Error::Error("SELinux was not enabled during the compile time!".to_string());
+            if required {
+                return Err(selinux_disabled_error);
+            } else {
+                show_error_if_needed(&selinux_disabled_error);
+            }
+        }
 
         let options = Self {
             attributes_only: matches.get_flag(options::ATTRIBUTES_ONLY),
@@ -801,7 +867,7 @@ impl Options {
                             return Err(Error::InvalidArgument(format!(
                                 "invalid argument {} for \'sparse\'",
                                 val
-                            )))
+                            )));
                         }
                     }
                 } else {
@@ -812,7 +878,7 @@ impl Options {
             backup_suffix,
             overwrite,
             no_target_dir,
-            preserve_attributes,
+            attributes,
             recursive,
             target_dir,
             progress_bar: matches.get_flag(options::PROGRESS_BAR),
@@ -826,12 +892,10 @@ impl Options {
     }
 
     fn preserve_hard_links(&self) -> bool {
-        for attribute in &self.preserve_attributes {
+        match self.attributes.links {
-            if *attribute == Attribute::Links {
+            Preserve::No => false,
-                return true;
+            Preserve::Yes { .. } => true,
-            }
         }
-        false
     }
 
     /// Whether to force overwriting the destination file.
@@ -951,6 +1015,22 @@ fn preserve_hardlinks(
     Ok(found_hard_link)
 }
 
+/// When handling errors, we don't always want to show them to the user. This function handles that.
+/// If the error is printed, returns true, false otherwise.
+fn show_error_if_needed(error: &Error) -> bool {
+    match error {
+        // When using --no-clobber, we don't want to show
+        // an error message
+        Error::NotAllFilesCopied => (),
+        Error::Skipped => (),
+        _ => {
+            show_error!("{}", error);
+            return true;
+        }
+    }
+    false
+}
+
 /// Copy all `sources` to `target`.  Returns an
 /// `Err(Error::NotAllFilesCopied)` if at least one non-fatal error was
 /// encountered.
@@ -1005,15 +1085,8 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu
                     options,
                     &mut symlinked_files,
                 ) {
-                    match error {
+                    if show_error_if_needed(&error) {
-                        // When using --no-clobber, we don't want to show
+                        non_fatal_errors = true;
-                        // an error message
-                        Error::NotAllFilesCopied => (),
-                        Error::Skipped => (),
-                        _ => {
-                            show_error!("{}", error);
-                            non_fatal_errors = true;
-                        }
                     }
                 }
             }
@@ -1100,114 +1173,138 @@ impl OverwriteMode {
     }
 }
 
+/// Handles errors for attributes preservation. If the attribute is not required, and
+/// errored, tries to show error (see `show_error_if_needed` for additional behavior details).
+/// If it's required, then the error is thrown.
+fn handle_preserve<F: Fn() -> CopyResult<()>>(p: &Preserve, f: F) -> CopyResult<()> {
+    match p {
+        Preserve::No => {}
+        Preserve::Yes { required } => {
+            let result = f();
+            if *required {
+                result?;
+            } else if let Err(error) = result {
+                show_error_if_needed(&error);
+            }
+        }
+    };
+    Ok(())
+}
+
 /// Copy the specified attributes from one path to another.
 pub(crate) fn copy_attributes(
     source: &Path,
     dest: &Path,
-    attributes: &[Attribute],
+    attributes: &Attributes,
 ) -> CopyResult<()> {
-    for attribute in attributes {
-        copy_attribute(source, dest, attribute)?;
-    }
-    Ok(())
-}
-
-fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResult<()> {
     let context = &*format!("{} -> {}", source.quote(), dest.quote());
     let source_metadata = fs::symlink_metadata(source).context(context)?;
-    match *attribute {
+
-        Attribute::Mode => {
+    // Ownership must be changed first to avoid interfering with mode change.
-            // The `chmod()` system call that underlies the
+    #[cfg(unix)]
-            // `fs::set_permissions()` call is unable to change the
+    handle_preserve(&attributes.ownership, || -> CopyResult<()> {
-            // permissions of a symbolic link. In that case, we just
+        use std::os::unix::prelude::MetadataExt;
-            // do nothing, since every symbolic link has the same
+        use uucore::perms::wrap_chown;
-            // permissions.
+        use uucore::perms::Verbosity;
-            if !dest.is_symlink() {
+        use uucore::perms::VerbosityLevel;
-                fs::set_permissions(dest, source_metadata.permissions()).context(context)?;
+
-                // FIXME: Implement this for windows as well
+        let dest_uid = source_metadata.uid();
-                #[cfg(feature = "feat_acl")]
+        let dest_gid = source_metadata.gid();
-                exacl::getfacl(source, None)
+
-                    .and_then(|acl| exacl::setfacl(&[dest], &acl, None))
+        wrap_chown(
-                    .map_err(|err| Error::Error(err.to_string()))?;
+            dest,
-            }
+            &dest.symlink_metadata().context(context)?,
+            Some(dest_uid),
+            Some(dest_gid),
+            false,
+            Verbosity {
+                groups_only: false,
+                level: VerbosityLevel::Normal,
+            },
+        )
+        .map_err(Error::Error)?;
+
+        Ok(())
+    })?;
+
+    handle_preserve(&attributes.mode, || -> CopyResult<()> {
+        // The `chmod()` system call that underlies the
+        // `fs::set_permissions()` call is unable to change the
+        // permissions of a symbolic link. In that case, we just
+        // do nothing, since every symbolic link has the same
+        // permissions.
+        if !dest.is_symlink() {
+            fs::set_permissions(dest, source_metadata.permissions()).context(context)?;
+            // FIXME: Implement this for windows as well
+            #[cfg(feature = "feat_acl")]
+            exacl::getfacl(source, None)
+                .and_then(|acl| exacl::setfacl(&[dest], &acl, None))
+                .map_err(|err| Error::Error(err.to_string()))?;
         }
-        #[cfg(unix)]
-        Attribute::Ownership => {
-            use std::os::unix::prelude::MetadataExt;
-            use uucore::perms::wrap_chown;
-            use uucore::perms::Verbosity;
-            use uucore::perms::VerbosityLevel;
 
-            let dest_uid = source_metadata.uid();
+        Ok(())
-            let dest_gid = source_metadata.gid();
+    })?;
 
-            wrap_chown(
+    handle_preserve(&attributes.timestamps, || -> CopyResult<()> {
-                dest,
+        let atime = FileTime::from_last_access_time(&source_metadata);
-                &dest.symlink_metadata().context(context)?,
+        let mtime = FileTime::from_last_modification_time(&source_metadata);
-                Some(dest_uid),
+        if dest.is_symlink() {
-                Some(dest_gid),
+            filetime::set_symlink_file_times(dest, atime, mtime)?;
-                false,
+        } else {
-                Verbosity {
+            filetime::set_file_times(dest, atime, mtime)?;
-                    groups_only: false,
+        }
-                    level: VerbosityLevel::Normal,
+
-                },
+        Ok(())
+    })?;
+
+    #[cfg(feature = "feat_selinux")]
+    handle_preserve(&attributes.context, || -> CopyResult<()> {
+        let context = selinux::SecurityContext::of_path(source, false, false).map_err(|e| {
+            format!(
+                "failed to get security context of {}: {}",
+                source.display(),
+                e
             )
-            .map_err(Error::Error)?;
+        })?;
-        }
+        if let Some(context) = context {
-        Attribute::Timestamps => {
+            context.set_for_path(dest, false, false).map_err(|e| {
-            let atime = FileTime::from_last_access_time(&source_metadata);
-            let mtime = FileTime::from_last_modification_time(&source_metadata);
-            if dest.is_symlink() {
-                filetime::set_symlink_file_times(dest, atime, mtime)?;
-            } else {
-                filetime::set_file_times(dest, atime, mtime)?;
-            }
-        }
-        #[cfg(feature = "feat_selinux")]
-        Attribute::Context => {
-            let context = selinux::SecurityContext::of_path(source, false, false).map_err(|e| {
                 format!(
-                    "failed to get security context of {}: {}",
+                    "failed to set security context for {}: {}",
-                    source.display(),
+                    dest.display(),
                     e
                 )
             })?;
-            if let Some(context) = context {
-                context.set_for_path(dest, false, false).map_err(|e| {
-                    format!(
-                        "failed to set security context for {}: {}",
-                        dest.display(),
-                        e
-                    )
-                })?;
-            }
         }
-        Attribute::Links => {}
+
-        Attribute::Xattr => {
+        Ok(())
-            #[cfg(unix)]
+    })?;
-            {
+
-                let xattrs = xattr::list(source)?;
+    handle_preserve(&attributes.xattr, || -> CopyResult<()> {
-                for attr in xattrs {
+        #[cfg(unix)]
-                    if let Some(attr_value) = xattr::get(source, attr.clone())? {
+        {
-                        xattr::set(dest, attr, &attr_value[..])?;
+            let xattrs = xattr::list(source)?;
-                    }
+            for attr in xattrs {
+                if let Some(attr_value) = xattr::get(source, attr.clone())? {
+                    xattr::set(dest, attr, &attr_value[..])?;
                 }
             }
-            #[cfg(not(unix))]
-            {
-                // The documentation for GNU cp states:
-                //
-                // > Try to preserve SELinux security context and
-                // > extended attributes (xattr), but ignore any failure
-                // > to do that and print no corresponding diagnostic.
-                //
-                // so we simply do nothing here.
-                //
-                // TODO Silently ignore failures in the `#[cfg(unix)]`
-                // block instead of terminating immediately on errors.
-            }
         }
-    };
+        #[cfg(not(unix))]
+        {
+            // The documentation for GNU cp states:
+            //
+            // > Try to preserve SELinux security context and
+            // > extended attributes (xattr), but ignore any failure
+            // > to do that and print no corresponding diagnostic.
+            //
+            // so we simply do nothing here.
+            //
+            // TODO Silently ignore failures in the `#[cfg(unix)]`
+            // block instead of terminating immediately on errors.
+        }
+
+        Ok(())
+    })?;
 
     Ok(())
 }
@@ -1580,7 +1677,8 @@ fn copy_file(
         // the user does not have permission to write to the file.
         fs::set_permissions(dest, dest_permissions).ok();
     }
-    copy_attributes(source, dest, &options.preserve_attributes)?;
+
+    copy_attributes(source, dest, &options.attributes)?;
 
     if let Some(progress_bar) = progress_bar {
         progress_bar.inc(fs::metadata(source)?.len());

tests/by-util/test_cp.rs

@@ -22,9 +22,7 @@ use filetime::FileTime;
 use rlimit::Resource;
 #[cfg(any(target_os = "linux", target_os = "android"))]
 use std::fs as std_fs;
-#[cfg(not(target_os = "freebsd"))]
 use std::thread::sleep;
-#[cfg(not(target_os = "freebsd"))]
 use std::time::Duration;
 
 static TEST_EXISTING_FILE: &str = "existing_file.txt";
@@ -907,6 +905,91 @@ fn test_cp_preserve_no_args() {
     }
 }
 
+#[test]
+fn test_cp_preserve_all() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let src_file = "a";
+    let dst_file = "b";
+
+    // Prepare the source file
+    at.touch(src_file);
+    #[cfg(unix)]
+    at.set_mode(src_file, 0o0500);
+
+    // TODO: create a destination that does not allow copying of xattr and context
+    // Copy
+    ucmd.arg(src_file)
+        .arg(dst_file)
+        .arg("--preserve=all")
+        .succeeds();
+
+    #[cfg(all(unix, not(target_os = "freebsd")))]
+    {
+        // Assert that the mode, ownership, and timestamps are preserved
+        // NOTICE: the ownership is not modified on the src file, because that requires root permissions
+        let metadata_src = at.metadata(src_file);
+        let metadata_dst = at.metadata(dst_file);
+        assert_metadata_eq!(metadata_src, metadata_dst);
+    }
+}
+
+#[test]
+#[cfg(all(unix, not(target_os = "android")))]
+fn test_cp_preserve_xattr() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let src_file = "a";
+    let dst_file = "b";
+
+    // Prepare the source file
+    at.touch(src_file);
+    #[cfg(unix)]
+    at.set_mode(src_file, 0o0500);
+
+    // Sleep so that the time stats are different
+    sleep(Duration::from_secs(1));
+
+    // TODO: create a destination that does not allow copying of xattr and context
+    // Copy
+    ucmd.arg(src_file)
+        .arg(dst_file)
+        .arg("--preserve=xattr")
+        .succeeds();
+
+    // FIXME: macos copy keeps the original mtime
+    #[cfg(not(any(target_os = "freebsd", target_os = "macos")))]
+    {
+        // Assert that the mode, ownership, and timestamps are *NOT* preserved
+        // NOTICE: the ownership is not modified on the src file, because that requires root permissions
+        let metadata_src = at.metadata(src_file);
+        let metadata_dst = at.metadata(dst_file);
+        assert_ne!(metadata_src.mtime(), metadata_dst.mtime());
+        // TODO: verify access time as well. It shouldn't change, however, it does change in this test.
+    }
+}
+
+#[test]
+#[cfg(all(target_os = "linux", not(feature = "feat_selinux")))]
+fn test_cp_preserve_all_context_fails_on_non_selinux() {
+    new_ucmd!()
+        .arg(TEST_COPY_FROM_FOLDER_FILE)
+        .arg(TEST_HELLO_WORLD_DEST)
+        .arg("--preserve=all,context")
+        .fails();
+}
+
+#[test]
+#[cfg(any(target_os = "android"))]
+fn test_cp_preserve_xattr_fails_on_android() {
+    // Because of the SELinux extended attributes used on Android, trying to copy extended
+    // attributes has to fail in this case, since we specify `--preserve=xattr` and this puts it
+    // into the required attributes
+    new_ucmd!()
+        .arg(TEST_COPY_FROM_FOLDER_FILE)
+        .arg(TEST_HELLO_WORLD_DEST)
+        .arg("--preserve=xattr")
+        .fails();
+}
+
 #[test]
 // For now, disable the test on Windows. Symlinks aren't well support on Windows.
 // It works on Unix for now and it works locally when run from a powershell