Re: [HACKERS] pg_upgrade and pg_config dependency

2012-03-19 Thread Bruce Momjian
On Fri, Mar 16, 2012 at 08:11:17PM -0300, Alvaro Herrera wrote:
> 
> Excerpts from Bruce Momjian's message of vie mar 16 20:06:28 -0300 2012:
> > Àlvaro told me he got a Spanish-language report that pg_upgrade
> > failed because it required pg_config, and pg_config is only supplied
> > with the devel packages.
> > 
> > I initially thought that it was a packaging problem, but I later
> > realized the pg_config is mostly developer settings, and that using
> > pg_config was not getting any change to libdir by dynamic_library_path
> > in postgresql.conf, and that I should just merge the pg_upgrade_support
> > detection code into the existing shared library detection "LOAD" code I
> > already had.
> > 
> > This avoids the pg_config dependency, works better for libdir, and
> > reduces the amount of code.
> 
> Bruce also found a reference to this old bug report:
> http://archives.postgresql.org/pgsql-bugs/2011-12/msg00254.php
> This includes a link to a Debian bug report by Peter.
> 
> > Patch attached.  Should this be backpatched to PG 9.1;  I think so.
> 
> +1

Applied, and backpatched to 9.1.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +

-- 
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] pg_upgrade and pg_config dependency

2012-03-16 Thread Alvaro Herrera

Excerpts from Bruce Momjian's message of vie mar 16 20:06:28 -0300 2012:
> Àlvaro told me he got a Spanish-language report that pg_upgrade
> failed because it required pg_config, and pg_config is only supplied
> with the devel packages.
> 
> I initially thought that it was a packaging problem, but I later
> realized the pg_config is mostly developer settings, and that using
> pg_config was not getting any change to libdir by dynamic_library_path
> in postgresql.conf, and that I should just merge the pg_upgrade_support
> detection code into the existing shared library detection "LOAD" code I
> already had.
> 
> This avoids the pg_config dependency, works better for libdir, and
> reduces the amount of code.

Bruce also found a reference to this old bug report:
http://archives.postgresql.org/pgsql-bugs/2011-12/msg00254.php
This includes a link to a Debian bug report by Peter.

> Patch attached.  Should this be backpatched to PG 9.1;  I think so.

+1

-- 
Álvaro Herrera 
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

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


[HACKERS] pg_upgrade and pg_config dependency

2012-03-16 Thread Bruce Momjian
Àlvaro told me he got a Spanish-language report that pg_upgrade
failed because it required pg_config, and pg_config is only supplied
with the devel packages.

I initially thought that it was a packaging problem, but I later
realized the pg_config is mostly developer settings, and that using
pg_config was not getting any change to libdir by dynamic_library_path
in postgresql.conf, and that I should just merge the pg_upgrade_support
detection code into the existing shared library detection "LOAD" code I
already had.

This avoids the pg_config dependency, works better for libdir, and
reduces the amount of code.

Patch attached.  Should this be backpatched to PG 9.1;  I think so.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

  + It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 9fbfd40..03382d9
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*** static void check_is_super_user(ClusterI
*** 20,26 
  static void check_for_prepared_transactions(ClusterInfo *cluster);
  static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
  static void check_for_reg_data_type_usage(ClusterInfo *cluster);
- static void check_for_support_lib(ClusterInfo *cluster);
  static void get_bin_version(ClusterInfo *cluster);
  
  #ifndef WIN32
--- 20,25 
*** check_cluster_versions(void)
*** 270,277 
  void
  check_cluster_compatibility(bool live_check)
  {
- 	check_for_support_lib(&new_cluster);
- 
  	/* get/check pg_control data of servers */
  	get_control_data(&old_cluster, live_check);
  	get_control_data(&new_cluster, false);
--- 269,274 
*** check_for_reg_data_type_usage(ClusterInf
*** 841,885 
  }
  
  
- /*
-  * Test pg_upgrade_support.so is in the proper place.	 We cannot copy it
-  * ourselves because install directories are typically root-owned.
-  */
- static void
- check_for_support_lib(ClusterInfo *cluster)
- {
- 	char		cmd[MAXPGPATH];
- 	char		libdir[MAX_STRING];
- 	char		libfile[MAXPGPATH];
- 	FILE	   *lib_test;
- 	FILE	   *output;
- 
- 	snprintf(cmd, sizeof(cmd), "\"%s/pg_config\" --pkglibdir", cluster->bindir);
- 
- 	if ((output = popen(cmd, "r")) == NULL ||
- 		fgets(libdir, sizeof(libdir), output) == NULL)
- 		pg_log(PG_FATAL, "Could not get pkglibdir data using %s: %s\n",
- 			   cmd, getErrorText(errno));
- 
- 
- 	pclose(output);
- 
- 	/* Remove trailing newline */
- 	if (strchr(libdir, '\n') != NULL)
- 		*strchr(libdir, '\n') = '\0';
- 
- 	snprintf(libfile, sizeof(libfile), "%s/pg_upgrade_support%s", libdir,
- 			 DLSUFFIX);
- 
- 	if ((lib_test = fopen(libfile, "r")) == NULL)
- 		pg_log(PG_FATAL,
- 			   "The pg_upgrade_support module must be created and installed in the %s cluster.\n",
- CLUSTER_NAME(cluster));
- 
- 	fclose(lib_test);
- }
- 
- 
  static void
  get_bin_version(ClusterInfo *cluster)
  {
--- 838,843 
diff --git a/contrib/pg_upgrade/function.c b/contrib/pg_upgrade/function.c
new file mode 100644
index 3225039..fe8fb40
*** a/contrib/pg_upgrade/function.c
--- b/contrib/pg_upgrade/function.c
***
*** 13,18 
--- 13,19 
  
  #include "access/transam.h"
  
+ #define PG_UPGRADE_SUPPORT	"$libdir/pg_upgrade_support"
  
  /*
   * install_support_functions_in_new_db()
*** get_loadable_libraries(void)
*** 154,170 
  		PQfinish(conn);
  	}
  
  	/* Allocate what's certainly enough space */
! 	if (totaltups > 0)
! 		os_info.libraries = (char **) pg_malloc(totaltups * sizeof(char *));
! 	else
! 		os_info.libraries = NULL;
  
  	/*
  	 * Now remove duplicates across DBs.  This is pretty inefficient code, but
  	 * there probably aren't enough entries to matter.
  	 */
  	totaltups = 0;
  
  	for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
  	{
--- 155,171 
  		PQfinish(conn);
  	}
  
+ 	totaltups++;	/* reserve for pg_upgrade_support */
+ 
  	/* Allocate what's certainly enough space */
! 	os_info.libraries = (char **) pg_malloc(totaltups * sizeof(char *));
  
  	/*
  	 * Now remove duplicates across DBs.  This is pretty inefficient code, but
  	 * there probably aren't enough entries to matter.
  	 */
  	totaltups = 0;
+ 	os_info.libraries[totaltups++] = pg_strdup(PG_UPGRADE_SUPPORT);
  
  	for (dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
  	{
*** check_loadable_libraries(void)
*** 256,261 
--- 257,268 
  		if (PQresultStatus(res) != PGRES_COMMAND_OK)
  		{
  			found = true;
+ 
+ 			/* exit and report missing support library with special message */
+ 			if (strcmp(lib, PG_UPGRADE_SUPPORT) == 0)
+ pg_log(PG_FATAL,
+    "The pg_upgrade_support module must be created and installed in the new cluster.\n");
+ 
  			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
  pg_log(PG_FATAL, "Could not open file \"%s\": %s\n",
  	   output_path, getErrorText(errn