Re: Bugfix and new feature for PGXS

Le lundi 24 juin 2013 19:40:19, Andrew Dunstan a écrit :
> On 06/18/2013 09:52 AM, Cédric Villemain wrote:
> > I've rebased the current set of pending patches I had, to fix pgxs and
> > added 2 new patches.
> >
> > Bugfixes have already been presented and sent in another thread, except
> > the last one which only fix comment in pgxs.mk.
> >
> > The new feature consists in a new variable to allow the installation of
> > contrib header files.
> > A new variable MODULEHEADER can be used in extension Makefile to list the
> > header files to install.
> >
> > The installation path default to
> > $includedir/$includedir_contrib/$extension. 2 new variables are set to
> > define directories: $includedir_contrib and $includedir_extension, the
> > former default to include/contrib and the later to
> > $includedir_contrib/$extension ($extension is the name of the
> > extension).
> >
> > This allows for example to install hstore header and be able to include
> > them
> >
> > in another extension like that:
> >    # include "contrib/hstore/hstore.h"
> >
> > For packagers of PostgreSQL, this allows to have a
> > postgresql-X.Y-contrib-dev package.
> > For developers of extension it's killing the copy-and-paste-this-function
> > dance previously required.
> >
> > I've not updated contribs' Makfefile: I'm not sure what we want to
> > expose.
> >
> > I've 2 other patches to write to automatize a bit more the detection of
> > things to do when building with USE_PGXS, based on the layout. Better
> > get a good consensus on this before writing them.
> >
> >
> > Bugfix:
> > 0001-fix-SHLIB_PREREQS-when-building-with-USE_PGXS.patch
> > 0002-Create-data-directory-if-extension-when-required.patch
> > 0003-set-VPATH-for-out-of-tree-extension-builds.patch
> > 0004-adds-support-for-VPATH-with-USE_PGXS.patch
> > 0006-Fix-suggested-layout-for-extension.patch
> >
> > New feature:
> > 0005-Allows-extensions-to-install-header-file.patch
> >
> > I'll do a documentation patch based on what is accepted in HEAD and/or
> > previous branches.
>
> I have just spent an hour or two wrestling with the first four of these.
> Items 5 and six are really separate items, which I'm not considering at
> the moment.
>
> The trouble I have is that there are no instructions on how to modify
> your Makefile or prepare a vpath tree, so I have been flying blind. I
> started out doing what Postgres does, namely to prepare the tree using
> config/prep_buildtree. This doesn't work at all - the paths don't get
> set properly unless you invoke make with the path to the real makefile,
> and I'm not sure they all get set correctly then. But that's not what we
> expect of a vpath setup, or at least not what I expect. I expect that
> with an appropriately set up make file and a correctly set up build tree
> I can use an identical make invocation to the one I would use for an
> in-tree build. That is, after setting up I should be able to ignore the
> fact that it's a vpath build.
>
> FYI I deliberately chose a non-core extension with an uncommon layout
> for testing: json_build <https://github.com/pgexperts/json_build>
>
> So, patches 1, 2 and 6 look sane enough, and I'm inclined to commit them
> just to get them out of the way. That means two of the commitfest items
> I am signed up for will be committed and two marked "Returned with
> comments". I think we need some more discussion about how VPATH builds
> should work with extensions, and subject to what limitations if any.

Thank you for reviewing those patches.
I'm sorry I haven't reproduced nor explained that much what is expected with
VPATH.

We have 2 main options with core postgresql: in-tree build or out-of-tree. The
former is the classical build process, the later is what's frequently use for
packaging because it allows not to pollute the source tree with intermediate
or fully built files.

You're right that we don't need to set VPATH explicitely, it is set by
'configure' when you exec configure out-of-source-tree. It is transparent to the
user.

Those 2 options are working as expected.

Now, the extensions/contribs. We have 2^2 ways to build them.

Either with PGXS or not, either with VPATH or not.
NO_PGXS is how we build contribs shipped with postgresql.
USE_PGXS is used by most of the others extensions (and contribs).

WIth extension, we do have to set VPATH explicitely if we want to use VPATH
(note that contribs/extensions must not care that postgresql has been built
with or without VPATH set). My patches try to fix that.

So the point is to be able to do exactly that :

$ mkdir /tmp/json_build && cd /tmp/json_build
$ make USE_PGXS=1 -f /path/to/json_build/Makefile

There is another way to do the same thing which is:
$ mkdir /tmp/json_build
$ make USE_PGXS=1 -C /tmp/json_build -f /path/to/json_build/Makefile

Thus packagers can now use :
$ make USE_PGXS=1 -C $vtarget -f /path/to/Makefile
DESTDIR=/my/packaging/area/json_build

instead of (short version):
$ make USE_PGXS=1 -C $vtarget -f /path/to/Makefile
DESTDIR="$srcdir/debian/$package"  VPATH="$srcdir" srcdir="$srcdir"

(please note the $srcdir to workaround the pgxs way of working)

For patch 0003, the point is to make the usage of VPATH transparent for the
users (as we do for core postgresql)

For patch 0004, the point is to take the required files from the good location.
$^ and $< contains the filepath which is where 'make' found the files, but
$(DATA) and others PGXS variables contains the list of required files, not
their filepath. As long as you build in-tree you won't hit this problem, but if
you try to build out-of-source tree, the .control file for example, then make
won't find the .control you just *built*, it'll try to copy it from original
source-tree to follow this recipe:
$(INSTALL_DATA) $(addprefix $(srcdir)/, $(addsuffix .control, $(EXTENSION)))
'$(DESTDIR)$(datadir)/extension/'

But this is 'make' job to find the file, we should not write that strongly as we
do. This should be:
$(INSTALL_DATA) $< '$(DESTDIR)$(datadir)/extension/'

This is what this patch fixes.

Are those explanations ok ?

PS: out-of-source tree builds are a nice way to prevent collision between
32bit and 64bits built share object, or even more worse situation with
kfreeBSD vs linux, etc. this also makes 'make clean' as fast as a a 'drop
table'.
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation