Re: [PATCH] Support built control file in PGXS VPATH builds

On Mon, 9 Mar 2020, 17:27 Peter Eisentraut, <
peter(dot)eisentraut(at)2ndquadrant(dot)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.