On 30/10/2025 09:17, Bruno Haible wrote:
Collin Funk wrote:
I don't think we should have 'sort' behave differently on GNU/Linux and
OpenBSD, for example. ...
On GNU/Linux:
$ ./src/sort -S 1k --compress-program=missing Makefile | wc -l
sort: could not run compress program 'missing': No such file or directory
30278
On OpenBSD:
$ ./src/sort -S 1k --compress-program=missing Makefile
sort: close failed: /tmp/sortVFCThm: Broken pipe
I agree that the latter way of failing is suboptimal, from the diagnostics
point of view.
The 3 ideas I had were the following:
Another idea is to do what some other callers of posix_spawn do, namely
call find_in_given_path (from Gnulib module 'findprog-in').
This is done in
- gnulib/lib/spawn-pipe.c line 172,
- GNU make:
https://sources.debian.org/src/make-dfsg/4.4.1-2/src/job.c?hl=2450#L2450
The child process has no way of returning ENOENT to
the parent process in posix_spawn if exec fails.
There are three reasons posix_spawn can fail:
A. Reasons tied to the program to be executed (e.g. 'missing').
B. Other reasons before exec().
C. Reasons after exec().
Use of find_in_given_path will produce a decent error message for case A,
independently of the platform.
The error message for case B will still be platform dependent. But this
case is rare, and most often not in the responsibility of the user.
Therefore it should be acceptable that this is platform dependent?
In case C, nothing changes.
Find attached a proposed patch. With it, I get on both GNU/Linux and
OpenBSD:
$ LC_ALL=C ./src/sort -S 1k --compress-program=missing Makefile|wc -l
sort: could not run compress program 'missing': No such file or directory
30279
and the test failure on OpenBSD is gone.
I like that, and it's not too much overhead, either in code or runtime.
I've amended with the following comment and will push later.
thank you!
Padraig
diff --git a/src/sort.c b/src/sort.c
index fe6886d76..548281bb4 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -1066,6 +1066,10 @@ pipe_child (pid_t *pid, int pipefds[2], int tempfd, bool
decompress,
posix_spawnattr_t attr;
posix_spawn_file_actions_t actions;
+ /* Lookup the program before we spawn, so that we consistently
+ handle access issues to COMPRESS_PROGRAM, because on systems
+ that emulate posix_spawn with fork/exec we would otherwise
+ only a generic (fatal) error in that case. */
resolved_compress_program =
find_in_given_path (compress_program, getenv ("PATH"), NULL, false);
if (resolved_compress_program == NULL)
