public inbox for [email protected]
help / color / mirror / Atom feedFrom: Matheus Alcantara <[email protected]>
To: David E. Wheeler <[email protected]>
Cc: Christoph Berg <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: [email protected]
Subject: Re: extension_control_path and "directory"
Date: Wed, 23 Apr 2025 17:34:26 -0300
Message-ID: <CAFY6G8c=W8CMFwcx2fA0RSFtvw=5BDGKOjGSV=nEUiZ6use2Bw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CAFY6G8dDjimxaSQV68ExWMmrAi8xNhiOT3RZOD8oQZmdGCG5Jg@mail.gmail.com>
<[email protected]>
<[email protected]>
On Wed, Apr 23, 2025 at 10:57 AM David E. Wheeler <[email protected]> wrote:
>
> On Apr 23, 2025, at 09:50, Christoph Berg <[email protected]> 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:
[application/octet-stream] v1-0001-Make-directory-work-with-extension-control-path.patch (4.6K, 2-v1-0001-Make-directory-work-with-extension-control-path.patch)
download | inline diff:
From c07bfd1556502592c1a63cceafe0542f28dfe0a4 Mon Sep 17 00:00:00 2001
From: Matheus Alcantara <[email protected]>
Date: Wed, 23 Apr 2025 16:11:24 -0300
Subject: [PATCH v1] Make "directory" work with extension control path
Previously extensions installed on a custom path that is available via
extension_control_path GUC that set the "directory" field on .control
file was not being able to CREATE. This was happening because on
get_extension_script_directory was hard coded to search for the script
files only on the share system dir.
This commit fix this issue by using the control->control_dir as a share
dir to return the path of the extension script files.
---
src/backend/commands/extension.c | 43 +++++++++++++++++--
.../t/001_extension_control_path.pl | 6 ++-
2 files changed, 43 insertions(+), 6 deletions(-)
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);
+
pfree(piece);
/* Canonicalize the path based on the OS and add to the list */
@@ -422,6 +430,9 @@ find_extension_control_filename(ExtensionControlFile *control)
ecp = Extension_control_path;
if (strlen(ecp) == 0)
ecp = "$system";
+ else if (strcmp(ecp, "$system") != 0)
+ ecp = psprintf("%s/extension", ecp);
+
result = find_in_path(basename, ecp, "extension_control_path", "$system", system_dir);
if (result)
@@ -439,8 +450,11 @@ find_extension_control_filename(ExtensionControlFile *control)
static char *
get_extension_script_directory(ExtensionControlFile *control)
{
- char sharepath[MAXPGPATH];
char *result;
+ int ctrldir_len;
+ int prefix_len;
+ int dir_len;
+ int suffix_len = strlen("extension");
/*
* The directory parameter can be omitted, absolute, or relative to the
@@ -452,9 +466,30 @@ get_extension_script_directory(ExtensionControlFile *control)
if (is_absolute_path(control->directory))
return pstrdup(control->directory);
- get_share_path(my_exec_path, sharepath);
- result = (char *) palloc(MAXPGPATH);
- snprintf(result, MAXPGPATH, "%s/%s", sharepath, control->directory);
+ ctrldir_len = strlen(control->control_dir);
+
+ /*
+ * 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);
+
+ /*
+ * At this point we have the control->directory and control->control_dir
+ * filled. As the extension is using a custom "directory" value we need to
+ * replace the "/extension" suffix from "control_dir" with the value of
+ * "directory" so that we can find the correct script files.
+ */
+ prefix_len = ctrldir_len - suffix_len;
+ dir_len = strlen(control->directory);
+
+ /* don't forget null terminator */
+ result = (char *) palloc(prefix_len + dir_len + 1);
+
+ memcpy(result, control->control_dir, prefix_len);
+ memcpy(result + prefix_len, control->directory, dir_len);
+ result[prefix_len + dir_len] = '\0'; /* don't forget null terminator */
return result;
}
diff --git a/src/test/modules/test_extensions/t/001_extension_control_path.pl b/src/test/modules/test_extensions/t/001_extension_control_path.pl
index c186c1470f7..7616011105f 100644
--- a/src/test/modules/test_extensions/t/001_extension_control_path.pl
+++ b/src/test/modules/test_extensions/t/001_extension_control_path.pl
@@ -5,6 +5,7 @@ use warnings FATAL => 'all';
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
+use File::Path qw( make_path );
my $node = PostgreSQL::Test::Cluster->new('node');
@@ -12,9 +13,10 @@ $node->init;
# Create a temporary directory for the extension control file
my $ext_dir = PostgreSQL::Test::Utils::tempdir();
+make_path("$ext_dir/extension");
my $ext_name = "test_custom_ext_paths";
-my $control_file = "$ext_dir/$ext_name.control";
-my $sql_file = "$ext_dir/$ext_name--1.0.sql";
+my $control_file = "$ext_dir/extension/$ext_name.control";
+my $sql_file = "$ext_dir/extension/$ext_name--1.0.sql";
# Create .control .sql file
open my $cf, '>', $control_file or die "Could not create control file: $!";
--
2.39.5 (Apple Git-154)
view thread (20+ 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]
Subject: Re: extension_control_path and "directory"
In-Reply-To: <CAFY6G8c=W8CMFwcx2fA0RSFtvw=5BDGKOjGSV=nEUiZ6use2Bw@mail.gmail.com>
* 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