Re: [PATCH] Support built control file in PGXS VPATH builds - Mailing list pgsql-hackers
| From | Craig Ringer |
|---|---|
| Subject | Re: [PATCH] Support built control file in PGXS VPATH builds |
| Date | |
| Msg-id | CAMsr+YHHHcRa8vdxCLkLO51FZG1cSzX9WhwMBwjDNMuB40=Vuw@mail.gmail.com Whole thread Raw |
| In response to | Re: [PATCH] Support built control file in PGXS VPATH builds (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>) |
| Responses |
Re: [PATCH] Support built control file in PGXS VPATH builds
(Craig Ringer <craig@2ndquadrant.com>)
|
| List | pgsql-hackers |
On Mon, 9 Mar 2020, 17:27 Peter Eisentraut, <peter.eisentraut@2ndquadrant.com> wrote:
On 2020-02-07 04:14, Craig Ringer wrote:
> The attached patch fixes this by having PGXS resolve
> $(EXTENSION).control along the VPATH.
Simpler patch:
diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 271e7eaba8..1cd750eecd 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -229,7 +229,7 @@ endif # MODULE_big
install: all installdirs
ifneq (,$(EXTENSION))
- $(INSTALL_DATA) $(addprefix $(srcdir)/, $(addsuffix .control, $(EXTENSION))) '$(DESTDIR)$(datadir)/extension/'
+ $(INSTALL_DATA) $(call vpathsearch,$(addsuffix .control, $(EXTENSION))) '$(DESTDIR)$(datadir)/extension/'
endif # EXTENSION
ifneq (,$(DATA)$(DATA_built))
$(INSTALL_DATA) $(addprefix $(srcdir)/, $(DATA)) $(DATA_built) '$(DESTDIR)$(datadir)/$(datamoduledir)/'
Does that work for you?
It wouldn't be my preference because it relies on the VPATH variable. AFAICS the extension cannot use finer grained vpath directives for this. And if anything relies on VPATH it must be set so you can't really benefit from vpath directives for anything else.
Currently it's possible to build extensions by unsetting VPATH after including pgxs.mk and defining vpath directives only the things you want to search for. This is immensely useful. You can prevent make from looking for build products in the srcdir so you don't get issues with stale files if someone does a vpath build from a dirty worktree that has files left in it from a previous in tree build. Lots of other things too.
So while your patch would work it would definitely not be my preference. Frankly I'd rather be moving on the other direction - doing away with all this DATA vs DATA_BUILT mess entirely and switch everything to using make vpath directives + automatic variable path resolution.
Our vpathsearch function is IMO a bit of a hack we shouldn't need to use at all. The only time it's necessary is when we absolutely need to get the vpath resolved path into a Make variable. Since I don't think make offers its own vpath directive aware search function there's no convenient way to get a make var with the resolved path in it before the target is invoked.
So really I think we should be letting make resolve the targets for us using automatic variables like $< $^ and $@ with the target search logic.
BTW it's definitely rather frustrating that make doesn't appear to have a $(vpathsearch foo) or $(vpathlookup foo) or whatever of its own. Seems very silly to not have that when there are vpath directives.
pgsql-hackers by date: