> On 8 Oct 2021, at 06:24, Noah Misch <noah@leadboat.com> wrote:
>
> On Thu, Oct 07, 2021 at 11:39:11PM -0400, Tom Lane wrote:
>> Noah Misch <noah@leadboat.com> writes:
>>> On Thu, Oct 07, 2021 at 03:44:48PM -0400, Tom Lane wrote:
>>>>> (1) I'm distrustful of the idea that perl 5.8.x will compile
>>>>> cleanly, or at all, on modern platforms. Certainly Postgres
>>>>> releases of similar vintage won't.
>>
>>> perlbrew uses the patchperl system to build old Perl in modern environments.
>>> This year, I used it to get 5.8.0. Building unpatched 5.8.0 does fail.
>>
>> Oh, cool.
>>
>>>> I propose that what might be more useful than the existing last
>>>> section of src/test/perl/README is something along the lines of:
>>
>>> -1. This would replace a useful recipe with, essentially, a restatement of
>>> that recipe in English words. That just leaves the user to rediscover the
>>> actual recipe.
>>
>> Well, I think the existing text does the reader a disservice
>> by stating a specific recipe without any context. Notably,
>> it says nothing about restricting which Perl modules you use.
>
> That's obvious from "cpanm install IPC::Run". Surely if any other non-core
> module were allowed, the recipe would list it in a similar way.
The proposed changes talks about with core modules are allowed to use, I think
that's a different thing. The distinction between core and non-core modules
may not be known/clear to people who haven't used Perl in the past.
> This is a source tree README; it shouldn't try to hold the reader's hand like
> the user-facing docs do. We've not had commits add usage of other modules, so
> there's no evidence of actual doubt on this point.
This README isn't primarily targeting committers though IMO, but new developers
onboarding onto postgres who are trying to learn the dev environment.
>> What do you think of using my proposed text followed by
>>
>> One way to test against an old Perl version is to use
>> perlbrew.
>> << more or less the existing text here >>
>> Bear in mind that you will still need to install IPC::Run,
>> and what you will get is a current version not the one
>> distributed with Perl 5.8.3. You will also need to update
>> Test::More because the version distributed with Perl 5.8.3
>> is too old to run our TAP tests. So this recipe does not create
>> a perfect reproduction of a back-in-the-day Perl installation,
>> but it will probably catch any problems that might surface in
>> the buildfarm.
>
> I don't see an improvement in there.
I respectfully disagree, the current text reads as if 5.8.0 is required for
running the test, not that using perlbrew is a great way to verify that your
tests pass in all supported Perl versions.
> If there's something to change, it's improving the actual recipe:
That we should do as well.
--
Daniel Gustafsson https://vmware.com/
