fix #4767: chown -v 0 nf isn't showing a message

[djedi23]

This PR modify chown to print a message to stdout when the file doesn't exist.
It's a fix for #4767 .

The following cases are implemented:

chown -v 0 nf 2> /dev/null
failed to change ownership of 'nf' to 0

$ chown -v user nf
failed to change ownership of 'nf' to user

$ chown -v user:group nf
failed to change ownership of 'nf' to user:group

$ touch f
$ chown -v --reference=f nf
failed to change ownership of 'nf' to user:group

To access the owner as specified by the user in the command, I had to modify the signature of the tuple returns by parse_gid_uid_and_filter and add a field in the struct ChownExecutor.

Note: the first part of the GNU tests/chown/basic is expected to fail (ownership of 'f' retained as root) but the rest is not.

[tertsdiepraam]

Cool! I like it, but I think we can do a bit of cleanup.

[tertsdiepraam]

Maybe it makes sense to make a struct for the (Option<u32>, Option<u32>, String, IfFrom)? That would make it a bit easier to read (and make the types shorter).

[tertsdiepraam]

Also the String might need to be an OsString if it can contain invalid utf-8.

[djedi23]

The raw_owner: String is constructed by either:

• matches.get_one::<String>( ... ) or
• format of entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string())

I don't know if these two functions are robust enough to invalid utf-8.
If not, it might imply to refactor at least uucore/*/perms.rs, uu/*/chown.rs, uu/*/chgrp.rs. Then it should be useful to open a dedicated issue for it.

[djedi23]

I refactored (Option<u32>, Option<u32>, String, IfFrom) to a struct GidUidOwnerFilter.

[tertsdiepraam]

Interesting that this message is different from the one that was commented out. Was that just a mistake?

[tertsdiepraam]

You could also add this assertion to the rest of the expression:

scene
    .ucmd()
    ...
    .fails()
    .stdout_contains(...);

[djedi23]

The actual test tests a non existing file with verbose flag:

 chown -v 0 nf

The string "retained as" is printed by the command:

touch f
chown -v --from=42 43 f

These two cases came from the GNU test suite: tests/chown/basic.sh

[sylvestre]

I guess you know but the gnu test still fails with:

2023-04-23T08:02:21.3828552Z 
2023-04-23T08:02:21.3828634Z FAIL: tests/chown/basic
2023-04-23T08:02:21.3828814Z =======================
2023-04-23T08:02:21.3829111Z 
2023-04-23T08:02:21.3829286Z --- exp	2023-04-23 08:02:10.738059549 +0000
2023-04-23T08:02:21.3829573Z +++ out	2023-04-23 08:02:10.734059549 +0000
2023-04-23T08:02:21.3829808Z @@ -1 +0,0 @@
2023-04-23T08:02:21.3830053Z -ownership of 'f' retained as root
2023-04-23T08:02:21.3830396Z chown: cannot dereference 'nf': No such file or directory
2023-04-23T08:02:21.3830676Z FAIL tests/chown/basic.sh (exit status: 1)
2023-04-23T08:02:21.3830825Z 

[djedi23]

There are two tests failing in tests/chown/basic:

# Make sure the correct diagnostic is output
# Note we output a name even though an id was specified.
chown -v --from=42 43 f > out || fail=1
printf "ownership of 'f' retained as $(id -nu)\n" > exp
compare exp out || fail=1

# Ensure diagnostics work for non existent files.
returns_ 1 chown -v 0 nf > out || fail=1
printf "failed to change ownership of 'nf' to 0\n" > exp
compare exp out || fail=1

This PR fix the second test: "failed to change ownership of"
The first one ("ownership of 'f' retained as") is fixed by the PR #4758.

So when theses two PRs will be merged to main, the gnu test will pass.

[github-actions]

GNU testsuite comparison:

GNU test failed: tests/tail-2/inotify-dir-recreate. tests/tail-2/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?

[cakebaker]

Thanks for your PR :)