pgsql: extension_control_path

Started by Peter Eisentraut12 months ago20 messages
Jump to latest
#1Peter Eisentraut
peter_e@gmx.net

extension_control_path

The new GUC extension_control_path specifies a path to look for
extension control files. The default value is $system, which looks in
the compiled-in location, as before.

The path search uses the same code and works in the same way as
dynamic_library_path.

Some use cases of this are: (1) testing extensions during package
builds, (2) installing extensions outside security-restricted
containers like Python.app (on macOS), (3) adding extensions to
PostgreSQL running in a Kubernetes environment using operators such as
CloudNativePG without having to rebuild the base image for each new
extension.

There is also a tweak in Makefile.global so that it is possible to
install extensions using PGXS into an different directory than the
default, using 'make install prefix=/else/where'. This previously
only worked when specifying the subdirectories, like 'make install
datadir=/else/where/share pkglibdir=/else/where/lib', for purely
implementation reasons. (Of course, without the path feature,
installing elsewhere was rarely useful.)

Author: Peter Eisentraut <peter@eisentraut.org>
Co-authored-by: Matheus Alcantara <matheusssilv97@gmail.com>
Reviewed-by: David E. Wheeler <david@justatheory.com>
Reviewed-by: Gabriele Bartolini <gabriele.bartolini@enterprisedb.com>
Reviewed-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Reviewed-by: Niccolò Fei <niccolo.fei@enterprisedb.com>
Discussion: /messages/by-id/E7C7BFFB-8857-48D4-A71F-88B359FADCFD@justatheory.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/4f7f7b0375854e2f89876473405a8f21c95012af

Modified Files
--------------
doc/src/sgml/config.sgml | 68 ++++
doc/src/sgml/extend.sgml | 19 +-
doc/src/sgml/ref/create_extension.sgml | 6 +-
src/Makefile.global.in | 19 +-
src/backend/commands/extension.c | 403 +++++++++++++--------
src/backend/utils/fmgr/dfmgr.c | 77 ++--
src/backend/utils/misc/guc_tables.c | 13 +
src/backend/utils/misc/postgresql.conf.sample | 1 +
src/include/commands/extension.h | 2 +
src/include/fmgr.h | 3 +
src/test/modules/test_extensions/Makefile | 1 +
src/test/modules/test_extensions/meson.build | 5 +
.../t/001_extension_control_path.pl | 80 ++++
13 files changed, 512 insertions(+), 185 deletions(-)

#2Christoph Berg
myon@debian.org
In reply to: Peter Eisentraut (#1)
extension_control_path and "directory"

Re: Peter Eisentraut

The new GUC extension_control_path specifies a path to look for
extension control files. The default value is $system, which looks in
the compiled-in location, as before.

Is the expectation that this also works for the extension "directory"?

semver is still failing because it's shipping its .sql files in a
separate directory:

2025-04-23 09:06:24.864 UTC [422345] myon@contrib_regression ERROR: could not open directory "/usr/share/postgresql/18/semver": No such file or directory
2025-04-23 09:06:24.864 UTC [422345] myon@contrib_regression STATEMENT: CREATE EXTENSION semver;

$ cat debian/postgresql-18-semver/usr/share/postgresql/18/extension/semver.control
# semver extension
comment = 'Semantic version data type'
default_version = '0.40.0'

directory = 'semver'
module_pathname = '$libdir/semver'
relocatable = true

$ ls debian/postgresql-18-semver/usr/share/postgresql/18/semver/
semver--0.10.0--0.11.0.sql semver--0.17.0--0.20.0.sql semver--0.30.0--0.31.0.sql semver--0.32.1--0.40.0.sql
semver--0.11.0--0.12.0.sql semver--0.20.0--0.21.0.sql semver--0.3.0--0.4.0.sql semver--0.40.0.sql
semver--0.12.0--0.13.0.sql semver--0.21.0--0.22.0.sql semver--0.31.0--0.31.1.sql semver--0.5.0--0.10.0.sql
semver--0.13.0--0.15.0.sql semver--0.2.1--0.2.4.sql semver--0.31.1--0.31.2.sql semver.sql
semver--0.15.0--0.16.0.sql semver--0.22.0--0.30.0.sql semver--0.31.2--0.32.0.sql semver--unpackaged--0.2.1.sql
semver--0.16.0--0.17.0.sql semver--0.2.4--0.3.0.sql semver--0.32.0--0.32.1.sql

Christoph

#3Matheus Alcantara
matheusssilv97@gmail.com
In reply to: Christoph Berg (#2)
Re: extension_control_path and "directory"

On Wed, Apr 23, 2025 at 6:39 AM Christoph Berg <myon@debian.org> wrote:

Re: Peter Eisentraut

The new GUC extension_control_path specifies a path to look for
extension control files. The default value is $system, which looks in
the compiled-in location, as before.

Is the expectation that this also works for the extension "directory"?

semver is still failing because it's shipping its .sql files in a
separate directory:

2025-04-23 09:06:24.864 UTC [422345] myon@contrib_regression ERROR: could not open directory "/usr/share/postgresql/18/semver": No such file or directory
2025-04-23 09:06:24.864 UTC [422345] myon@contrib_regression STATEMENT: CREATE EXTENSION semver;

$ cat debian/postgresql-18-semver/usr/share/postgresql/18/extension/semver.control
# semver extension
comment = 'Semantic version data type'
default_version = '0.40.0'

directory = 'semver'
module_pathname = '$libdir/semver'
relocatable = true

$ ls debian/postgresql-18-semver/usr/share/postgresql/18/semver/
semver--0.10.0--0.11.0.sql semver--0.17.0--0.20.0.sql semver--0.30.0--0.31.0.sql semver--0.32.1--0.40.0.sql
semver--0.11.0--0.12.0.sql semver--0.20.0--0.21.0.sql semver--0.3.0--0.4.0.sql semver--0.40.0.sql
semver--0.12.0--0.13.0.sql semver--0.21.0--0.22.0.sql semver--0.31.0--0.31.1.sql semver--0.5.0--0.10.0.sql
semver--0.13.0--0.15.0.sql semver--0.2.1--0.2.4.sql semver--0.31.1--0.31.2.sql semver.sql
semver--0.15.0--0.16.0.sql semver--0.22.0--0.30.0.sql semver--0.31.2--0.32.0.sql semver--unpackaged--0.2.1.sql
semver--0.16.0--0.17.0.sql semver--0.2.4--0.3.0.sql semver--0.32.0--0.32.1.sql

I've tried to implement some kind of "SHAREDIR search path" as we've
discussed on another thread [1]/messages/by-id/0F50D989-B82D-4F59-9F13-C08A4673322C@justatheory.com but it shows out that we need some
considerable refactoring to make it work.

Talking with Peter privately IIUC we concluded that we may document
this limitation that using extension control path with an extension that
uses a custom "directory" field on .control file will not work. I think
that we may add a note section on "extension_control_path" doc on [2]https://www.postgresql.org/docs/devel/runtime-config-client.html#GUC-EXTENSION-CONTROL-PATH,
what do you think?

[1]: /messages/by-id/0F50D989-B82D-4F59-9F13-C08A4673322C@justatheory.com
[2]: https://www.postgresql.org/docs/devel/runtime-config-client.html#GUC-EXTENSION-CONTROL-PATH

--
Matheus Alcantara

#4Christoph Berg
myon@debian.org
In reply to: Matheus Alcantara (#3)
Re: extension_control_path and "directory"

Re: Matheus Alcantara

I've tried to implement some kind of "SHAREDIR search path" as we've
discussed on another thread [1] but it shows out that we need some
considerable refactoring to make it work.

Remembering which path the .control file was found in and from there
open the extension "directory" doesn't sound too hard. Why does it
have to be more complicated?

Also, re-running a search path discovery for the directory is probably
just wrong, if there are different extension versions in the "control"
search path and the "extensions" search path, it might lead to weird
version skew problems.

Talking with Peter privately IIUC we concluded that we may document
this limitation that using extension control path with an extension that
uses a custom "directory" field on .control file will not work. I think
that we may add a note section on "extension_control_path" doc on [2],
what do you think?

Seen from Debian, this would be a regression since it worked with my
custom patch.

The number of extensions using that feature is limited, though, so it
wouldn't be a huge problem:

$ grep directory */*/*.control
pgfincore/pgfincore/pgfincore.control:directory = pgfincore
postgresql-pgmp/postgresql-pgmp/pgmp.control:directory = 'pgmp'
postgresql-semver/postgresql-semver/semver.control:directory = 'semver'

(Not including extensions generating the .control file at build time.)

Christoph

#5David E. Wheeler
david@kineticode.com
In reply to: Christoph Berg (#4)
Re: extension_control_path and "directory"

On Apr 23, 2025, at 09:50, Christoph Berg <myon@debian.org> wrote:

Remembering which path the .control file was found in and from there
open the extension "directory" doesn't sound too hard. Why does it
have to be more complicated?

This was my question, as well. Do you have a WIP patch to share, Matheus?

Also, re-running a search path discovery for the directory is probably
just wrong, if there are different extension versions in the "control"
search path and the "extensions" search path, it might lead to weird
version skew problems.

I assumed we would just have one or the other GUCs, not both.

The number of extensions using that feature is limited, though, so it
wouldn't be a huge problem:

FWIW it’s a a simple patch to make semver work, and probably also for the others. It’s just the reverse of this change[1]https://github.com/theory/pg-semver/commit/88b3abd:

```patch
--- a/Makefile
+++ b/Makefile
@@ -5,7 +5,6 @@ EXTVERSION   = $(shell grep -m 1 '[[:space:]]\{8\}"version":' META.json | \
 DISTVERSION  = $(shell grep -m 1 '[[:space:]]\{3\}"version":' META.json | \
                sed -e 's/[[:space:]]*"version":[[:space:]]*"\([^"]*\)",\{0,1\}/\1/')
  -MODULEDIR    = semver
 DATA         = $(wildcard sql/*.sql)
 DOCS         = $(wildcard doc/*.mmd)
 TESTS        = $(wildcard test/sql/*.sql)
--- a/semver.control
+++ b/semver.control
@@ -1,7 +1,5 @@
 # semver extension
 comment = 'Semantic version data type'
 default_version = '0.32.1'
-
-directory = 'semver'
 module_pathname = '$libdir/semver'
 relocatable = true

```

I think I’ll write a blog post this week recommending people not use these directives, and also to remove `$lib/` from `module_pathname`.

Best,

David

[1]: https://github.com/theory/pg-semver/commit/88b3abd

#6Matheus Alcantara
matheusssilv97@gmail.com
In reply to: David E. Wheeler (#5)
Re: extension_control_path and "directory"

On Wed, Apr 23, 2025 at 10:57 AM David E. Wheeler <david@justatheory.com> wrote:

On Apr 23, 2025, at 09:50, Christoph Berg <myon@debian.org> wrote:

Remembering which path the .control file was found in and from there
open the extension "directory" doesn't sound too hard. Why does it
have to be more complicated?

This was my question, as well. Do you have a WIP patch to share, Matheus?

I spent some time trying to implement this and somehow I got lost in the
changes I thought I would need to make to the "find_in_path" function
and others it calls, but reading these messages and looking at the code
again I think that the change is much simpler than I thought.

Attached is a draft patch that uses the path that the .control file was
found to search for the script files when the "directory" is set on the
.control file.

I've tested with the semver extension and it seems to work fine with
this patch. Can you please check on your side to see if it's also
working?

I still want to make some polish on this patch and also include some
more test cases using the "directory" on the .control file but I think
that is stable to make some tests, make check and check-world is happy.

--
Matheus Alcantara

Attachments:

v1-0001-Make-directory-work-with-extension-control-path.patchapplication/octet-stream; name=v1-0001-Make-directory-work-with-extension-control-path.patchDownload+43-7
#7Christoph Berg
myon@debian.org
In reply to: Matheus Alcantara (#6)
Re: extension_control_path and "directory"

Re: Matheus Alcantara

I've tested with the semver extension and it seems to work fine with
this patch. Can you please check on your side to see if it's also
working?

Hi Matheus,

thanks for the patch, it does indeed work.

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 180f4af9be3..d68efd59118 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -376,6 +376,14 @@ get_extension_control_directories(void)
 			/* Substitute the path macro if needed */
 			mangled = substitute_path_macro(piece, "$system", system_dir);
+
+			/*
+			 * Append "extension" suffix in case is a custom extension control
+			 * path.
+			 */
+			if (strcmp(piece, "$system") != 0)
+				mangled = psprintf("%s/extension", mangled);

This would look prettier if it was something like

mangled = substitute_path_macro(piece, "$system", system_dir "/extension");

... but I'm wondering if it wouldn't be saner if the control path
should be stored without "extension" in that struct. Then opening the
control file would be path + "extension/" + filename and the extra
directory would be path + directory, without any on-the-fly stripping
of trailing components.

The extension_control_path GUC could also be adjusted to refer to the
directory one level above the extension/foo.control location.

+	/*
+	 * Assert that the control->control_dir end with /extension suffix so that
+	 * we can replace with the value from control->directory.
+	 */
+	Assert(ctrldir_len >= suffix_len &&
+		   strcmp(control->control_dir + ctrldir_len - suffix_len, "extension") == 0);

If control_dir is coming from extension_control_path, it might have a
different suffix. Replace the Assert by elog(ERROR). (Or see above.)

Christoph

#8Matheus Alcantara
matheusssilv97@gmail.com
In reply to: Christoph Berg (#7)
Re: extension_control_path and "directory"

On Thu, Apr 24, 2025 at 7:21 AM Christoph Berg <myon@debian.org> wrote:

Re: Matheus Alcantara

I've tested with the semver extension and it seems to work fine with
this patch. Can you please check on your side to see if it's also
working?

Hi Matheus,

thanks for the patch, it does indeed work.

Thanks for testing and for reviewing.

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 180f4af9be3..d68efd59118 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -376,6 +376,14 @@ get_extension_control_directories(void)
/* Substitute the path macro if needed */
mangled = substitute_path_macro(piece, "$system", system_dir);
+
+                       /*
+                        * Append "extension" suffix in case is a custom extension control
+                        * path.
+                        */
+                       if (strcmp(piece, "$system") != 0)
+                               mangled = psprintf("%s/extension", mangled);

This would look prettier if it was something like

mangled = substitute_path_macro(piece, "$system", system_dir "/extension");

... but I'm wondering if it wouldn't be saner if the control path
should be stored without "extension" in that struct. Then opening the
control file would be path + "extension/" + filename and the extra
directory would be path + directory, without any on-the-fly stripping
of trailing components.

The extension_control_path GUC could also be adjusted to refer to the
directory one level above the extension/foo.control location.

Storing the control path directly without any code to remove the
/extension at the end would be more trick I think, because we would need
to change the find_in_path() function to return the path without the
suffix.

In this new version I've introduced a new "basedir" field on
ExtensionControlFile so that we can save the base directory to search
for .control files and scripts. With this new field, on
get_extension_script_directory() we just need to join control->basedir
with control->directory. Note that we still need to handle the removal
of the /extension at the end of control path but I think that on this
new version the code looks a bit better (IMHO) since we just need to
handle on find_extension_control_filename(). WYT?

+       /*
+        * Assert that the control->control_dir end with /extension suffix so that
+        * we can replace with the value from control->directory.
+        */
+       Assert(ctrldir_len >= suffix_len &&
+                  strcmp(control->control_dir + ctrldir_len - suffix_len, "extension") == 0);

If control_dir is coming from extension_control_path, it might have a
different suffix. Replace the Assert by elog(ERROR). (Or see above.)

In v2 I've moved the logic to remove the /extension to
parse_extension_control_file(), do you think that this Assert on this
function would still be wrong? IIUC we should always have /extension at
the end of "control_dir" at this place, because the
extension_control_path GUC will omit the /extension at the end and we
will force it to have the suffix on the path at
find_extension_control_filename() and
get_extension_control_directories() functions. I'm missing something
here?

I've also included some more TAP tests on this new version.

--
Matheus Alcantara

Attachments:

v2-0001-Make-directory-work-with-extension-control-path.patchapplication/octet-stream; name=v2-0001-Make-directory-work-with-extension-control-path.patchDownload+84-33
#9David E. Wheeler
david@kineticode.com
In reply to: Matheus Alcantara (#8)
Re: extension_control_path and "directory"

On Apr 24, 2025, at 11:18, Matheus Alcantara <matheusssilv97@gmail.com> wrote:

In v2 I've moved the logic to remove the /extension to
parse_extension_control_file(), do you think that this Assert on this
function would still be wrong? IIUC we should always have /extension at
the end of "control_dir" at this place, because the
extension_control_path GUC will omit the /extension at the end and we
will force it to have the suffix on the path at
find_extension_control_filename() and
get_extension_control_directories() functions. I'm missing something
here?

I took this patch for a spin and managed to make it core dump. How? Well I installed semver with this command:

```sh
make prefix=/Users/david/Downloads install
```

Then set the search paths and restarted:

```ini
extension_control_path = '/Users/david/Downloads/share/extension:$system'
dynamic_library_path = '/Users/david/Downloads/lib:$libdir'
```

Then I connected and ran `CREATE EXTENSION semver` and it segfaulted. I poked around for a few minutes and realized that my prefix is not what I expected. Because it doesn’t contain the string “postgres”, PGXS helpfully adds it. The actual paths are:

```ini
extension_control_path = '/Users/david/Downloads/share/postgresql/extension:$system'
dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir'
```

With that fix it no longer segafulted.

So I presume something crashes when a directory or file doesn’t exist.

But I am not at all sure we want this prefix behavior for installing extensions. I get that has been the behavior for setting the main sharedir and libdir for Postgres, but I don’t know that it makes sense for extension prefixes.

Best,

David

#10Matheus Alcantara
matheusssilv97@gmail.com
In reply to: David E. Wheeler (#9)
Re: extension_control_path and "directory"

On Thu, Apr 24, 2025 at 7:27 PM David E. Wheeler <david@justatheory.com> wrote:

On Apr 24, 2025, at 11:18, Matheus Alcantara <matheusssilv97@gmail.com> wrote:

In v2 I've moved the logic to remove the /extension to
parse_extension_control_file(), do you think that this Assert on this
function would still be wrong? IIUC we should always have /extension at
the end of "control_dir" at this place, because the
extension_control_path GUC will omit the /extension at the end and we
will force it to have the suffix on the path at
find_extension_control_filename() and
get_extension_control_directories() functions. I'm missing something
here?

I took this patch for a spin and managed to make it core dump. How? Well I installed semver with this command:

```sh
make prefix=/Users/david/Downloads install
```

Then set the search paths and restarted:

```ini
extension_control_path = '/Users/david/Downloads/share/extension:$system'
dynamic_library_path = '/Users/david/Downloads/lib:$libdir'
```

Then I connected and ran `CREATE EXTENSION semver` and it segfaulted. I poked around for a few minutes and realized that my prefix is not what I expected. Because it doesn’t contain the string “postgres”, PGXS helpfully adds it. The actual paths are:

```ini
extension_control_path = '/Users/david/Downloads/share/postgresql/extension:$system'
dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir'
```

With that fix it no longer segafulted.

So I presume something crashes when a directory or file doesn’t exist.

Yes, you are right. The problem was where I was asserting
control->control_dir != NULL. I've moved the assert after the "if
(!filename)" check that returns an error if the extension was not found.

Attached v3 with this fix and also a TAP test for this scenario.

I'm just a bit confused how you get it working using /extension at the
end of extension_control_path since with this patch this suffix is not
necessary and since we hard coded append this it should return an error
when trying to search on something like
/Users/david/Downloads/share/postgresql/extension/extension

--
Matheus Alcantara

Attachments:

v3-0001-Make-directory-work-with-extension-control-path.patchapplication/octet-stream; name=v3-0001-Make-directory-work-with-extension-control-path.patchDownload+92-33
#11David E. Wheeler
david@kineticode.com
In reply to: Matheus Alcantara (#10)
Re: extension_control_path and "directory"

On Apr 25, 2025, at 09:25, Matheus Alcantara <matheusssilv97@gmail.com> wrote:

Yes, you are right. The problem was where I was asserting
control->control_dir != NULL. I've moved the assert after the "if
(!filename)" check that returns an error if the extension was not found.

Attached v3 with this fix and also a TAP test for this scenario.

That fixes the segfault, thank you.

I'm just a bit confused how you get it working using /extension at the
end of extension_control_path since with this patch this suffix is not
necessary and since we hard coded append this it should return an error
when trying to search on something like

It worked with

extension_control_path = '/Users/david/Downloads/share/postgresql/extension:$system’

But not with

extension_control_path = '/Users/david/Downloads/share/postgresql:$system’

And here is where the control file actually is:

❯ ll ~/Downloads/share/postgresql/extension total 8
-rw-r--r-- 1 david staff 161B Apr 24 18:07 semver.control

So I don’t know the answer to your question, but it’d be handy to have functions that return a list of resolved paths from extension_control_path and dynamic_library_path, since they get mangled.

Best,

David

#12Matheus Alcantara
matheusssilv97@gmail.com
In reply to: David E. Wheeler (#11)
Re: extension_control_path and "directory"

On Fri, Apr 25, 2025 at 4:13 PM David E. Wheeler <david@justatheory.com> wrote:

On Apr 25, 2025, at 09:25, Matheus Alcantara <matheusssilv97@gmail.com> wrote:

Yes, you are right. The problem was where I was asserting
control->control_dir != NULL. I've moved the assert after the "if
(!filename)" check that returns an error if the extension was not found.

Attached v3 with this fix and also a TAP test for this scenario.

That fixes the segfault, thank you.

Great, thanks for testing!

I'm just a bit confused how you get it working using /extension at the
end of extension_control_path since with this patch this suffix is not
necessary and since we hard coded append this it should return an error
when trying to search on something like

It worked with

extension_control_path = '/Users/david/Downloads/share/postgresql/extension:$system’

But not with

extension_control_path = '/Users/david/Downloads/share/postgresql:$system’

And here is where the control file actually is:

❯ ll ~/Downloads/share/postgresql/extension total 8
-rw-r--r-- 1 david staff 161B Apr 24 18:07 semver.control

So I don’t know the answer to your question, but it’d be handy to have functions that return a list of resolved paths from extension_control_path and dynamic_library_path, since they get mangled.

Ok, I was testing using extension_control_path = '$system:/my/custom/path'
(starting with the macro) and it was working as expected, testing with
the macro at the end does not work.

The problem was on find_extension_control_filename() that was appending
the /extension at the end of the entire extension_control_path GUC value
instead of just the custom paths.

To append the /extension at each path on extension_control_path would
require some changes on find_in_path() that
find_extension_control_filename() calls, which I think that it would
make the function more complicated. I've them created a similar
find_in_paths() function that works in the same way but it receives a
List of paths instead of the string of paths separated by ":". We can
get this List of paths using get_extension_control_directories() that
also handle the macro substitution like find_in_path().

Attached v4 with these fixes. I hope that now you should be able to omit
the /extension from the GUC value.

--
Matheus Alcantara

Attachments:

v4-0001-Make-directory-work-with-extension-control-path.patchapplication/octet-stream; name=v4-0001-Make-directory-work-with-extension-control-path.patchDownload+144-49
#13David E. Wheeler
david@kineticode.com
In reply to: Matheus Alcantara (#12)
Re: extension_control_path and "directory"

On Apr 25, 2025, at 17:18, Matheus Alcantara <matheusssilv97@gmail.com> wrote:

Ok, I was testing using extension_control_path = '$system:/my/custom/path'
(starting with the macro) and it was working as expected, testing with
the macro at the end does not work.

Great example of why it’s useful to do as much testing as possible! That’s an entirely reasonable place to start testing :-)

The problem was on find_extension_control_filename() that was appending
the /extension at the end of the entire extension_control_path GUC value
instead of just the custom paths.

Oh yeah, lol, that wouldn’t work.

To append the /extension at each path on extension_control_path would
require some changes on find_in_path() that
find_extension_control_filename() calls, which I think that it would
make the function more complicated. I've them created a similar
find_in_paths() function that works in the same way but it receives a
List of paths instead of the string of paths separated by ":". We can
get this List of paths using get_extension_control_directories() that
also handle the macro substitution like find_in_path().

Attached v4 with these fixes. I hope that now you should be able to omit
the /extension from the GUC value.

Yes! It now works with this configuration:

```ini
extension_control_path = '/Users/david/Downloads/share/postgresql:$system'
dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir’
```

Which is nicely more consistent. Kind of want that first one to be called “share_path” now, though, since it’s not just extensions. Although I guess it’s only extension control file searching that uses it (for now).

If I understand this bit correctly:

```c
/* Substitute the path macro if needed */
mangled = substitute_path_macro(piece, "$system", system_dir);

/*
* Append "extension" suffix in case is a custom extension control
* path.
*/
if (strcmp(piece, "$system") != 0)
mangled = psprintf("%s/extension", mangled);
```

The value of `piece` is a single path from the search path, right? If so, I think it’s either `$system` or something else; I don’t it would ever be that `$system` is a substring of a single path. Is that right?

Other than that, I think this patch is good to go. Thanks!

Best,

David

`

#14Matheus Alcantara
matheusssilv97@gmail.com
In reply to: David E. Wheeler (#13)
Re: extension_control_path and "directory"

On Mon, Apr 28, 2025 at 5:49 PM David E. Wheeler <david@justatheory.com> wrote:

To append the /extension at each path on extension_control_path would
require some changes on find_in_path() that
find_extension_control_filename() calls, which I think that it would
make the function more complicated. I've them created a similar
find_in_paths() function that works in the same way but it receives a
List of paths instead of the string of paths separated by ":". We can
get this List of paths using get_extension_control_directories() that
also handle the macro substitution like find_in_path().

Attached v4 with these fixes. I hope that now you should be able to omit
the /extension from the GUC value.

Yes! It now works with this configuration:

```ini
extension_control_path = '/Users/david/Downloads/share/postgresql:$system'
dynamic_library_path = '/Users/david/Downloads/lib/postgresql:$libdir’
```

Which is nicely more consistent. Kind of want that first one to be called “share_path” now, though, since it’s not just extensions. Although I guess it’s only extension control file searching that uses it (for now).

Thanks for testing!

If I understand this bit correctly:

```c
/* Substitute the path macro if needed */
mangled = substitute_path_macro(piece, "$system", system_dir);

/*
* Append "extension" suffix in case is a custom extension control
* path.
*/
if (strcmp(piece, "$system") != 0)
mangled = psprintf("%s/extension", mangled);
```

The value of `piece` is a single path from the search path, right? If so, I think it’s either `$system` or something else; I don’t it would ever be that `$system` is a substring of a single path. Is that right?

Yes, it is a single path from the search path, in your case it will be
"/Users/david/Downloads/share/postgresql" and "$system". We split these
paths based on the system path separator and get the next "piece" here:

char *piece = first_path_var_separator(ecp);

The first_path_var_separator() changes the "ecp" parameter on every call,
it returns the next path on "ecp" and changes it to have the remaining
paths to iterate over it.

Other than that, I think this patch is good to go. Thanks!

Thanks for reviewing!

--
Matheus Alcantara

#15David E. Wheeler
david@kineticode.com
In reply to: Matheus Alcantara (#14)
Re: extension_control_path and "directory"

On Apr 29, 2025, at 09:49, Matheus Alcantara <matheusssilv97@gmail.com> wrote:

Yes, it is a single path from the search path, in your case it will be
"/Users/david/Downloads/share/postgresql" and "$system". We split these
paths based on the system path separator and get the next "piece" here:

char *piece = first_path_var_separator(ecp);

The first_path_var_separator() changes the "ecp" parameter on every call,
it returns the next path on "ecp" and changes it to have the remaining
paths to iterate over it.

Right. My point is a minor one, but I thin you can use an if/ else there:

```c
if (strcmp(piece, "$system") == 0) {
/* Substitute the path macro if needed */
mangled = substitute_path_macro(piece, "$system", system_dir);
} else {
/*
* Append "extension" suffix in case is a custom extension
* control path.
*/
mangled = psprintf("%s/extension", mangled);
}
```

Best,

David

#16Matheus Alcantara
matheusssilv97@gmail.com
In reply to: David E. Wheeler (#15)
Re: extension_control_path and "directory"

On Tue, Apr 29, 2025 at 11:08 AM David E. Wheeler <david@justatheory.com> wrote:

Right. My point is a minor one, but I thin you can use an if/ else there:

```c
if (strcmp(piece, "$system") == 0) {
/* Substitute the path macro if needed */
mangled = substitute_path_macro(piece, "$system", system_dir);
} else {
/*
* Append "extension" suffix in case is a custom extension
* control path.
*/
mangled = psprintf("%s/extension", mangled);
}
```

The substitute_path_macro() already handles the if/else on "piece" but I
think that this if/else version looks nicer. Fixed.

I've also included some documentation changes for this v5 version to
remove the "extension" from the examples and also mention the scenario
when using the "directory" on the .control file.

--
Matheus Alcantara

Attachments:

v5-0001-Make-directory-work-with-extension-control-path.patchapplication/octet-stream; name=v5-0001-Make-directory-work-with-extension-control-path.patchDownload+161-57
#17David E. Wheeler
david@kineticode.com
In reply to: Matheus Alcantara (#16)
Re: extension_control_path and "directory"

On Apr 29, 2025, at 11:06, Matheus Alcantara <matheusssilv97@gmail.com> wrote:

The substitute_path_macro() already handles the if/else on "piece" but I
think that this if/else version looks nicer. Fixed.

I've also included some documentation changes for this v5 version to
remove the "extension" from the examples and also mention the scenario
when using the "directory" on the .control file.

Nice, thanks. I’ve made a PR in my GitHub clone for anyone who likes to look it over that way.

https://github.com/theory/postgres/pull/11/files

Best,

David

#18Peter Eisentraut
peter_e@gmx.net
In reply to: Matheus Alcantara (#16)
Re: extension_control_path and "directory"

On 29.04.25 17:06, Matheus Alcantara wrote:

On Tue, Apr 29, 2025 at 11:08 AM David E. Wheeler <david@justatheory.com> wrote:

Right. My point is a minor one, but I thin you can use an if/ else there:

```c
if (strcmp(piece, "$system") == 0) {
/* Substitute the path macro if needed */
mangled = substitute_path_macro(piece, "$system", system_dir);
} else {
/*
* Append "extension" suffix in case is a custom extension
* control path.
*/
mangled = psprintf("%s/extension", mangled);
}
```

The substitute_path_macro() already handles the if/else on "piece" but I
think that this if/else version looks nicer. Fixed.

I've also included some documentation changes for this v5 version to
remove the "extension" from the examples and also mention the scenario
when using the "directory" on the .control file.

Thanks, I have committed this. I did a bit of code reformatting and
adjusted the documentation a bit. It's good to get this in before beta1
so that we don't have to change the valid values of
extension_control_path past beta1.

#19Matheus Alcantara
matheusssilv97@gmail.com
In reply to: Peter Eisentraut (#18)
Re: extension_control_path and "directory"

On Fri, May 2, 2025 at 11:51 AM Peter Eisentraut <peter@eisentraut.org> wrote:

Thanks, I have committed this. I did a bit of code reformatting and
adjusted the documentation a bit. It's good to get this in before beta1
so that we don't have to change the valid values of
extension_control_path past beta1.

Thanks Peter!

--
Matheus Alcantara

#20Christoph Berg
myon@debian.org
In reply to: Matheus Alcantara (#19)
Re: extension_control_path and "directory"

Re: Matheus Alcantara

Thanks, I have committed this. I did a bit of code reformatting and
adjusted the documentation a bit. It's good to get this in before beta1
so that we don't have to change the valid values of
extension_control_path past beta1.

Thanks Peter!

And thanks everyone!

Christoph