Re: Repeated pg_upgrade buildfarm failures on binturon

Oskari Saarenmaa <os(at)ohmu(dot)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 pathname of the current working directory that is no longer
than {PATH_MAX} bytes including the terminating null byte, and the value
does not contain any components that are dot or dot-dot, then the shell
shall set PWD to the value from the environment. Otherwise, if a value
for PWD is passed to the shell in the environment when it is executed,
the value is an 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 that would be output by pwd
-P. Otherwise, the sh utility sets PWD to the pathname that would be
output by pwd -P. In cases where PWD is set to the value from the
environment, the value can contain components that refer to files of
type symbolic link. In cases where PWD is set to the pathname that would
be output by pwd -P, if there is insufficient permission on 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 value
of 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