Re: pg_regress: lookup shellprog in $PATH

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Gurjeet Singh <gurjeet(at)singh(dot)im>, Postgres Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_regress: lookup shellprog in $PATH
Date: 2022-08-25 14:13:46
Message-ID: 1469975.1661436826@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> But what we've got is:

> [rhaas pgsql]$ git grep execl\(
> src/bin/pg_ctl/pg_ctl.c: (void) execl("/bin/sh", "/bin/sh", "-c", cmd,
> (char *) NULL);
> src/test/regress/pg_regress.c: execl(shellprog, shellprog, "-c",
> cmdline2, (char *) NULL);

Right. I wouldn't really feel a need to change anything, except
that we have this weird inconsistency between the way pg_ctl does
it and the way pg_regress does it. I think we should settle on
just one way.

> We could do as you propose and I don't think we would be worse off
> than we are today. But I'm confused why the correct formulation
> wouldn't be exactly what POSIX specifies, namely execl(shellprog,
> "sh", "-c", ...). That way, if somebody has a system where they do set
> $SHELL properly but do not have /bin/sh, things would still work.

My point is that that *isn't* what POSIX specifies. They say in so
many words that the path actually used by system(3) is unspecified.
They do NOT say that it's the value of $SHELL, and given that you're
allowed to set $SHELL to a non-POSIX-compatible shell, using that
is really wrong. We've gotten away with it so far because we
resolve $SHELL at build time not run time, but it's still shaky.

Interestingly, if you look at concrete man pages, you tend to find
something else. Linux says

The system() library function uses fork(2) to create a child process
that executes the shell command specified in command using execl(3) as
follows:
execl("/bin/sh", "sh", "-c", command, (char *) 0);

My BSD machines say "the command is handed to sh(1)", without committing
to just how that's found ... but guess what, "which sh" finds /bin/sh.

In any case, I can't find any system(3) that relies on $SHELL,
so my translation wasn't correct according to either the letter
of POSIX or common practice. It's supposed to be more or less
a hard-wired path, they just don't want to commit to which path.

Moreover, leaving aside the question of whether pg_regress'
current behavior is actually bug-compatible with system(3),
what is the argument that it needs to be? We have at this
point sufficient experience with pg_ctl's use of /bin/sh
to be pretty confident that that works everywhere. So let's
standardize on the simpler way, not the more complex way.

(It looks like pg_ctl has used /bin/sh since 6bcce25801c3f
of Oct 2015.)

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-08-25 14:19:39 Re: replacing role-level NOINHERIT with a grant-level option
Previous Message Robert Haas 2022-08-25 13:50:18 Re: pg_regress: lookup shellprog in $PATH