Skip to content

fix(chown,chgrp): print --verbose success lines to stdout - #13419

Merged
cakebaker merged 2 commits into
uutils:mainfrom
l46983284-cpu:fix/chown-chgrp-verbose-stdout-gnu-compat
Jul 18, 2026
Merged

fix(chown,chgrp): print --verbose success lines to stdout#13419
cakebaker merged 2 commits into
uutils:mainfrom
l46983284-cpu:fix/chown-chgrp-verbose-stdout-gnu-compat

Conversation

@l46983284-cpu

Copy link
Copy Markdown
Contributor

Summary

chown/chgrp --verbose was printing successful changed/retained lines to stderr with a chown:/chgrp: prefix because report_ownership_change_success and the wrap_chown Ok path used show_error!.

GNU prints those informational lines to stdout with no util-name prefix. This PR routes success-path verbose/changes output through println! and leaves real errors on stderr.

Changes

  • src/uucore/src/lib/features/perms.rs: success verbose/changes → stdout
  • tests/by-util/test_chown.rs, tests/by-util/test_chgrp.rs: assert retained/changed lines on stdout

Test plan

  • CI test_chown / test_chgrp
  • Manual: chgrp --verbose "$(id -gn)" file >out 2>err → line in out, empty err, no chgrp: prefix

Fixes #13408

@codspeed-hq

codspeed-hq Bot commented Jul 16, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 339 untouched benchmarks
⏩ 46 skipped benchmarks1


Comparing l46983284-cpu:fix/chown-chgrp-verbose-stdout-gnu-compat (d97fdbd) with main (b40aaf6)

Open in CodSpeed

Footnotes

  1. 46 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.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/tail-n0f (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/retry (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout-group (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/rm/many-dir-entries-vs-OOM is now being skipped but was previously passing.
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would use writeln! instead of println!, otherwise the example from #13408 will panic:

$ cargo run --features=unix -q chgrp --verbose "$(id -gn)" "$tmp/f" > /dev/full ; echo "exit=$?"
chgrp: cannot dereference '/f': No such file or directory

thread 'main' (79884) panicked at /rustc/2d8144b7880597b6e6d3dfd63a9a9efae3f533d3/library/std/src/io/stdio.rs:1166:9:
failed printing to stdout: No space left on device (os error 28)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
exit=101

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks — switched the verbose success path to writeln! so a full/closed stdout reports a write error instead of panicking on println!.

Pushed as a small helper in perms.rs; focused cargo check / clippy on uu_chown + uu_chgrp are green.

Main already writes some verbose lines with writeln! and reports
/dev/full write errors, but successful retained/changed messages from
wrap_chown and report_ownership_change_success still went through
show_error! (stderr + util-name prefix).

Route those informational lines through write_verbose_line so GNU-style
stdout output is preserved and write failures still surface on flush.
Update chown/chgrp tests accordingly.

Fixes uutils#13408

Signed-off-by: Alex Chen <l46983284@gmail.com>
@l46983284-cpu
l46983284-cpu force-pushed the fix/chown-chgrp-verbose-stdout-gnu-compat branch from 47fd467 to 66bf90a Compare July 17, 2026 17:18
@l46983284-cpu

Copy link
Copy Markdown
Contributor Author

Rebased onto current main.

main already landed the /dev/full non-panic write-error path (058a2a6). This branch now only finishes the remaining GNU gap from #13408: successful --verbose / --changes retained/changed lines go through write_verbose_line (stdout) instead of show_error! (stderr + util-name prefix).

Local: test_chown 33/33 and test_chgrp 31/31 under --features unix.

Returning early after write_verbose_line skipped -R children and
left recursive/symlink-cycle suites looking for retained lines that
were never printed. Write the line and continue into safe_dive/dive.

Also drop accidental backslash-escaped quotes in the recursive test
needles so they match path.quote() output.

Signed-off-by: Alex Chen <l46983284@gmail.com>
@l46983284-cpu

Copy link
Copy Markdown
Contributor Author

Pushed a small residual fix for the recursive path. After routing verbose retained lines to stdout, the root traverse was returning early and never visited children under -R, which is why CI only saw ownership of 'a'/'z'. The root arm now writes the line and continues into dive, and the recursive test needles match path.quote() single quotes again.

Local: test_chown::test_chown_recursive + test_chown_symlink_cycles PASS; full test_chown:: was 33/33 earlier.

@cakebaker
cakebaker merged commit fa2cb3c into uutils:main Jul 18, 2026
157 of 159 checks passed
@cakebaker

Copy link
Copy Markdown
Contributor

Thanks for your PR!

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.

chgrp/chown --verbose prints "changed/retained" lines to stderr with a util-name prefix instead of stdout

2 participants