Обсуждение: pgsql: Skip \password TAP test on old IPC::Run versions
Skip \password TAP test on old IPC::Run versions IPC::Run versions prior to 0.98 cause the interactive session to time out, so SKIP the test in case these versions are detected (they are within the base requirement for our TAP tests in general). Error reported by the BF and investigation by Tom Lane. Discussion: https://postgr.es/m/414A86BD-986B-48A7-A1E4-EEBCE5AF08CB@yesql.se Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/2e57ffe12f6b5c1498f29cb7c0d9e17c797d9da6 Modified Files -------------- src/test/authentication/t/001_password.pl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
On 2023-04-08 Sa 09:57, Daniel Gustafsson wrote:
Skip \password TAP test on old IPC::Run versions IPC::Run versions prior to 0.98 cause the interactive session to time out, so SKIP the test in case these versions are detected (they are within the base requirement for our TAP tests in general). Error reported by the BF and investigation by Tom Lane. Discussion: https://postgr.es/m/414A86BD-986B-48A7-A1E4-EEBCE5AF08CB@yesql.se
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'; }"
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
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. Can't we write "use IPC::Run 0.98;" and let
some other code manage the version comparison?
regards, tom lane
> 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.
--
Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
> On 8 Apr 2023, at 18:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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 });
Maybe I'm missing something, but I was envisioning
eval { require IO::Pty; use IPC::Run 0.98; }
with no need to do more than check if the eval traps an error.
regards, tom lane
On 2023-04-08 Sa 16:20, Daniel Gustafsson wrote:
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
On 2023-04-08 Sa 16:30, Tom Lane wrote:
Daniel Gustafsson <daniel@yesql.se> writes:On 8 Apr 2023, at 18:23, Tom Lane <tgl@sss.pgh.pa.us> wrote: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 });Maybe I'm missing something, but I was envisioning eval { require IO::Pty; use IPC::Run 0.98; } with no need to do more than check if the eval traps an error.
You need to be careful with "use". It is executed in the compile phase, so I'd avoid it here.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
> On 8 Apr 2023, at 22:37, Andrew Dunstan <andrew@dunslane.net> wrote:
> On 2023-04-08 Sa 16:20, Daniel Gustafsson wrote:
>>> 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'); }
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