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 does what I'd like to
> see.

This is building contrib with PGXS and dependance to PostgreSQL source tree 
with VPATH and modification of current contribs' Makefile, but it uses the same 
set of commands users are expected when building something (configure, make, 
make install).

I believe you are right that users are not supposed to call make with -f 
parameters to find the Makefile. They are supposed to execute the configure 
script.
It should be possible to find a way to do that without breaking anything. I am 
just not sure (yet) it is worth the effort.

Andrew, I'll understand you have something more important to cook for this CF, 
we can come back to this use case in the next CF if you prefer.

Thanks again for the time you spent in the review, 
-- 
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to