cat: diagnose splice errors

[MuntasirSZN]

Note

Needs #12832

The splice() fast path swallowed every error as a silent read/write
fallback, so a splice failure after some bytes were copied (e.g. an
EIO injected by strace in tests/cat/splice.sh) produced no diagnostic.
Once any data has been copied, treat a splice error as fatal and report
it as cat: <input>: … on the read side or cat: write error: … on the
write side; failures before the first byte still
fall back to read/write. Write errors elsewhere now also omit the input
file name.

Now tests/cat/splice.sh passes.

Signed-off-by: MuntasirSZN muntasir.joypurhat@gmail.com

[codspeed-hq]

Merging this PR will not alter performance

✅ 347 untouched benchmarks
⏩ 46 skipped benchmarks¹

---

_{Comparing MuntasirSZN:splice-cat (aed178a) with main (a8efa79)}

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. ↩

[sylvestre]

Please add a test to make sure we don't regress in the future

[MuntasirSZN]

Done @sylvestre

[oech3]

I don't agree with this approach. This should be shared at uucore for all utils. Also distioction for read/write is not splice specific.

[oech3]

Why cannot we simply use .map_err_context?

[oech3]

Now tests/cat/splice.sh passes.

Note that error distinction is disabled until next releas of GNU at backporting at https://github.com/uutils/coreutils/blob/main/util/fetch-gnu.sh .
The test is failing by different reason (probably due to CI specific reason?, passing locally without PR).

[oech3]

I meant we should improve existing code. Not introducing new fn. We probably should have several PRs for it instead of 1 PR e.g. https://github.com/uutils/coreutils/pull/12832/changes

[MuntasirSZN]

This means we need to wait for your pr + anything that I might create then this one.

[MuntasirSZN]

Dropped the new uucore function and specialized types — the loop now lives in cat with the first cycle split out, and write errors are built via .map_err_context(||
translate!("common-write-error")) (shared UIoError). CatError::Write is only a marker so cat_files doesn't prefix the input filename during multi-file aggregation. Once #12832 and
the read/write separation in uucore::pipes land, I'll switch cat to the improved shared helper and delete the local copy.

[MuntasirSZN]

Now tests/cat/splice.sh passes.

Note that error distinction is disabled until next releas of GNU at backporting at https://github.com/uutils/coreutils/blob/main/util/fetch-gnu.sh .
The test is failing by different reason (probably due to CI specific reason?, passing locally without PR).

Hmm, but not for me? For me locally without pr it doesn't pass.

[MuntasirSZN]

@xtqqczze I cannot resolve review that is conflicting, @oech3 is asking for something and you are pushing against it (which he pushed against before) pls read the resolved review comments....

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/cat/splice is no longer failing!
Congrats! The gnu test tests/cut/mb-non-utf8 is no longer failing!
Congrats! The gnu test tests/mv/mv-exchange is no longer failing!
Congrats! The gnu test tests/od/od-j is no longer failing!
Congrats! The gnu test tests/dd/no-allocate is now passing!
Note: The gnu test tests/misc/write-errors was skipped on 'main' but is now failing.

[oech3]

I still don't understand this large diff just for appending a new message to existing error message.
New function, enum and macro is unnecessary. No?

[MuntasirSZN]

I still don't understand this large diff just for appending a new message to existing error message.
New function, enum and macro is unnecessary. No?

Function, it is (will move to uucore once your prs are done). Enum, no, because it is the idiomatic way as Errors should be in a large enum. Macro, well sylvestre asked for it.

[oech3]

We should remove CatResult and CatError before fixing this to avoid producing specialized types for mostly all utils at least for.

[MuntasirSZN]

We should remove CatResult and CatError before fixing this to avoid producing specialized types for mostly all utils at least for.

Isn't that a good thing? In uucore it can have its own error, though CatResult and CatError isn't created by me, so its the maintainers decision.

[oech3]

If we want to share logic for error with file path and distinction, utility specific abstriction is large barrier for sharing code base in my opinion.

[xtqqczze]

I don’t have any further comments regarding this PR.