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.
--
Michael
signature.asc
Description: PGP signature
