On 3/23/21 7:09 PM, Andrew Dunstan wrote:
> On 3/23/21 6:36 PM, Michael Paquier wrote:
>> On Thu, Jan 28, 2021 at 10:19:57AM -0500, Andrew Dunstan wrote:
>>> +BEGIN
>>> +{
>>> +
>>> +    # putting this in a BEGIN block means it's run and checked by perl -c
>>> +
>>> +
>>> +    # everything other than info and get_new_node that we need to override.
>>> +    # they are all instance methods, so we can use the same template for 
>>> all.
>>> +    my @instance_overrides = qw(init backup start kill9 stop reload restart
>>> +      promote logrotate safe_psql psql background_psql
>>> +      interactive_psql poll_query_until command_ok
>>> +      command_fails command_like command_checks_all
>>> +      issues_sql_like run_log pg_recvlogical_upto
>>> +    );
>> No actual objections here, but it would be easy to miss the addition
>> of a new routine.  Would an exclusion filter be more adapted, aka
>> override everything except get_new_node() and info()?
>
>
> Actually, following a brief offline discussion today I've thought of a
> way that doesn't require subclassing. Will post that probably tomorrow.
>


And here it is. No subclass, no eval, no magic :-) Some of my colleagues
are a lot happier ;-)

The downside is that we need to litter PostgresNode with a bunch of
lines like:

    local %ENV = %ENV;
    _set_install_env($self);

The upside is that there's no longer a possibility that someone will add
a new routine to PostgresNode and forget to update the subclass.

Here is my simple test program:

    #!/usr/bin/perl

    use lib '/home/andrew/pgl/pg_head/src/test/perl';

    # PostgresNode (via TestLib) hijacks stdout, so make a dup before it
    gets a chance
    use vars qw($out);
    BEGIN
    {
        open ($out, ">&STDOUT");
    }

    use PostgresNode;

    my $node = PostgresNode->get_new_node('v12', install_path =>
    '/home/andrew/pgl/inst.12.5711');

    $ENV{PG_REGRESS} = '/bin/true'; # stupid but necessary

    $node->init();

    $node->start();

    my $version = $node->safe_psql('postgres', 'select version()');

    $node->stop();

    print $out "Version: $version\n";
    print $out $node->info();


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 97e05993be..5eabea4b2b 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -355,6 +355,7 @@ sub info
 	print $fh "Archive directory: " . $self->archive_dir . "\n";
 	print $fh "Connection string: " . $self->connstr . "\n";
 	print $fh "Log file: " . $self->logfile . "\n";
+	print $fh "Install Path: " , $self->{_install_path} . "\n" if $self->{_install_path};
 	close $fh or die;
 	return $_info;
 }
@@ -428,6 +429,9 @@ sub init
 	my $pgdata = $self->data_dir;
 	my $host   = $self->host;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	$params{allows_streaming} = 0 unless defined $params{allows_streaming};
 	$params{has_archiving}    = 0 unless defined $params{has_archiving};
 
