Repository navigation
chown, chgrp: report failed -R changes with -v as for operands - #15126
abendrothj wants to merge 4 commits into
Conversation
Merging this PR will degrade performance by 19.67%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | three_39_bit_primes |
583.2 ms | 943.5 ms | -38.19% |
| ❌ | Simulation | five_38_bit_primes |
1.6 s | 1.8 s | -10.01% |
| ❌ | Memory | du_all_wide_tree[(5000, 500)] |
1,010.5 KB | 1,084.6 KB | -6.83% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing abendrothj:chown-verbose-failures (a37ca9a) with main (6292c03)
Footnotes
-
54 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. ↩
| // As for an operand, see `report_chown`. | ||
| if self.verbosity.level == VerbosityLevel::Verbose { | ||
| let from = (owner.uid(), owner.gid()); | ||
| let to = ( |
There was a problem hiding this comment.
same dest_uid.unwrap_or / failed_change_line code as in report_replaced, could be dedup into a small helper, no?
There was a problem hiding this comment.
done, both go through a small failed_line helper now.
| at.mkdir("dir"); | ||
| at.touch("dir/file"); | ||
|
|
||
| ucmd.args(&["-R", "-v", "root", "dir"]) |
There was a problem hiding this comment.
this will pass for root and the test fails, please skip it when geteuid().is_root()
There was a problem hiding this comment.
done, it returns early as root. checked as root in a container: it failed before and is skipped now.
3fd3860 to
b99a774
Compare
b99a774 to
efea4f3
Compare
--from was checked on a stat of the name and the change was then made by name, so a rename landing in between changed a file that never had the required owner. On Linux, hold the file open with O_PATH, check that it is the one that was stat'd and still passes --from, and change it through that descriptor with fchownat(AT_EMPTY_PATH). This covers operands and -R entries. A file found replaced is left alone; as with GNU, that fails without a message, and -v reports the change that was not made.
On Linux, a directory operand was changed through a descriptor opened for reading, and when that open failed it was skipped without a message. Change it by name then, or through an O_PATH descriptor with --from, as for any other file. Under -R it is still left alone and the failure to read it reported, as with GNU.
Under -R -H, a symlink met in the tree was stat'd itself, while the change went to the file it points to. --from was judged on, and -v reported, the link's owner. Use the target's, as GNU does.
With -R on Linux, a change below the operand that failed was reported without the "failed to change" line that -v adds for the operand and on other platforms.
efea4f3 to
a37ca9a
Compare
On Linux,
chown -R -vprinted no "failed to change ownership of …" line for an entry below the operand whose change failed; the operand itself got one. GNU 9.12 prints it for every file.-R entries now get the same -v failure line as the operand. Like the operand's, it goes to stderr; GNU prints these on stdout, which is the existing TODO in test_chown.rs and isn't changed here. Test:
test_chown_recursive_verbose_reports_failed_entry.Based on #15125.
Closes #15115