diff --git a/src/uu/mv/locales/en-US.ftl b/src/uu/mv/locales/en-US.ftl index be6462b0b31..34ae3a80fdc 100644 --- a/src/uu/mv/locales/en-US.ftl +++ b/src/uu/mv/locales/en-US.ftl @@ -39,6 +39,7 @@ mv-error-no-symlink-support = your operating system does not support symlinks mv-error-permission-denied = Permission denied mv-error-inter-device-move-failed = inter-device move failed: {$from} to {$to}; unable to remove target: {$err} mv-error-setting-attribute = setting attribute {$name}: {$err} +mv-error-setting-attributes = setting attributes for {$path}: {$err} mv-error-exchange-two-operands = --exchange requires exactly two operands mv-error-exchange-not-supported = --exchange is not supported on this platform diff --git a/src/uu/mv/locales/fr-FR.ftl b/src/uu/mv/locales/fr-FR.ftl index abf3f552ac9..c7baf666351 100644 --- a/src/uu/mv/locales/fr-FR.ftl +++ b/src/uu/mv/locales/fr-FR.ftl @@ -39,6 +39,7 @@ mv-error-no-symlink-support = votre système d'exploitation ne prend pas en char mv-error-permission-denied = Permission refusée mv-error-inter-device-move-failed = échec du déplacement inter-périphérique : {$from} vers {$to} ; impossible de supprimer la cible : {$err} mv-error-setting-attribute = définition de l'attribut {$name} : {$err} +mv-error-setting-attributes = définition des attributs pour {$path} : {$err} mv-error-exchange-two-operands = --exchange nécessite exactement deux opérandes mv-error-exchange-not-supported = --exchange n'est pas pris en charge sur cette plateforme diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 68cc9c21887..3ae71bdd16b 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -1459,8 +1459,8 @@ fn rename_file_fallback( target_os = "netbsd" ))] { - if let Ok(failed) = fsxattr::copy_xattrs_fd_ignore_unsupported(&src_file, &dst_file) { - show_xattr_failures(failed); + if let Ok(failed) = fsxattr::copy_xattrs_fd(&src_file, &dst_file) { + show_xattr_failures_for(to, failed); } } @@ -1491,6 +1491,42 @@ fn rename_file_fallback( Ok(()) } +/// Like [`show_xattr_failures`], but for a file moved to `to`. When `to` does +/// not support xattrs at all, GNU 9.12 warns once for the file rather than once +/// per attribute. It says nothing when only a POSIX ACL could not be set. +#[cfg(any( + target_os = "freebsd", + target_os = "hurd", + target_os = "linux", + target_os = "android", + target_os = "netbsd" +))] +fn show_xattr_failures_for(to: &Path, failed: Vec<(OsString, io::Error)>) { + use uucore::error::strip_errno; + use uucore::show_error; + + let (unsupported, failed): (Vec<_>, Vec<_>) = failed + .into_iter() + .partition(|(_, err)| fsxattr::is_xattr_unsupported(err)); + let is_acl = |name: &OsString| { + matches!( + name.to_str(), + Some("system.posix_acl_access" | "system.posix_acl_default") + ) + }; + if let Some((_, err)) = unsupported.iter().find(|(name, _)| !is_acl(name)) { + show_error!( + "{}", + translate!( + "mv-error-setting-attributes", + "path" => to.quote(), + "err" => strip_errno(err) + ) + ); + } + show_xattr_failures(failed); +} + /// Report each xattr that a cross-device move could not copy. Like GNU, these /// are only warnings: the move itself still succeeds. #[cfg(any( diff --git a/src/uucore/src/lib/features/fsxattr.rs b/src/uucore/src/lib/features/fsxattr.rs index 373fd1b770a..b8f6faf06a5 100644 --- a/src/uucore/src/lib/features/fsxattr.rs +++ b/src/uucore/src/lib/features/fsxattr.rs @@ -17,7 +17,7 @@ use std::path::Path; /// True if the error is `ENOTSUP` / `EOPNOTSUPP` (same errno on Linux, /// distinct on the BSDs). #[cfg(unix)] -fn is_xattr_unsupported(err: &std::io::Error) -> bool { +pub fn is_xattr_unsupported(err: &std::io::Error) -> bool { matches!( err.raw_os_error(), Some(e) if e == libc::ENOTSUP || e == libc::EOPNOTSUPP @@ -25,7 +25,7 @@ fn is_xattr_unsupported(err: &std::io::Error) -> bool { } #[cfg(not(unix))] -fn is_xattr_unsupported(_err: &std::io::Error) -> bool { +pub fn is_xattr_unsupported(_err: &std::io::Error) -> bool { false } diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index b2bf30e8732..c4ef49df128 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -3175,28 +3175,92 @@ fn test_mv_no_prompt_unwriteable_file_with_no_tty() { assert!(at.file_exists("target_notty")); } -/// Test mv silently succeeds when dest filesystem doesn't support xattrs (ENOTSUP) +/// Moving a file with xattrs onto a file system without xattr support warns +/// once for the file, however many attributes it has, and still succeeds, as +/// GNU 9.12 does. A file without xattrs, or with only a POSIX ACL, moves +/// without a word. Mounting ramfs needs root, so this skips without sudo. #[test] #[cfg(target_os = "linux")] -fn test_mv_xattr_enotsup_silent() { - use std::process::Command; +fn test_mv_xattr_enotsup_warns_once() { + // spell-checker:ignore noxattr ramfs + use rustc_hash::FxHashMap; + use std::ffi::OsString; + use uucore::fsxattr::apply_xattrs; + use uutests::util::PATH; + 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()) + let set_xattrs = |name: &str, attrs: &[(&str, &[u8])]| { + at.write(name, "content"); + let attrs: FxHashMap> = attrs + .iter() + .map(|(attr, value)| (OsString::from(attr), value.to_vec())) + .collect(); + apply_xattrs(at.plus(name), attrs) + }; + // user::rw-, user:0:r--, group::r--, mask::r--, other::---. The named user + // keeps the kernel from folding it into the mode bits. + let acl: &[u8] = &[ + 2, 0, 0, 0, 1, 0, 6, 0, 255, 255, 255, 255, 2, 0, 4, 0, 0, 0, 0, 0, 4, 0, 4, 0, 255, 255, + 255, 255, 16, 0, 4, 0, 255, 255, 255, 255, 32, 0, 0, 0, 255, 255, 255, 255, + ]; + if set_xattrs("one_attr", &[("user.test", b"v")]).is_err() + || set_xattrs("acl_only", &[("system.posix_acl_access", acl)]).is_err() { - 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(); + println!("test skipped: the test directory does not accept xattrs and ACLs"); + return; + } + let three: &[(&str, &[u8])] = &[("user.one", b"1"), ("user.two", b"2"), ("user.three", b"3")]; + set_xattrs("three_attrs", three).unwrap(); + set_xattrs("same_fs", &[("user.test", b"v")]).unwrap(); + at.write("no_attrs", "content"); + + at.mkdir("noxattr"); + let mount = scene + .cmd("sudo") + .env("PATH", PATH) + .args(&["-E", "--non-interactive", "mount", "-t", "ramfs"]) + .args(&["-o", "mode=0777", "ramfs", "noxattr"]) + .run(); + if !mount.succeeded() { + println!("test skipped: mounting ramfs requires root"); + return; } + + let move_onto_ramfs = |name: &str| { + let dest = format!("noxattr/{name}"); + let result = scene.ucmd().arg(name).arg(&dest).run(); + let moved = !at.file_exists(name) && at.file_exists(&dest); + (name.to_owned(), result, moved) + }; + let warned = [move_onto_ramfs("one_attr"), move_onto_ramfs("three_attrs")]; + let silent = [move_onto_ramfs("no_attrs"), move_onto_ramfs("acl_only")]; + + // Unmount before the asserts so a failure does not leave it mounted. + scene + .cmd("sudo") + .env("PATH", PATH) + .args(&["-E", "--non-interactive", "umount", "noxattr"]) + .succeeds(); + + for (name, result, moved) in warned { + assert!(moved, "{name} was not moved"); + result.success().stderr_is(format!( + "mv: setting attributes for 'noxattr/{name}': Operation not supported\n" + )); + } + for (name, result, moved) in silent { + assert!(moved, "{name} was not moved"); + result.success().no_stderr(); + } + + scene + .ucmd() + .arg("same_fs") + .arg("same_fs_moved") + .succeeds() + .no_stderr(); } /// Cross-device mv of a directory must preserve the directory's own xattrs.