public inbox for [email protected]  

help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: Matheus Alcantara <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: Gabriele Bartolini <[email protected]>
Cc: Craig Ringer <[email protected]>
Cc: David E. Wheeler <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: RFC: Additional Directory for Extensions
Date: Tue, 11 Mar 2025 16:58:55 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAFY6G8cGeUV0f5K8v-Du0ts3iZyRE6Q5dNtYjQq8cjS4epLX5A@mail.gmail.com>
References: <CA+TgmoaMk==vOUmWwJ5ZA-EueNqa9RQaWMEiHCF1ZKW_YCcj8g@mail.gmail.com>
	<[email protected]>
	<CAGRY4ny=Oy-F0qCRFQa=XtEqyUbMygvbqqKahAvbae7v4LTOrQ@mail.gmail.com>
	<[email protected]>
	<CAGRY4nx8Fmea4udk--PKdvfgpNdqC+rcueAmGwuw2PD9dv+zqQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAFY6G8dzuLXFjONX5E-TvyD5E9r21b5vp1My_nMcqdGD0VYfYw@mail.gmail.com>
	<[email protected]>
	<CAFY6G8dGcv1D8tCkjrz+9DvAYG9h4U=hBm0VXjJEXExy44hauQ@mail.gmail.com>
	<CAFY6G8eOMkGXLqzUz=qXrwt4X_HvmVZ13570DAbgAiw+3q03DA@mail.gmail.com>
	<CAFY6G8fV5FTqn7p8pxbvPYUD5+ffik-J50GChjKH2Gg7d=fRPg@mail.gmail.com>
	<[email protected]>
	<CAFY6G8cGeUV0f5K8v-Du0ts3iZyRE6Q5dNtYjQq8cjS4epLX5A@mail.gmail.com>

On 10.03.25 21:25, Matheus Alcantara wrote:
> On Thu, Mar 6, 2025 at 10:46 AM Peter Eisentraut <[email protected]> wrote:
>> This looks very good to me.  I have one issue to point out:  The logic
>> in get_extension_control_directories() needs to be a little bit more
>> careful to align with the rules in find_in_path().  For example, it
>> should use first_path_var_separator() to get the platform-specific path
>> separator, and probably also substitute_path_macro() and
>> canonicalize_path() etc., to keep everything consistent.
>>
> I fixed this hardcoded path separator issue on the TAP test and forgot
> to fix it also on code, sorry, fixed on this new version.

>> (Maybe it would be ok to move the function to dfmgr.c to avoid having
>> to export too many things from there.)
>>
> I've exported substitute_path_macro because adding a new function on
> dfmgr would require #include nodes/pg_list.h and I'm not sure what
> approach would be better, please let me know what you think.

Yes, that structure looks ok.  But you can remove one level of block in 
get_extension_control_directories().

I found a bug that was already present in my earlier patch versions:

@@ -423,7 +424,7 @@ find_extension_control_filename(ExtensionControlFile 
*control)
     ecp = Extension_control_path;
     if (strlen(ecp) == 0)
         ecp = "$system";
-   result = find_in_path(basename, Extension_control_path, 
"extension_control_path", "$system", system_dir);
+   result = find_in_path(basename, ecp, "extension_control_path", 
"$system", system_dir);

Without this, it won't work if you set extension_control_path empty. 
(Maybe add a test for that?)

I think this all works now, but I think the way 
pg_available_extensions() works is a bit strange and inefficient.  After 
it finds a candidate control file, it calls 
read_extension_control_file() with the extension name, that calls 
parse_extension_control_file(), that calls 
find_extension_control_filename(), and that calls find_in_path(), which 
searches that path again!

There should be a simpler way into this.  Maybe 
pg_available_extensions() should fill out the ExtensionControlFile 
structure itself, set ->control_dir with where it found it, then call 
directly to parse_extension_control_file(), and that should skip the 
finding if the directory is already set.  Or something similar.






view thread (47+ messages)  latest in thread

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: RFC: Additional Directory for Extensions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox