pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedNon-portable shell code in pg_upgrade tap tests
17+ messages / 6 participants
[nested] [flat]
Commits from this thread:
2018-07-21 dfbad3f454 04269320ae 04e3492c5c 2cc81f95e6 b73ab5b4b5 b782b7c57f ab57f48f68 Further portability hacking in pg_upgrade's test script.
2018-07-21 0c0908d926 5e5d131202 Be more paranoid about quoting in pg_upgrade's test script.
2018-07-20 986127e3db e6105686dc Avoid unportable shell syntax in pg_upgrade's test script.
* Non-portable shell code in pg_upgrade tap tests
@ 2018-07-20 12:38 Victor Wagner <vitus@wagner.pp.ru>
0 siblings, 1 reply; 17+ messages in thread
From: Victor Wagner @ 2018-07-20 12:38 UTC (permalink / raw)
To: pgsql-hackers
Collegues,
I've discovered that in the branch REL_11_STABLE there is shell script
src/bin/pg_upgrade/test.sh which doesn't work under Solaris 10.
(it uses $(command) syntax with is not compatible with original
Solaris /bin/sh)
I was quite surprised that this problem goes unnoticed on big buildfarm,
but after some investigation found out that both Solaris machines in
that buildfarm don't configure postgres with --enable-tap-tests.
Offending code is:
# make sure all directories and files have group permissions, on Unix hosts
# Windows hosts don't support Unix-y permissions.
case $testhost in
MINGW*) ;;
*) if [ $(find ${PGDATA} -type f ! -perm 640 | wc -l) -ne 0 ]; then
echo "files in PGDATA with permission != 640";
exit 1;
fi ;;
esac
case $testhost in
MINGW*) ;;
*) if [ $(find ${PGDATA} -type d ! -perm 750 | wc -l) -ne 0 ]; then
echo "directories in PGDATA with permission != 750";
exit 1;
fi ;;
esac
It is quite easy to replace $() syntax with backticks. Backticks are
not nestable and considered unsafe by modern shell scripting style
guides, but they do work with older shells.
--
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-20 14:25 Tom Lane <tgl@sss.pgh.pa.us>
parent: Victor Wagner <vitus@wagner.pp.ru>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2018-07-20 14:25 UTC (permalink / raw)
To: Victor Wagner <vitus@wagner.pp.ru>; +Cc: pgsql-hackers
Victor Wagner <vitus@wagner.pp.ru> writes:
> I've discovered that in the branch REL_11_STABLE there is shell script
> src/bin/pg_upgrade/test.sh which doesn't work under Solaris 10.
> (it uses $(command) syntax with is not compatible with original
> Solaris /bin/sh)
OK ...
> It is quite easy to replace $() syntax with backticks. Backticks are
> not nestable and considered unsafe by modern shell scripting style
> guides, but they do work with older shells.
Please send a patch. Most of us do not have access to old shells
we could test this on. (The oldest machine I have, and it's old
enough to vote, does run that script ... I doubt very many other
developers have anything older.)
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-20 14:55 Victor Wagner <vitus@wagner.pp.ru>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 17+ messages in thread
From: Victor Wagner @ 2018-07-20 14:55 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
On Fri, 20 Jul 2018 10:25:47 -0400
Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Victor Wagner <vitus@wagner.pp.ru> writes:
> > I've discovered that in the branch REL_11_STABLE there is shell
> > script src/bin/pg_upgrade/test.sh which doesn't work under Solaris
> > 10. (it uses $(command) syntax with is not compatible with original
> > Solaris /bin/sh)
>
> Please send a patch. Most of us do not have access to old shells
Here it goes. Previous letter was written before fixed tests were
completed, because this old machine is slow.
> we could test this on. (The oldest machine I have, and it's old
> enough to vote, does run that script ... I doubt very many other
> developers have anything older.)
>
> regards, tom lane
Attachments:
original_systemV_sh_compatibility.patch (text/x-patch, 1.0K) download
commit efb0bbcefeccd826ba951ac31057689e752b79d0
Author: Victor Wagner <vitus@wagner.pp.ru>
Date: Fri Jul 20 16:06:50 2018 +0300
Fixed incompatibility of src/bin/pg_upgrade/test.sh with solaris /bin/sh
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 45ccd8fa66..775dd5729d 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -234,7 +234,7 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B
# Windows hosts don't support Unix-y permissions.
case $testhost in
MINGW*) ;;
- *) if [ $(find ${PGDATA} -type f ! -perm 640 | wc -l) -ne 0 ]; then
+ *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
echo "files in PGDATA with permission != 640";
exit 1;
fi ;;
@@ -242,7 +242,7 @@ esac
case $testhost in
MINGW*) ;;
- *) if [ $(find ${PGDATA} -type d ! -perm 750 | wc -l) -ne 0 ]; then
+ *) if [ `find ${PGDATA} -type d ! -perm 750 | wc -l` -ne 0 ]; then
echo "directories in PGDATA with permission != 750";
exit 1;
fi ;;
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-20 15:09 Tom Lane <tgl@sss.pgh.pa.us>
parent: Victor Wagner <vitus@wagner.pp.ru>
1 sibling, 1 reply; 17+ messages in thread
From: Tom Lane @ 2018-07-20 15:09 UTC (permalink / raw)
To: Victor Wagner <vitus@wagner.pp.ru>; +Cc: pgsql-hackers
Victor Wagner <vitus@wagner.pp.ru> writes:
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Please send a patch. Most of us do not have access to old shells
> Here it goes. Previous letter was written before fixed tests were
> completed, because this old machine is slow.
Thanks. Will check on my own dinosaurs, and push if I don't find
a problem.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-20 21:18 Tels <nospam-pg-abuse@bloodgate.com>
parent: Victor Wagner <vitus@wagner.pp.ru>
1 sibling, 1 reply; 17+ messages in thread
From: Tels @ 2018-07-20 21:18 UTC (permalink / raw)
To: Victor Wagner <vitus@wagner.pp.ru>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers
Moin,
On Fri, July 20, 2018 10:55 am, Victor Wagner wrote:
> On Fri, 20 Jul 2018 10:25:47 -0400
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
>> Victor Wagner <vitus@wagner.pp.ru> writes:
>> > I've discovered that in the branch REL_11_STABLE there is shell
>> > script src/bin/pg_upgrade/test.sh which doesn't work under Solaris
>> > 10. (it uses $(command) syntax with is not compatible with original
>> > Solaris /bin/sh)
>
>>
>> Please send a patch. Most of us do not have access to old shells
>
> Here it goes. Previous letter was written before fixed tests were
> completed, because this old machine is slow.
+ *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
Shouldn't ${PGDATA} in the above as argument to find be quoted, otherwise
the shell would get confused if it contains spaces or other special
characters?
Regards,
Tels
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-21 15:29 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tels <nospam-pg-abuse@bloodgate.com>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2018-07-21 15:29 UTC (permalink / raw)
To: Tels <nospam-pg-abuse@bloodgate.com>; +Cc: Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
"Tels" <nospam-pg-abuse@bloodgate.com> writes:
> + *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
> Shouldn't ${PGDATA} in the above as argument to find be quoted, otherwise
> the shell would get confused if it contains spaces or other special
> characters?
Hmm. Yeah, probably. I don't think this script is meant to be run with
arbitrary values of PGDATA, but most of the other uses are quoted, so
for consistency's sake this should be too.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-21 16:47 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 17+ messages in thread
From: ilmari@ilmari.org @ 2018-07-21 16:47 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Tels <nospam-pg-abuse@bloodgate.com>; Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
Tom Lane <tgl@sss.pgh.pa.us> writes:
> "Tels" <nospam-pg-abuse@bloodgate.com> writes:
>> + *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
>
>> Shouldn't ${PGDATA} in the above as argument to find be quoted, otherwise
>> the shell would get confused if it contains spaces or other special
>> characters?
>
> Hmm. Yeah, probably. I don't think this script is meant to be run with
> arbitrary values of PGDATA, but most of the other uses are quoted, so
> for consistency's sake this should be too.
PGDATA is built from `pwd`, so it breaks if the build directory has a
space in it. Because it's testing for the absence of files, it doesn't
actually break the test, but would fail to detect the bugs it's trying
to.
+ find /home/ilmari/src/post gresql/src/bin/pg_upgrade/tmp_check/data -type f ! -perm 640
+ wc -l
find: ‘/home/ilmari/src/post’: No such file or directory
find: ‘gresql/src/bin/pg_upgrade/tmp_check/data’: No such file or directory
+ [ 0 -ne 0 ]
+ find /home/ilmari/src/post gresql/src/bin/pg_upgrade/tmp_check/data -type d ! -perm 750
+ wc -l
find: ‘/home/ilmari/src/post’: No such file or directory
find: ‘gresql/src/bin/pg_upgrade/tmp_check/data’: No such file or directory
+ [ 0 -ne 0 ]
Attached is a patch fixing this. I checked the rest of the script, and
this seems to be the only place lacking quoting.
- ilmari
--
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
the consequences of." -- Skud's Meta-Law
Attachments:
0001-Quote-PGDATA-in-pg_upgrade-test.sh.patch (text/x-diff, 1.2K) download
From cabd43aa1988fb9f33743981266c9bf2278681a2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Sat, 21 Jul 2018 17:42:39 +0100
Subject: [PATCH] Quote ${PGDATA} in pg_upgrade/test.sh
---
src/bin/pg_upgrade/test.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 775dd5729d..0e285c5c17 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -234,7 +234,7 @@ pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B
# Windows hosts don't support Unix-y permissions.
case $testhost in
MINGW*) ;;
- *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
+ *) if [ `find "${PGDATA}" -type f ! -perm 640 | wc -l` -ne 0 ]; then
echo "files in PGDATA with permission != 640";
exit 1;
fi ;;
@@ -242,7 +242,7 @@ esac
case $testhost in
MINGW*) ;;
- *) if [ `find ${PGDATA} -type d ! -perm 750 | wc -l` -ne 0 ]; then
+ *) if [ `find "${PGDATA}" -type d ! -perm 750 | wc -l` -ne 0 ]; then
echo "directories in PGDATA with permission != 750";
exit 1;
fi ;;
--
2.18.0
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-21 18:04 Tom Lane <tgl@sss.pgh.pa.us>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
1 sibling, 0 replies; 17+ messages in thread
From: Tom Lane @ 2018-07-21 18:04 UTC (permalink / raw)
To: ilmari@ilmari.org; +Cc: Tels <nospam-pg-abuse@bloodgate.com>; Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Hmm. Yeah, probably. I don't think this script is meant to be run with
>> arbitrary values of PGDATA, but most of the other uses are quoted, so
>> for consistency's sake this should be too.
> Attached is a patch fixing this. I checked the rest of the script, and
> this seems to be the only place lacking quoting.
Done already, but thanks.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-21 22:49 Tels <nospam-pg-abuse@bloodgate.com>
parent: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
1 sibling, 1 reply; 17+ messages in thread
From: Tels @ 2018-07-21 22:49 UTC (permalink / raw)
To: "Dagfinn Ilmari Mannsåker" <ilmari@ilmari.org>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
Moin,
On Sat, July 21, 2018 12:47 pm, Dagfinn Ilmari Mannsåker wrote:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>
>> "Tels" <nospam-pg-abuse@bloodgate.com> writes:
>>> + *) if [ `find ${PGDATA} -type f ! -perm 640 | wc -l` -ne 0 ]; then
>>
>>> Shouldn't ${PGDATA} in the above as argument to find be quoted,
>>> otherwise
>>> the shell would get confused if it contains spaces or other special
>>> characters?
>>
>> Hmm. Yeah, probably. I don't think this script is meant to be run with
>> arbitrary values of PGDATA, but most of the other uses are quoted, so
>> for consistency's sake this should be too.
>
> PGDATA is built from `pwd`, so it breaks if the build directory has a
> space in it. Because it's testing for the absence of files, it doesn't
> actually break the test, but would fail to detect the bugs it's trying
> to.
Thanx for the fix. But perhaps I should have been a bit more specific in
my first message, spaces are not the only character this can break at.
Looking at your new patch, I notice you used "" for quoting, not ''. (Not
sure which variant Tom used when pushing a patch).
I'm not a shell expert, but I think '' are safer, as "" still has some
interpolation from the shell (at least on the systems I use regulary):
te@pc:~$ mkdir 'test$test'
te@pc:~$ touch 'test$test/test'
te@pc:~$ find 'test$test/test' -type f
test$test/test
te@pc:~$ find "test$test/test" -type f
find: ‘test/test’: No such file or directory
So I've grown the habit to always use '' instead of "". Not sure if this
is specific to bash vs. sh or PC vs Mac, tho.
Best wishes,
Tels
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-21 23:20 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tels <nospam-pg-abuse@bloodgate.com>
0 siblings, 0 replies; 17+ messages in thread
From: Tom Lane @ 2018-07-21 23:20 UTC (permalink / raw)
To: Tels <nospam-pg-abuse@bloodgate.com>; +Cc: "Dagfinn Ilmari Mannsåker" <ilmari@ilmari.org>; Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
"Tels" <nospam-pg-abuse@bloodgate.com> writes:
> Looking at your new patch, I notice you used "" for quoting, not ''. (Not
> sure which variant Tom used when pushing a patch).
> I'm not a shell expert, but I think '' are safer, as "" still has some
> interpolation from the shell (at least on the systems I use regulary):
We can't do that here because '' would suppress interpolation of the
variable's value, which is sort of the point.
AFAIK, the locution "$foo" is safe regardless of what is in $foo,
as long as only one pass of shell evaluation is involved. The shell
will treat the value of $foo as one not-further-interpreted command
argument.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-22 07:29 Noah Misch <noah@leadboat.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 17+ messages in thread
From: Noah Misch @ 2018-07-22 07:29 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
On Fri, Jul 20, 2018 at 11:09:13AM -0400, Tom Lane wrote:
> Victor Wagner <vitus@wagner.pp.ru> writes:
> > Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Please send a patch. Most of us do not have access to old shells
>
> > Here it goes. Previous letter was written before fixed tests were
> > completed, because this old machine is slow.
>
> Thanks. Will check on my own dinosaurs, and push if I don't find
> a problem.
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 Solaris 10 /bin/sh can't even run most of "configure", but Solaris 10 also
provides /bin/ksh and /usr/xpg4/bin/sh with the usual modern features.
("configure" works on Solaris 10 by finding a good shell and re-execing
itself.)
Maintaining shell scripts to an even lower common denominator than Autoconf
would be a good deal less reliable.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-22 14:46 Tom Lane <tgl@sss.pgh.pa.us>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 17+ messages in thread
From: Tom Lane @ 2018-07-22 14:46 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Victor Wagner <vitus@wagner.pp.ru>; 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
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-22 17:38 Noah Misch <noah@leadboat.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 17+ messages in thread
From: Noah Misch @ 2018-07-22 17:38 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
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.
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-22 18:53 Tom Lane <tgl@sss.pgh.pa.us>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 2 replies; 17+ messages in thread
From: Tom Lane @ 2018-07-22 18:53 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
Noah Misch <noah@leadboat.com> writes:
> On Sun, Jul 22, 2018 at 10:46:03AM -0400, Tom Lane wrote:
>> 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.
Oh! Hm, I wonder whether we shouldn't do that, ie add SHELL = @SHELL@
to Makefile.global.in.
A quick trawl of the buildfarm logs says most of our animals compute
SHELL = /bin/sh anyway, and so would be unaffected. There's a sizable
population that find /bin/bash though, and one active critter that finds
/bin/ksh.
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-23 03:39 Michael Paquier <michael@paquier.xyz>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 17+ messages in thread
From: Michael Paquier @ 2018-07-23 03:39 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Noah Misch <noah@leadboat.com>; Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
On Sun, Jul 22, 2018 at 02:53:51PM -0400, Tom Lane wrote:
> Oh! Hm, I wonder whether we shouldn't do that, ie add SHELL = @SHELL@
> to Makefile.global.in.
That sounds like a good idea to me.
> A quick trawl of the buildfarm logs says most of our animals compute
> SHELL = /bin/sh anyway, and so would be unaffected. There's a sizable
> population that find /bin/bash though, and one active critter that finds
> /bin/ksh.
Except for the FreeBSD boxes, right? I thought that using directly
/bin/ and not /usr/local/bin/ was considered an abuse of Linux in their
universe.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-23 04:01 Tom Lane <tgl@sss.pgh.pa.us>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 17+ messages in thread
From: Tom Lane @ 2018-07-23 04:01 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Noah Misch <noah@leadboat.com>; Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
Michael Paquier <michael@paquier.xyz> writes:
> On Sun, Jul 22, 2018 at 02:53:51PM -0400, Tom Lane wrote:
>> A quick trawl of the buildfarm logs says most of our animals compute
>> SHELL = /bin/sh anyway, and so would be unaffected. There's a sizable
>> population that find /bin/bash though, and one active critter that finds
>> /bin/ksh.
> Except for the FreeBSD boxes, right? I thought that using directly
> /bin/ and not /usr/local/bin/ was considered an abuse of Linux in their
> universe.
No, I'm pretty sure that "sh" wins a place in /bin even on FreeBSD ;-)
regards, tom lane
^ permalink raw reply [nested|flat] 17+ messages in thread
* Re: Non-portable shell code in pg_upgrade tap tests
@ 2018-07-23 07:00 Noah Misch <noah@leadboat.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 17+ messages in thread
From: Noah Misch @ 2018-07-23 07:00 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Victor Wagner <vitus@wagner.pp.ru>; pgsql-hackers
On Sun, Jul 22, 2018 at 02:53:51PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Sun, Jul 22, 2018 at 10:46:03AM -0400, Tom Lane wrote:
> >> 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.
>
> Oh! Hm, I wonder whether we shouldn't do that, ie add SHELL = @SHELL@
> to Makefile.global.in.
I see that as the right way forward.
^ permalink raw reply [nested|flat] 17+ messages in thread
end of thread, other threads:[~2018-07-23 07:00 UTC | newest]
Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-07-20 12:38 Non-portable shell code in pg_upgrade tap tests Victor Wagner <vitus@wagner.pp.ru>
2018-07-20 14:25 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-20 14:55 ` Victor Wagner <vitus@wagner.pp.ru>
2018-07-20 15:09 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-22 07:29 ` Noah Misch <noah@leadboat.com>
2018-07-22 14:46 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-22 17:38 ` Noah Misch <noah@leadboat.com>
2018-07-22 18:53 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-23 03:39 ` Michael Paquier <michael@paquier.xyz>
2018-07-23 04:01 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-23 07:00 ` Noah Misch <noah@leadboat.com>
2018-07-20 21:18 ` Tels <nospam-pg-abuse@bloodgate.com>
2018-07-21 15:29 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-21 16:47 ` Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
2018-07-21 18:04 ` Tom Lane <tgl@sss.pgh.pa.us>
2018-07-21 22:49 ` Tels <nospam-pg-abuse@bloodgate.com>
2018-07-21 23:20 ` Tom Lane <tgl@sss.pgh.pa.us>
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