Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #4615: chown isnt showing a message #4758

Merged
merged 14 commits into from
May 24, 2023

Conversation

djedi23
Copy link
Contributor

@djedi23 djedi23 commented Apr 21, 2023

This PR add a message when the current user is not matched by the from argument and the verbosity is increased.
Two different messages are displayed depending if a group is set or not to match the observed behavior on GNU chown.

This PR fixes #4615.

@sylvestre
Copy link
Contributor

Seems that the GNU test isn't fixed:


FAIL: tests/chown/basic
=======================

chown: ownership of 'f' retained as root
--- exp	2023-04-21 06:05:45.448369817 +0000
+++ out	2023-04-21 06:05:45.444369729 +0000
@@ -1 +0,0 @@
-ownership of 'f' retained as root
make[5]: *** [Makefile:21405: tests/test-suite-root.log] Error 1
chown: cannot dereference 'nf': No such file or directory
--- exp	2023-04-21 06:05:45.452369904 +0000
+++ out	2023-04-21 06:05:45.448369817 +0000
@@ -1 +0,0 @@
-failed to change ownership of 'nf' to 0
FAIL tests/chown/basic.sh (exit status: 1)

@djedi23

This comment was marked as resolved.

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/tail-2/inotify-dir-recreate is no longer failing!

@djedi23
Copy link
Contributor Author

djedi23 commented Apr 21, 2023

Printing the message in stdout fixes the first part of the GNU test.
The test still fails on the diagnostics for non existent files...

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/tail-2/inotify-dir-recreate is no longer failing!

@@ -260,6 +260,27 @@ impl ChownExecutor {
}
}
} else {
if self.verbosity.level == VerbosityLevel::Verbose {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move this into a function?

fn print_verbose(&self, path: &Path, uid: u32, gid: Option<u32>) {
    if self.verbosity.level == VerbosityLevel::Verbose {
        match gid {
            Some(gid) => {
                println!(
                    "ownership of {} retained as {}:{}",
                    path.quote(),
                    entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()),
                    entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string()),
                );
            }
            None => {
                println!(
                    "ownership of {} retained as {}",
                    path.quote(),
                    entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()),
                );
            }
        }
    }
}

and call:
print_verbose(path, uid, self.dest_gid.map(|_| meta.gid()));

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/tail-2/inotify-dir-recreate is no longer failing!

@cakebaker cakebaker force-pushed the 4615_chown_isnt_showing_a_message branch from 3678b4a to aabf5de Compare May 22, 2023 05:20
@cakebaker
Copy link
Contributor

With GNU chown 9.3, the following commands:

$ chown -v --from=42 :43 f
$ chown -v --from=:42 :43 f
$ chown -v --from=42:42 :43 f

all output:

group of 'f' retained as dho

uutils chown, on the other hand, returns in all three cases:

ownership of 'f' retained as dho:dho

@djedi23
Copy link
Contributor Author

djedi23 commented May 24, 2023

Corrected.

@cakebaker cakebaker merged commit b29f8b0 into uutils:main May 24, 2023
@cakebaker
Copy link
Contributor

Thanks!

@djedi23 djedi23 deleted the 4615_chown_isnt_showing_a_message branch May 24, 2023 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chown -v --from=42 43 isn't showing a message
3 participants