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

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Skip \password TAP test on old IPC::Run versions
Date: 2023-04-08 20:46:08
Message-ID: B05CB1A2-4AB9-484D-9B5D-A492349E208C@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

> On 8 Apr 2023, at 22:37, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> 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'); }

Because I was trying to get "use IPC::Run" to work and that required multiple
evals, and got stuck on that track. I agree that your version is better and
does indeed work (and offloads version comparison to the UNIVERSAL class), so
I'll go ahead with the below.

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

--
Daniel Gustafsson

In response to

Browse pgsql-committers by date

  From Date Subject
Next Message Daniel Gustafsson 2023-04-08 21:38:48 pgsql: Simplify version check for SKIP clause
Previous Message Andrew Dunstan 2023-04-08 20:40:12 Re: pgsql: Skip \password TAP test on old IPC::Run versions