> On 8 Oct 2021, at 06:24, Noah Misch <n...@leadboat.com> wrote: > > On Thu, Oct 07, 2021 at 11:39:11PM -0400, Tom Lane wrote: >> Noah Misch <n...@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/