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.
Bruno
>From c60fb5798e002d1b7d76056bd0d64f5c5091568a Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email protected]>
Date: Thu, 30 Oct 2025 10:15:09 +0100
Subject: [PATCH] sort: Report problems with the compress program before
posix_spawnp
* bootstrap.conf (gnulib_modules): Add findprog-in.
* src/sort.c: Include findprog.h.
(pipe_child): Look up the compress_program in PATH and report errors
such as ENOENT or EACCES before invoking posix_spawnp.
---
bootstrap.conf | 1 +
src/sort.c | 25 ++++++++++++++++++++++---
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/bootstrap.conf b/bootstrap.conf
index 6654f61ef..3a5a4cfa8 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -107,6 +107,7 @@ gnulib_modules="
filemode
filenamecat
filevercmp
+ findprog-in
flexmember
fnmatch-gnu
fopen-safer
diff --git a/src/sort.c b/src/sort.c
index 7127f671b..fcf427d72 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -36,6 +36,7 @@
#include "c-ctype.h"
#include "fadvise.h"
#include "filevercmp.h"
+#include "findprog.h"
#include "flexmember.h"
#include "hard-locale.h"
#include "hash.h"
@@ -1034,6 +1035,8 @@ static int
pipe_child (pid_t *pid, int pipefds[2], int tempfd, bool decompress,
size_t tries)
{
+ char const *resolved_compress_program;
+ char *compress_program_to_free;
struct tempnode *saved_temphead;
double wait_retry = 0.25;
struct cs_status cs;
@@ -1041,12 +1044,21 @@ pipe_child (pid_t *pid, int pipefds[2], int tempfd, bool decompress,
posix_spawnattr_t attr;
posix_spawn_file_actions_t actions;
+ resolved_compress_program =
+ find_in_given_path (compress_program, getenv ("PATH"), NULL, false);
+ if (resolved_compress_program == NULL)
+ return errno;
+ compress_program_to_free = nullptr;
+ if (resolved_compress_program != compress_program)
+ compress_program_to_free = (char *) resolved_compress_program;
+
if ((result = posix_spawnattr_init (&attr)))
return result;
if ((result = posix_spawnattr_setflags (&attr, POSIX_SPAWN_USEVFORK))
|| (result = posix_spawn_file_actions_init (&actions)))
{
posix_spawnattr_destroy (&attr);
+ free (compress_program_to_free);
return result;
}
@@ -1055,6 +1067,7 @@ pipe_child (pid_t *pid, int pipefds[2], int tempfd, bool decompress,
int saved_errno = errno;
posix_spawnattr_destroy (&attr);
posix_spawn_file_actions_destroy (&actions);
+ free (compress_program_to_free);
return saved_errno;
}
@@ -1080,11 +1093,16 @@ pipe_child (pid_t *pid, int pipefds[2], int tempfd, bool decompress,
close (pipefds[1]);
posix_spawnattr_destroy (&attr);
posix_spawn_file_actions_destroy (&actions);
+ free (compress_program_to_free);
return result;
}
- char const *const argv[] = { compress_program, decompress ? "-d" : nullptr,
- nullptr };
+ char const *const argv[] =
+ {
+ resolved_compress_program,
+ decompress ? "-d" : nullptr,
+ nullptr
+ };
/* At least NMERGE + 1 subprocesses are needed. More could be created, but
uncontrolled subprocess generation can hurt performance significantly.
@@ -1104,7 +1122,7 @@ pipe_child (pid_t *pid, int pipefds[2], int tempfd, bool decompress,
saved_temphead = temphead;
temphead = nullptr;
- result = posix_spawnp (pid, compress_program, &actions, &attr,
+ result = posix_spawnp (pid, resolved_compress_program, &actions, &attr,
(char * const *) argv, environ);
temphead = saved_temphead;
@@ -1125,6 +1143,7 @@ pipe_child (pid_t *pid, int pipefds[2], int tempfd, bool decompress,
posix_spawnattr_destroy (&attr);
posix_spawn_file_actions_destroy (&actions);
+ free (compress_program_to_free);
if (result)
{
--
2.51.0
