Hi Matheus, Thanks for testing this and for the review.
> 1: The SHAREDIR vs extension_control_path wording
Agreed. With extension_control_path = '$system', a control file in
SHAREDIR/extension with directory = 'scripts' loads its scripts from
SHAREDIR/scripts, so "the element used to locate the primary control
file" could indeed be read as SHAREDIR/extension. v4-0001 uses your
suggested wording:
Unless an absolute path is given, the name is relative to the parent
of the extension directory where the primary control file was found,
that is, the installation's SHAREDIR directory, or the corresponding
extension_control_path element.
> 2: Commit message
Fixed; it now says the path is resolved against "the base directory,
i.e., the parent of the "extension" directory in which the primary
control file was found".
> The attached v3 includes a 0002 that fixes the docs for this.
I can reproduce it: with extension_control_path = '$system/foo',
pg_available_extensions returns no rows and CREATE EXTENSION fails with
"component in parameter "extension_control_path" is not an absolute
path". It's even more confusing when combined with a valid element:
with extension_control_path = '$system/foo:$system',
pg_available_extensions lists the extensions from $system (the bad
element is silently skipped as a nonexistent directory), but CREATE
EXTENSION on any of them still fails with the same error, because
find_in_paths() rejects the first element before it gets to $system.
0002 looks good and is included in v4 unchanged.
Two related things I noticed while testing:
- The long_desc of extension_control_path in guc_parameters.dat
(guc_tables.c on 18) has the same "loaded from the same directory
where the primary control file was found" sentence that 0001 fixes in
config.sgml. The new 0003 adjusts it to match. Since it's a
translatable string, I've kept it separate and meant it for master
only, so 0001 and 0002 can be backpatched without it.
- The '$system/foo:$system' inconsistency comes from find_in_paths()
and pg_available_extensions handling an invalid element differently,
and from nothing validating the value when it's set. Would a check
hook that rejects such elements at SET time be worth pursuing? One
concern is that it could break existing setups: a value like
'$system:$system/foo' works today for any extension found in $system,
but with a check hook it would be rejected, and in postgresql.conf
that would prevent the server from starting after an upgrade.
dynamic_library_path also only validates its elements when they're
used. If there's interest I can start a separate thread for that;
it's not addressed in v4.
v4-0001 and v4-0002 are rebased on current master and apply cleanly to
REL_19_STABLE and REL_18_STABLE; the docs build without warnings.
v4-0003 applies on master (and REL_19_STABLE, if wanted there); it
builds and passes make check.
v4-0001-doc-Fix-description-of-relative-extension-directo.patch
Description: Binary data
v4-0002-doc-Fix-description-of-system-in-extension_contro.patch
Description: Binary data
v4-0003-Fix-long-description-of-extension_control_path.patch
Description: Binary data