@@ -555,6 +559,9 @@ sub backup
 	my $backup_path = $self->backup_dir . '/' . $backup_name;
 	my $name        = $self->name;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	print "# Taking pg_basebackup $backup_name from node \"$name\"\n";
 	TestLib::system_or_bail(
 		'pg_basebackup', '-D', $backup_path, '-h',
@@ -780,22 +787,22 @@ sub start
 	my $name   = $self->name;
 	my $ret;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	BAIL_OUT("node \"$name\" is already running") if defined $self->{_pid};
 
 	print("### Starting node \"$name\"\n");
 
-	{
-		# Temporarily unset PGAPPNAME so that the server doesn't
-		# inherit it.  Otherwise this could affect libpqwalreceiver
-		# connections in confusing ways.
-		local %ENV = %ENV;
-		delete $ENV{PGAPPNAME};
-
-		# Note: We set the cluster_name here, not in postgresql.conf (in
-		# sub init) so that it does not get copied to standbys.
-		$ret = TestLib::system_log('pg_ctl', '-D', $self->data_dir, '-l',
+	# Temporarily unset PGAPPNAME so that the server doesn't
+	# inherit it.  Otherwise this could affect libpqwalreceiver
+	# connections in confusing ways.
+	delete $ENV{PGAPPNAME};
+
+	# Note: We set the cluster_name here, not in postgresql.conf (in
+	# sub init) so that it does not get copied to standbys.
+	$ret = TestLib::system_log('pg_ctl', '-D', $self->data_dir, '-l',
 			$self->logfile, '-o', "--cluster-name=$name", 'start');
-	}
 
 	if ($ret != 0)
 	{
@@ -826,6 +833,10 @@ sub kill9
 	my ($self) = @_;
 	my $name = $self->name;
 	return unless defined $self->{_pid};
+
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	print "### Killing node \"$name\" using signal 9\n";
 	# kill(9, ...) fails under msys Perl 5.8.8, so fall back on pg_ctl.
 	kill(9, $self->{_pid})
@@ -852,6 +863,10 @@ sub stop
 	my $port   = $self->port;
 	my $pgdata = $self->data_dir;
 	my $name   = $self->name;
+
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	$mode = 'fast' unless defined $mode;
 	return unless defined $self->{_pid};
 	print "### Stopping node \"$name\" using mode $mode\n";
@@ -874,6 +889,10 @@ sub reload
 	my $port   = $self->port;
 	my $pgdata = $self->data_dir;
 	my $name   = $self->name;
+
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	print "### Reloading node \"$name\"\n";
 	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, 'reload');
 	return;
@@ -895,15 +914,15 @@ sub restart
 	my $logfile = $self->logfile;
 	my $name    = $self->name;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	print "### Restarting node \"$name\"\n";
 
-	{
-		local %ENV = %ENV;
-		delete $ENV{PGAPPNAME};
+	delete $ENV{PGAPPNAME};
 
-		TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
+	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
 			'restart');
-	}
 
 	$self->_update_pid(1);
 	return;
@@ -924,6 +943,10 @@ sub promote
 	my $pgdata  = $self->data_dir;
 	my $logfile = $self->logfile;
 	my $name    = $self->name;
+
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	print "### Promoting node \"$name\"\n";
 	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
 		'promote');
@@ -945,6 +968,10 @@ sub logrotate
 	my $pgdata  = $self->data_dir;
 	my $logfile = $self->logfile;
 	my $name    = $self->name;
+
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	print "### Rotating log in node \"$name\"\n";
 	TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
 		'logrotate');
@@ -1117,6 +1144,14 @@ By default, all nodes use the same PGHOST value.  If specified, generate a
 PGHOST specific to this node.  This allows multiple nodes to use the same
 port.
 
+=item install_path => '/path/to/postgres/installation'
+
+Using this parameter is it possible to have nodes pointing to different
+installations, for testing different versions together or the same version
+with different build parameters. The provided path must be the parent of the
+installation's 'bin' and 'lib' directories. In the common case where this is
+not provided, Postgres binaries will be found in the caller's PATH.
+
 =back
 
 For backwards compatibility, it is also exported as a standalone function,
@@ -1165,12 +1200,65 @@ sub get_new_node
 	# Lock port number found by creating a new node
 	my $node = $class->new($name, $host, $port);
 
+	if ($params{install_path})
+	{
+		$node->{_install_path} = $params{install_path};
+	}
+
 	# Add node to list of nodes
 	push(@all_nodes, $node);
 
 	return $node;
 }
 
+# Private routine to set the PATH and (DY)LD_LIBRARY_PATH correctly when there
+# is an install path set for the node.
+# Routines that call Postgres binaries need to call this routine like this:
+#
+#    local %ENV = %ENV;
+#    _set_install_env{$self);
+#
+# This is effectively a no-op if the install path isn't set.
+#
+# The install path set in get_new_node needs to be a directory containing
+# bin and lib subdirectories as in a standard PostgreSQL installation, so this
+# can't be used with installations where the bin and lib directories don't have
+# a common parent directory.
+sub _set_install_env
+{
+	my $self = shift;
+	my $inst = $self->{_install_path};
+	return unless $inst;
+	# the caller must have set up a local copy of %ENV
+	if ($TestLib::windows_os)
+	{
+		# Windows picks up DLLs from the PATH rather than *LD_LIBRARY_PATH
+		# choose the right path separator
+		if ($Config{osname} eq 'MSWin32')
+		{
+			$ENV{PATH} = "$inst/bin;$inst/lib;$ENV{PATH}";
+		}
+		else
+		{
+			$ENV{PATH} = "$inst/bin:$inst/lib:$ENV{PATH}";
+		}
+	}
+	else
+	{
+		my $dylib_name =
+		  $Config{osname} eq 'darwin' ? "DYLD_LIBRARY_PATH" : "LD_LIBRARY_PATH";
+		$ENV{PATH} = "$inst/bin:$ENV{PATH}";
+		if (exists $ENV{$dylib_name})
+		{
+			$ENV{$dylib_name} = "$inst/lib:$ENV{$dylib_name}";
+		}
+		else
+		{
+			$ENV{$dylib_name} = "$inst/lib";
+		}
+	}
+}
+
 =pod
 
 =item get_free_port()
