Re: [HACKERS] Bugfix and new feature for PGXS
On 11/19/14 11:11 PM, Peter Eisentraut wrote: I noticed this item was still in the 9.4 code. Looking at the original submission (http://www.postgresql.org/message-id/201306181552.36673.ced...@2ndquadrant.com, patch 0001), I think the original reason for adding this was wrong to begin with. I have applied all three patches to 9.4 and 9.5. So this issue is now resolved. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 12/4/14 11:38 AM, Peter Eisentraut wrote: On 11/19/14 11:11 PM, Peter Eisentraut wrote: I noticed this item was still in the 9.4 code. Looking at the original submission (http://www.postgresql.org/message-id/201306181552.36673.ced...@2ndquadrant.com, patch 0001), I think the original reason for adding this was wrong to begin with. I have applied all three patches to 9.4 and 9.5. So this issue is now resolved. Apparently, some buildfarm module is unhappy with this: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=sittelladt=2014-12-04%2017%3A16%3A29stg=RedisFDW-build Is there some custom code running there? I don't know how that error would happen otherwise? -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 12/04/2014 02:44 PM, Peter Eisentraut wrote: On 12/4/14 11:38 AM, Peter Eisentraut wrote: On 11/19/14 11:11 PM, Peter Eisentraut wrote: I noticed this item was still in the 9.4 code. Looking at the original submission (http://www.postgresql.org/message-id/201306181552.36673.ced...@2ndquadrant.com, patch 0001), I think the original reason for adding this was wrong to begin with. I have applied all three patches to 9.4 and 9.5. So this issue is now resolved. Apparently, some buildfarm module is unhappy with this: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=sittelladt=2014-12-04%2017%3A16%3A29stg=RedisFDW-build Is there some custom code running there? I don't know how that error would happen otherwise? You have broken two buildfarm instances that build and test external modules - in one case the Redis FDW module and in the other the File Text Array FDW. I will see what can be retrieved. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 12/04/2014 03:47 PM, Andrew Dunstan wrote: On 12/04/2014 02:44 PM, Peter Eisentraut wrote: On 12/4/14 11:38 AM, Peter Eisentraut wrote: On 11/19/14 11:11 PM, Peter Eisentraut wrote: I noticed this item was still in the 9.4 code. Looking at the original submission (http://www.postgresql.org/message-id/201306181552.36673.ced...@2ndquadrant.com, patch 0001), I think the original reason for adding this was wrong to begin with. I have applied all three patches to 9.4 and 9.5. So this issue is now resolved. Apparently, some buildfarm module is unhappy with this: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=sittelladt=2014-12-04%2017%3A16%3A29stg=RedisFDW-build Is there some custom code running there? I don't know how that error would happen otherwise? You have broken two buildfarm instances that build and test external modules - in one case the Redis FDW module and in the other the File Text Array FDW. I will see what can be retrieved. I think this needs to be reverted. This has broken two modules that are not even using vpath builds. You can see the relevant Makefiles at: https://github.com/adunstan/file_text_array_fdw/blob/master/Makefile and https://github.com/pg-redis-fdw/redis_fdw/blob/master/Makefile. IIRC, the code you found convoluted and removed was required precisely to prevent this sort of error. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 12/4/14 3:47 PM, Andrew Dunstan wrote: You have broken two buildfarm instances that build and test external modules - in one case the Redis FDW module and in the other the File Text Array FDW. I will see what can be retrieved. This has been fixed. One thing I'll look into sometime is splitting up Makefile.global into parts that apply to PGXS and those that don't (and possibly common parts), because there are too many ifdef PGXS's popping up all over the place in an unorganized fashion. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On Wed, Nov 19, 2014 at 11:11 PM, Peter Eisentraut pete...@gmx.net wrote: The applicable parts of that patch could be backpatched as a bug fix (but evidently no one cares about building contrib with pgxs (except when I submit a patch to remove it)). Touché. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 6/18/13 9:52 AM, Cédric Villemain wrote: 0006-Fix-suggested-layout-for-extension.patch I have committed this patch. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 1/31/14 9:28 PM, Bruce Momjian wrote: On Fri, Jan 31, 2014 at 09:28:06PM -0500, Andrew Dunstan wrote: On 01/31/2014 09:19 PM, Bruce Momjian wrote: On Thu, Oct 10, 2013 at 11:00:30PM -0400, Andrew Dunstan wrote: On 10/10/2013 09:35 PM, Peter Eisentraut wrote: On Tue, 2013-10-08 at 10:04 -0400, Andrew Dunstan wrote: On 10/07/2013 08:47 PM, Peter Eisentraut wrote: I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). suggested fix is below. Hmm, this would duplicate information about shared library naming in a place outside of Makefile.shlib. That doesn't look right. Please suggest an alternative. Did we ever address this? No. I never got a reply, AFAIK. OK, seems there is no known fix then. Thanks. I noticed this item was still in the 9.4 code. Looking at the original submission (http://www.postgresql.org/message-id/201306181552.36673.ced...@2ndquadrant.com, patch 0001), I think the original reason for adding this was wrong to begin with. Attached patch 0001 fixes that. The applicable parts of that patch could be backpatched as a bug fix (but evidently no one cares about building contrib with pgxs (except when I submit a patch to remove it)). In that topic, I also propose attached patch 0002 for 9.4, which removes the use of the USE_VPATH variable and just uses VPATH. There is no need for this additional indirection. Also, USE_FOO variables are typically Boolean, so this is misnamed. I note that the documentation in this topic (http://www.postgresql.org/docs/devel/static/extend-pgxs.html) suggests the use of config/prep_buildtree, but that is not installed, so I don't know how much use that advice is. (I don't know at this point whether just installing it would be a solution.) Thirdly, I propose attached patch 0003, which reverts patch 0004 in the original submission. I believe that patch was unnecessary, and I think the new code is uglier. (Of course, I'm biased, because I wrote the old code.) Technically, this ought to be for 9.5 only. Finally, I have written a test suite for PGXS to support all these outrageous claims: https://github.com/petere/test_pgxs. This tests most of the variables supported in PGXS makefiles and tests that both documented ways of vpath builds work. I don't propose this for inclusion at the moment, because that would require more work, but it's a good reference. From 2f7a2530ab7dbd7a737ff856d9d33b4aaa699dd6 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut peter_e@gmx.net Date: Wed, 19 Nov 2014 21:45:24 -0500 Subject: [PATCH 1/3] Fix SHLIB_PREREQS use in contrib, allowing PGXS builds dblink and postgres_fdw use SHLIB_PREREQS = submake-libpq to build libpq first. This doesn't work in a PGXS build, because there is no libpq to build. So just omit setting SHLIB_PREREQS in this case. Note that PGXS users can still use SHLIB_PREREQS (although it is not documented). The problem here is only that contrib modules can be built in-tree or using PGXS, and the prerequisite is only applicable in the former case. Commit 6697aa2bc25c83b88d6165340348a31328c35de6 previously attempted to address this by creating a somewhat fake submake-libpq target in Makefile.global. That was not the right fix, and it was also done in a nonportable way, so revert that. --- contrib/dblink/Makefile | 2 +- contrib/postgres_fdw/Makefile | 2 +- src/Makefile.global.in| 12 +--- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/contrib/dblink/Makefile b/contrib/dblink/Makefile index 09032f8..7cb0237 100644 --- a/contrib/dblink/Makefile +++ b/contrib/dblink/Makefile @@ -4,7 +4,6 @@ MODULE_big = dblink OBJS = dblink.o PG_CPPFLAGS = -I$(libpq_srcdir) SHLIB_LINK = $(libpq) -SHLIB_PREREQS = submake-libpq EXTENSION = dblink DATA = dblink--1.1.sql dblink--1.0--1.1.sql dblink--unpackaged--1.0.sql @@ -21,6 +20,7 @@ PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) else +SHLIB_PREREQS = submake-libpq subdir = contrib/dblink top_builddir = ../.. include $(top_builddir)/src/Makefile.global diff --git a/contrib/postgres_fdw/Makefile b/contrib/postgres_fdw/Makefile index 8c49720..c0f4160 100644 --- a/contrib/postgres_fdw/Makefile +++ b/contrib/postgres_fdw/Makefile @@ -5,7 +5,6 @@ OBJS = postgres_fdw.o option.o deparse.o connection.o PG_CPPFLAGS = -I$(libpq_srcdir) SHLIB_LINK = $(libpq) -SHLIB_PREREQS = submake-libpq EXTENSION = postgres_fdw DATA = postgres_fdw--1.0.sql @@ -20,6 +19,7 @@ PG_CONFIG = pg_config PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) else +SHLIB_PREREQS = submake-libpq subdir = contrib/postgres_fdw top_builddir = ../.. include $(top_builddir)/src/Makefile.global diff --git a/src/Makefile.global.in b/src/Makefile.global.in index aa54f94..d7a83c8 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -456,23 +456,13 @@ ifeq ($(PORTNAME),cygwin) libpq_pgport += $(LDAP_LIBS_FE)
Re: [HACKERS] Bugfix and new feature for PGXS
On Thu, Oct 10, 2013 at 11:00:30PM -0400, Andrew Dunstan wrote: On 10/10/2013 09:35 PM, Peter Eisentraut wrote: On Tue, 2013-10-08 at 10:04 -0400, Andrew Dunstan wrote: On 10/07/2013 08:47 PM, Peter Eisentraut wrote: I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). suggested fix is below. Hmm, this would duplicate information about shared library naming in a place outside of Makefile.shlib. That doesn't look right. Please suggest an alternative. Did we ever address this? -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 01/31/2014 09:19 PM, Bruce Momjian wrote: On Thu, Oct 10, 2013 at 11:00:30PM -0400, Andrew Dunstan wrote: On 10/10/2013 09:35 PM, Peter Eisentraut wrote: On Tue, 2013-10-08 at 10:04 -0400, Andrew Dunstan wrote: On 10/07/2013 08:47 PM, Peter Eisentraut wrote: I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). suggested fix is below. Hmm, this would duplicate information about shared library naming in a place outside of Makefile.shlib. That doesn't look right. Please suggest an alternative. Did we ever address this? No. I never got a reply, AFAIK. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On Fri, Jan 31, 2014 at 09:28:06PM -0500, Andrew Dunstan wrote: On 01/31/2014 09:19 PM, Bruce Momjian wrote: On Thu, Oct 10, 2013 at 11:00:30PM -0400, Andrew Dunstan wrote: On 10/10/2013 09:35 PM, Peter Eisentraut wrote: On Tue, 2013-10-08 at 10:04 -0400, Andrew Dunstan wrote: On 10/07/2013 08:47 PM, Peter Eisentraut wrote: I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). suggested fix is below. Hmm, this would duplicate information about shared library naming in a place outside of Makefile.shlib. That doesn't look right. Please suggest an alternative. Did we ever address this? No. I never got a reply, AFAIK. OK, seems there is no known fix then. Thanks. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le jeudi 10 octobre 2013 21:37:24 Peter Eisentraut a écrit : On Mon, 2013-10-07 at 22:00 -0400, Andrew Dunstan wrote: The code has been sitting in HEAD for several months, and I committed on the back branches because it was wanted. New features normally go through a full development cycle including extensive beta testing, especially when they contain changes to external interfaces. The fact that the patch author wanted his feature released immediately (of course) doesn't warrant a free pass in this case, IMO. What's new feature ? PGXS break when 19e231b was commited, the patch around this special submake is trying to bugfix that. See http://www.postgresql.org/message-id/201305281410.32535.ced...@2ndquadrant.com There are also reports like this one (and thread): http://www.postgresql.org/message-id/cabrt9rcj6rvgmxbyebcgymwbwiobko_w6zvwrzn75_f+usp...@mail.gmail.com where you can read that I am not 'the' only guy who want to have this commited. And believeing this is worth a backpatch as it stands for a bugfix. Here also the problem is stated as something to fix. http://www.postgresql.org/message-id/20130516165318.gf15...@eldon.alvh.no-ip.org That's being said, you are correct about the problem you found and it's good to be able to release new version without a bug for OSX. I'm just sad you didn't get time to voice a bit before Andrew spent too much time on that. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On Tue, 2013-10-08 at 10:04 -0400, Andrew Dunstan wrote: On 10/07/2013 08:47 PM, Peter Eisentraut wrote: I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). suggested fix is below. Hmm, this would duplicate information about shared library naming in a place outside of Makefile.shlib. That doesn't look right. diff --git a/src/Makefile.global.in b/src/Makefile.global.in index bb732bb..b562378 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -422,7 +422,11 @@ ifndef PGXS submake-libpq: $(MAKE) -C $(libpq_builddir) all else -submake-libpq: $(libdir)/libpq.so ; +ifneq ($(PORTNAME),cygwin) +submake-libpq: $(libdir)/libpq$(DLSUFFIX) ; +else +submake-libpq: $(libdir)/cygpq$(DLSUFFIX) ; +endif endif ifndef PGXS -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On Mon, 2013-10-07 at 22:00 -0400, Andrew Dunstan wrote: The code has been sitting in HEAD for several months, and I committed on the back branches because it was wanted. New features normally go through a full development cycle including extensive beta testing, especially when they contain changes to external interfaces. The fact that the patch author wanted his feature released immediately (of course) doesn't warrant a free pass in this case, IMO. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 10/10/2013 09:37 PM, Peter Eisentraut wrote: On Mon, 2013-10-07 at 22:00 -0400, Andrew Dunstan wrote: The code has been sitting in HEAD for several months, and I committed on the back branches because it was wanted. New features normally go through a full development cycle including extensive beta testing, especially when they contain changes to external interfaces. The fact that the patch author wanted his feature released immediately (of course) doesn't warrant a free pass in this case, IMO. Perhaps you should have stated your objection when this was being discussed, and saved me some time. I frankly think we can be a bit more tolerant about build system features than we would be about the actual software it builds. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 10/10/2013 09:35 PM, Peter Eisentraut wrote: On Tue, 2013-10-08 at 10:04 -0400, Andrew Dunstan wrote: On 10/07/2013 08:47 PM, Peter Eisentraut wrote: I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). suggested fix is below. Hmm, this would duplicate information about shared library naming in a place outside of Makefile.shlib. That doesn't look right. Please suggest an alternative. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 10/07/2013 08:47 PM, Peter Eisentraut wrote: I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). suggested fix is below. cheers andrew diff --git a/src/Makefile.global.in b/src/Makefile.global.in index bb732bb..b562378 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -422,7 +422,11 @@ ifndef PGXS submake-libpq: $(MAKE) -C $(libpq_builddir) all else -submake-libpq: $(libdir)/libpq.so ; +ifneq ($(PORTNAME),cygwin) +submake-libpq: $(libdir)/libpq$(DLSUFFIX) ; +else +submake-libpq: $(libdir)/cygpq$(DLSUFFIX) ; +endif endif ifndef PGXS -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On Sun, 2013-09-29 at 19:09 -0400, Andrew Dunstan wrote: On 09/03/2013 04:04 AM, Cédric Villemain wrote: Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. I applied this one line version of the patch, which seemed more elegant than the later one supplied. I backpatched that and the rest of the VPATH changes for extensiuons to 9.1 where we first provided for extensions, so people can have a reasonably uniform build system for their extensions. I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). Given that this supposedly small and noninvasive set of changes has caused a number of problems already, I suggest we revert this entire thing until we have had time to actually test it somewhere other than in the the stable branches. As it stands, it is a new feature being developed in the stable branches, which is clearly not acceptable. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 10/07/2013 08:47 PM, Peter Eisentraut wrote: On Sun, 2013-09-29 at 19:09 -0400, Andrew Dunstan wrote: On 09/03/2013 04:04 AM, Cédric Villemain wrote: Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. I applied this one line version of the patch, which seemed more elegant than the later one supplied. I backpatched that and the rest of the VPATH changes for extensiuons to 9.1 where we first provided for extensions, so people can have a reasonably uniform build system for their extensions. I suspect this line submake-libpq: $(libdir)/libpq.so ; will cause problems on platforms with a different extension (e.g. OS X). Given that this supposedly small and noninvasive set of changes has caused a number of problems already, I suggest we revert this entire thing until we have had time to actually test it somewhere other than in the the stable branches. As it stands, it is a new feature being developed in the stable branches, which is clearly not acceptable. If you want to revert it then go ahead, but the last statement is simply incorrect. The code has been sitting in HEAD for several months, and I committed on the back branches because it was wanted. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le lundi 30 septembre 2013 00:10:09 Andrew Dunstan a écrit : On 09/29/2013 07:09 PM, Andrew Dunstan wrote: On 09/03/2013 04:04 AM, Cédric Villemain wrote: Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. I applied this one line version of the patch, which seemed more elegant than the later one supplied. I backpatched that and the rest of the VPATH changes for extensiuons to 9.1 where we first provided for extensions, so people can have a reasonably uniform build system for their extensions. I have reverted this in the 9.2 and 9.1 branches until I can work out why it's breaking the buildfarm. 9.3 seems to be fine. Yes, please take the longer but better patch following the small one. There are eveidences that it fixes compilation to always build installdirs first. Previously installdirs may be build after copying files, so it fails. Chritstoph authored this good patch. I'm sorry not to have been clear on that. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 09/03/2013 04:04 AM, Cédric Villemain wrote: Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. I applied this one line version of the patch, which seemed more elegant than the later one supplied. I backpatched that and the rest of the VPATH changes for extensiuons to 9.1 where we first provided for extensions, so people can have a reasonably uniform build system for their extensions. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 09/29/2013 07:09 PM, Andrew Dunstan wrote: On 09/03/2013 04:04 AM, Cédric Villemain wrote: Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. I applied this one line version of the patch, which seemed more elegant than the later one supplied. I backpatched that and the rest of the VPATH changes for extensiuons to 9.1 where we first provided for extensions, so people can have a reasonably uniform build system for their extensions. I have reverted this in the 9.2 and 9.1 branches until I can work out why it's breaking the buildfarm. 9.3 seems to be fine. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. Also, the bugfixes were supposed to be backported. The behavior is not changed, nothing new, just fixing problem for some kind of extension builds. (However the USE_VPATH is new and is not needed in 9.3) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et FormationFrom fb9d5ed99397b40e7fc33224fa31cd5c54c7b5d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= ced...@2ndquadrant.fr Date: Tue, 3 Sep 2013 09:55:05 +0200 Subject: [PATCH] Add installdirs to PGXS order-only-prerequisites prerequisites are not ordered but installdirs is required by others. This patch is the simplest one, another solution being to remove completely installdirs prerequisite. Spotted by Christoph Berg with plr build. --- src/makefiles/pgxs.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index 8618aa1..ac12f7d 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -124,7 +124,7 @@ all: all-lib endif # MODULE_big -install: all installdirs installcontrol installdata installdatatsearch installdocs installscripts +install: all installcontrol installdata installdatatsearch installdocs installscripts | installdirs ifdef MODULES $(INSTALL_SHLIB) $(addsuffix $(DLSUFFIX), $(MODULES)) '$(DESTDIR)$(pkglibdir)/' endif # MODULES -- 1.8.4.rc3 signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 09/03/2013 04:02 AM, Cédric Villemain wrote: Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. Just a remember that the doc is written and is waiting to be commited. There is also an issue spoted by Christoph with the installdirs prerequisite, the attached patch fix that. Also, the bugfixes were supposed to be backported. The behavior is not changed, nothing new, just fixing problem for some kind of extension builds. (However the USE_VPATH is new and is not needed in 9.3) Darn, I knew I had forgotten something. Mea maxima culpa. Well, the 9.3 tarballs have been cut, unfortunately, but I'll get to this as soon as I can. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le lundi 8 juillet 2013 21:46:39, Andrew Dunstan a écrit : On 07/08/2013 03:40 PM, Josh Berkus wrote: On 07/04/2013 06:18 AM, Andrew Dunstan wrote: On 07/04/2013 09:14 AM, Cédric Villemain wrote: ah yes, good catch, I though .control file were unique per contrib, but there aren't. It's already been fixed. Does it look like this patch will get into committable shape in the next couple of days? I think everything has been committed - as I think the CF app shows. The only thing left in this srea from Cédric is the insallation of headers, which Peter is down to review and is in Waiting on Author status. which is returned with feedback now, I can update if someone really wants it. We are owed a docco patch though. Simple one, attached. I didn't document USE_VPATH, not sure how to explain that clearly. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation From 1380870e061362b871c375c25517005f82358dc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= ced...@2ndquadrant.fr Date: Fri, 12 Jul 2013 23:39:11 +0200 Subject: [PATCH] add documentation for commit 6697aa2 Document that PGXS offers VPATH build and add a sample like in installation.sgml Also update the part about the PGXS global makefile inclusion. It was suggested to put it at the end, but if the Makefile contains a target before the include of pgxs then the default all: target is removed and the Makefile must define all: itself. --- doc/src/sgml/extend.sgml | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml index 60fa1a8..8f082e4 100644 --- a/doc/src/sgml/extend.sgml +++ b/doc/src/sgml/extend.sgml @@ -935,7 +935,7 @@ include $(PGXS) To use the acronymPGXS/acronym infrastructure for your extension, you must write a simple makefile. In the makefile, you need to set some variables -and finally include the global acronymPGXS/acronym makefile. +and include the global acronymPGXS/acronym makefile. Here is an example that builds an extension module named literalisbn_issn/literal, consisting of a shared library containing some C code, an extension control file, a SQL script, and a documentation @@ -1161,6 +1161,19 @@ include $(PGXS) or on the literalmake/literal command line. /para + para +You can also run literalmake/literal in a directory outside the source +tree of your extension, if you want to keep the build directory separate. +This procedure is also called a +indextermprimaryVPATH/primary/indextermfirsttermVPATH/firstterm +build. Here's how: +screen + userinputmkdir build_dir/userinput + userinputcd build_dir/userinput + userinputmake -f /path/to/extension/source/tree/Makefile/userinput + userinputmake -f /path/to/extension/source/tree/Makefile install/userinput +/screen + /para caution para Changing varnamePG_CONFIG/varname only works when building -- 1.7.10.4 signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 07/08/2013 12:51 PM, Josh Berkus wrote: I think everything has been committed - as I think the CF app shows. The only thing left in this srea from Cédric is the insallation of headers, which Peter is down to review and is in Waiting on Author status. Yeah, that's the one I'm asking about. is that going to get done in the next few days, or does it bounce? OK, I'm setting this to returned with feedback because there has been no further discussion of this patch here in the last several days. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 07/04/2013 06:18 AM, Andrew Dunstan wrote: On 07/04/2013 09:14 AM, Cédric Villemain wrote: ah yes, good catch, I though .control file were unique per contrib, but there aren't. It's already been fixed. Does it look like this patch will get into committable shape in the next couple of days? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 07/08/2013 03:40 PM, Josh Berkus wrote: On 07/04/2013 06:18 AM, Andrew Dunstan wrote: On 07/04/2013 09:14 AM, Cédric Villemain wrote: ah yes, good catch, I though .control file were unique per contrib, but there aren't. It's already been fixed. Does it look like this patch will get into committable shape in the next couple of days? I think everything has been committed - as I think the CF app shows. The only thing left in this srea from Cédric is the insallation of headers, which Peter is down to review and is in Waiting on Author status. We are owed a docco patch though. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
I think everything has been committed - as I think the CF app shows. The only thing left in this srea from Cédric is the insallation of headers, which Peter is down to review and is in Waiting on Author status. Yeah, that's the one I'm asking about. is that going to get done in the next few days, or does it bounce? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
I'm not sure whether this is as simple as changing $ to $^ in the pgxs.mk's installcontrol rule, or if something more is required. Could you take a look? will do. ah yes, good catch, I though .control file were unique per contrib, but there aren't. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation
Re: [HACKERS] Bugfix and new feature for PGXS
On 07/04/2013 09:14 AM, Cédric Villemain wrote: I'm not sure whether this is as simple as changing $ to $^ in the pgxs.mk's installcontrol rule, or if something more is required. Could you take a look? will do. ah yes, good catch, I though .control file were unique per contrib, but there aren't. It's already been fixed. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On Mon, Jul 1, 2013 at 5:04 PM, Andrew Dunstan and...@dunslane.net wrote: These changes are fairly small and mostly non-invasive, but if I've broken something we should find out about it fairly quickly, I hope. Turns out you broke something. Specifically, contrib/spi now only installs autoinc.control instead of all the control files for extensions in that directory. Before: /usr/bin/install -c -m 644 ./autoinc.control ./insert_username.control ./moddatetime.control ./refint.control ./timetravel.control '/Users/rhaas/project/share/postgresql/extension/' /usr/bin/install -c -m 644 ./autoinc--1.0.sql ./autoinc--unpackaged--1.0.sql ./insert_username--1.0.sql ./insert_username--unpackaged--1.0.sql ./moddatetime--1.0.sql ./moddatetime--unpackaged--1.0.sql ./refint--1.0.sql ./refint--unpackaged--1.0.sql ./timetravel--1.0.sql ./timetravel--unpackaged--1.0.sql '/Users/rhaas/project/share/postgresql/extension/' /usr/bin/install -c -m 755 autoinc.so insert_username.so moddatetime.so refint.so timetravel.so '/Users/rhaas/project/lib/postgresql/' /usr/bin/install -c -m 644 ./autoinc.example ./insert_username.example ./moddatetime.example ./refint.example ./timetravel.example '/Users/rhaas/project/share/doc//postgresql/extension/' Now: /usr/bin/install -c -m 644 autoinc.control '/Users/rhaas/project/share/postgresql/extension/' /usr/bin/install -c -m 644 autoinc--1.0.sql autoinc--unpackaged--1.0.sql insert_username--1.0.sql insert_username--unpackaged--1.0.sql moddatetime--1.0.sql moddatetime--unpackaged--1.0.sql refint--1.0.sql refint--unpackaged--1.0.sql timetravel--1.0.sql timetravel--unpackaged--1.0.sql '/Users/rhaas/project/share/postgresql/extension/' /usr/bin/install -c -m 644 autoinc.example insert_username.example moddatetime.example refint.example timetravel.example '/Users/rhaas/project/share/doc//postgresql/extension/' /usr/bin/install -c -m 755 autoinc.so insert_username.so moddatetime.so refint.so timetravel.so '/Users/rhaas/project/lib/postgresql/' I'm not sure whether this is as simple as changing $ to $^ in the pgxs.mk's installcontrol rule, or if something more is required. Could you take a look? Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 07/03/2013 05:52 PM, Robert Haas wrote: On Mon, Jul 1, 2013 at 5:04 PM, Andrew Dunstan and...@dunslane.net wrote: These changes are fairly small and mostly non-invasive, but if I've broken something we should find out about it fairly quickly, I hope. Turns out you broke something. Specifically, contrib/spi now only installs autoinc.control instead of all the control files for extensions in that directory. ... I'm not sure whether this is as simple as changing $ to $^ in the pgxs.mk's installcontrol rule, or if something more is required. Could you take a look? will do. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le samedi 29 juin 2013 22:00:34, Josh Berkus a écrit : On 06/29/2013 11:42 AM, Andrew Dunstan wrote: I think we can survive for now without an additional tool. What I did was a proof of concept, it was not intended as a suggestion that we should install prep_buildtree. I am only suggesting that, in addition to your changes, if USE_VPATH is set then that path is used instead of a path inferred from the name of the Makefile. SO is this patch returned with feedback? Only the 0005-Allows-extensions-to-install-header-file.patch , others are in the hands of Andrew. Additional patch required for documentation. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 6/29/13 1:54 PM, Andrew Dunstan wrote: I haven't seen a response to this. One thing we are missing is documentation. Given that I'm inclined to commit all of this (i.e. cedric's patches 1,2,3, and 4 plus my addition). Could someone post an updated set of patches that is currently under consideration? I'm also inclined to backpatch it, since without that it seems to me unlikely packagers will be able to make practical use of it for several years, and the risk is very low. Actually, the risk of makefile changes is pretty high, especially in cases involving advanced features such as vpath. GNU make hasn't been as stable is one might think, lately. We should carefully consider exactly which parts are worth backpatching. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 07/01/2013 04:39 PM, Peter Eisentraut wrote: On 6/29/13 1:54 PM, Andrew Dunstan wrote: I haven't seen a response to this. One thing we are missing is documentation. Given that I'm inclined to commit all of this (i.e. cedric's patches 1,2,3, and 4 plus my addition). Could someone post an updated set of patches that is currently under consideration? See what I actually committed today. I'm also inclined to backpatch it, since without that it seems to me unlikely packagers will be able to make practical use of it for several years, and the risk is very low. Actually, the risk of makefile changes is pretty high, especially in cases involving advanced features such as vpath. GNU make hasn't been as stable is one might think, lately. We should carefully consider exactly which parts are worth backpatching. These changes are fairly small and mostly non-invasive, but if I've broken something we should find out about it fairly quickly, I hope. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/26/2013 10:52 AM, Andrew Dunstan wrote: On 06/25/2013 11:29 AM, Cédric Villemain wrote: Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : On 06/24/2013 07:24 PM, Cédric Villemain wrote: Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. I still think this is premature. I have spent some more time trying to make it work the way I think it should, so far without success. I think we need more discussion about how we'd like VPATH to work for PGXS would. There's no urgency about this - we've lived with the current situation for quite a while. Sure... I did a quick and dirty patch (I just validate without lot of testing), attached to this email to fix json_build (at least for make, make install) As you're constructing targets based on commands to execute in the srcdir directory, and because srcdir is only set in pgxs.mk, it is possible to define srcdir early in the json_build/Makefile and use it. This still doesn't do what I really want, which is to be able to invoke make without anything special in the invocation that triggers VPATH processing. Here's what I did that works like I think it should. First the attached patch on top of yours. Second, I did: mkdir vpath.json_build cd vpath.json_build sh/path/to/pgsource/config/prep_buildtree ../json_build . ln -s ../json_build/json_build.control . Then I created vpath.mk with these contents: ext_srcdir = ../json_build USE_VPATH = $(ext_srcdir) Finally I vpath-ized the Makefile (see attached). Given all of that, I was able to do, in the vpath directory: make make install make installcheck make clean and it all just worked, with exactly the same make invocations as work in an in-tree build. So what's lacking to make this nice is a tool to automate the second and third steps above. Maybe there are other ways of doing this, but this does what I'd like to see. I haven't seen a response to this. One thing we are missing is documentation. Given that I'm inclined to commit all of this (i.e. cedric's patches 1,2,3, and 4 plus my addition). I'm also inclined to backpatch it, since without that it seems to me unlikely packagers will be able to make practical use of it for several years, and the risk is very low. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
[it seems my first email didn't make it, sent again] Le mercredi 26 juin 2013 16:52:01, Andrew Dunstan a écrit : On 06/25/2013 11:29 AM, Cédric Villemain wrote: Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : On 06/24/2013 07:24 PM, Cédric Villemain wrote: Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. I still think this is premature. I have spent some more time trying to make it work the way I think it should, so far without success. I think we need more discussion about how we'd like VPATH to work for PGXS would. There's no urgency about this - we've lived with the current situation for quite a while. Sure... I did a quick and dirty patch (I just validate without lot of testing), attached to this email to fix json_build (at least for make, make install) As you're constructing targets based on commands to execute in the srcdir directory, and because srcdir is only set in pgxs.mk, it is possible to define srcdir early in the json_build/Makefile and use it. This still doesn't do what I really want, which is to be able to invoke make without anything special in the invocation that triggers VPATH processing. I believe it is the case currently (with my patches applyed), we just have to invoke Makefile like we invoke configure for PostgreSQL, except that we don't need a configure stage with the contribs. Obviously it is not exactly the same because 'configure' is a script to execute, but 'make' is a command with a configuration file (the Makefile) For PostgreSQL it is: $ mkdir build_dir $ cd build_dir $ path/to/source/tree/configure [options go here] $ gmake For contribs it is: $ mkdir build_dir $ cd build_dir $ gmake -f /path/to/source/tree/Makefile [options go here] Here's what I did that works like I think it should. First the attached patch on top of yours. Second, I did: mkdir vpath.json_build cd vpath.json_build sh/path/to/pgsource/config/prep_buildtree ../json_build . ln -s ../json_build/json_build.control . OK, this creates supposedly required directories in the build process. This looks a bit wrong and more work than required: not all the source directories are required by the build process. But I understand the point. Second, config/prep_buildtree is not installed (by make install), so it is not suitable to use it with PGXS (PGXS is found with pg_config, when the postgresql source tree is not accessible anymore). We may add config/prep_buildtree to INSTALL_PROGRAM. The 'ln -s' can probably be copied by prep_buildtree. Then I created vpath.mk with these contents: ext_srcdir = ../json_build USE_VPATH = $(ext_srcdir) OK. Finally I vpath-ized the Makefile (see attached). I don't see how this is more pretty than other solution. Given all of that, I was able to do, in the vpath directory: make make install make installcheck make clean and it all just worked, with exactly the same make invocations as work in an in-tree build. It breaks others: mkdir /tmp/auth_delay cd /tmp/auth_delay sh /path/prep_buildtree /path/contrib/auth_delay/ . (no .control, it's not an extension) make make: *** Pas de règle pour fabriquer la cible « auth_delay.so », nécessaire pour « all ». Arrêt. This works: cd /tmp/auth_delay rm -rf * make -f /path/contrib/auth_delay/Makefile PS: I exported USE_PGXS=1 because of the switch case in the makefile. So what's lacking to make this nice is a tool to automate the second and third steps above. If the second and third steps are automatized, you'll still have to invoke them at some point. How ? mkdir /tmp/json_build cd /tmp/json_build make make install make installcheck make clean - this will never work, make won't find anything to do. You'll need the equivalent of configure (as we do for PostgreSQL, I agree) to trigger prep_buildtree: mkdir /tmp/json_build cd /tmp/json_build $ path/to/source/tree/configure [options go here] make make install make installcheck make clean
Re: [HACKERS] Bugfix and new feature for PGXS
Le samedi 29 juin 2013 19:54:53, Andrew Dunstan a écrit : On 06/26/2013 10:52 AM, Andrew Dunstan wrote: On 06/25/2013 11:29 AM, Cédric Villemain wrote: Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : On 06/24/2013 07:24 PM, Cédric Villemain wrote: Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. I still think this is premature. I have spent some more time trying to make it work the way I think it should, so far without success. I think we need more discussion about how we'd like VPATH to work for PGXS would. There's no urgency about this - we've lived with the current situation for quite a while. Sure... I did a quick and dirty patch (I just validate without lot of testing), attached to this email to fix json_build (at least for make, make install) As you're constructing targets based on commands to execute in the srcdir directory, and because srcdir is only set in pgxs.mk, it is possible to define srcdir early in the json_build/Makefile and use it. This still doesn't do what I really want, which is to be able to invoke make without anything special in the invocation that triggers VPATH processing. Here's what I did that works like I think it should. First the attached patch on top of yours. Second, I did: mkdir vpath.json_build cd vpath.json_build sh/path/to/pgsource/config/prep_buildtree ../json_build . ln -s ../json_build/json_build.control . Then I created vpath.mk with these contents: ext_srcdir = ../json_build USE_VPATH = $(ext_srcdir) Finally I vpath-ized the Makefile (see attached). Given all of that, I was able to do, in the vpath directory: make make install make installcheck make clean and it all just worked, with exactly the same make invocations as work in an in-tree build. So what's lacking to make this nice is a tool to automate the second and third steps above. Maybe there are other ways of doing this, but this does what I'd like to see. I haven't seen a response to this. One thing we are missing is documentation. Given that I'm inclined to commit all of this (i.e. cedric's patches 1,2,3, and 4 plus my addition). I did so I sent the mail again. I believe your addition need some changes to allow the PGXS+VPATH case as it currently depends on the source-tree tool. So it needs to be in postgresql-server-devel packages, thus installed by postgresql make install. I'm going to update the doc. It's probably worth to have some examples. I'm also inclined to backpatch it, since without that it seems to me unlikely packagers will be able to make practical use of it for several years, and the risk is very low. Yes, it is valuable to help packagers here, thanks. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/29/2013 02:27 PM, Cédric Villemain wrote: I haven't seen a response to this. One thing we are missing is documentation. Given that I'm inclined to commit all of this (i.e. cedric's patches 1,2,3, and 4 plus my addition). I did so I sent the mail again. I believe your addition need some changes to allow the PGXS+VPATH case as it currently depends on the source-tree tool. So it needs to be in postgresql-server-devel packages, thus installed by postgresql make install. I'm going to update the doc. It's probably worth to have some examples. I think we can survive for now without an additional tool. What I did was a proof of concept, it was not intended as a suggestion that we should install prep_buildtree. I am only suggesting that, in addition to your changes, if USE_VPATH is set then that path is used instead of a path inferred from the name of the Makefile. A tool to set up a full vpath build tree for extensions or external modules seems to be beyond the scope of this. I'm also inclined to backpatch it, since without that it seems to me unlikely packagers will be able to make practical use of it for several years, and the risk is very low. Yes, it is valuable to help packagers here, thanks. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/29/2013 11:42 AM, Andrew Dunstan wrote: I think we can survive for now without an additional tool. What I did was a proof of concept, it was not intended as a suggestion that we should install prep_buildtree. I am only suggesting that, in addition to your changes, if USE_VPATH is set then that path is used instead of a path inferred from the name of the Makefile. SO is this patch returned with feedback? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 26 juin 2013 16:52:01, Andrew Dunstan a écrit : On 06/25/2013 11:29 AM, Cédric Villemain wrote: Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : On 06/24/2013 07:24 PM, Cédric Villemain wrote: Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. I still think this is premature. I have spent some more time trying to make it work the way I think it should, so far without success. I think we need more discussion about how we'd like VPATH to work for PGXS would. There's no urgency about this - we've lived with the current situation for quite a while. Sure... I did a quick and dirty patch (I just validate without lot of testing), attached to this email to fix json_build (at least for make, make install) As you're constructing targets based on commands to execute in the srcdir directory, and because srcdir is only set in pgxs.mk, it is possible to define srcdir early in the json_build/Makefile and use it. This still doesn't do what I really want, which is to be able to invoke make without anything special in the invocation that triggers VPATH processing. I believe it is the case currently (with my patches applyed), we just have to invoke Makefile like we invoke configure for PostgreSQL, except that we don't need a configure stage with the contribs. Obviously it is not exactly the same because 'configure' is a script to execute, but 'make' is a command with a configuration file (the Makefile) For PostgreSQL it is: $ mkdir build_dir $ cd build_dir $ path/to/source/tree/configure [options go here] $ gmake For contribs it is: $ mkdir build_dir $ cd build_dir $ gmake -f /path/to/source/tree/Makefile [options go here] Here's what I did that works like I think it should. First the attached patch on top of yours. Second, I did: mkdir vpath.json_build cd vpath.json_build sh/path/to/pgsource/config/prep_buildtree ../json_build . ln -s ../json_build/json_build.control . OK, this creates supposedly required directories in the build process. This looks a bit wrong and more work than required: not all the source directories are required by the build process. But I understand the point. Second, config/prep_buildtree is not installed (by make install), so it is not suitable to use it with PGXS (PGXS is found with pg_config, when the postgresql source tree is not accessible anymore). We may add config/prep_buildtree to INSTALL_PROGRAM. The 'ln -s' can probably be copied by prep_buildtree. Then I created vpath.mk with these contents: ext_srcdir = ../json_build USE_VPATH = $(ext_srcdir) OK. Finally I vpath-ized the Makefile (see attached). I don't see how this is more pretty than other solution. Given all of that, I was able to do, in the vpath directory: make make install make installcheck make clean and it all just worked, with exactly the same make invocations as work in an in-tree build. It breaks others: mkdir /tmp/auth_delay cd /tmp/auth_delay sh /path/prep_buildtree /path/contrib/auth_delay/ . (no .control, it's not an extension) make make: *** Pas de règle pour fabriquer la cible « auth_delay.so », nécessaire pour « all ». Arrêt. This works: cd /tmp/auth_delay rm -rf * make -f /path/contrib/auth_delay/Makefile PS: I exported USE_PGXS=1 because of the switch case in the makefile. So what's lacking to make this nice is a tool to automate the second and third steps above. If the second and third steps are automatized, you'll still have to invoke them at some point. How ? mkdir /tmp/json_build cd /tmp/json_build make make install make installcheck make clean - this will never work, make won't find anything to do. You'll need the equivalent of configure (as we do for PostgreSQL, I agree) to trigger prep_buildtree: mkdir /tmp/json_build cd /tmp/json_build $ path/to/source/tree/configure [options go here] make make install make installcheck make clean Maybe there are other ways of doing this, but this
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/25/2013 11:29 AM, Cédric Villemain wrote: Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : On 06/24/2013 07:24 PM, Cédric Villemain wrote: Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. I still think this is premature. I have spent some more time trying to make it work the way I think it should, so far without success. I think we need more discussion about how we'd like VPATH to work for PGXS would. There's no urgency about this - we've lived with the current situation for quite a while. Sure... I did a quick and dirty patch (I just validate without lot of testing), attached to this email to fix json_build (at least for make, make install) As you're constructing targets based on commands to execute in the srcdir directory, and because srcdir is only set in pgxs.mk, it is possible to define srcdir early in the json_build/Makefile and use it. This still doesn't do what I really want, which is to be able to invoke make without anything special in the invocation that triggers VPATH processing. Here's what I did that works like I think it should. First the attached patch on top of yours. Second, I did: mkdir vpath.json_build cd vpath.json_build sh/path/to/pgsource/config/prep_buildtree ../json_build . ln -s ../json_build/json_build.control . Then I created vpath.mk with these contents: ext_srcdir = ../json_build USE_VPATH = $(ext_srcdir) Finally I vpath-ized the Makefile (see attached). Given all of that, I was able to do, in the vpath directory: make make install make installcheck make clean and it all just worked, with exactly the same make invocations as work in an in-tree build. So what's lacking to make this nice is a tool to automate the second and third steps above. Maybe there are other ways of doing this, but this does what I'd like to see. cheers andrew diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index f70d5f7..42e3654 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -61,18 +61,21 @@ ifdef PGXS top_builddir := $(dir $(PGXS))../.. include $(top_builddir)/src/Makefile.global -# If Makefile is not in current directory we are building the extension with -# VPATH so we set the variable here -# XXX what about top_srcdir ? -ifeq ($(CURDIR),$(dir $(firstword $(MAKEFILE_LIST top_srcdir = $(top_builddir) +# If USE_VPATH is set or Makefile is not in current directory we are building +# the extension with VPATH so we set the variable here +ifdef USE_VPATH +srcdir = $(USE_VPATH) +VPATH = $(USE_VPATH) +else +ifeq ($(CURDIR),$(dir $(firstword $(MAKEFILE_LIST srcdir = . VPATH = else -top_srcdir = $(top_builddir) srcdir = $(dir $(firstword $(MAKEFILE_LIST))) VPATH = $(srcdir) endif +endif # These might be set in Makefile.global, but if they were not found # during the build of PostgreSQL, supply default values so that users EXTENSION= json_build ifeq ($(wildcard vpath.mk),vpath.mk) include vpath.mk else ext_srcdir = . endif EXTVERSION = $(shell grep default_version $(EXTENSION).control | sed -e s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/) DATA = $(filter-out $(wildcard sql/*--*.sql),$(wildcard sql/*.sql)) DOCS = $(wildcard $(ext_srcdir)/doc/*.md) USE_MODULE_DB = 1 TESTS= $(wildcard $(ext_srcdir)/test/sql/*.sql) REGRESS_OPTS = --inputdir=$(ext_srcdir)/test --outputdir=test \ --load-extension=$(EXTENSION) REGRESS = $(patsubst $(ext_srcdir)/test/sql/%.sql,%,$(TESTS)) MODULE_big = $(EXTENSION) OBJS = $(patsubst $(ext_srcdir)/src/%.c,src/%.o,$(wildcard $(ext_srcdir)/src/*.c)) PG_CONFIG= pg_config all: sql/$(EXTENSION)--$(EXTVERSION).sql sql/$(EXTENSION)--$(EXTVERSION).sql: $(ext_srcdir)/sql/$(EXTENSION).sql cp $ $@ DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql DATA = $(filter-out $(ext_srcdir)/sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard $(ext_srcdir)/sql/*--*.sql)) EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) # we put all the tests in a test subdir, but
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/24/2013 07:24 PM, Cédric Villemain wrote: Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. I still think this is premature. I have spent some more time trying to make it work the way I think it should, so far without success. I think we need more discussion about how we'd like VPATH to work for PGXS would. There's no urgency about this - we've lived with the current situation for quite a while. When I have more time I will work on it some more. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a écrit : On 06/24/2013 07:24 PM, Cédric Villemain wrote: Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. I still think this is premature. I have spent some more time trying to make it work the way I think it should, so far without success. I think we need more discussion about how we'd like VPATH to work for PGXS would. There's no urgency about this - we've lived with the current situation for quite a while. Sure... I did a quick and dirty patch (I just validate without lot of testing), attached to this email to fix json_build (at least for make, make install) As you're constructing targets based on commands to execute in the srcdir directory, and because srcdir is only set in pgxs.mk, it is possible to define srcdir early in the json_build/Makefile and use it. When I have more time I will work on it some more. Thank you -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation diff --git a/Makefile b/Makefile index ce10008..87582d1 100644 --- a/Makefile +++ b/Makefile @@ -1,24 +1,28 @@ EXTENSION= json_build -EXTVERSION = $(shell grep default_version $(EXTENSION).control | sed -e s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/) -DATA = $(filter-out $(wildcard sql/*--*.sql),$(wildcard sql/*.sql)) -DOCS = $(wildcard doc/*.md) +srcdir = $(dir $(firstword $(MAKEFILE_LIST))) +EXTVERSION = $(shell grep default_version $(srcdir)/$(EXTENSION).control | sed -e s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/) + +DATA = $(filter-out $(wildcard $(srcdir)/sql/*--*.sql),$(wildcard $(srcdir)/sql/*.sql)) +DOCS = $(wildcard $(srcdir)/doc/*.md) USE_MODULE_DB = 1 -TESTS= $(wildcard test/sql/*.sql) +TESTS= $(wildcard $(srcdir)/test/sql/*.sql) REGRESS_OPTS = --inputdir=test --outputdir=test \ --load-extension=$(EXTENSION) -REGRESS = $(patsubst test/sql/%.sql,%,$(TESTS)) +REGRESS = $(patsubst $(srcdir)/test/sql/%.sql,%,$(TESTS)) MODULE_big = $(EXTENSION) -OBJS = $(patsubst src/%.c,src/%.o,$(wildcard src/*.c)) +OBJS = $(patsubst $(srcdir)/src/%.c,$(srcdir)/src/%.o,$(wildcard $(srcdir)/src/*.c)) PG_CONFIG= pg_config all: sql/$(EXTENSION)--$(EXTVERSION).sql sql/$(EXTENSION)--$(EXTVERSION).sql: sql/$(EXTENSION).sql + $(MKDIR_P) sql cp $ $@ + DATA_built = sql/$(EXTENSION)--$(EXTVERSION).sql -DATA = $(filter-out sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard sql/*--*.sql)) +DATA = $(filter-out $(srcdir)/sql/$(EXTENSION)--$(EXTVERSION).sql, $(wildcard $(srcdir)/sql/*--*.sql)) EXTRA_CLEAN = sql/$(EXTENSION)--$(EXTVERSION).sql PGXS := $(shell $(PG_CONFIG) --pgxs) signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
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. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 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
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. 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 It doesn't work: [andrew@emma vpath.json_build]$ PATH=../../inst.vpgxs.5706/bin:$PATH make -f `pwd`/../json_build/Makefile grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -fpic -shared -o json_build.so -L/home/pgl/npgl/inst.vpgxs.5706/lib -Wl,--as-needed -Wl,-rpath,'/home/pgl/npgl/inst.vpgxs.5706/lib',--enable-new-dtags cp /home/andrew/pgl/extns/vpath.json_build/../json_build/sql/json_build.sql sql/json_build--.sql cp: cannot create regular file `sql/json_build--.sql': No such file or directory make: *** [sql/json_build--.sql] Error 1 [andrew@emma vpath.json_build]$ cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a écrit : On 06/24/2013 04:02 PM, Cédric Villemain wrote: 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. No, this is exactly what I'm objecting to. I want to be able to do: invoke_vpath_magic standard_make_commands_as_for_non_vpath Your patches have been designed to overcome your particular issues, but they don't meet the general case, IMNSHO. This is why I want to have more discussion about how vpath builds could work for PGXS modules. The patch does not restrict anything, it is not supposed to lead to regression. The assignment of VPATH and srcdir are wrong in the PGXS case, the patch correct them. You're still free to do make VPATH=/mypath ... the VPATH provided won't be erased by the pgxs.mk logic. 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 It doesn't work: [andrew@emma vpath.json_build]$ PATH=../../inst.vpgxs.5706/bin:$PATH make -f `pwd`/../json_build/Makefile grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory grep: json_build.control: No such file or directory gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -fpic -shared -o json_build.so -L/home/pgl/npgl/inst.vpgxs.5706/lib -Wl,--as-needed -Wl,-rpath,'/home/pgl/npgl/inst.vpgxs.5706/lib',--enable-new-dtags cp /home/andrew/pgl/extns/vpath.json_build/../json_build/sql/json_build.sql sql/json_build--.sql cp: cannot create regular file `sql/json_build--.sql': No such file or directory make: *** [sql/json_build--.sql] Error 1 [andrew@emma vpath.json_build]$ Ah, you make the point : your Makefile does not support VPATH or I failed to consider some cases. It seems to me that : EXTVERSION = $(shell grep default_version $(EXTENSION).control | sed -e s/default_version[[:space:]]*=[[:space:]]*'\([^']*\)'/\1/) is not working. I'm going to check GNU Make guidelines about that case (should $(command) be executed on each path in the VPATH or not ?) [...] thinking a bit more... I suppose gmake expects the Makefile to list $(EXTENSION).control file as a prerequisite (thus its paths will be known during make invocation and your command will work) -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Le jeudi 20 juin 2013 05:26:21, Peter Eisentraut a écrit :
On Wed, 2013-06-19 at 20:58 +0200, Cédric Villemain wrote:
I believe he answered the proposal to put all headers on the same flat
directory, instead of a tree.
The headers are used as
#include hstore.h
#include ltree.h
etc.
in the existing source code.
If you want to install the for use by others, you can do one of three
things:
1) Install them into $(pg_config --includedir-server), so other users
will just include them in the same way as shown above.
I don't like this one because extension can overwrite sever headers.
2) Install them in a different directory, but keep the same #include
lines. That would require PGXS changes, perhaps a new pg_config option,
or something that produces the right -I option to find them.
Patch of PGXS is not a problem.
pg_config patch is a requirement only if users set their own
$(includedir_contrib) variable. I didn't though about it, but it is probably
better to add that. This looks trivial too.
To include hstore header we write «#include hstore.h» but we add :
-I$(includedir_contrib)/hstore to the FLAGS in the extension makefile which
does require hstore. It changes nothing to hstore Makefile itself.
The main difference is that before we had to -I$(top_srcdir)/../contrib/hstore
*iff* we are not building with PGXS (else we need to have the hstore source
available somewhere), now we can do -I$(includedir_contrib)/hstore with PGXS
(hstore need to be installed first, as we USE_PGXS)
Then PGXS offers to catch both case transparently so we can do :
-I${include_contrib_header) in Makefile
and pgxs.mk takes care of adjusting include_contrib_header (or whatever its
name) according to USE_PGXS value.
3) Install them in a different directory and require a different
#include line. But then you have to change the existing uses as well,
which would probably require moving files around.
Having to change existing source code do keep the same behavior is not
attractive at all.
Both 2 and 3 require a lot of work, possibly compatibility breaks, for
no obvious reason.
2 is a good solution and not a lot of work, IMO.
Removing the need of setting -I$(include...) in the contrib Makefile can be
done later by adding a PGXS variable to define the contrib requirements, this
is something different from the current feature (build contrib depending on
another(s) without full source tree)
--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation
signature.asc
Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Opinions all? * Give up on being able to use one extension's header from another entirely, except in the special case of in-tree builds There are already a good number of use cases with only hstore and intarray, I'm in favor of this feature. * Install install-enabled contrib headers into an include/contrib/ that's always on the pgxs include path, so you can still just #include hstore.h . For in-tree builds, explicit add other modules' contrib dirs as Peter has done in the proposed plperl_hstore and plpython_hstore. (Use unqualified names). I don't like the idea to share a flat directory with different header files, filenames can overlap. * Install install-enabled contrib headers to include/contrib/[modulename]/ . Within the module, use the unqualified header name. When referring to it from outside the module, use #include contrib/modulename/header.h. Add $(top_srcdir) to the include path when building extensions in-tree. not either, see my other mail. we still #include hstore.h when we want, we just add that the header so it is available for PGXS builds. Contrib Makefile still need to -I$(includedir_contrib)/hstore. What's new is that the header is installed, not that we don't have to set FLAGS. Personally, I'm all for just using the local path when building the module, and the qualified path elsewhere. It requires only a relatively simple change, and I don't think that using hstore.h within hstore, and contrib/hstore/hstore.h elsewhere is of great concern. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 6/20/13 1:21 AM, Craig Ringer wrote: As you note, the other option is to just refer to extension headers by their unqualified name. I'm *really* not a fan of that idea due to the obvious clash possibilities with Pg's own headers or system headers, especially given that the whole idea of extensions is that they're user supplied. Not having any kind of namespacing is worrying. We already install all PostgreSQL server header files into a separate directory, so the only clashes you have to worry about are with other extensions and with the backend. And you have to do that anyway, because you will have namespace issues for the shared libraries, the symbols in those libraries, and the extension names. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 6/20/13 1:21 AM, Craig Ringer wrote: Personally, I'm all for just using the local path when building the module, and the qualified path elsewhere. It requires only a relatively simple change, and I don't think that using hstore.h within hstore, and contrib/hstore/hstore.h elsewhere is of great concern. It doesn't work if those header files include other header files (as in the case of plpython, for example). -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 04:48:11, Peter Eisentraut a écrit : On Tue, 2013-06-18 at 15:52 +0200, Cédric Villemain wrote: This allows for example to install hstore header and be able to include them in another extension like that: # include contrib/hstore/hstore.h That's not going to work. hstore's header file is included as #include hstore.h (cf. hstore source code). Having it included under different paths in different contexts will be a mess. OK. At the beginning I though of putting headers in include/contrib but feared that some header may have the same name. If you think that it is suitable, I can do that ? (and include the clean: recipe that I missed on the first shot) Also, do we want to keep the word 'contrib' ? include/extension looks fine too... -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 6/19/13 5:55 AM, Cédric Villemain wrote: Le mercredi 19 juin 2013 04:48:11, Peter Eisentraut a écrit : On Tue, 2013-06-18 at 15:52 +0200, Cédric Villemain wrote: This allows for example to install hstore header and be able to include them in another extension like that: # include contrib/hstore/hstore.h That's not going to work. hstore's header file is included as #include hstore.h (cf. hstore source code). Having it included under different paths in different contexts will be a mess. OK. At the beginning I though of putting headers in include/contrib but feared that some header may have the same name. If you think that it is suitable, I can do that ? (and include the clean: recipe that I missed on the first shot) I don't think there is any value in moving the contrib header files around. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/19/2013 10:06 AM, Peter Eisentraut wrote: On 6/19/13 5:55 AM, Cédric Villemain wrote: Le mercredi 19 juin 2013 04:48:11, Peter Eisentraut a écrit : On Tue, 2013-06-18 at 15:52 +0200, Cédric Villemain wrote: This allows for example to install hstore header and be able to include them in another extension like that: # include contrib/hstore/hstore.h That's not going to work. hstore's header file is included as #include hstore.h (cf. hstore source code). Having it included under different paths in different contexts will be a mess. OK. At the beginning I though of putting headers in include/contrib but feared that some header may have the same name. If you think that it is suitable, I can do that ? (and include the clean: recipe that I missed on the first shot) I don't think there is any value in moving the contrib header files around. What are they going to be used for anyway? I rubbed up against this not too long ago. Things will blow up if you use stuff from the module and the module isn't already loaded. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 6/19/13 10:19 AM, Andrew Dunstan wrote: What are they going to be used for anyway? I rubbed up against this not too long ago. Things will blow up if you use stuff from the module and the module isn't already loaded. See transforms. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 18:20:11, Andrew Dunstan a écrit : On 06/19/2013 11:26 AM, Peter Eisentraut wrote: On 6/19/13 10:19 AM, Andrew Dunstan wrote: What are they going to be used for anyway? I rubbed up against this not too long ago. Things will blow up if you use stuff from the module and the module isn't already loaded. See transforms. So you're saying to install extension headers, but into the main directory where we put server headers? At the same level than server headers, but I'm open for suggestion. $ tree -d include include ├── libpq └── postgresql ├── contrib │ └── hstore ├── informix │ └── esql ├── internal │ └── libpq └── server And all subidrs of server/ -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/19/2013 11:26 AM, Peter Eisentraut wrote: On 6/19/13 10:19 AM, Andrew Dunstan wrote: What are they going to be used for anyway? I rubbed up against this not too long ago. Things will blow up if you use stuff from the module and the module isn't already loaded. See transforms. So you're saying to install extension headers, but into the main directory where we put server headers? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/19/2013 12:32 PM, Cédric Villemain wrote: Le mercredi 19 juin 2013 18:20:11, Andrew Dunstan a écrit : On 06/19/2013 11:26 AM, Peter Eisentraut wrote: On 6/19/13 10:19 AM, Andrew Dunstan wrote: What are they going to be used for anyway? I rubbed up against this not too long ago. Things will blow up if you use stuff from the module and the module isn't already loaded. See transforms. So you're saying to install extension headers, but into the main directory where we put server headers? At the same level than server headers, but I'm open for suggestion. $ tree -d include include ├── libpq └── postgresql ├── contrib │ └── hstore ├── informix │ └── esql ├── internal │ └── libpq └── server And all subidrs of server/ This is what Peter was arguing against, isn't it? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 6/19/13 12:20 PM, Andrew Dunstan wrote: So you're saying to install extension headers, but into the main directory where we put server headers? Yes, if we choose to install some extension headers, that is where we should put them. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 18:48:21, Andrew Dunstan a écrit : On 06/19/2013 12:32 PM, Cédric Villemain wrote: Le mercredi 19 juin 2013 18:20:11, Andrew Dunstan a écrit : On 06/19/2013 11:26 AM, Peter Eisentraut wrote: On 6/19/13 10:19 AM, Andrew Dunstan wrote: What are they going to be used for anyway? I rubbed up against this not too long ago. Things will blow up if you use stuff from the module and the module isn't already loaded. See transforms. So you're saying to install extension headers, but into the main directory where we put server headers? At the same level than server headers, but I'm open for suggestion. $ tree -d include include ├── libpq └── postgresql ├── contrib │ └── hstore ├── informix │ └── esql ├── internal │ └── libpq └── server And all subidrs of server/ This is what Peter was arguing against, isn't it? Now I have a doubt :) I believe he answered the proposal to put all headers on the same flat directory, instead of a tree. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
Peter Eisentraut wrote: On 6/19/13 12:20 PM, Andrew Dunstan wrote: So you're saying to install extension headers, but into the main directory where we put server headers? Yes, if we choose to install some extension headers, that is where we should put them. The question of the name of the directory still stands. contrib would be the easiest answer, but it's slightly wrong because externally-supplied modules could also want to install headers. extension might be it, but there are things that aren't extensions (right? if not, that would be my choice). -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
Le mercredi 19 juin 2013 21:06:23, Alvaro Herrera a écrit : Peter Eisentraut wrote: On 6/19/13 12:20 PM, Andrew Dunstan wrote: So you're saying to install extension headers, but into the main directory where we put server headers? Yes, if we choose to install some extension headers, that is where we should put them. The question of the name of the directory still stands. contrib would be the easiest answer, but it's slightly wrong because externally-supplied modules could also want to install headers. extension might be it, but there are things that aren't extensions (right? if not, that would be my choice). yes, I think the same. auth_delay for example is not an extension as in CREATE EXTENSION. So...it is probably better to postpone this decision and keep on the idea to just install headers where there should be will traditional name (contrib). -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Bugfix and new feature for PGXS
On Wed, 2013-06-19 at 20:58 +0200, Cédric Villemain wrote: I believe he answered the proposal to put all headers on the same flat directory, instead of a tree. The headers are used as #include hstore.h #include ltree.h etc. in the existing source code. If you want to install the for use by others, you can do one of three things: 1) Install them into $(pg_config --includedir-server), so other users will just include them in the same way as shown above. 2) Install them in a different directory, but keep the same #include lines. That would require PGXS changes, perhaps a new pg_config option, or something that produces the right -I option to find them. 3) Install them in a different directory and require a different #include line. But then you have to change the existing uses as well, which would probably require moving files around. Both 2 and 3 require a lot of work, possibly compatibility breaks, for no obvious reason. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Bugfix and new feature for PGXS
On 06/20/2013 11:26 AM, Peter Eisentraut wrote:
3) Install them in a different directory and require a different
#include line. But then you have to change the existing uses as well,
which would probably require moving files around.
If I understand you correctly, your concern seems to be with referring
to the extensions headers within that extension. For example, a pgxs
build of hstore using contrib/hstore/hstore.h wouldn't work if
hstore.h was in the current location, the root of the hstore contrib
module. It'd be fine for a non-pgxs build, since we'd just add
$(top_srcdir) to the include path when building contrib modules in-tree.
So, for pgxs, we'd have to construct a temporary include dir, copying
hstore.h into a temporary 'contrib/hstore' subdir for the build. Which
is messy, but would work, and would confine the change mostly to pgxs.
Or we'd have to move it there permanently in the source tree, which
seems kind of excessive. There's a certain appeal in having extensions
follow the same model as the main server code:
hstore/
src/
contrib
hstore
crc32.c hstore_compat.c hstore_gin.c hstore_gist.c
hstore_io.c hstore_op.c
include
contrib
hstore
hstore.h
... but that's pretty significant disruption for something I was hoping
would be a rather small change.
As you note, the other option is to just refer to extension headers by
their unqualified name. I'm *really* not a fan of that idea due to the
obvious clash possibilities with Pg's own headers or system headers,
especially given that the whole idea of extensions is that they're user
supplied. Not having any kind of namespacing is worrying. What could
possibly work would be to install extension headers in
contrib/[modulename]/ and automatically have pgxs and the in-tree build
add appropriate -I directives to those subdirs based on dependencies
declared in the Makefile. Again, though, that makes the whole thing a
lot more complex than I'd like.
Drat.
A final option: When building the extension its self, use hstore.h.
When referring to it from elsewhere, use contrib/hstore/hstore.h. In
pgxs's case it'd be installed, in an in-tree build it'd be handled by
adding ${top_srcdir} to the include path when we're building contribs.
Opinions all?
* Give up on being able to use one extension's header from another
entirely, except in the special case of in-tree builds
* Install install-enabled contrib headers into an include/contrib/
that's always on the pgxs include path, so you can still just #include
hstore.h . For in-tree builds, explicit add other modules' contrib dirs
as Peter has done in the proposed plperl_hstore and plpython_hstore.
(Use unqualified names).
* Install install-enabled contrib headers to
include/contrib/[modulename]/ . Within the module, use the unqualified
header name. When referring to it from outside the module, use #include
contrib/modulename/header.h. Add $(top_srcdir) to the include path
when building extensions in-tree.
Personally, I'm all for just using the local path when building the
module, and the qualified path elsewhere. It requires only a relatively
simple change, and I don't think that using hstore.h within hstore,
and contrib/hstore/hstore.h elsewhere is of great concern.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Bugfix and new feature for PGXS
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. -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation From c23041f31b5a312702d79bbe759a56628f3e37e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= ced...@2ndquadrant.fr Date: Tue, 28 May 2013 14:11:18 +0200 Subject: [PATCH 1/6] fix SHLIB_PREREQS when building with USE_PGXS commit 19e231b introduced SHLIB_PREREQS but failed to port that to PGXS build. The issue is that submake-* can not be built with PGXS, in this case they must check that expected files are present (and installed). Maybe it is better to only check if they have been built ? This fix the build of dblink and postgres_fdw (make USE_PGXS=1) --- src/Makefile.global.in | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Makefile.global.in b/src/Makefile.global.in index 8bfb77d..c3c595e 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -415,13 +415,24 @@ libpq_pgport = -L$(top_builddir)/src/port -lpgport \ -L$(top_builddir)/src/common -lpgcommon $(libpq) endif - +# If PGXS is not defined, builds as usual: +# build dependancies required by SHLIB_PREREQS +# If the build is with PGXS, then any requirement is supposed to be already +# build and we just take care that the expected files exist +ifndef PGXS submake-libpq: $(MAKE) -C $(libpq_builddir) all +else +submake-libpq: $(libdir)/libpq.so ; +endif +ifndef PGXS submake-libpgport: $(MAKE) -C $(top_builddir)/src/port all $(MAKE) -C $(top_builddir)/src/common all +else +submake-libpgport: $(libdir)/libpgport.a $(libdir)/libpgcommon.a ; +endif .PHONY: submake-libpq submake-libpgport -- 1.7.10.4 From 3d3f4df6792c0e98b0a915b8704504f27738bf26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= ced...@2ndquadrant.fr Date: Tue, 28 May 2013 14:17:04 +0200 Subject: [PATCH 2/6] Create data directory if extension when required There is a hack to link the regression data files from the srcdir to the builddir when doing 'make VPATH'. but it failed when used in conjunction with USE_PGXS and out-of-tree build of an extension. Issue is the absence of the data/ directory in the builddir. --- src/makefiles/pgxs.mk |1 + 1 file changed, 1 insertion(+) diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index bbcfe04..6a19b0f 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -263,6 +263,7 @@ test_files_build := $(patsubst $(srcdir)/%, $(abs_builddir)/%, $(test_files_src) all: $(test_files_build) $(test_files_build): $(abs_builddir)/%: $(srcdir)/% + $(MKDIR_P) $(dir $@) ln -s $ $@ endif # VPATH -- 1.7.10.4 From 66b394ae867bde2ad968027f0708ae59a140d81b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Villemain?= ced...@2ndquadrant.fr Date: Tue, 28 May 2013 14:51:43 +0200 Subject: [PATCH 3/6] set VPATH for out-of-tree extension builds If the makefile is not in the current directory (where we launch 'make') then assume we are building out-of-src tree and set the VPATH to the directory of the first makefile... Thus it fixes: mkdir /tmp/a cd /tmp/a make -f extension_src/Makefile USE_PGXS=1 --- src/makefiles/pgxs.mk |9 + 1 file changed, 9 insertions(+)
Re: [HACKERS] Bugfix and new feature for PGXS
On Tue, 2013-06-18 at 15:52 +0200, Cédric Villemain wrote: This allows for example to install hstore header and be able to include them in another extension like that: # include contrib/hstore/hstore.h That's not going to work. hstore's header file is included as #include hstore.h (cf. hstore source code). Having it included under different paths in different contexts will be a mess. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
