Thread: 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
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] 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], what do you think? [1] https://www.postgresql.org/message-id/0F50D989-B82D-4F59-9F13-C08A4673322C%40justatheory.com [2] https://www.postgresql.org/docs/devel/runtime-config-client.html#GUC-EXTENSION-CONTROL-PATH -- Matheus Alcantara
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
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]:
```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
Attachment
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
Attachment
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
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
Attachment
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 thatmy prefix is not what I expected. Because it doesn’t contain the string “postgres”, PGXS helpfully adds it. The actualpaths 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 settingthe main sharedir and libdir for Postgres, but I don’t know that it makes sense for extension prefixes. Best, David
Attachment
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 thatmy prefix is not what I expected. Because it doesn’t contain the string “postgres”, PGXS helpfully adds it. The actualpaths 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
Attachment
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 fromextension_control_path and dynamic_library_path, since they get mangled. Best, David
Attachment
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 fromextension_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
Attachment
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
`
Attachment
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
justextensions. 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
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
Attachment
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
Attachment
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
Attachment
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.
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
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