diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index a1bda0e7690..5f3f8926483 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -215,3 +215,5 @@ TUNABLES tunables VMULL vmull +ENOTSUP +enotsup diff --git a/src/uu/cp/locales/en-US.ftl b/src/uu/cp/locales/en-US.ftl index a0b95cf6c66..f4e9df00668 100644 --- a/src/uu/cp/locales/en-US.ftl +++ b/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 diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 3048f38b711..22e67702e39 100644 --- a/src/uu/cp/src/cp.rs +++ b/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}"); } @@ -1629,6 +1648,10 @@ 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 CopyResult<()>>(p: &Preserve, f: F) -> CopyResult<()> { match p { Preserve::No { .. } => {} @@ -1636,8 +1659,12 @@ fn handle_preserve CopyResult<()>>(p: &Preserve, f: F) -> CopyResult< 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); + } } } } @@ -1674,8 +1701,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(()) } diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index aa34a6294ae..da13d72c26e 100644 --- a/src/uu/mv/src/mv.rs +++ b/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)] @@ -1153,13 +1158,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); } } @@ -1201,18 +1204,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()) } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index dfd07ec4bd0..092ce4321ca 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2605,7 +2605,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")] @@ -3117,9 +3117,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] @@ -7498,3 +7497,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(); + } +} diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 3c69d65a78d..557f55010f7 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2856,3 +2856,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(); + } +}