Message-ID: <51C88483.7090900@dunslane.net>
Date: Mon, 24 Jun 2013 13:40:19 -0400
From: Andrew Dunstan <andrew@dunslane.net>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:17.0) Gecko/20130110 Thunderbird/17.0.2
MIME-Version: 1.0
To: cedric@2ndquadrant.com
CC: pgsql-hackers@postgresql.org, Craig Ringer <craig@2ndquadrant.com>
Subject: Re: Bugfix and new feature for PGXS
References: <201306181552.36673.cedric@2ndquadrant.com>
In-Reply-To: <201306181552.36673.cedric@2ndquadrant.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


On 06/18/2013 09:52 AM, C=C3=A9dric Villemain wrote:
> I've rebased the current set of pending patches I had, to fix pgxs and ad=
ded 2
> new patches.
>
> Bugfixes have already been presented and sent in another thread, except t=
he
> 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/$extensi=
on.
> 2 new variables are set to define directories: $includedir_contrib and
> $includedir_extension, the former default to include/contrib and the late=
r to
> $includedir_contrib/$extension ($extension is the name of the extension).
>
> This allows for example to install hstore header and be able to include t=
hem
> 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 t=
hings
> 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.=20
Items 5 and six are really separate items, which I'm not considering at=20
the moment.

The trouble I have is that there are no instructions on how to modify=20
your Makefile or prepare a vpath tree, so I have been flying blind. I=20
started out doing what Postgres does, namely to prepare the tree using=20
config/prep_buildtree. This doesn't work at all - the paths don't get=20
set properly unless you invoke make with the path to the real makefile,=20
and I'm not sure they all get set correctly then. But that's not what we=20
expect of a vpath setup, or at least not what I expect. I expect that=20
with an appropriately set up make file and a correctly set up build tree=20
I can use an identical make invocation to the one I would use for an=20
in-tree build. That is, after setting up I should be able to ignore the=20
fact that it's a vpath build.

FYI I deliberately chose a non-core extension with an uncommon layout=20
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=20
just to get them out of the way. That means two of the commitfest items=20
I am signed up for will be committed and two marked "Returned with=20
comments". I think we need some more discussion about how VPATH builds=20
should work with extensions, and subject to what limitations if any.

cheers

andrew


--=20
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers