make installcheck-world in a clean environment

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

make installcheck-world in a clean environment

Alexander Lakhin-2
Hello,

I am trying to perform "make installcheck-world" after "make clean" (or after installing a precompiled binaries), but it fails.
The error I get is related to ECPG regression test:
make -C test installcheck
make[2]: Entering directory `/home/postgres/postgres/src/interfaces/ecpg/test'
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 '-I../../../../src/port' '-I../../../../src/test/regress' '-DHOST_TUPLE="x86_64-pc-linux-gnu"' '-DSHELLPROG="/bin/sh"' '-DDLSUFFIX=".so"' -I../../../../src/include  -D_GNU_SOURCE   -c -o pg_regress_ecpg.o pg_regress_ecpg.c
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -L../../../../src/port -L../../../../src/common -Wl,--as-needed -Wl,-rpath,'/usr/local/pgsql/lib',--enable-new-dtags  pg_regress_ecpg.o ../../../../src/test/regress/pg_regress.o -lpgcommon -lpgport -lpthread -lz -lreadline -lrt -lcrypt -ldl -lm  -o pg_regress
make -C connect all
make[3]: Entering directory `/home/postgres/postgres/src/interfaces/ecpg/test/connect'
make[3]: *** No rule to make target `test1.c', needed by `test1.o'.  Stop.
make[3]: Leaving directory `/home/postgres/postgres/src/interfaces/ecpg/test/connect'

Is it a supported scenario to make installcheck-world without performing "make" first?
(If I do "make -C src/interfaces/ecpg" and then "make installcheck-world", then this error is gone. And when I set up all the extensions, all tests passed successfully.)
And even if we need to perform make, I wonder, should the recompiled ecpg binary be checked instead of installed one?
I tried to modify Makefile to target installed ecpg binary and it's libs
(see the patch attached), it works, but this fix is only suited for installcheck.
So if this scenario should be supported, a more elaborated fix is needed.

Best regards,
------
Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


ecpg-installcheck.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
02.04.2018 12:12, Alexander Lakhin wrote:

Is it a supported scenario to make installcheck-world without performing "make" first?
(If I do "make -C src/interfaces/ecpg" and then "make installcheck-world", then this error is gone. And when I set up all the extensions, all tests passed successfully.)
And even if we need to perform make, I wonder, should the recompiled ecpg binary be checked instead of installed one?
I tried to modify Makefile to target installed ecpg binary and it's libs
(see the patch attached), it works, but this fix is only suited for installcheck.
So if this scenario should be supported, a more elaborated fix is needed.
To avoid overheating of this pretty hot discussion, I would like just to propose "a more elaborated fix" (for REL_10_STABLE and master).
In fact, when we perform "make installcheck" it not only requires us to build ecpg, but it also rebuilds libpostgres, libpgport and libpq (for installcheck-world).
I believe that the larger testing surface (coverage), the better, so using installed assets (libs, headers) is more useful.

Regarding "remote installcheck", that was discussed recently, the proposed patch complicates this, but opens a way to implement it correctly.
Think of distinct target "remotecheck", that will not define USE_INSTALLED_ASSETS, but will account for remote connection to server and run only supported tests.

Best regards,
------
Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

make-checkinstall-10.diff (6K) Download Attachment
make-checkinstall-master.diff (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
06.04.2018 09:19, Alexander Lakhin wrote:
To avoid overheating of this pretty hot discussion, I would like just to propose "a more elaborated fix" (for REL_10_STABLE and master).
In fact, when we perform "make installcheck" it not only requires us to build ecpg, but it also rebuilds libpostgres, libpgport and libpq (for installcheck-world).
I believe that the larger testing surface (coverage), the better, so using installed assets (libs, headers) is more useful.
In hope to keep a lid on this explosive topic, I propose the improved fix.

Best regards,
------
Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


make-checkinstall-10.patch (7K) Download Attachment
make-checkinstall-master.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Robert Haas
In reply to this post by Alexander Lakhin-2
On Mon, Apr 2, 2018 at 5:12 AM, Alexander Lakhin <[hidden email]> wrote:
> Is it a supported scenario to make installcheck-world without performing
> "make" first?

The evidence suggests that the answer is "no".

> (If I do "make -C src/interfaces/ecpg" and then "make installcheck-world",
> then this error is gone. And when I set up all the extensions, all tests
> passed successfully.)

Your proposed fix involves inventing something called
USE_INSTALLED_ASSETS, but we don't need that anywhere else, so why do
we need it for ECPG?

I am not opposed to fixing this, but one might also ask whether it's a
good use of time, since the workaround is straightforward.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
04.05.2018 14:58, Robert Haas wrote:

Thanks for your involvement!

Your proposed fix involves inventing something called
USE_INSTALLED_ASSETS, but we don't need that anywhere else, so why do
we need it for ECPG?
We need that not only for ECPG, but for libpostgres, libpgport, and libpq too, if we want to check the installed ones (instead of recompiled).
Imagine that we will recompile psql (and use it) to perform installcheck for the server instance. It will work but we'll miss an opportunity to check whether the installed psql is working. I believe, it's the same with ecpg (as now, we can even remove installed ecpg and have all the tests passed).
I am not opposed to fixing this, but one might also ask whether it's a
good use of time, since the workaround is straightforward.

I'm not seeing a workaround to perform more complete installcheck without modifying makefiles. So for me the question is whether the increasing the testing surface justifies this use of time.


Best regards,

------
Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Robert Haas
On Fri, May 4, 2018 at 8:30 AM, Alexander Lakhin <[hidden email]> wrote:
> I'm not seeing a workaround to perform more complete installcheck without
> modifying makefiles. So for me the question is whether the increasing the
> testing surface justifies this use of time.

After thinking about this some more, I think the question here is
definitional.  A first attempt at defining 'make installcheck' is to
say that it runs the tests from the build tree against the running
server.  Certainly, we intend to use the SQL files that are present in
the build tree, not the ones that were present in the build tree where
the running server was built.  But what about client programs that we
use to connect to the server?  You're suggesting that we use the
pre-installed ones, but that is actually pretty problematic because
the ones we see as installed might correspond neither to the contents
of the build tree nor to the running server.  Conceivably we could end
up having a mix of assets from three different places: (1) the running
server, (2) the build tree, (3) whatever is in our path at the moment.
That seems very confusing.  So now I think it's probably right to
define 'make installcheck' as using the assets from the build tree to
test the running server.  Under that definition, we're missing some
dependencies, but USE_INSTALLED_ASSETS isn't a thing we need.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Tom Lane-2
Robert Haas <[hidden email]> writes:
> On Fri, May 4, 2018 at 8:30 AM, Alexander Lakhin <[hidden email]> wrote:
>> I'm not seeing a workaround to perform more complete installcheck without
>> modifying makefiles. So for me the question is whether the increasing the
>> testing surface justifies this use of time.

> After thinking about this some more, I think the question here is
> definitional.  A first attempt at defining 'make installcheck' is to
> say that it runs the tests from the build tree against the running
> server.  Certainly, we intend to use the SQL files that are present in
> the build tree, not the ones that were present in the build tree where
> the running server was built.  But what about client programs that we
> use to connect to the server?  You're suggesting that we use the
> pre-installed ones, but that is actually pretty problematic because
> the ones we see as installed might correspond neither to the contents
> of the build tree nor to the running server.  Conceivably we could end
> up having a mix of assets from three different places: (1) the running
> server, (2) the build tree, (3) whatever is in our path at the moment.
> That seems very confusing.  So now I think it's probably right to
> define 'make installcheck' as using the assets from the build tree to
> test the running server.  Under that definition, we're missing some
> dependencies, but USE_INSTALLED_ASSETS isn't a thing we need.

Nah, I disagree with this.  To me, the purpose of "make installcheck"
is to verify the correctness of an installation, which I take to include
the client programs as well as the server.  I think that "make
installcheck" ought to use the installed version of any file that we
actually install, and go to the build tree only for things we don't
install (e.g. SQL test scripts).

If the user has screwed up his PATH or other environmental aspects so that
what he's testing isn't a single installation, that's his error, not
something that "make installcheck" ought to work around.  Indeed, maybe
such aspects of his setup are intentional, and second-guessing them would
completely defeat his purpose.  In any case, if you want to test the
build-tree assets, that's what "make check" is for.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
07.05.2018 20:07, Tom Lane wrote:
Robert Haas [hidden email] writes:
After thinking about this some more, I think the question here is
definitional.  A first attempt at defining 'make installcheck' is to
say that it runs the tests from the build tree against the running
server.  Certainly, we intend to use the SQL files that are present in
the build tree, not the ones that were present in the build tree where
the running server was built.  But what about client programs that we
use to connect to the server?  You're suggesting that we use the
pre-installed ones, but that is actually pretty problematic because
the ones we see as installed might correspond neither to the contents
of the build tree nor to the running server.
If the contents of the source tree doesn't correspond to the running server, then I'm afraid, we can't installcheck it for sure.
I think it's supposed that we use for installcheck exactly the same source files that was used to build the server.
Regarding clients program, if we will not use/check it while installchecking, then by what means they can be tested when installed?
I think, the pgsql-packagers would like to check whether the whole installation of PostgreSQL is working, not only the server.
For me, very realistic and most useful scenario of installcheck is:
install postgresql-x.rpm
install postgresql-x.src.rpm
./configure --prefix=$target_path --enable-tap-tests, etc...
make installcheck(-world)
  Conceivably we could end
up having a mix of assets from three different places: (1) the running
server, (2) the build tree, (3) whatever is in our path at the moment.
That seems very confusing.  So now I think it's probably right to
define 'make installcheck' as using the assets from the build tree to
test the running server.  Under that definition, we're missing some
dependencies, but USE_INSTALLED_ASSETS isn't a thing we need.
I see another scenario, that was discussed a month ago - remote (or server-only) installcheck.
( https://www.postgresql.org/message-id/flat/CAM0nTJ6iorX_tmg5MX0mQU3z3w%3D9wk%2BpGK8zrvn7DNWqnyv%2BsQ%40mail.gmail.com )
It can be useful too and if it will be supported, then USE_INSTALLED_ASSETS usage should be extended to psql, etc.
Nah, I disagree with this.  To me, the purpose of "make installcheck"
is to verify the correctness of an installation, which I take to include
the client programs as well as the server.  I think that "make
installcheck" ought to use the installed version of any file that we
actually install, and go to the build tree only for things we don't
install (e.g. SQL test scripts).
Yes, that's the proposed patches intended for.  I didn't encounter any problems with it during internal testing with Linux and mingw-builds.
If the user has screwed up his PATH or other environmental aspects so that
what he's testing isn't a single installation, that's his error, not
something that "make installcheck" ought to work around.  Indeed, maybe
such aspects of his setup are intentional, and second-guessing them would
completely defeat his purpose.  In any case, if you want to test the
build-tree assets, that's what "make check" is for.
Even modified configs could lead to test failures (for example, lc_messages can break server log checking), so the installcheck should be performed against some clean and determinated installation anyway.

Best regards,
------
Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Peter Eisentraut-6
Exactly what order of steps are you executing that doesn't work?

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
Hello Peter,
06.07.2018 00:39, Peter Eisentraut wrote:
Exactly what order of steps are you executing that doesn't work?
In Centos 7, using the master branch from git:
./configure --enable-tap-tests
make install
make install -C contrib
chown -R postgres:postgres /usr/local/pgsql/
/usr/local/pgsql/bin/initdb -D /usr/local/pgsql/data
/usr/local/pgsql/bin/pg_ctl start -l logfile -D /usr/local/pgsql/data
make clean
# Also you can just install binary packages to get the same state.

make installcheck-world
# This check fails.

Best regards,
------

Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Tom Lane-2
Alexander Lakhin <[hidden email]> writes:
> 06.07.2018 00:39, Peter Eisentraut wrote:
>> Exactly what order of steps are you executing that doesn't work?

> In Centos 7, using the master branch from git:
> ./configure --enable-tap-tests
> make install
> make install -C contrib
> chown -R postgres:postgres /usr/local/pgsql/
> /usr/local/pgsql/bin/initdb -D /usr/local/pgsql/data
> /usr/local/pgsql/bin/pg_ctl start -l logfile -D /usr/local/pgsql/data
> /make clean/
> # Also you can just install binary packages to get the same state.

> make installcheck-world
> # This check fails.

I do not think that should be expected to work.  It would require that
"make installcheck" first invoke "make all" (to rebuild the stuff you
threw away with "make clean"), which is rather antithetical to its
purpose.  Specifically, installcheck is supposed to be checking something
you already built; so having it do a fresh build seems to introduce
version-skew hazards that we don't need.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
Hello Tom,

11.07.2018 23:15, Tom Lane wrote:

/make clean/
# Also you can just install binary packages to get the same state.

      
make installcheck-world
# This check fails.
I do not think that should be expected to work.  It would require that
"make installcheck" first invoke "make all" (to rebuild the stuff you
threw away with "make clean"), which is rather antithetical to its
purpose.  Specifically, installcheck is supposed to be checking something
you already built; so having it do a fresh build seems to introduce
version-skew hazards that we don't need.
If I understand correctly, the installcheck target should not use the stuff in the build directory (that is thrown away with 'make clean'), but instead should use/check the installed assets.
In fact with REL_10_STABLE you can run "make clean && make installcheck" and it works just fine (without calling 'make all'). It's sufficient to have a working instance running. And I think, this behavior is correct — "installcheck" supposed to check not something we built ("check" is supposed to do that), but something we installed.
And only if I run "make installcheck-world" with REL_10_STABLE I get an error related to ECPG, I've referred to in the first message in this thread.
In the master branch there was some changes that prevent "make clean && make installcheck" scenario to run, but I think it can (and should) be fixed too.
(When I prepared the patches, there were no differences between these branches in this aspect.)
So for me the question is what assets should the installcheck target be checking? Installed or built ones? For example, if psql/pg_dump/ecpg/... is installed in /usr/local/pgsql/bin/, should it be checked by the installcheck? And if we target the installed stuff then why do we need to build something (or have something built) for installcheck?

Best regards,
------

Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Peter Eisentraut-6
In reply to this post by Alexander Lakhin-2
On 06.07.18 09:45, Alexander Lakhin wrote:

> ./configure --enable-tap-tests
> make install
> make install -C contrib
> chown -R postgres:postgres /usr/local/pgsql/
> /usr/local/pgsql/bin/initdb -D /usr/local/pgsql/data
> /usr/local/pgsql/bin/pg_ctl start -l logfile -D /usr/local/pgsql/data
> /make clean/
> # Also you can just install binary packages to get the same state.
>
> make installcheck-world
> # This check fails.

For me, this fails at

In file included from ../../src/include/postgres.h:47,
                 from chklocale.c:17:
../../src/include/utils/elog.h:71:10: fatal error: utils/errcodes.h: No
such file or directory

That's probably fixable.  But it's different from what you had initially
reported.

--
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
Hello Peter,
14.07.2018 13:57, Peter Eisentraut wrote:

> On 06.07.18 09:45, Alexander Lakhin wrote:
>> ./configure --enable-tap-tests
>> make install
>> make install -C contrib
>> chown -R postgres:postgres /usr/local/pgsql/
>> /usr/local/pgsql/bin/initdb -D /usr/local/pgsql/data
>> /usr/local/pgsql/bin/pg_ctl start -l logfile -D /usr/local/pgsql/data
>> /make clean/
>> # Also you can just install binary packages to get the same state.
>>
>> make installcheck-world
>> # This check fails.
> For me, this fails at
>
> In file included from ../../src/include/postgres.h:47,
>                  from chklocale.c:17:
> ../../src/include/utils/elog.h:71:10: fatal error: utils/errcodes.h: No
> such file or directory
>
> That's probably fixable.  But it's different from what you had initially
> reported.
This error wasn't present in master at the time of initial report (in
April). As "git bisect" shows such errors appeared after 372728b.
So in REL_10_STABLE (or in master before 372728b) you could "make
installcheck" (but not installcheck-world) in a clean environment.


Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Tom Lane-2
Alexander Lakhin <[hidden email]> writes:

> 14.07.2018 13:57, Peter Eisentraut wrote:
>> On 06.07.18 09:45, Alexander Lakhin wrote:
>>> ./configure --enable-tap-tests
>>> make install
>>> make install -C contrib
>>> chown -R postgres:postgres /usr/local/pgsql/
>>> /usr/local/pgsql/bin/initdb -D /usr/local/pgsql/data
>>> /usr/local/pgsql/bin/pg_ctl start -l logfile -D /usr/local/pgsql/data
>>> /make clean/
>>> # Also you can just install binary packages to get the same state.
>>>
>>> make installcheck-world
>>> # This check fails.

I remain pretty skeptical that this is a sensible way to proceed,
especially not if what you're testing is installed binary packages.
You're creating yet *another* hazard for version-skew-like problems,
namely that there's no certainty that you gave configure arguments
that're compatible with what the installed packages used.

But having said that ...

>> For me, this fails at
>> In file included from ../../src/include/postgres.h:47,
>> from chklocale.c:17:
>> ../../src/include/utils/elog.h:71:10: fatal error: utils/errcodes.h: No
>> such file or directory
>> That's probably fixable.  But it's different from what you had initially
>> reported.

> This error wasn't present in master at the time of initial report (in
> April). As "git bisect" shows such errors appeared after 372728b.
> So in REL_10_STABLE (or in master before 372728b) you could "make
> installcheck" (but not installcheck-world) in a clean environment.

... I think this was actually induced by 3b8f6e75f, for which we already
had to make some adjustments to ensure that the generated headers got
built when needed.  This seems like another such case, so I stuck in a
couple more submake-generated-headers dependencies to fix it.

The original complaint about ecpg remains; I'm not terribly excited
about messing with that.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Tom Lane-2
I wrote:
> The original complaint about ecpg remains; I'm not terribly excited
> about messing with that.

Well, I got curious as to why we were seeing such a weird error,
and eventually traced it to this stuff in ecpg/test/Makefile.regress:

# Standard way to invoke the ecpg preprocessor
ECPG = ../../preproc/ecpg --regression -I$(srcdir)/../../include -I$(srcdir)

# Files that most or all ecpg preprocessor test outputs depend on
ECPG_TEST_DEPENDENCIES = ../../preproc/ecpg$(X) \
        $(srcdir)/../regression.h \
        $(srcdir)/../../include/sqlca.h \
        $(srcdir)/../../include/sqlda.h \
        $(srcdir)/../../include/sqltypes.h \
        $(srcdir)/../../include/sql3types.h

%.c: %.pgc $(ECPG_TEST_DEPENDENCIES)
        $(ECPG) -o $@ $<

Now, the fine gmake manual quoth:

    `%' in a prerequisite of a pattern rule stands for the same stem
    that was matched by the `%' in the target.  In order for the pattern
    rule to apply, its target pattern must match the file name under
    consideration and all of its prerequisites (after pattern substitution)
    must name files that exist or can be made.

So the problem is that (after make clean) ../../preproc/ecpg doesn't
exist, and the Makefiles under test/ have no idea how to build it,
and thus this pattern rule is inapplicable.  Thus you end up getting
"No rule to make target" errors.

While it seems straightforward to fix this for "make check" scenarios ---
just go make ecpg before trying to make the tests --- I think these rules
are very surprising for "make installcheck" cases.  You'd expect "make
installcheck" to test the installed ecpg, as indeed Alexander pointed out
at the start of the thread.  But it's not doing that.

Alexander's USE_INSTALLED_ASSETS patch attempted to fix that, and I now
see the point of it, but it still seems pretty hacky and invasive (why
should an ecpg-only problem be touching stuff outside ecpg)?  Also,
unless I'm still missing something, it doesn't fix the problem with
"make clean check": ecpg won't have been built before we try to build the
test programs.

I'd suggest trying to simplify the USE_INSTALLED_ASSETS patch so it
doesn't muck with the rules for building pg_regress.  I don't find
that to be very relevant to the problem.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
In reply to this post by Tom Lane-2
Hello Tom,

31.07.2018 01:16, Tom Lane wrote:
Alexander Lakhin [hidden email] writes:
14.07.2018 13:57, Peter Eisentraut wrote:
On 06.07.18 09:45, Alexander Lakhin wrote:
./configure --enable-tap-tests
make install
make install -C contrib
chown -R postgres:postgres /usr/local/pgsql/
/usr/local/pgsql/bin/initdb -D /usr/local/pgsql/data
/usr/local/pgsql/bin/pg_ctl start -l logfile -D /usr/local/pgsql/data
/make clean/
# Also you can just install binary packages to get the same state.

make installcheck-world
# This check fails.
I remain pretty skeptical that this is a sensible way to proceed,
especially not if what you're testing is installed binary packages.
You're creating yet *another* hazard for version-skew-like problems,
namely that there's no certainty that you gave configure arguments
that're compatible with what the installed packages used.
I believe that `installed_instance_path\bin\pg_config --configure` can show the arguments, which can be used to perform ./configure and then make installcheck for binary packages.
I understand that it should be done on the same platform and with exactly the same PG version, but I think it's the only right way to check the binaries (to perform user-centric testing).

Best regards,

------

Alexander Lakhin

Postgres Professional: http://www.postgrespro.com

The Russian Postgres Company
Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
In reply to this post by Tom Lane-2
31.07.2018 02:42, Tom Lane wrote:
I wrote:
The original complaint about ecpg remains; I'm not terribly excited
about messing with that.
Well, I got curious as to why we were seeing such a weird error,
and eventually traced it to this stuff in ecpg/test/Makefile.regress:

# Standard way to invoke the ecpg preprocessor
ECPG = ../../preproc/ecpg --regression -I$(srcdir)/../../include -I$(srcdir)

# Files that most or all ecpg preprocessor test outputs depend on
ECPG_TEST_DEPENDENCIES = ../../preproc/ecpg$(X) \
	$(srcdir)/../regression.h \
	$(srcdir)/../../include/sqlca.h \
	$(srcdir)/../../include/sqlda.h \
	$(srcdir)/../../include/sqltypes.h \
	$(srcdir)/../../include/sql3types.h

%.c: %.pgc $(ECPG_TEST_DEPENDENCIES)
	$(ECPG) -o $@ $<

Now, the fine gmake manual quoth:

    `%' in a prerequisite of a pattern rule stands for the same stem
    that was matched by the `%' in the target.  In order for the pattern
    rule to apply, its target pattern must match the file name under
    consideration and all of its prerequisites (after pattern substitution)
    must name files that exist or can be made.

So the problem is that (after make clean) ../../preproc/ecpg doesn't
exist, and the Makefiles under test/ have no idea how to build it,
and thus this pattern rule is inapplicable.  Thus you end up getting
"No rule to make target" errors.
Yes, it's exactly the problem I was trying to fix.
While it seems straightforward to fix this for "make check" scenarios ---
just go make ecpg before trying to make the tests --- I think these rules
are very surprising for "make installcheck" cases.  You'd expect "make
installcheck" to test the installed ecpg, as indeed Alexander pointed out
at the start of the thread.  But it's not doing that.

Alexander's USE_INSTALLED_ASSETS patch attempted to fix that, and I now
see the point of it, but it still seems pretty hacky and invasive (why
should an ecpg-only problem be touching stuff outside ecpg)?  Also,
unless I'm still missing something, it doesn't fix the problem with
"make clean check": ecpg won't have been built before we try to build the
test programs.
In fact, after fixing ECPG usage with installcheck, I found that "make installcheck" then rebuilds libpostgres, libpgport and libpq (for installcheck-world). (I mentioned it in this thread before.) Later I proposed a more comprehensive patch that allows us to make installcheck cleanly (without building something).
So the problem is not ecpg-only, the ecpg's failure to build is prominent part of it, but there are another assets getting built under the hood.
I'd suggest trying to simplify the USE_INSTALLED_ASSETS patch so it
doesn't muck with the rules for building pg_regress.  I don't find
that to be very relevant to the problem.
I can simplify the patch to fix only the ECPG failure, and then prepare a distinct patch for libs, if it makes sense.


Best regards,
------

Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Tom Lane-2
Alexander Lakhin <[hidden email]> writes:
> 31.07.2018 02:42, Tom Lane wrote:
>> Alexander's USE_INSTALLED_ASSETS patch attempted to fix that, and I now
>> see the point of it, but it still seems pretty hacky and invasive (why
>> should an ecpg-only problem be touching stuff outside ecpg)?  Also,
>> unless I'm still missing something, it doesn't fix the problem with
>> "make clean check": ecpg won't have been built before we try to build the
>> test programs.

> In fact, after fixing ECPG usage with installcheck, I found that "make
> installcheck" then rebuilds libpostgres, libpgport and libpq (for
> installcheck-world). (I mentioned it in this thread before.) Later I
> proposed a more comprehensive patch that allows us to make installcheck
> cleanly (without building something).
> So the problem is not ecpg-only, the ecpg's failure to build is
> prominent part of it, but there are another assets getting built under
> the hood.

Well, there's a lot of moving parts here, and it's not clear to me what
makes the most sense.  We potentially have the following things that
ECPG "make check" generates or references in the build tree, but we might
wish for "make installcheck" to use preinstalled versions of instead:

1. Server executables and other "installed" server support files.
2. PG cluster (datadir and running server).
3. pg_regress and libraries it depends on, such as libpq.
4. ecpg preprocessor.
5. ecpg's exported C header files, needed to compile test programs.
6. ecpg's libraries, used by test programs; also libpq.

Currently, installcheck references an existing running cluster (#2)
and therefore a fortiori #1 as well.  That's fine.  I believe we
reference a local pg_regress and libraries (#3) in both cases,
and that's also fine, because we don't install pg_regress at all.
(Well, PGXS does, but ecpg tests shouldn't rely on that.)
So it boils down to what to do about #4/#5/#6.

I suppose we could define "make installcheck" as referring only
to using an installed server, not any ecpg support, in which
case we don't need any big surgery to the ecpg makefiles.
I'm not sure how consistent a definition that is, but maybe it
makes sense by analogy to pg_regress.

Another point here is that if you did "make check" and then
"make installcheck", it presumably would not rebuild the test
programs, meaning they'd still have been built using the local
resources not the installed ones (or vice versa for the other
order).  So there's also a difference between "build" and
"test execution", which seems relevant, and it's one we don't
really have any make-target nomenclature to distinguish.

Given that you'd expect "make all" to use local copies of the
build resources, perhaps there should be a separate target
named along the lines of "make tests-from-install" that uses
installed ecpg+other resources to compile the test programs.

Anyway, as long as the installcheck target is dependent on "all" not
something else, maybe it's fine as is.  I'm definitely not sure what
would be a consistent design for doing it differently.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: make installcheck-world in a clean environment

Alexander Lakhin-2
Hello Tom,

31.07.2018 22:01, Tom Lane wrote:
Well, there's a lot of moving parts here, and it's not clear to me what
makes the most sense.  We potentially have the following things that
ECPG "make check" generates or references in the build tree, but we might
wish for "make installcheck" to use preinstalled versions of instead:

1. Server executables and other "installed" server support files.
2. PG cluster (datadir and running server).
3. pg_regress and libraries it depends on, such as libpq.
4. ecpg preprocessor.
5. ecpg's exported C header files, needed to compile test programs.
6. ecpg's libraries, used by test programs; also libpq.

Currently, installcheck references an existing running cluster (#2)
and therefore a fortiori #1 as well.  That's fine.  I believe we
reference a local pg_regress and libraries (#3) in both cases,
and that's also fine, because we don't install pg_regress at all.
(Well, PGXS does, but ecpg tests shouldn't rely on that.)
So it boils down to what to do about #4/#5/#6.
I would split 3 to:
3. client libraries, such as libpq.
7. pg_regress.
I missed the fact that pg_regress is installed in lib/pgxs/src/test/regress/, and now it raises a question for me - why this pg_regress instance should be used only for PGXS tests?
Leaving the question aside, I propose the fix to use the preinstalled versions of 3-6.
I suppose we could define "make installcheck" as referring only
to using an installed server, not any ecpg support, in which
case we don't need any big surgery to the ecpg makefiles.
I'm not sure how consistent a definition that is, but maybe it
makes sense by analogy to pg_regress.

Another point here is that if you did "make check" and then
"make installcheck", it presumably would not rebuild the test
programs, meaning they'd still have been built using the local
resources not the installed ones (or vice versa for the other
order).  So there's also a difference between "build" and
"test execution", which seems relevant, and it's one we don't
really have any make-target nomenclature to distinguish.
Yes, this is important question and I have no complete answer for now, but maybe these test programs could be built as temporary?
Given that you'd expect "make all" to use local copies of the
build resources, perhaps there should be a separate target
named along the lines of "make tests-from-install" that uses
installed ecpg+other resources to compile the test programs.
If I understand you correctly, the attached patch does just this.
To illustrate it I also attached the log of "make -C src/interfaces/ecpg; make installcheck" and the log of "make installcheck" with patch applied (both procedures executed immediately after "make clean").
Anyway, as long as the installcheck target is dependent on "all" not
something else, maybe it's fine as is.  I'm definitely not sure what
would be a consistent design for doing it differently.
I would propose the following design for "make installcheck":
1. For every thing that has installed version, use it.
2. For every thing that has no installed version and is not a subject to build (e.g. .../t/*.sql), use it's version in the source tree.
3. For every thing that has no installed version and should be built (e.g. src/interfaces/ecpg/test/connect/*.pgc), build one-time version and use it.

Regarding to ecpg tests it means that we should use installed ecpg, installed libs, installed *.h, and compile src/interfaces/ecpg/test/connect/*.pgc into a temporary binary.

Maybe I miss some use cases, where existing design suits better, but in general I think that the more test coverage (more installed things checked), the better.


Best regards,
------

Alexander Lakhin
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


make-checkinstall-master.patch (8K) Download Attachment
installcheck-patched.log (159K) Download Attachment
installcheck.log (264K) Download Attachment
12