Re: [HACKERS] [patch] extensions_path GUC

[David E. Wheeler]

On Oct 23, 2015, at 9:26 AM, Jim Nasby  wrote:

> I would love it if make check worked. make installcheck adds extra effort to 
> extension develoopment, not to mention leaving your actual install in a less 
> than pristine state.

I’ve wanted this for a long time. I think it would have to create a temporary 
cluster, fire up a server, install the extension(s), run the tests, shut down 
the server and delete the cluster.

> Possibly related to this... I'd also like to have other options for running 
> unit tests, besides pg_regress. I looked at it briefly and the big PITA about 
> doing it was having to manage the temporary database (and ideally temporary 
> cluster). If standing those up was separated from pg_regress it would make it 
> a lot easier for someone to customize how testing works under PGXS.

Right, then pg_regress could just be the default test framework.

Dvaid



smime.p7s
Description: S/MIME cryptographic signature

[HACKERS] [patch] extensions_path GUC

[Sandro Santilli]

One problem we have with PostGIS is you cannot test an extension
unless you have access to the system extension dir.

The following patch tries to address that by allowing to specify
a per-cluster extension path via an "extensions_path" GUC.

It is more a request-for-comments rather than a ready patch, as
I hadn't considered all use cases like upgrades of already-loaded
extensions and the possibility ot have a list of directories to
seek for extensions.

Anyway, patch is attached.

--strk;

  ()   Free GIS & Flash consultant/developer
  /\   http://strk.keybit.net/services.html
>From 9bba783d6581bc445b8a24fc8e615969fc16ab90 Mon Sep 17 00:00:00 2001
From: Sandro Santilli 
Date: Fri, 23 Oct 2015 12:00:51 +0200
Subject: [PATCH] Add extensions_path GUC

---
 src/backend/commands/extension.c | 37 ++---
 src/backend/utils/misc/guc.c | 14 ++
 src/include/commands/extension.h |  3 +++
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 67b16a7..df4e5df 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -64,6 +64,7 @@
 /* Globally visible state variables */
 bool		creating_extension = false;
 Oid			CurrentExtensionObject = InvalidOid;
+char		*Extension_path = NULL; /* set by guc.c */
 
 /*
  * Internal data structure to hold the results of parsing a control file
@@ -348,15 +349,29 @@ is_extension_script_filename(const char *filename)
 	return (extension != NULL) && (strcmp(extension, ".sql") == 0);
 }
 
+static void
+get_extension_path(char *ret_path)
+{
+	char		sharepath[MAXPGPATH];
+	if ( Extension_path )
+	{
+		snprintf(ret_path, MAXPGPATH, "%s", Extension_path);
+	}
+	else
+	{
+		get_share_path(my_exec_path, sharepath);
+		snprintf(ret_path, MAXPGPATH, "%s/extension", sharepath);
+	}
+}
+
 static char *
 get_extension_control_directory(void)
 {
-	char		sharepath[MAXPGPATH];
 	char	   *result;
 
-	get_share_path(my_exec_path, sharepath);
 	result = (char *) palloc(MAXPGPATH);
-	snprintf(result, MAXPGPATH, "%s/extension", sharepath);
+
+	get_extension_path(result);
 
 	return result;
 }
@@ -364,13 +379,13 @@ get_extension_control_directory(void)
 static char *
 get_extension_control_filename(const char *extname)
 {
-	char		sharepath[MAXPGPATH];
-	char	   *result;
+	char		extpath[MAXPGPATH];
+	char	  *result;
 
-	get_share_path(my_exec_path, sharepath);
+	get_extension_path(extpath);
 	result = (char *) palloc(MAXPGPATH);
-	snprintf(result, MAXPGPATH, "%s/extension/%s.control",
-			 sharepath, extname);
+	snprintf(result, MAXPGPATH, "%s/%s.control",
+			 extpath, extname);
 
 	return result;
 }
@@ -378,7 +393,7 @@ get_extension_control_filename(const char *extname)
 static char *
 get_extension_script_directory(ExtensionControlFile *control)
 {
-	char		sharepath[MAXPGPATH];
+	char		extpath[MAXPGPATH];
 	char	   *result;
 
 	/*
@@ -391,9 +406,9 @@ get_extension_script_directory(ExtensionControlFile *control)
 	if (is_absolute_path(control->directory))
 		return pstrdup(control->directory);
 
-	get_share_path(my_exec_path, sharepath);
+	get_extension_path(extpath);
 	result = (char *) palloc(MAXPGPATH);
-	snprintf(result, MAXPGPATH, "%s/%s", sharepath, control->directory);
+	snprintf(result, MAXPGPATH, "%s/%s", extpath, control->directory);
 
 	return result;
 }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fda0fb9..f76d07b 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -33,6 +33,7 @@
 #include "access/xact.h"
 #include "catalog/namespace.h"
 #include "commands/async.h"
+#include "commands/extension.h"
 #include "commands/prepare.h"
 #include "commands/vacuum.h"
 #include "commands/variable.h"
@@ -2943,6 +2944,19 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
+		{"extensions_path", PGC_SUSET, CLIENT_CONN_OTHER,
+			gettext_noop("Sets the path for extensions."),
+			gettext_noop("If an extension control file needs to be opened "
+		 "the system will search this path for "
+		 "the specified file."),
+			GUC_SUPERUSER_ONLY
+		},
+		_path,
+		NULL, /* will be handled by get_extension_control_directory */
+		NULL, NULL, NULL
+	},
+
+	{
 		{"krb_server_keyfile", PGC_SIGHUP, CONN_AUTH_SECURITY,
 			gettext_noop("Sets the location of the Kerberos server key file."),
 			NULL,
diff --git a/src/include/commands/extension.h b/src/include/commands/extension.h
index 0423350..bf6f44b 100644
--- a/src/include/commands/extension.h
+++ b/src/include/commands/extension.h
@@ -27,6 +27,9 @@
 extern PGDLLIMPORT bool creating_extension;
 extern Oid	CurrentExtensionObject;
 
+/* Path to find extension control files in,
+ * defaults to sharedir/extension */
+extern char *Extension_path;
 
 extern ObjectAddress CreateExtension(CreateExtensionStmt *stmt);
 
-- 
1.9.1


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to 

Re: [HACKERS] [patch] extensions_path GUC

[Heikki Linnakangas]

On 10/23/2015 01:33 PM, Sandro Santilli wrote:

One problem we have with PostGIS is you cannot test an extension
unless you have access to the system extension dir.

The following patch tries to address that by allowing to specify
a per-cluster extension path via an "extensions_path" GUC.

It is more a request-for-comments rather than a ready patch, as
I hadn't considered all use cases like upgrades of already-loaded
extensions and the possibility ot have a list of directories to
seek for extensions.


This was discussed a while ago already, see 
http://www.postgresql.org/message-id/51ae0845.8010...@ocharles.org.uk.


In short, I don't think just setting extensions_path is enough or 
desirable, but I would welcome a patch that makes "make check" work for 
extensions, by creating a temporary installation.


- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] extensions_path GUC

[Michael Paquier]

On Fri, Oct 23, 2015 at 7:46 PM, Heikki Linnakangas wrote:
> On 10/23/2015 01:33 PM, Sandro Santilli wrote:
> In short, I don't think just setting extensions_path is enough or desirable,
> but I would welcome a patch that makes "make check" work for extensions, by
> creating a temporary installation.

Isn't that the existing EXTRA_INSTALL?
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] extensions_path GUC

