Re: Bugfix and new feature for PGXS

From: Cédric Villemain <cedric(at)2ndquadrant(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Craig Ringer <craig(at)2ndquadrant(dot)com>
Subject: Re: Bugfix and new feature for PGXS
Date: 2013-06-29 18:27:57
Message-ID: 201306292027.58057.cedric@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2013-06-29 18:38:26 Re: vacuumlo - use a cursor
Previous Message Cédric Villemain 2013-06-29 18:18:58 Re: Bugfix and new feature for PGXS