From: Tom Lane <tgl@sss.pgh.pa.us>
To: Noah Misch <noah@leadboat.com>
cc: Victor Wagner <vitus@wagner.pp.ru>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Non-portable shell code in pg_upgrade tap tests
In-reply-to: <20180722072904.GA3951499@rfd.leadboat.com>
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>
Comments: In-reply-to Noah Misch <noah@leadboat.com>
	message dated "Sun, 22 Jul 2018 07:29:04 -0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6518.1532270763.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Sun, 22 Jul 2018 10:46:03 -0400
Message-ID: <6519.1532270763@sss.pgh.pa.us>
Precedence: bulk

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 impr=
oved
>   shell, such as bash or ksh, that was discovered during configure, so t=
hat
>   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.  However, I'd expect the Autoconf guys
to target a pretty darn low common denominator shell-wise.  It's unlikely
that the patches we just repaired were completely port-tested before
commit, and we now know that the buildfarm has been falling down on the
job as well.  (Partly my fault, since gaur/pademelon don't run the
TAP tests either.  I'm hesitant to triple their already long cycle
times, but ...)

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.

In the long run, no doubt it'd be better to rewrite test.sh in Perl,
but this is what we've got today.

			regards, tom lane