cp/mv: suppress xattr ENOTSUP errors for optional preservation (#10083)

* cp/mv: suppress xattr ENOTSUP errors for optional preservation

* mv: copy xattrs for symlink fallback and format tests

* cp: fix test expectation for GNU-compatible error format

* fix test portability for Android and xattr detection

---------

Co-authored-by: Sylvestre Ledru <sylvestre@debian.org>

Author: ChrisDryden

.vscode/cspell.dictionaries/jargon.wordlist.txt

@@ -220,5 +220,7 @@ TUNABLES
 tunables
 VMULL
 vmull
+ENOTSUP
+enotsup
 SETFL
 tmpfs

src/uu/cp/locales/en-US.ftl

@@ -91,6 +91,7 @@ cp-error-failed-to-create-whole-tree = failed to create whole tree
 cp-error-failed-to-create-directory = Failed to create directory: { $error }
 cp-error-backup-format = cp: { $error }
   Try '{ $exec } --help' for more information.
+cp-error-setting-attributes = setting attributes for { $path }
 
 # Debug enum strings
 cp-debug-enum-no = no

src/uu/cp/src/cp.rs

@@ -1316,6 +1316,20 @@ fn parse_path_args(
     Ok((paths, target))
 }
 
+/// Check if an error is ENOTSUP/EOPNOTSUPP (operation not supported).
+/// This is used to suppress xattr errors on filesystems that don't support them.
+fn is_enotsup_error(error: &CpError) -> bool {
+    #[cfg(unix)]
+    const EOPNOTSUPP: i32 = libc::EOPNOTSUPP;
+    #[cfg(not(unix))]
+    const EOPNOTSUPP: i32 = 95;
+
+    match error {
+        CpError::IoErr(e) | CpError::IoErrContext(e, _) => e.raw_os_error() == Some(EOPNOTSUPP),
+        _ => false,
+    }
+}
+
 /// When handling errors, we don't always want to show them to the user. This function handles that.
 fn show_error_if_needed(error: &CpError) {
     match error {
@@ -1328,6 +1342,11 @@ fn show_error_if_needed(error: &CpError) {
             // touch a b && echo "n"|cp -i a b && echo $?
             // should return an error from GNU 9.2
         }
+        // Format IoErrContext using strip_errno to remove "(os error N)" suffix
+        // for GNU-compatible output
+        CpError::IoErrContext(io_err, context) => {
+            show_error!("{}: {}", context, uucore::error::strip_errno(io_err));
+        }
         _ => {
             show_error!("{error}");
         }
@@ -1630,15 +1649,23 @@ 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.
+///
+/// Note: ENOTSUP/EOPNOTSUPP errors are silently ignored when not required, as per GNU cp
+/// documentation: "Try to preserve SELinux security context and extended attributes (xattr),
+/// but ignore any failure to do that and print no corresponding diagnostic."
 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);
+            } else if let Err(ref error) = result {
+                // Suppress ENOTSUP errors when preservation is optional.
+                // This matches GNU cp behavior for -a and --preserve=all.
+                if !is_enotsup_error(error) {
+                    show_error_if_needed(error);
+                }
             }
         }
     }
@@ -1675,8 +1702,13 @@ fn copy_extended_attrs(source: &Path, dest: &Path) -> CopyResult<()> {
         fs::set_permissions(dest, revert_perms)?;
     }
 
-    // If copying xattrs failed, propagate that error now.
-    copy_xattrs_result?;
+    // If copying xattrs failed, propagate that error now with context.
+    copy_xattrs_result.map_err(|e| {
+        CpError::IoErrContext(
+            e,
+            translate!("cp-error-setting-attributes", "path" => dest.quote()),
+        )
+    })?;
 
     Ok(())
 }

src/uu/mv/src/mv.rs

@@ -908,7 +908,12 @@ fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> {
 #[cfg(unix)]
 fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> {
     let path_symlink_points_to = fs::read_link(from)?;
-    unix::fs::symlink(path_symlink_points_to, to).and_then(|_| fs::remove_file(from))
+    unix::fs::symlink(path_symlink_points_to, to)?;
+    #[cfg(not(any(target_os = "macos", target_os = "redox")))]
+    {
+        let _ = copy_xattrs_if_supported(from, to);
+    }
+    fs::remove_file(from)
 }
 
 #[cfg(windows)]
@@ -1147,13 +1152,11 @@ fn copy_file_with_hardlinks_helper(
         rename_symlink_fallback(from, to)?;
     } else {
         // Copy a regular file.
+        fs::copy(from, to)?;
+        // Copy xattrs, ignoring ENOTSUP errors (filesystem doesn't support xattrs)
         #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
         {
-            fs::copy(from, to).and_then(|_| fsxattr::copy_xattrs(&from, &to))?;
-        }
-        #[cfg(any(target_os = "macos", target_os = "redox"))]
-        {
-            fs::copy(from, to)?;
+            let _ = copy_xattrs_if_supported(from, to);
         }
     }
 
@@ -1195,18 +1198,32 @@ fn rename_file_fallback(
     }
 
     // Regular file copy
-    #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
     fs::copy(from, to)
-        .and_then(|_| fsxattr::copy_xattrs(&from, &to))
-        .and_then(|_| fs::remove_file(from))
         .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
-    #[cfg(any(target_os = "macos", target_os = "redox", not(unix)))]
-    fs::copy(from, to)
-        .and_then(|_| fs::remove_file(from))
+
+    // Copy xattrs, ignoring ENOTSUP errors (filesystem doesn't support xattrs)
+    #[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
+    {
+        let _ = copy_xattrs_if_supported(from, to);
+    }
+
+    fs::remove_file(from)
         .map_err(|err| io::Error::new(err.kind(), translate!("mv-error-permission-denied")))?;
     Ok(())
 }
 
