mv: keep copying xattrs after one fails on a cross-device move by krosci · Pull Request #14629 · uutils/coreutils

krosci · 2026-09-16T22:19:36Z

When a cross-device move copies a file or directory that carries several xattrs, a single rejected attribute (e.g. one larger than the destination filesystem allows) used to abort the copy loop, silently dropping every remaining xattr, and the move reported success. GNU mv reports the failing attribute on stderr, copies on through the others, and still exits 0.

copy_xattrs*/apply_xattrs* in uucore now report each failing attribute, keep copying the rest, and return the first error. mv no longer aborts the directory path on a reported failure. cp uses a silent marker so the already-reported per-attribute diagnostics are not echoed a second time.

Fixes #14598.

sylvestre · 2026-09-16T22:21:23Z

    #[error("{}", translate!("cp-error-not-all-files-copied"))]
    NotAllFilesCopied,

+    /// Extended-attribute copying failed, but every failure was already


i think it can be a one line comment

sylvestre · 2026-09-16T22:21:43Z


    // If copying xattrs failed, propagate that error now with context.
+    //
+    // `copy_xattrs*` already reported each failing attribute on stderr, so


same, doesn't have to be that long

github-actions · 2026-09-17T06:00:40Z

GNU testsuite comparison:

GNU test failed: tests/df/skip-rootfs. tests/df/skip-rootfs is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/id/smack. tests/id/smack is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/mkdir/smack-root. tests/mkdir/smack-root is passing on 'main'. Maybe you have to rebase?

codspeed · 2026-09-20T18:46:28Z

Merging this PR will not alter performance

✅ 377 untouched benchmarks
⏩ 54 skipped benchmarks¹

_{Comparing krosci:mv-copy-xattr-continue-on-error (25ac6f2) with main (5eb255f)}

54 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

sylvestre · 2026-09-21T11:29:13Z

+        } else {
+            "setting attribute"
+        };
+        show_error!("{action} {}: {}", attr_name.quote(), strip_errno(&err));


please use translate!() for these two strings
also, GNU says "setting attribute 'user.x' for 'dest': ...", could you please check the LANG=C output and match it (the "for dest" part is missing here)?

sylvestre · 2026-09-21T11:29:39Z

+/// failing `user.m_big` is rejected.
+#[test]
+#[cfg(target_os = "linux")]
+fn test_mv_cross_device_dir_xattr_partial_failure_completes() {


this test is ~90% the same as the one above, could be dedup, no?

You're right, I'm done!

sylvestre · 2026-09-21T11:29:46Z

+    assert_eq!(small_z_out.stdout, b"87654321");
+
+    let big_out = Command::new("getfattr")
+        .args(["-n", "user.m_big", "--only-values", "--absolute-names"])


the getfattr + assert block is repeated 5 times in this file, a small helper please :)

You're right, I'm done!

sylvestre · 2026-09-21T11:29:52Z

+/// source. Returns `None` when this machine's filesystem combination cannot
+/// produce that failure, in which case the test should be skipped.
+#[cfg(target_os = "linux")]
+fn tmpfs_to_target_failing_xattr_value(dest_dir: &Path) -> Option<String> {


same probe as in the uucore unit test and test_cp, please share it (uutests?)

Done, moved to uutests::util::tmpfs_to_target_failing_xattr_value.

sylvestre reviewed Sep 16, 2026

View reviewed changes

krosci requested a review from sylvestre September 16, 2026 23:14

krosci force-pushed the mv-copy-xattr-continue-on-error branch 5 times, most recently from c11d4b9 to 7018e7c Compare September 20, 2026 10:33

sylvestre force-pushed the mv-copy-xattr-continue-on-error branch from 7018e7c to 9c3117a Compare September 20, 2026 19:29

sylvestre reviewed Sep 21, 2026

View reviewed changes

Comment thread .github/workflows/GnuComment.yml Outdated

sylvestre reviewed Sep 21, 2026

View reviewed changes

Comment thread build.rs

krosci force-pushed the mv-copy-xattr-continue-on-error branch 3 times, most recently from c66613d to add8620 Compare September 21, 2026 12:53

mv: keep copying xattrs after one fails on a cross-device move

25ac6f2

krosci force-pushed the mv-copy-xattr-continue-on-error branch from add8620 to 25ac6f2 Compare September 21, 2026 16:13

Uh oh!

Conversation

krosci commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codspeed Bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

krosci commented Sep 16, 2026 •

edited

Loading

github-actions Bot commented Sep 17, 2026 •

edited

Loading

codspeed Bot commented Sep 20, 2026 •

edited

Loading