On 24/10/2025 20:52, Collin Funk wrote:
Pádraig Brady <[email protected]> writes:
The logic in the patch looks sound.
Actually the fall-back logic in maybe_create_temp() looks wrong.Previously any
fork failure (usually due to resource contraints)
would result in a fallback to writing to temp file directly.
Now any posix_spawn() failure results in immediate exit.
Should we treat the --compress-program as fully optional
(which it was anyway in the case of resource contraints)
and always fall-back to writing the temp file directly?
(We'd write an error for each spawn failure of course).
I'm inclined to think we should do that.
An untested patch to do that:
I made that choice after looking at the behavior of
--compress-program=missing:
$ seq 100000 | shuf > input
$ /bin/sort -S 1K --compress-program=missing input | wc -l
couldn't execute compress program: errno 2
[...]
couldn't execute compress program: errno 2
sort: 'missing' [-d] terminated abnormally
couldn't execute compress program: errno 2
0
I guess that is a failure from exec, not fork, though.
Here is the behavior without your proposed change:
$ ./src/sort -S 1K --compress-program=missing input | wc -l
sort: could not run compress program 'missing': No such file or directory
0
$ ./src/sort -S 1K --compress-program=missing input > /dev/null
$ echo $?
2
Here is the behavior with your proposed change:
$ ./src/sort -S 1K --compress-program=missing input | wc -l
sort: could not run compress program 'missing': No such file or directory
100000
$ ./src/sort -S 1K --compress-program=missing input > /dev/null
sort: could not run compress program 'missing': No such file or directory
$ echo $?
0
That seems reasonable to me. But should the exit status be changed? If
the warning is printed to standard error before the sorted lines are
written to standard output, I worry it will never be seen.
Yes that's a fair point re writing to stderr while exiting success.
Though in this case posix_spawn() is conflating many possible errors,
essentially all of which are non fatal, though you would want some
indication of them since they're non usual.
Given we minimize the duplicate errors I'm OK with the warnings.
You want some indication if the use typos the --compress-program at least.
I suppose you could special case certain errnos, but in this case
I think it might be ok to just warn.
Technically this is a change in behavior, so I suppose one would need
to change coreutils.texi describe --compress-program more accurately.
Alternatively one could warn with EAGAIN/ENOMEM and fail otherwise,
but I'm inclined to keep the logic and change the docs,
so that we're consistent over all errnos.
cheers,
Padraig