[Heikki Linnakangas]

On 10/23/2015 02:59 PM, Michael Paquier wrote:

On Fri, Oct 23, 2015 at 7:46 PM, Heikki Linnakangas wrote:

On 10/23/2015 01:33 PM, Sandro Santilli wrote:
In short, I don't think just setting extensions_path is enough or desirable,
but I would welcome a patch that makes "make check" work for extensions, by
creating a temporary installation.


Isn't that the existing EXTRA_INSTALL?


No. The problem is that "make check" doesn't work extensions built 
outside the server source tree. You get an error:


"make check" is not supported.
Do "make install", then "make installcheck" instead.

- Heikki



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] extensions_path GUC

[Jim Nasby]

On 10/23/15 11:02 AM, Heikki Linnakangas wrote:

On 10/23/2015 02:59 PM, Michael Paquier wrote:

On Fri, Oct 23, 2015 at 7:46 PM, Heikki Linnakangas wrote:

On 10/23/2015 01:33 PM, Sandro Santilli wrote:
In short, I don't think just setting extensions_path is enough or
desirable,
but I would welcome a patch that makes "make check" work for
extensions, by
creating a temporary installation.


Isn't that the existing EXTRA_INSTALL?


No. The problem is that "make check" doesn't work extensions built
outside the server source tree. You get an error:

"make check" is not supported.
Do "make install", then "make installcheck" instead.


I would love it if make check worked. make installcheck adds extra 
effort to extension develoopment, not to mention leaving your actual 
install in a less than pristine state.


Possibly related to this... I'd also like to have other options for 
running unit tests, besides pg_regress. I looked at it briefly and the 
big PITA about doing it was having to manage the temporary database (and 
ideally temporary cluster). If standing those up was separated from 
pg_regress it would make it a lot easier for someone to customize how 
testing works under PGXS.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [patch] extensions_path GUC

[Oskari Saarenmaa]

23.10.2015, 13:33, Sandro Santilli kirjoitti:

One problem we have with PostGIS is you cannot test an extension
unless you have access to the system extension dir.

The following patch tries to address that by allowing to specify
a per-cluster extension path via an "extensions_path" GUC.

It is more a request-for-comments rather than a ready patch, as
I hadn't considered all use cases like upgrades of already-loaded
extensions and the possibility ot have a list of directories to
seek for extensions.

Anyway, patch is attached.


This would be very useful. I proposed this previously in February, but 
that implementation (almost identical to yours) was rejected.


https://commitfest.postgresql.org/5/170/
http://www.postgresql.org/message-id/54e3c31f.8010...@ohmu.fi

Both of these implementations miss a way to override the path for 
extension .so files.


/ Oskari


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers