"make check" in src/test/isolation is unworkable

Started by Tom Laneover 14 years ago11 messages
#1Tom Lane
tgl@sss.pgh.pa.us

I believe that the "make check" target in src/test/isolation is
fundamentally unportable, as is illustrated by the fact that buildfarm
member coypu is currently choking on it. The reason is that the
pg_isolation_regress program depends on libpq, and in particular it
depends on having an *installed* libpq. Anyplace where it appears to
work, it's because you already installed Postgres, or at least libpq.

Apparently coypu is the only buildfarm member that hasn't got a
reasonably recent libpq already installed in system directories; or
maybe it's just the first such that's tried to run the isolation-test
script step.

While we could maybe hack this to the point where it works (on some
platforms) by dynamically linking libpq from the source tree, I don't
think it's worth the trouble.

Recommendations:

1. Modify the buildfarm script to run "make installcheck" in the
isolation-test step, and of course move that to after doing the install
step.

2. Get rid of the "check" target in src/test/isolation/Makefile.
We don't need to be dealing with bug reports from people who try to
use it and get either a link failure (easily diagnosed) or a libpq
version compatibility problem (not so easily diagnosed).

regards, tom lane

#2Greg Stark
gsstark@mit.edu
In reply to: Tom Lane (#1)
Re: "make check" in src/test/isolation is unworkable

On Mon, May 9, 2011 at 12:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

While we could maybe hack this to the point where it works (on some
platforms) by dynamically linking libpq from the source tree, I don't
think it's worth the trouble.

How is this different from the regular case with pg_regress?

--
greg

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Greg Stark (#2)
Re: "make check" in src/test/isolation is unworkable

Greg Stark <gsstark@mit.edu> writes:

How is this different from the regular case with pg_regress?

pg_regress doesn't link in libpq.

regards, tom lane

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#1)
Re: "make check" in src/test/isolation is unworkable

On 05/08/2011 07:35 PM, Tom Lane wrote:

I believe that the "make check" target in src/test/isolation is
fundamentally unportable, as is illustrated by the fact that buildfarm
member coypu is currently choking on it. The reason is that the
pg_isolation_regress program depends on libpq, and in particular it
depends on having an *installed* libpq. Anyplace where it appears to
work, it's because you already installed Postgres, or at least libpq.

darn, you're right.

Apparently coypu is the only buildfarm member that hasn't got a
reasonably recent libpq already installed in system directories; or
maybe it's just the first such that's tried to run the isolation-test
script step.

Most aren't running the test because they aren't updated yet. There are
six machines running the tests:

pgbfprod=# select distinct sysname from build_status_log where
log_stage ~ 'isolation' and snapshot > now() - interval '2 months';
sysname
---------
anchovy
coypu
crake
bobcat
chough
kite

chough is doing the wrong thing anyway, because I got distracted and
forgot to fill in the MSVC piece of the puzzle.

While we could maybe hack this to the point where it works (on some
platforms) by dynamically linking libpq from the source tree, I don't
think it's worth the trouble.

Recommendations:

1. Modify the buildfarm script to run "make installcheck" in the
isolation-test step, and of course move that to after doing the install
step.

working on that.

I have pushed a quick fix disabling the test for now until I come up
with proper coding for this tomorrow. See

<https://github.com/PGBuildFarm/client-code/commit/bb1d2f972205d0a8f438bfde86a0fc99ffdeb24a&gt;

2. Get rid of the "check" target in src/test/isolation/Makefile.
We don't need to be dealing with bug reports from people who try to
use it and get either a link failure (easily diagnosed) or a libpq
version compatibility problem (not so easily diagnosed).

+1.

cheers

andrew

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#4)
Re: "make check" in src/test/isolation is unworkable

On 05/08/2011 09:54 PM, Andrew Dunstan wrote:

On 05/08/2011 07:35 PM, Tom Lane wrote:

I believe that the "make check" target in src/test/isolation is
fundamentally unportable, as is illustrated by the fact that buildfarm
member coypu is currently choking on it. The reason is that the
pg_isolation_regress program depends on libpq, and in particular it
depends on having an *installed* libpq. Anyplace where it appears to
work, it's because you already installed Postgres, or at least libpq.

darn, you're right.

OK, I have crake running the installation checks:
<http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=crake&amp;dt=2011-05-09%2004%3A17%3A01&amp;stg=isolation-check&gt;,
so I have checked in a hot fix for the buildfarm client:
<https://github.com/PGBuildFarm/client-code/commit/c3c20a6457f57efcdcecb83e9c8168791f33f699&gt;

What's a bit annoying is that these tests were checked in without a
vestige of MSVC support, and nobody pinged the usual suspects (i.e.
Magnus and me) to ask for help in providing it, unless my memory is even
worse than usual. We have a bit of work to do to enable that, which I'll
try to get done before pgcon.

cheers

andrew

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#5)
Re: "make check" in src/test/isolation is unworkable

Andrew Dunstan <andrew@dunslane.net> writes:

What's a bit annoying is that these tests were checked in without a
vestige of MSVC support, and nobody pinged the usual suspects (i.e.
Magnus and me) to ask for help in providing it,

Speaking of pinging Windows people, have either of you noticed the
reports that CREATE/ALTER USER VALID UNTIL 'infinity' is crashing on
Windows?

http://archives.postgresql.org/pgsql-bugs/2011-05/msg00009.php
http://archives.postgresql.org/pgsql-bugs/2011-05/msg00030.php

regards, tom lane

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#6)
Bug #6005

[subject changed]

On 05/09/2011 09:44 AM, Tom Lane wrote:

Andrew Dunstan<andrew@dunslane.net> writes:

What's a bit annoying is that these tests were checked in without a
vestige of MSVC support, and nobody pinged the usual suspects (i.e.
Magnus and me) to ask for help in providing it,

Speaking of pinging Windows people, have either of you noticed the
reports that CREATE/ALTER USER VALID UNTIL 'infinity' is crashing on
Windows?

http://archives.postgresql.org/pgsql-bugs/2011-05/msg00009.php
http://archives.postgresql.org/pgsql-bugs/2011-05/msg00030.php

It appears to have been fixed (tested on Mingw-w64/W7).

What exactly are the other tests you want run?

cheers

andrew

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#7)
Re: Bug #6005

Andrew Dunstan <andrew@dunslane.net> writes:

Speaking of pinging Windows people, have either of you noticed the
reports that CREATE/ALTER USER VALID UNTIL 'infinity' is crashing on
Windows?

It appears to have been fixed (tested on Mingw-w64/W7).
What exactly are the other tests you want run?

Check to see if timezone_abbrevations has a sane value and the
pg_timezone_abbrevs view has sane contents (ie, not empty).

regards, tom lane

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#8)
Re: Bug #6005

On 05/12/2011 07:34 PM, Tom Lane wrote:

Andrew Dunstan<andrew@dunslane.net> writes:

Speaking of pinging Windows people, have either of you noticed the
reports that CREATE/ALTER USER VALID UNTIL 'infinity' is crashing on
Windows?

It appears to have been fixed (tested on Mingw-w64/W7).
What exactly are the other tests you want run?

Check to see if timezone_abbrevations has a sane value and the
pg_timezone_abbrevs view has sane contents (ie, not empty).

Value is 'Default'. View is not empty and looks sane.

cheers

andrew

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#1)
Re: "make check" in src/test/isolation is unworkable

On sön, 2011-05-08 at 19:35 -0400, Tom Lane wrote:

I believe that the "make check" target in src/test/isolation is
fundamentally unportable, as is illustrated by the fact that buildfarm
member coypu is currently choking on it. The reason is that the
pg_isolation_regress program depends on libpq, and in particular it
depends on having an *installed* libpq. Anyplace where it appears to
work, it's because you already installed Postgres, or at least libpq.

I came across this old issue. Unless I'm missing something, there is no
reason why pg_isolation_regress needs to be linked with libpq at all,
and it works fine without it. If we removed the libpq link, then it
would work just like pg_regress and could support "make check".

Apparently, -Wl,--as-needed isn't working too well here.

#11Andrew Dunstan
andrew@dunslane.net
In reply to: Peter Eisentraut (#10)
Re: "make check" in src/test/isolation is unworkable

On 02/29/2012 02:33 PM, Peter Eisentraut wrote:

On sön, 2011-05-08 at 19:35 -0400, Tom Lane wrote:

I believe that the "make check" target in src/test/isolation is
fundamentally unportable, as is illustrated by the fact that buildfarm
member coypu is currently choking on it. The reason is that the
pg_isolation_regress program depends on libpq, and in particular it
depends on having an *installed* libpq. Anyplace where it appears to
work, it's because you already installed Postgres, or at least libpq.

I came across this old issue. Unless I'm missing something, there is no
reason why pg_isolation_regress needs to be linked with libpq at all,
and it works fine without it. If we removed the libpq link, then it
would work just like pg_regress and could support "make check".

Apparently, -Wl,--as-needed isn't working too well here.

I believe we can't rely on it working. If we really don't need libpq
then why not just filter it out?

cheers

andrew