On 2013-11-09 15:13:14 +0100, Andres Freund wrote:
> Hi,
> 
> On 2013-11-08 20:02:30 +0000, Robert Haas wrote:
> > Fix pg_isolation_regress to work outside its build directory.
> 
> > This makes it possible to, for example, use the isolation tester to
> > test a contrib module.
> 
> Unfortunately this isn't working all that well on anole and currawong :/.
> Looking at the output on the former the reason becomes clear:
> ./pg_isolation_regress --temp-install=./tmp_check --inputdir=. 
> --top-builddir=../../.. --schedule=./isolation_schedule
> /usr/lib/hpux64/dld.so: Unable to find library 'libpq.so.5'.
> sh: 15000 Killed
> 
> isolationtester doesn't find libpq during find_other_exec(). I think that is 
> because it normally
> only gets run when pg_regress.c has done:
>               /*
>                * Set up shared library paths to include the temp install.
>                *
>                * LD_LIBRARY_PATH covers many platforms.  DYLD_LIBRARY_PATH 
> works on
>                * Darwin, and maybe other Mach-based systems.  LIBPATH is for 
> AIX.
>                * Windows needs shared libraries in PATH (only those linked 
> into
>                * executables, not dlopen'ed ones). Feel free to account for 
> others
>                * as well.
>                */
>               add_to_path("LD_LIBRARY_PATH", ':', libdir);
>               add_to_path("DYLD_LIBRARY_PATH", ':', libdir);
>               add_to_path("LIBPATH", ':', libdir);
> #if defined(WIN32)
>               add_to_path("PATH", ';', libdir);
> #elif defined(__CYGWIN__)
>               add_to_path("PATH", ':', libdir);
> #endif
> 
> but the initialization functions are run earlier:
>       /*
>        * We call the initialization function here because that way we can set
>        * default parameters and let them be overwritten by the commandline.
>        */
>       ifunc(argc, argv);
> 
> The only hack I currently can think of is to store argv[0] in a global
> variable and to run the find_other_exec() in the first run through
> isolation_start_test() :/.

A patch to that end attached.

Alternatively we could add a "startup_function startup" argument to
regression_main() that's called later, when the environment is setup.

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From af98a67ac0bcbf43e9a0039439e722af4008c6db Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 12 Nov 2013 12:49:54 +0100
Subject: [PATCH] Fix pg_isolation_regress to find_other_exec() after
 environment setup.

The previous behaviour of doing find_other_exec() in the
initialization function called by regression_main() failed to forsee
that libraries in --temp-install mode might not be in the library
search path at that point. The environment is only initialized after
performing commandline parsing which has to happen after the
initialization functions are executed so defaults setup there can be
overridden.
Instead do the lookup the first time through the test routine.

This hopefully should fix buildfarm members anole and currawong.
---
 src/test/isolation/isolation_main.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/test/isolation/isolation_main.c b/src/test/isolation/isolation_main.c
index b3a8ff0..94f01b8 100644
--- a/src/test/isolation/isolation_main.c
+++ b/src/test/isolation/isolation_main.c
@@ -12,7 +12,10 @@
 
 #include "pg_regress.h"
 
+char saved_argv0[MAXPGPATH];
 char isolation_exec[MAXPGPATH];
+bool looked_up_isolation_exec = false;
+
 #define PG_ISOLATION_VERSIONSTR "isolationtester (PostgreSQL) " PG_VERSION "\n"
 
 /*
@@ -32,6 +35,19 @@ isolation_start_test(const char *testname,
 	char		psql_cmd[MAXPGPATH * 3];
 	size_t		offset = 0;
 
+	/* need to do the path lookup here, check isolation_init() for details */
+	if (!looked_up_isolation_exec)
+	{
+		/* look for isolationtester binary */
+		if (find_other_exec(saved_argv0, "isolationtester",
+							PG_ISOLATION_VERSIONSTR, isolation_exec) != 0)
+		{
+			fprintf(stderr, _("could not find proper isolationtester binary\n"));
+			exit(2);
+		}
+		looked_up_isolation_exec = true;
+	}
+
 	/*
 	 * Look for files in the output dir first, consistent with a vpath search.
 	 * This is mainly to create more reasonable error messages if the file is
@@ -82,13 +98,16 @@ isolation_start_test(const char *testname,
 static void
 isolation_init(int argc, char **argv)
 {
-	/* look for isolationtester binary */
-	if (find_other_exec(argv[0], "isolationtester",
-						PG_ISOLATION_VERSIONSTR, isolation_exec) != 0)
-	{
-		fprintf(stderr, _("could not find proper isolationtester binary\n"));
-		exit(2);
-	}
+	/*
+	 * We unfortunately cannot do the find_other_exec() lookup to find the
+	 * "isolationtester" binary here.  regression_main() calls the
+	 * initialization functions before parsing the commandline arguments and
+	 * thus hasn't changed the library search path at this point which in turn
+	 * can cause the "isolationtester -V" invocation that find_other_exec()
+	 * does to fail since it's linked to libpq.  So we instead copy argv[0]
+	 * and do the lookup the first time through isolation_start_test().
+	 */
+	strncpy(saved_argv0, argv[0], MAXPGPATH);
 
 	/* set default regression database name */
 	add_stringlist_item(&dblist, "isolationtest");
-- 
1.8.5.rc1.dirty

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

Reply via email to