coreutils icon indicating copy to clipboard operation
coreutils copied to clipboard

nohup: added a bunch of nohup fixes based on the failures in the GNU tests

Open ChrisDryden opened this issue 2 weeks ago • 4 comments

This is a bit of a bigger PR but the original goal was to try and add the support of always setting the umask of the output file and when running the tests against the GNU tests I found a few inconsistencies that I'm going to summarize below:

First is that the GNU implementation will always try to set umask of nohop.out to 600 which we werent doing in our tests, I added a wrapper to it to make it always set that way.

Second the error message is based on whether the input or/and the output terminal is a terminal, the current implementation printed the same error message for all use cases and I added error messages specific to each use case.

Third, the error code for failure is set based on the ENV variable POSIXLY_CORRECT which we were already doing in some cases but we need to do that for all cases where it would return a 125 failure

Lastly the show_error macro didn't work for this use case because its possible to fail the output to stdout and then its expected to give a different error code based on whether the stderr was written successfully.

I added rust integ tests for each of these scenarios too, the way the tests are set up in nohup is a bit all over the place, when this gets merged in I'd like to make them all use the same macros and reduce the verbosity a bit

ChrisDryden avatar Dec 10 '25 17:12 ChrisDryden

GNU testsuite comparison:

Congrats! The gnu test tests/misc/nohup is no longer failing!

github-actions[bot] avatar Dec 10 '25 17:12 github-actions[bot]

CodSpeed Performance Report

Merging #9628 will not alter performance

Comparing ChrisDryden:nohup_gnu_fixes (0aeea3b) with main (2000af8)

Summary

✅ 127 untouched
⏩ 6 skipped[^skipped]

[^skipped]: 6 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.

codspeed-hq[bot] avatar Dec 10 '25 18:12 codspeed-hq[bot]

GNU testsuite comparison:

Congrats! The gnu test tests/misc/nohup is no longer failing!

github-actions[bot] avatar Dec 10 '25 18:12 github-actions[bot]

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/misc/nohup is no longer failing!

github-actions[bot] avatar Dec 10 '25 19:12 github-actions[bot]

GNU testsuite comparison:

Congrats! The gnu test tests/misc/nohup is no longer failing!

github-actions[bot] avatar Dec 12 '25 15:12 github-actions[bot]

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/misc/nohup is no longer failing!

github-actions[bot] avatar Dec 14 '25 19:12 github-actions[bot]

I'm going to try and split this up into many smaller PR's to make it easier to review.

ChrisDryden avatar Dec 17 '25 14:12 ChrisDryden

Made two PR's from the code in here that are subsets of this original PR: https://github.com/uutils/coreutils/pull/9684 and https://github.com/uutils/coreutils/pull/9685

The other code in here relies on the ENV Variable code being merged in first so will wait on that before splitting the rest of the code up.

ChrisDryden avatar Dec 17 '25 15:12 ChrisDryden

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/misc/nohup is no longer failing!
Skip an intermittent issue tests/tail/inotify-dir-recreate (was skipped on 'main', now failing)

github-actions[bot] avatar Dec 17 '25 15:12 github-actions[bot]