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

On 2023-04-08 Sa 16:20, Daniel Gustafsson wrote:
>> On 8 Apr 2023, at 18:23, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>> Andrew Dunstan<andrew(at)dunslane(dot)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