Message-ID: <51CB0011.8020506@dunslane.net>
Date: Wed, 26 Jun 2013 10:52:01 -0400
From: Andrew Dunstan <andrew@dunslane.net>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:17.0) Gecko/20130110 Thunderbird/17.0.2
MIME-Version: 1.0
To: cedric@2ndquadrant.com
CC: pgsql-hackers@postgresql.org, Craig Ringer <craig@2ndquadrant.com>
Subject: Re: Bugfix and new feature for PGXS
References: <201306181552.36673.cedric@2ndquadrant.com>
 <201306250124.54610.cedric@2ndquadrant.com> <51C9B4DB.8000007@dunslane.net>
 <201306251729.47025.cedric@2ndquadrant.com>
In-Reply-To: <201306251729.47025.cedric@2ndquadrant.com>
Content-Type: multipart/mixed; boundary="------------020901090908030405000005"
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

This is a multi-part message in MIME format.
--------------020901090908030405000005
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable


On 06/25/2013 11:29 AM, C=C3=A9dric Villemain wrote:
> Le mardi 25 juin 2013 17:18:51, Andrew Dunstan a =C3=A9crit :
>> On 06/24/2013 07:24 PM, C=C3=A9dric Villemain wrote:
>>> Le mardi 25 juin 2013 00:18:26, Andrew Dunstan a =C3=A9crit :
>>>> On 06/24/2013 04:02 PM, C=C3=A9dric 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=3D/mypath ..." the VP=
ATH
>>> 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 d=
efine
> 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=20
make without anything special in the invocation that triggers VPATH=20
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 =3D ../json_build
     USE_VPATH =3D $(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=20
in an in-tree build.

So what's lacking to make this nice is a tool to automate the second and=20
third steps above.

Maybe there are other ways of doing this, but this does what I'd like to=20
see.

cheers

andrew


--------------020901090908030405000005
Content-Type: text/x-patch;
 name="pgxs-usevpath.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="pgxs-usevpath.patch"

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

--------------020901090908030405000005
Content-Type: text/plain; charset=UTF-8;
 name="Makefile"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="Makefile"

RVhURU5TSU9OICAgID0ganNvbl9idWlsZAoKaWZlcSAoJCh3aWxkY2FyZCB2
cGF0aC5tayksdnBhdGgubWspCmluY2x1ZGUgdnBhdGgubWsKZWxzZQpleHRf
c3JjZGlyID0gLgplbmRpZgoKRVhUVkVSU0lPTiAgID0gJChzaGVsbCBncmVw
IGRlZmF1bHRfdmVyc2lvbiAkKEVYVEVOU0lPTikuY29udHJvbCB8IHNlZCAt
ZSAicy9kZWZhdWx0X3ZlcnNpb25bWzpzcGFjZTpdXSo9W1s6c3BhY2U6XV0q
J1woW14nXSpcKScvXDEvIikKREFUQSAgICAgICAgID0gJChmaWx0ZXItb3V0
ICQod2lsZGNhcmQgc3FsLyotLSouc3FsKSwkKHdpbGRjYXJkIHNxbC8qLnNx
bCkpCkRPQ1MgICAgICAgICA9ICQod2lsZGNhcmQgJChleHRfc3JjZGlyKS9k
b2MvKi5tZCkKVVNFX01PRFVMRV9EQiA9IDEKVEVTVFMgICAgICAgID0gJCh3
aWxkY2FyZCAkKGV4dF9zcmNkaXIpL3Rlc3Qvc3FsLyouc3FsKQpSRUdSRVNT
X09QVFMgPSAtLWlucHV0ZGlyPSQoZXh0X3NyY2RpcikvdGVzdCAtLW91dHB1
dGRpcj10ZXN0IFwKCS0tbG9hZC1leHRlbnNpb249JChFWFRFTlNJT04pClJF
R1JFU1MgICAgICA9ICQocGF0c3Vic3QgJChleHRfc3JjZGlyKS90ZXN0L3Nx
bC8lLnNxbCwlLCQoVEVTVFMpKQpNT0RVTEVfYmlnICAgICAgPSAkKEVYVEVO
U0lPTikKT0JKUyAgICAgICAgID0gJChwYXRzdWJzdCAkKGV4dF9zcmNkaXIp
L3NyYy8lLmMsc3JjLyUubywkKHdpbGRjYXJkICQoZXh0X3NyY2Rpcikvc3Jj
LyouYykpClBHX0NPTkZJRyAgICA9IHBnX2NvbmZpZwoKYWxsOiBzcWwvJChF
WFRFTlNJT04pLS0kKEVYVFZFUlNJT04pLnNxbAoKc3FsLyQoRVhURU5TSU9O
KS0tJChFWFRWRVJTSU9OKS5zcWw6ICQoZXh0X3NyY2Rpcikvc3FsLyQoRVhU
RU5TSU9OKS5zcWwKCWNwICQ8ICRACgpEQVRBX2J1aWx0ID0gc3FsLyQoRVhU
RU5TSU9OKS0tJChFWFRWRVJTSU9OKS5zcWwKREFUQSA9ICQoZmlsdGVyLW91
dCAkKGV4dF9zcmNkaXIpL3NxbC8kKEVYVEVOU0lPTiktLSQoRVhUVkVSU0lP
Tikuc3FsLCAkKHdpbGRjYXJkICQoZXh0X3NyY2Rpcikvc3FsLyotLSouc3Fs
KSkKRVhUUkFfQ0xFQU4gPSBzcWwvJChFWFRFTlNJT04pLS0kKEVYVFZFUlNJ
T04pLnNxbAoKUEdYUyA6PSAkKHNoZWxsICQoUEdfQ09ORklHKSAtLXBneHMp
CmluY2x1ZGUgJChQR1hTKQoKIyB3ZSBwdXQgYWxsIHRoZSB0ZXN0cyBpbiBh
IHRlc3Qgc3ViZGlyLCBidXQgcGd4cyBleHBlY3RzIHVzIG5vdCB0bywgZGFy
biBpdApvdmVycmlkZSBwZ19yZWdyZXNzX2NsZWFuX2ZpbGVzID0gdGVzdC9y
ZXN1bHRzLyB0ZXN0L3JlZ3Jlc3Npb24uZGlmZnMgdGVzdC9yZWdyZXNzaW9u
Lm91dCB0bXBfY2hlY2svIGxvZy8K

--------------020901090908030405000005
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


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

--------------020901090908030405000005--