Date: Thu, 24 Apr 2025 12:21:06 +0200
From: Christoph Berg <myon@debian.org>
To: Matheus Alcantara <matheusssilv97@gmail.com>
Cc: "David E. Wheeler" <david@justatheory.com>,
	Peter Eisentraut <peter@eisentraut.org>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: extension_control_path and "directory"
Message-ID: <aAoQkqwktHQI5KLO@msg.df7cb.de>
References: <E1tumbY-003Dl3-2o@gemulon.postgresql.org>
 <aAi1VACxhjMhjFnb@msg.df7cb.de>
 <CAFY6G8dDjimxaSQV68ExWMmrAi8xNhiOT3RZOD8oQZmdGCG5Jg@mail.gmail.com>
 <aAjwPjsIElh17Nby@msg.df7cb.de>
 <3C304C9E-976C-4FB9-A883-707E0B90756A@justatheory.com>
 <CAFY6G8c=W8CMFwcx2fA0RSFtvw=5BDGKOjGSV=nEUiZ6use2Bw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFY6G8c=W8CMFwcx2fA0RSFtvw=5BDGKOjGSV=nEUiZ6use2Bw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aAoQkqwktHQI5KLO%40msg.df7cb.de>
Precedence: bulk

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