cp: when copying a read only file, make sure that the xattrs can be set properly#7009
cp: when copying a read only file, make sure that the xattrs can be set properly#7009cakebaker merged 2 commits intouutils:mainfrom sylvestre:xattrs
Conversation
|
GNU testsuite comparison: |
There was a problem hiding this comment.
The following error message is shown when I run the tests: setfacl: a: No such file or directory. And all tests pass, even though I didn't apply the changes to cp.rs.
There was a problem hiding this comment.
indeed, thanks :)
Test improved!
There was a problem hiding this comment.
Hm, the test still passes without the changes applied to cp.rs. Is that the expected behavior?
There was a problem hiding this comment.
nope, it failed on my system
There was a problem hiding this comment.
actually, works on my system too
let me look at this
There was a problem hiding this comment.
the set command wasn't executed, it should be good now :)
---- test_cp::test_cp_preserve_xattr_readonly_source stdout ----
touch: /tmp/.tmpm2KYKU/a
run: /home/sylvestre/dev/debian/coreutils/target/debug/coreutils cp --preserve=xattr /tmp/.tmpm2KYKU/a /tmp/.tmpm2KYKU/e
thread 'test_cp::test_cp_preserve_xattr_readonly_source' panicked at tests/by-util/test_cp.rs:5988:10:
Command was expected to succeed. code: 1
stdout =
stderr = cp: Permission denied (os error 13)
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
tests/by-util/test_cp.rs
Outdated
| let xattr_key = "user.test"; | ||
| let xattr_value = "value"; |
There was a problem hiding this comment.
As the test is relatively long it might make sense to get rid of these variables to improve the readability and use the values directly on lines 5946 and 5948.
Update: I think it only makes sense to get rid of xattr_value. See my other comment about the use of xattr_key.
tests/by-util/test_cp.rs
Outdated
| stdout.contains("user.test"), | ||
| "Expected 'user.test' not found in getfattr output:\n{}", |
There was a problem hiding this comment.
Here you can use the xattr_key var you defined above.
| stdout.contains("user.test"), | |
| "Expected 'user.test' not found in getfattr output:\n{}", | |
| stdout.contains(xattr_key), | |
| "Expected '{xattr_key}' not found in getfattr output:\n{}", |
cakebaker
left a comment
There was a problem hiding this comment.
Overall it looks good :)
…et properly on the destination Should fix tests/misc/xattr.sh
|
GNU testsuite comparison: |
Cool :) |
on the destination
Should fix tests/misc/xattr.sh