Conversation
| #[error("{}", translate!("cp-error-not-all-files-copied"))] | ||
| NotAllFilesCopied, | ||
|
|
||
| /// Extended-attribute copying failed, but every failure was already |
There was a problem hiding this comment.
i think it can be a one line comment
|
|
||
| // If copying xattrs failed, propagate that error now with context. | ||
| // | ||
| // `copy_xattrs*` already reported each failing attribute on stderr, so |
There was a problem hiding this comment.
same, doesn't have to be that long
|
GNU testsuite comparison: |
c11d4b9 to
7018e7c
Compare
Merging this PR will degrade performance by 17.62%
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing 
Footnotes
|
7018e7c to
9c3117a
Compare
| } else { | ||
| "setting attribute" | ||
| }; | ||
| show_error!("{action} {}: {}", attr_name.quote(), strip_errno(&err)); |
There was a problem hiding this comment.
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)?
| /// failing `user.m_big` is rejected. | ||
| #[test] | ||
| #[cfg(target_os = "linux")] | ||
| fn test_mv_cross_device_dir_xattr_partial_failure_completes() { |
There was a problem hiding this comment.
this test is ~90% the same as the one above, could be dedup, no?
| assert_eq!(small_z_out.stdout, b"87654321"); | ||
|
|
||
| let big_out = Command::new("getfattr") | ||
| .args(["-n", "user.m_big", "--only-values", "--absolute-names"]) |
There was a problem hiding this comment.
the getfattr + assert block is repeated 5 times in this file, a small helper please :)
| /// 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> { |
There was a problem hiding this comment.
same probe as in the uucore unit test and test_cp, please share it (uutests?)
There was a problem hiding this comment.
Done, moved to uutests::util::tmpfs_to_target_failing_xattr_value.
5c5b404 to
6d1d122
Compare
|
This has just changed in latest coreutils, please check |
6d1d122 to
5ccddc9
Compare
I don't think so; I've rebased to upstream, but it doesn't do anything for me. |
|
In GNU coreutils there were changes to mv, can you check it's still ok ? |
This branch has not been deployed
3 participants
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.mvno longer aborts the directory path on a reported failure.cpuses a silent marker so the already-reported per-attribute diagnostics are not echoed a second time.Fixes #14598.