@@ -1330,6 +1418,9 @@ sub safe_psql
 {
 	my ($self, $dbname, $sql, %params) = @_;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	my ($stdout, $stderr);
 
 	my $ret = $self->psql(
@@ -1441,6 +1532,9 @@ sub psql
 {
 	my ($self, $dbname, $sql, %params) = @_;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	my $stdout            = $params{stdout};
 	my $stderr            = $params{stderr};
 	my $replication       = $params{replication};
@@ -1634,6 +1728,9 @@ sub background_psql
 {
 	my ($self, $dbname, $stdin, $stdout, $timer, %params) = @_;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	my $replication = $params{replication};
 
 	my @psql_params = (
@@ -1712,6 +1809,9 @@ sub interactive_psql
 {
 	my ($self, $dbname, $stdin, $stdout, $timer, %params) = @_;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	my @psql_params = ('psql', '-XAt', '-d', $self->connstr($dbname));
 
 	push @psql_params, @{ $params{extra_params} }
@@ -1755,6 +1855,9 @@ sub poll_query_until
 {
 	my ($self, $dbname, $query, $expected) = @_;
 
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	$expected = 't' unless defined($expected);    # default value
 
 	my $cmd = [ 'psql', '-XAt', '-c', $query, '-d', $self->connstr($dbname) ];
@@ -1810,8 +1913,11 @@ sub command_ok
 
 	my $self = shift;
 
-	local $ENV{PGHOST} = $self->host;
-	local $ENV{PGPORT} = $self->port;
+	local %ENV = %ENV;
+	_set_install_env($self);
+
+	$ENV{PGHOST} = $self->host;
+	$ENV{PGPORT} = $self->port;
 
 	TestLib::command_ok(@_);
 	return;
@@ -1831,8 +1937,11 @@ sub command_fails
 
 	my $self = shift;
 
-	local $ENV{PGHOST} = $self->host;
-	local $ENV{PGPORT} = $self->port;
+	local %ENV = %ENV;
+	_set_install_env($self);
+
+	$ENV{PGHOST} = $self->host;
+	$ENV{PGPORT} = $self->port;
 
 	TestLib::command_fails(@_);
 	return;
@@ -1852,8 +1961,11 @@ sub command_like
 
 	my $self = shift;
 
-	local $ENV{PGHOST} = $self->host;
-	local $ENV{PGPORT} = $self->port;
+	local %ENV = %ENV;
+	_set_install_env($self);
+
+	$ENV{PGHOST} = $self->host;
+	$ENV{PGPORT} = $self->port;
 
 	TestLib::command_like(@_);
 	return;
@@ -1874,8 +1986,11 @@ sub command_checks_all
 
 	my $self = shift;
 
-	local $ENV{PGHOST} = $self->host;
-	local $ENV{PGPORT} = $self->port;
+	local %ENV = %ENV;
+	_set_install_env($self);
+
+	$ENV{PGHOST} = $self->host;
+	$ENV{PGPORT} = $self->port;
 
 	TestLib::command_checks_all(@_);
 	return;
@@ -1899,8 +2014,11 @@ sub issues_sql_like
 
 	my ($self, $cmd, $expected_sql, $test_name) = @_;
 
-	local $ENV{PGHOST} = $self->host;
-	local $ENV{PGPORT} = $self->port;
+	local %ENV = %ENV;
+	_set_install_env($self);
+
+	$ENV{PGHOST} = $self->host;
+	$ENV{PGPORT} = $self->port;
 
 	truncate $self->logfile, 0;
 	my $result = TestLib::run_log($cmd);
@@ -1923,8 +2041,11 @@ sub run_log
 {
 	my $self = shift;
 
-	local $ENV{PGHOST} = $self->host;
-	local $ENV{PGPORT} = $self->port;
+	local %ENV = %ENV;
+	_set_install_env($self);
+
+	$ENV{PGHOST} = $self->host;
+	$ENV{PGPORT} = $self->port;
 
 	TestLib::run_log(@_);
 	return;
@@ -2174,6 +2295,10 @@ sub pg_recvlogical_upto
 {
 	my ($self, $dbname, $slot_name, $endpos, $timeout_secs, %plugin_options)
 	  = @_;
+
+	local %ENV = %ENV;
+	_set_install_env($self);
+
 	my ($stdout, $stderr);
 
 	my $timeout_exception = 'pg_recvlogical timed out';

Reply via email to