From: Noah Misch <noah@leadboat.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Victor Wagner <vitus@wagner.pp.ru>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Non-portable shell code in pg_upgrade tap tests
Date: Sun, 22 Jul 2018 10:38:57 -0700
Message-ID: <20180722173857.GA3998719@rfd.leadboat.com> (raw)
In-Reply-To: <6519.1532270763@sss.pgh.pa.us>
References: <20180720153820.69e9ae6c@fafnir.local.vm>
<17833.1532096747@sss.pgh.pa.us>
<20180720175508.07776219@fafnir.local.vm>
<20015.1532099353@sss.pgh.pa.us>
<20180722072904.GA3951499@rfd.leadboat.com>
<6519.1532270763@sss.pgh.pa.us>
On Sun, Jul 22, 2018 at 10:46:03AM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > I'd say the right way to fix this is the one specified in
> > https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/The-Make-Macro-SHELL.html,
> > in particular:
>
> > Using @SHELL@ means that your makefile will benefit from the same improved
> > shell, such as bash or ksh, that was discovered during configure, so that
> > you aren't fighting two different sets of shell bugs between the two
> > contexts.
>
> The pg_upgrade makefile does in fact use $(SHELL), so it will default to
> whatever shell configure used.
It will not, because we don't set $(SHELL) anywhere. $(SHELL) is not @SHELL@.
In our makefiles, $(SHELL) is always /bin/sh, the GNU make default.
> The bigger picture here is that it's worth maintaining a common coding
> style in this script. Randomly using `...` in some places and $(...)
> in others just increases the reader's cognitive load, as does spelling
> conditional blocks in multiple styles. So I'd have felt these changes
> were appropriate on style grounds even if there were no portability
> complaint.
For 986127e narrowly, I agree. However, I do want folks able to use $(...)
where it saves backslashes; that's more valuable than `...` uniformity. For
commit 0426932, as the comment above the changed code indicates, the
conditional blocks are copies of "configure" code. Consistency with their
source was more valuable than style, particularly since the code is
security-critical.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: noah@leadboat.com, tgl@sss.pgh.pa.us, vitus@wagner.pp.ru
Subject: Re: Non-portable shell code in pg_upgrade tap tests
In-Reply-To: <20180722173857.GA3998719@rfd.leadboat.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox