Re: pgsql: Skip \password TAP test on old IPC::Run versions

On 8 Apr 2023, at 18:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Stylistic nitpick: It's not necessary to have 2 evals here:

+   skip "IO::Pty and IPC::Run >= 0.98 required", 1 unless
+       (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= 
'0.98' });

just say "eval { require IO::Pty; return $IPC::Run::VERSION >= '0.98'; }"

What I was wondering about was if it's safe to depend on the VERSION
being fully numeric.  

Reading documentation online pointed at this being the established way, and
trying to read the Perl5 code it seems to essentially turn whatever is set in
$VERSION into a numeric. 

Can't we write "use IPC::Run 0.98;" and let
some other code manage the version comparison?

We can, but that AFAIK (Andrew might have a better answer) requires the below
diff which I think leaves some readability to be desired:

-   (eval { require IO::Pty; } && eval { $IPC::Run::VERSION >= '0.98' });
+   (eval { require IO::Pty; } && !!eval { IPC::Run->VERSION('0.98'); 1 });

This is needed since ->VERSION die()'s if the version isn't satisfied.  That
seems to work fine though, and will ensure that the UNIVERSAL version method
does the version comparison.  We can of course expand the comment on why that
construct is needed.

I still don't understand why you're using two evals here. This should be sufficient and seems simpler to me:

  unless eval { require IO:Pty; IPC::Run->VERSION('0.98'); }

cheers

andrew

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