Re: Non-portable shell code in pg_upgrade tap tests

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Victor Wagner <vitus(at)wagner(dot)pp(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Non-portable shell code in pg_upgrade tap tests
Date: 2018-07-22 14:46:03
Message-ID: 6519.1532270763@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Noah Misch <noah(at)leadboat(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David G. Johnston 2018-07-22 15:22:23 Re: [HACKERS] Bug in to_timestamp().
Previous Message Fabien COELHO 2018-07-22 14:19:50 Re: pgbench: improve --help and --version parsing