more: use OS errors instead of hardcoded locales - #14085
Conversation
Merging this PR will degrade performance by 4.83%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | numfmt_stream_to_si_precision |
330.4 ms | 348 ms | -5.05% |
| ❌ | Simulation | seq_formatted |
166.3 ms | 174.4 ms | -4.62% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing Devel08:more-use-os-error (86a2a6f) with main (259fdfc)2
Footnotes
-
50 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. ↩
-
No successful run was found on
main(71ebb92) during the generation of this report, so 259fdfc was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
|
GNU testsuite comparison: |
| more-error-is-directory = {$path} is a directory. | ||
| more-error-cannot-open-no-such-file = cannot open {$path}: No such file or directory | ||
| more-error-cannot-open-io-error = cannot open {$path}: {$error} | ||
| more-error-is-directory = {$path}: Is a directory |
There was a problem hiding this comment.
Where is the place more-error-is-directory cannot be replaced by os error? (Removing it at another PR is fine).
There was a problem hiding this comment.
File::open() doesn't throw an error when a file is a directory, thus we have to check that manually
There was a problem hiding this comment.
EISDIR (or EPERM) is returned when read was called for directory. So the arm
coreutils/src/uu/more/src/more.rs
Lines 42 to 51 in 92cbd2f
There was a problem hiding this comment.
yeah it's returned on read, but the message will look something like "more: Is a directory", the path will be missing, if we rely on the message that read returns, should we let read's message or the custom locale?
There was a problem hiding this comment.
I'll fix this at different PR using format! or something else.
There was a problem hiding this comment.
You can use the map_err_context function of the FromIo trait. It basically adds context (usually the filename) before the error message
This PR replaces hardcoded error message with native OS error