+/// Copy xattrs from source to destination, ignoring ENOTSUP/EOPNOTSUPP errors.
+/// These errors indicate the filesystem doesn't support extended attributes,
+/// which is acceptable when moving files across filesystems.
+#[cfg(all(unix, not(any(target_os = "macos", target_os = "redox"))))]
+fn copy_xattrs_if_supported(from: &Path, to: &Path) -> io::Result<()> {
+    match fsxattr::copy_xattrs(from, to) {
+        Ok(()) => Ok(()),
+        Err(e) if e.raw_os_error() == Some(libc::EOPNOTSUPP) => Ok(()),
+        Err(e) => Err(e),
+    }
+}
+
 fn is_empty_dir(path: &Path) -> bool {
     fs::read_dir(path).is_ok_and(|mut contents| contents.next().is_none())
 }

tests/by-util/test_cp.rs

@@ -2615,7 +2615,7 @@ fn test_cp_reflink_insufficient_permission() {
         .arg("unreadable")
         .arg(TEST_EXISTING_FILE)
         .fails()
-        .stderr_only("cp: 'unreadable' -> 'existing_file.txt': Permission denied (os error 13)\n");
+        .stderr_only("cp: 'unreadable' -> 'existing_file.txt': Permission denied\n");
 }
 
 #[cfg(target_os = "linux")]
@@ -3131,9 +3131,8 @@ fn test_cp_archive_on_nonexistent_file() {
         .arg(TEST_NONEXISTENT_FILE)
         .arg(TEST_EXISTING_FILE)
         .fails()
-        .stderr_only(
-            "cp: cannot stat 'nonexistent_file.txt': No such file or directory (os error 2)\n",
-        );
+        .stderr_contains("cannot stat 'nonexistent_file.txt'")
+        .stderr_contains("No such file or directory");
 }
 
 #[test]
@@ -7512,3 +7511,63 @@ fn test_cp_to_existing_file_permissions() {
     let new_dst_mode = std::fs::metadata(&dst_path).unwrap().permissions().mode();
     assert_eq!(dst_mode, new_dst_mode);
 }
+
+/// Test xattr ENOTSUP handling: -a/--preserve=all silent, --preserve=xattr errors
+#[test]
+#[cfg(target_os = "linux")]
+fn test_cp_xattr_enotsup_handling() {
+    use std::process::Command;
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.write("src", "x");
+
+    // Check if setfattr is available and source fs supports xattrs
+    if !Command::new("setfattr")
+        .args(["-n", "user.t", "-v", "v", &at.plus_as_string("src")])
+        .status()
+        .is_ok_and(|s| s.success())
+    {
+        return; // Skip: setfattr not available or source doesn't support xattrs
+    }
+
+    // Check if /dev/shm exists
+    if !std::path::Path::new("/dev/shm").exists() {
+        return; // Skip: /dev/shm not available
+    }
+
+    // Check if /dev/shm actually doesn't support xattrs by trying to set one
+    let shm_test_file = "/dev/shm/xattr_test_probe";
+    std::fs::write(shm_test_file, "test").ok();
+    let shm_supports_xattr = Command::new("setfattr")
+        .args(["-n", "user.t", "-v", "v", shm_test_file])
+        .status()
+        .is_ok_and(|s| s.success());
+    std::fs::remove_file(shm_test_file).ok();
+
+    if shm_supports_xattr {
+        return; // Skip: /dev/shm supports xattrs on this system
+    }
+
+    // -a: silent success
+    scene
+        .ucmd()
+        .args(&["-a", &at.plus_as_string("src"), "/dev/shm/t1"])
+        .succeeds()
+        .no_stderr();
+    // --preserve=all: silent success
+    scene
+        .ucmd()
+        .args(&["--preserve=all", &at.plus_as_string("src"), "/dev/shm/t2"])
+        .succeeds()
+        .no_stderr();
+    // --preserve=xattr: must fail with proper message
+    scene
+        .ucmd()
+        .args(&["--preserve=xattr", &at.plus_as_string("src"), "/dev/shm/t3"])
+        .fails()
+        .stderr_contains("setting attributes")
+        .stderr_contains("Operation not supported");
+    for f in ["/dev/shm/t1", "/dev/shm/t2", "/dev/shm/t3"] {
+        std::fs::remove_file(f).ok();
+    }
+}

tests/by-util/test_mv.rs

@@ -2821,3 +2821,27 @@ fn test_mv_no_prompt_unwriteable_file_with_no_tty() {
     assert!(!at.file_exists("source_notty"));
     assert!(at.file_exists("target_notty"));
 }
+
+/// Test mv silently succeeds when dest filesystem doesn't support xattrs (ENOTSUP)
+#[test]
+#[cfg(target_os = "linux")]
+fn test_mv_xattr_enotsup_silent() {
+    use std::process::Command;
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.write("src", "x");
+
+    if Command::new("setfattr")
+        .args(["-n", "user.t", "-v", "v", &at.plus_as_string("src")])
+        .status()
+        .is_ok_and(|s| s.success())
+    {
+        scene
+            .ucmd()
+            .arg(at.plus_as_string("src"))
+            .arg("/dev/shm/mv_test")
+            .succeeds()
+            .no_stderr();
+        std::fs::remove_file("/dev/shm/mv_test").ok();
+    }
+}