Re: RFC: Additional Directory for Extensions

From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>
Cc: Gabriele Bartolini <gabriele(dot)bartolini(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: RFC: Additional Directory for Extensions
Date: 2024-08-29 15:55:27
Message-ID: D30A91FA-A6D4-4737-941F-0BBB2984B730@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Aug 27, 2024, at 22:24, Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com> wrote:

> `pg_config` only cares about compile-time settings, so I would not
> expect its output to change.

Right, of course that’s its original purpose, but extensions depend on it to determine where to install extensions. Not just PGXS, but also pgrx and various Makefile customizations I’ve seen in the wild.

> I suspect we'd have to add PGXS extension-path awareness if going for
> per-extension subdir support. I'm not sure it makes sense to teach
> `pg_config` about this, since it'd need to have a different mode like
>
> pg_config --extension myextname --extension-sharedir
>
> since the extension's "sharedir" is
> $basedir/extensions/myextension/share or whatever.

Right. PGXS would just need to know where to put the directory for an extension. There should be a default for the project, and then it can be overridden with something like DESTDIR (but without full paths under that prefix).

> Supporting this looks to be a bit intrusive in the makefiles,
> requiring them to differentiate between "share dir for extensions" and
> "share dir for !extensions", etc. I'm not immediately sure if it can
> be done in a way that transparently converts unmodified extension PGXS
> makefiles to target the new paths; it might require an additional
> define, or use of new variables and an ifdef block to add
> backwards-compat to the extension makefile for building on older
> postgres.

Yeah, might just have to be an entirely new thing, though it sure would be nice for existing PGXS-using Makefiles to do the right thing. Maybe for the new version of the server with the proposed new pattern it would dispatch to the new thing somehow without modifying all the rest of its logic.

> Right. The proposed structure is rather a bigger change than I was
> thinking when I suggested supporting an extension search path not just
> a single extra path. But it's also a cleaner proposal; the
> per-extension directory would make it easier to ensure that the
> extension control file, sql scripts, and dynamic library all match the
> same extension and version if multiple ones are on the path. Which is
> desirable when doing things like testing a new version of an in-core
> extension.

💯

> Right. And I've been on the receiving end of having a small, focused
> patch derailed by others jumping in and scope-exploding it into
> something completely different to solve a much wider but related
> problem.

I’m not complaining, I would definitely prefer to see consensus on a cleaner proposal along the lines we’ve discussed and a commitment to time from parties able to get it done in time for v18. I’m willing to help where I can with my baby C! Failing that, we can fall back on the destdir patch.

> I'm definitely not trying to stand in the way of progress with this; I
> just want to make sure that it doesn't paint us into a corner that
> prevents a more general solution from being adopted later. That's why
> I'm suggesting making the config a multi-value string (list of paths)
> and raising a runtime "ERROR: searching multiple paths for extensions
> not yet supported" or something if >1 path configured.
>
> If that doesn't work, no problem.

I think the logic would have to be different, so they’d be different GUCs with their own semantics. But if the core team and committers are on board with the general idea of search paths and per-extension directory organization, it would be best to avoid getting stuck with maintaining the current patch’s GUC.

OTOH, we could get it committed now and revert it later if we get the better thing done and committed.

>> I think we should get some clarity on the proposal, and then consensus, as you say. I say “get some clarity” because my proposal doesn’t require recursing, and I’m not sure why it’d be needed.
>
> From what you and Gabriele are discussing (which I agree with), it wouldn’t.

Ah, great.

I’ll try to put some thought into a more formal proposal in a new thread next week. Unless your Gabriele beats me to it 😂.

Best,

David

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2024-08-29 15:56:44 Re: pl/pgperl Patch for adding $_FN detail just like triggers have for $_TD
Previous Message Nitin Motiani 2024-08-29 15:38:43 Re: race condition in pg_class