Re: multi-install PostgresNode

On 12/17/20 7:55 PM, Michael Paquier wrote:
> On Thu, Dec 17, 2020 at 04:37:54PM -0500, Andrew Dunstan wrote:
>> The proposed module would look something like this:
>>
>> [...]
>>
>>     use parent PostgresNode;
>>
>>     sub get_new_node
>>     {
>>         my $installpath= shift;
>>         my $node = PostgresNode::get_new_node(@_);
>>         bless $node; # re-bless into current class
>>         $node->{_installpath} = $installpath;
>>         return $node;
>>     }
> Passing down the installpath as argument and saving it within a
> PostgresNode or child class looks like the correct way of doing things
> to me.  This would require an extra routine to be able to get the
> install path from a node as _installpath would remain internal to the
> module file, right?  Shouldn't it be something that ought to be
> directly part of PostgresNode actually, where we could enforce the lib
> and bin paths to the output of pg_config if an _installpath is not
> provided by the caller?  In short, I am not sure that we need an extra
> module here.
>
>> and then  for each class method in PostgresNode.pm we'd have an override
>> something like:
>>
>>     sub foo
>>     {
>>         my $node=shift;
>>         my $inst = $node->{_installpath};
>>         local %ENV = %ENV;
>>         $ENV{PATH} = "$inst/bin:$ENV{PATH}";
>>         $ENV{LD_LIBRARY_PATH} = "$inst/lib:$ENV{LD_LIBRARY_PATH}";
>>         $node->SUPER::foo(@_);
>>     }
>>
>> There might be more elegant ways of doing this, but that's what I came
>> up with.
> As long as it does not become necessary to pass down _installpath to
> all indidivual binary calls we have in PostgresNode or the extra
> module, this gets a +1 from me.  So, if I am getting that right, the
> key point is the use of local %ENV here to make sure that PATH and
> LD_LIBRARY_PATH are only enforced when it comes to calls within
> PostgresNode.pm, right?  That's an elegant solution.  This is
> something I have wanted for a long time for pg_upgrade to be able to
> get rid of its test.sh.
>
>> My main question is: do we want something like this in the core code
>> (presumably in src/test/perl), or is it not of sufficiently general
>> interest? If it's wanted I'll submit a patch, probably for the March CF,
>> but January if I manage to get my running shoes on. If not, I'll put it
>> in the buildfarm code, but then any TAP tests that want it will likewise
>> need to live there.
> This facility gives us the possibility to clean up the test code of
> pg_upgrade and move it to a TAP test, so I'd say that it is worth
> having in the core code in the long-term.


This turns out to be remarkably short, with the use of a little eval magic.

Give the attached, this test program works just fine:

    #!/bin/perl
    use PostgresNodePath;
    $ENV{PG_REGRESS} =
    '/home/andrew/pgl/vpath.12/src/test/regress/pg_regress';
    my $node = get_new_node('/home/andrew/pgl/inst.12.5711','blurfl');
    print $node->info;
    print $node->connstr(),"\n";
    $node->init();

cheers


andrew

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