Re: Non-portable shell code in pg_upgrade tap tests - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Non-portable shell code in pg_upgrade tap tests
Date
Msg-id 6519.1532270763@sss.pgh.pa.us
Whole thread Raw
In response to Re: Non-portable shell code in pg_upgrade tap tests  (Noah Misch <noah@leadboat.com>)
Responses Re: Non-portable shell code in pg_upgrade tap tests  (Noah Misch <noah@leadboat.com>)
List pgsql-hackers
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


pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: pgbench: improve --help and --version parsing
Next
From: "David G. Johnston"
Date:
Subject: Re: [HACKERS] Bug in to_timestamp().