From: Tom Lane <tgl@sss.pgh.pa.us>
To: Noah Misch <noah@leadboat.com>
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:46:03 -0400
Message-ID: <6519.1532270763@sss.pgh.pa.us> (raw)
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>
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. 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
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: tgl@sss.pgh.pa.us, noah@leadboat.com, vitus@wagner.pp.ru
Subject: Re: Non-portable shell code in pg_upgrade tap tests
In-Reply-To: <6519.1532270763@sss.pgh.pa.us>
* 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