Re: make installcheck-world in a clean environment

From: Alexander Lakhin <exclusion(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: make installcheck-world in a clean environment
Date: 2018-08-01 21:08:57
Message-ID: 39b7758a-f31a-b4f8-011a-446cc005b99a@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
make-checkinstall-master.patch text/x-patch 8.6 KB
installcheck-patched.log text/x-log 144.4 KB
installcheck.log text/x-log 244.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2018-08-01 21:17:54 Re: [Patch] Checksums for SLRU files
Previous Message Alvaro Herrera 2018-08-01 21:03:11 Re: [report] memory leaks in COPY FROM on partitioned table