Re: Repeated pg_upgrade buildfarm failures on binturon - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Repeated pg_upgrade buildfarm failures on binturon
Date
Msg-id 25948.1436281999@sss.pgh.pa.us
Whole thread Raw
In response to Re: Repeated pg_upgrade buildfarm failures on binturon  (Oskari Saarenmaa <os@ohmu.fi>)
Responses Re: Repeated pg_upgrade buildfarm failures on binturon
List pgsql-hackers
Oskari Saarenmaa <os@ohmu.fi> writes:
> 07.07.2015, 14:21, Andres Freund kirjoitti:
>> Those seem to indicate something going seriously wrong to me.

> Binturong and Dingo run on the same host with a hourly cronjob to
> trigger the builds.  These failures are caused by concurrent test runs
> on different branches which use the same tmp_check directory for
> pg_upgrade tests, see
> http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dingo&dt=2015-07-07%2002%3A58%3A01&stg=check-pg_upgrade

Ouch.

> It looks like neither make (GNU Make 4.0) nor shell (default Solaris
> /bin/sh) updates $PWD to point to the current directory where test.sh is
> executed and test.sh puts the test cluster in the original working
> directory of the process that launched make.

Double ouch.  It's the responsibility of the shell, not gmake, that PWD
reflect reality.  POSIX 2008, under Shell Variables, quoth as follows:

PWD Set by the shell and by the cd utility. In the shell the value shall be initialized from the environment as
follows.If a value for PWD is passed to the shell in the environment when it is executed, the value is an absolute
pathnameof the current working directory that is no longer than {PATH_MAX} bytes including the terminating null byte,
andthe value does not contain any components that are dot or dot-dot, then the shell shall set PWD to the value from
theenvironment. Otherwise, if a value for PWD is passed to the shell in the environment when it is executed, the value
isan absolute pathname of the current working directory, and the value does not contain any components that are dot or
dot-dot,then it is unspecified whether the shell sets PWD to the value from the environment or sets PWD to the pathname
thatwould be output by pwd -P. Otherwise, the sh utility sets PWD to the pathname that would be output by pwd -P. In
caseswhere PWD is set to the value from the environment, the value can contain components that refer to files of type
symboliclink. In cases where PWD is set to the pathname that would be output by pwd -P, if there is insufficient
permissionon the current working directory, or on any parent of that directory, to determine what that pathname would
be,the value of PWD is unspecified. Assignments to this variable may be ignored. If an application sets or unsets the
valueof PWD, the behaviors of the cd and pwd utilities are unspecified.
 

On the other hand, there is no text at all about PWD in the predecessor
Single Unix Spec v2, which is what we frequently regard as our minimum
baseline.  So one could argue that the Solaris shell you're using is
a valid implementation of SUS v2.

> I've restricted builds to one at a time on that host to work around this
> issue for now.  Also attached a patch to explicitly set PWD=$(CURDIR) in
> the Makefile to make sure test.sh runs with the right directory.

Given the last sentence in the POSIX 2008 text, I think unconditionally
munging PWD as you're proposing is a bit risky.  What I suggest is that
we add code to set PWD only if it's not set, which is most easily done
in test.sh itself, along the lines of
# Very old shells may not set PWD for us.if [ x"$PWD" = x"" ]; then  PWD=`pwd -P`fi

A quick look around says that pg_upgrade/test.sh is the only place where
we're depending on shell PWD, so we only need to fix this one script.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: FPW compression leaks information
Next
From: David Christensen
Date:
Subject: [PATCH] correct the initdb.log path for pg_regress