Re: sort: prefer posix_spawnp to fork and execlp

[Collin Funk]

Pádraig Brady <p...@draigbrady.com> 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.

Collin