Thread: Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.
[ cc'ing Craig and Noah, as author/committer of the existing text ] Daniel Gustafsson <daniel@yesql.se> writes: > On 7 Oct 2021, at 21:02, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> BTW, looking at that a second time, I wonder if that advice is >> really of any use. > Yeah, I would have to agree. Reading that again I think what it perhaps should > be saying is that 5.8.3 is the Perl API level that the testcode must conform > to, but they should run with basically whichever recent Perl you have handy as > long as the required modules are installed. Not that we expect developers to > run 5.8.3 when executing TAP tests. Yeah. I propose that what might be more useful than the existing last section of src/test/perl/README is something along the lines of: Avoid using any bleeding-edge Perl features. We have buildfarm animals running Perl versions as old as 5.8.3, so your tests will be expected to pass on that. Also, do not use any non-core Perl modules except IPC::Run. Or, if you must do so for a particular test, arrange to skip the test when the needed module isn't present. regards, tom lane
> On 7 Oct 2021, at 21:44, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > [ cc'ing Craig and Noah, as author/committer of the existing text ] > > Daniel Gustafsson <daniel@yesql.se> writes: >> On 7 Oct 2021, at 21:02, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> BTW, looking at that a second time, I wonder if that advice is >>> really of any use. > >> Yeah, I would have to agree. Reading that again I think what it perhaps should >> be saying is that 5.8.3 is the Perl API level that the testcode must conform >> to, but they should run with basically whichever recent Perl you have handy as >> long as the required modules are installed. Not that we expect developers to >> run 5.8.3 when executing TAP tests. > > Yeah. I propose that what might be more useful than the existing last > section of src/test/perl/README is something along the lines of: > > Avoid using any bleeding-edge Perl features. We have buildfarm > animals running Perl versions as old as 5.8.3, so your tests will > be expected to pass on that. > > Also, do not use any non-core Perl modules except IPC::Run. > Or, if you must do so for a particular test, arrange to skip > the test when the needed module isn't present. Agreed, that's a lot more helpful. Since the set of core Perl modules change over time as modules are brought in (at least that's my understanding of it), that last paragraph might want to discourage use of modules that aren't expected to be in-core in commonly used systems? It might be overthinking it though. -- Daniel Gustafsson https://vmware.com/
On 2021-Oct-07, Daniel Gustafsson wrote: > Agreed, that's a lot more helpful. Since the set of core Perl modules change > over time as modules are brought in (at least that's my understanding of it), > that last paragraph might want to discourage use of modules that aren't > expected to be in-core in commonly used systems? It might be overthinking it > though. Maybe we can mention `corelist -a` as a way to find out the module versions shipped with each Perl version. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "The Gord often wonders why people threaten never to come back after they've been told never to return" (www.actsofgord.com)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes: > Maybe we can mention `corelist -a` as a way to find out the module > versions shipped with each Perl version. Hm, I don't see that on my RHEL box. It does exist on my Mac, but the output is very unhelpful: $ which corelist /usr/bin/corelist $ corelist -a The contents of this script should normally never run! The perl wrapper should pick the correct script in /usr/bin by appending the appropriate version. You can try appending the appropriate perl version number. See perlmacosx.pod for more information about multiple version support in Mac OS X. That hint leads me to notice $ ls /usr/bin/corelist* /usr/bin/corelist* /usr/bin/corelist5.18* /usr/bin/corelist5.30* but all three of those print the same thing. So this isn't looking promising :-( regards, tom lane
On 2021-Oct-07, Tom Lane wrote: > Alvaro Herrera <alvherre@alvh.no-ip.org> writes: > > Maybe we can mention `corelist -a` as a way to find out the module > > versions shipped with each Perl version. > > Hm, I don't see that on my RHEL box. Oh, that's strange. It's installed by the perl package on my system, so I had assumed it was a standard part of a Perl install. > It does exist on my Mac, but the output is very unhelpful: Wow, it looks like it's completely broken in macOS. > So this isn't looking promising :-( Looking in the archives, apparently people use perl -MModule::CoreList but I see that that module, at least in Debian, is distributed even less widely than corelist(1) itself, because it's a separate package -- even though it seems to be part of Perl's core. Also, the module's interface appears less helpful than `corelist -a`. Let's leave it at that, then. Your original is a step forward in any case. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
Alvaro Herrera <alvherre@alvh.no-ip.org> writes: > On 2021-Oct-07, Tom Lane wrote: >> So this isn't looking promising :-( > Looking in the archives, apparently people use > perl -MModule::CoreList > but I see that that module, at least in Debian, is distributed even less > widely than corelist(1) itself, because it's a separate package -- even > though it seems to be part of Perl's core. Also, the module's interface > appears less helpful than `corelist -a`. Hmm. I do see that Module::CoreList knows not only which modules are in core but when they were brought in, so that does seem like a really valuable reference to know about. Let's just say something like "You can consult Module::CoreList to find out whether and for long a module has been present in the Perl core." regards, tom lane
On 2021-Oct-07, Tom Lane wrote: > Hmm. I do see that Module::CoreList knows not only which modules > are in core but when they were brought in, so that does seem like > a really valuable reference to know about. Let's just say something > like "You can consult Module::CoreList to find out whether and for > long a module has been present in the Perl core." WFM. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ Essentially, you're proposing Kevlar shoes as a solution for the problem that you want to walk around carrying a loaded gun aimed at your foot. (Tom Lane)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes: > On 2021-Oct-07, Tom Lane wrote: >> Hmm. I do see that Module::CoreList knows not only which modules >> are in core but when they were brought in, so that does seem like >> a really valuable reference to know about. Let's just say something >> like "You can consult Module::CoreList to find out whether and for >> long a module has been present in the Perl core." > WFM. Concretely, then, I propose the attached. regards, tom lane diff --git a/src/test/perl/README b/src/test/perl/README index bfc7cdcfa3..94b00096fb 100644 --- a/src/test/perl/README +++ b/src/test/perl/README @@ -70,20 +70,15 @@ perldoc for the test modules, e.g.: perldoc src/test/perl/PostgresNode.pm -Required Perl -------------- - -Tests must run on perl 5.8.3 and newer. perlbrew is a good way to obtain such -a Perl; see http://perlbrew.pl . - -Just install and - - perlbrew --force install 5.8.3 - perlbrew use 5.8.3 - perlbrew install-cpanm - cpanm install IPC::Run - -then re-run configure to ensure the correct Perl is used when running -tests. To verify that the right Perl was found: - - grep ^PERL= config.log +Portability +----------- + +Avoid using any bleeding-edge Perl features. We have buildfarm animals +running Perl versions as old as 5.8.3, so your tests will be expected +to pass on that. + +Also, do not use any non-core Perl modules except IPC::Run. Or, if you +must do so for a particular test, arrange to skip the test when the needed +module isn't present. If unsure, you can consult Module::CoreList to find +out whether a given module is part of the Perl core, and which module +versions shipped with which Perl releases.
> On 7 Oct 2021, at 23:48, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Concretely, then, I propose the attached. LGTM. Good idea to change the section heading, Portability is a better title for this. -- Daniel Gustafsson https://vmware.com/
On 2021-Oct-07, Tom Lane wrote: > +Portability > +----------- > + > +Avoid using any bleeding-edge Perl features. We have buildfarm animals > +running Perl versions as old as 5.8.3, so your tests will be expected > +to pass on that. > + > +Also, do not use any non-core Perl modules except IPC::Run. Or, if you > +must do so for a particular test, arrange to skip the test when the needed > +module isn't present. If unsure, you can consult Module::CoreList to find > +out whether a given module is part of the Perl core, and which module > +versions shipped with which Perl releases. LGTM, thanks. -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/ "Find a bug in a program, and fix it, and the program will work today. Show the program how to find and fix a bug, and the program will work forever" (Oliver Silfridge)
On Thu, Oct 07, 2021 at 03:44:48PM -0400, Tom Lane wrote: > [ cc'ing Craig and Noah, as author/committer of the existing text ] > > Daniel Gustafsson <daniel@yesql.se> writes: > > On 7 Oct 2021, at 21:02, Tom Lane <tgl@sss.pgh.pa.us> wrote: > >> BTW, looking at that a second time, I wonder if that advice is > >> really of any use. > >> > >> (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. > >> (2) Unless perlbrew.pl is doing something a lot more magic than > >> I think, you're going to end up with current-not-historical > >> versions of whatever it has to pull from CPAN. That's going > >> to include at least IPC::Run and Test::More if you want to run > >> our TAP tests. Yes. If someone changed the recipe to install Test::More 0.87 and the oldest-acceptable IPC::Run, we'd detect more portability problems. I'd regard such a change as an improvement. > >> So maybe this advice is helpful, but I'm not very convinced. The rest of this thread is leaning on the above misconceptions: > I propose that what might be more useful than the existing last > section of src/test/perl/README is something along the lines of: > > Avoid using any bleeding-edge Perl features. We have buildfarm > animals running Perl versions as old as 5.8.3, so your tests will > be expected to pass on that. > > Also, do not use any non-core Perl modules except IPC::Run. > Or, if you must do so for a particular test, arrange to skip > the test when the needed module isn't present. -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.
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. 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. regards, tom lane
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. 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. > 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. If there's something to change, it's improving the actual recipe: --- a/src/test/perl/README +++ b/src/test/perl/README @@ -83,3 +83,4 @@ Just install and perlbrew install-cpanm - cpanm install IPC::Run + cpanm install Test::More@0.87 + cpanm install IPC::Run@tbd_old_version
> 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/
Daniel Gustafsson <daniel@yesql.se> writes: > On 8 Oct 2021, at 06:24, Noah Misch <noah@leadboat.com> wrote: >> 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. Yeah, I don't really think that this recipe makes it plain that we have a policy. It certainly fails to explain that you're allowed to use additional modules if you're willing to skip the relevant tests. > This README isn't primarily targeting committers though IMO, but new developers > onboarding onto postgres who are trying to learn the dev environment. Right. >> If there's something to change, it's improving the actual recipe: > That we should do as well. You're not going to get far with "improving the recipe", because it's just not possible. To check this, I installed perlbrew on a Fedora 34 machine, and found that it actually can install a mostly-working 5.8.3 (nice!). But as I suspected earlier, it can't reproduce the old module configuration: $ cpanm install Test::More@0.87 --> Working on install Fetching http://www.cpan.org/authors/id/D/DA/DAGOLDEN/install-0.01.tar.gz ... OK ==> Found dependencies: ExtUtils::MakeMaker --> Working on ExtUtils::MakeMaker Fetching http://www.cpan.org/authors/id/B/BI/BINGOS/ExtUtils-MakeMaker-7.62.tar.gz ... OK Configuring ExtUtils-MakeMaker-7.62 ... OK Building and testing ExtUtils-MakeMaker-7.62 ... OK Successfully installed ExtUtils-MakeMaker-7.62 (upgraded from 6.17) Configuring install-0.01 ... OK Building and testing install-0.01 ... OK Successfully installed install-0.01 ! Finding Test::More (== 0.87) on cpanmetadb failed. Found Test::More 1.302188 which doesn't satisfy == 0.87. 2 distributions installed Not only is that a fail on Test::More itself, but as un-asked-for side effects, it upgraded ExtUtils::MakeMaker to current, and installed a module that should not have been there (which kinda defeats the point of the exercise). I did find I could install IPC::Run 0.79, which matches prairiedog's version of that module: $ cpanm install IPC::Run@0.79 install is up to date. (0.01) ! Finding IPC::Run (== 0.79) on cpanmetadb failed. --> Working on IPC::Run Fetching http://cpan.metacpan.org/authors/id/R/RS/RSOD/IPC-Run-0.79.tar.gz ... OK Configuring IPC-Run-0.79 ... OK Building and testing IPC-Run-0.79 ... OK Successfully installed IPC-Run-0.79 1 distribution installed However, this just reflects the fact that prairiedog's installation is itself a bit Frankensteinan. What it has for Test::More and IPC::Run are just the oldest versions I could find on CPAN back in 2017 when I built that installation. I can't claim that they have any historical relevance. They are, however, a lot more likely to still be duplicatable from current CPAN than actually-old versions. So while this recipe is a lot more useful than I thought, it can't entirely reproduce the Perl environment of older buildfarm members. I think we really ought to document that. I also think it is useful to explicitly state the policy and then give this recipe as one way to (partially) test against the policy. regards, tom lane
On Fri, Oct 08, 2021 at 12:03:41PM -0400, Tom Lane wrote: > Daniel Gustafsson <daniel@yesql.se> writes: > > On 8 Oct 2021, at 06:24, Noah Misch <noah@leadboat.com> wrote: > >> 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. > > Yeah, I don't really think that this recipe makes it plain that we have > a policy. It certainly fails to explain that you're allowed to use > additional modules if you're willing to skip the relevant tests. True, +1 for mentioning that tests can use less-available modules if they skip when those modules are absent. I'm only -0 for adding the other English (unlike the -1 for the original proposal of removing the shell commands). > >> If there's something to change, it's improving the actual recipe: > > > That we should do as well. > > You're not going to get far with "improving the recipe", because it's > just not possible. To check this, I installed perlbrew on a Fedora 34 Your test result is evidence that "cpanm install Test::More@0.87" is the wrong shell command, but it's quite a leap to "just not possible". Surely there exist other shell commands that install http://backpan.perl.org/modules/by-authors/id/M/MS/MSCHWERN/Test-Simple-0.87_03.tar.gz. (Perhaps none of us will care enough to identify them, but they exist.) By the way, I suspect 93fb39e introduced a regression in the recipe. (I haven't tested, though.) Before commit 93fb39e, "cpanm install IPC::Run" would update Test::More. As of 5.8.3, the core version of Test::More is new enough for IPC::Run but not new enough for PostgreSQL. I recommend adding "cpanm install Test::More" to restore the pre-93fb39e functionality.
Noah Misch <noah@leadboat.com> writes: > On Fri, Oct 08, 2021 at 12:03:41PM -0400, Tom Lane wrote: >> You're not going to get far with "improving the recipe", because it's >> just not possible. To check this, I installed perlbrew on a Fedora 34 > Your test result is evidence that "cpanm install Test::More@0.87" is the wrong > shell command, but it's quite a leap to "just not possible". Surely there > exist other shell commands that install > http://backpan.perl.org/modules/by-authors/id/M/MS/MSCHWERN/Test-Simple-0.87_03.tar.gz. > (Perhaps none of us will care enough to identify them, but they exist.) Oh, I never heard of backpan before. Now I'm tempted to see how far I can downgrade prairiedog before it breaks ;-). However, I agree that most people won't care about that, and probably shouldn't need to. > By the way, I suspect 93fb39e introduced a regression in the recipe. (I > haven't tested, though.) Before commit 93fb39e, "cpanm install IPC::Run" > would update Test::More. As of 5.8.3, the core version of Test::More is new > enough for IPC::Run but not new enough for PostgreSQL. I recommend adding > "cpanm install Test::More" to restore the pre-93fb39e functionality. Good point. So how about like the attached? regards, tom lane diff --git a/src/test/perl/README b/src/test/perl/README index bfc7cdcfa3..ab986f14bc 100644 --- a/src/test/perl/README +++ b/src/test/perl/README @@ -70,20 +70,33 @@ perldoc for the test modules, e.g.: perldoc src/test/perl/PostgresNode.pm -Required Perl -------------- +Portability +----------- + +Avoid using any bleeding-edge Perl features. We have buildfarm animals +running Perl versions as old as 5.8.3, so your tests will be expected +to pass on that. -Tests must run on perl 5.8.3 and newer. perlbrew is a good way to obtain such -a Perl; see http://perlbrew.pl . +Also, do not use any non-core Perl modules except IPC::Run. Or, if you +must do so for a particular test, arrange to skip the test when the needed +module isn't present. If unsure, you can consult Module::CoreList to find +out whether a given module is part of the Perl core, and which module +versions shipped with which Perl releases. -Just install and +One way to test for compatibility with old Perl versions is to use +perlbrew; see http://perlbrew.pl . After installing that, do perlbrew --force install 5.8.3 perlbrew use 5.8.3 perlbrew install-cpanm + cpanm install Test::More cpanm install IPC::Run -then re-run configure to ensure the correct Perl is used when running -tests. To verify that the right Perl was found: +Then re-run Postgres' configure to ensure the correct Perl is used when +running tests. To verify that the right Perl was found: grep ^PERL= config.log + +(Note: this recipe does not create a perfect reproduction of a +back-in-the-day Perl installation, because it will update Test::More +and IPC::Run to current versions. In most cases that won't matter.)
On Sat, Oct 09, 2021 at 03:44:17PM -0400, Tom Lane wrote: > > By the way, I suspect 93fb39e introduced a regression in the recipe. (I > > haven't tested, though.) Before commit 93fb39e, "cpanm install IPC::Run" > > would update Test::More. As of 5.8.3, the core version of Test::More is new > > enough for IPC::Run but not new enough for PostgreSQL. I recommend adding > > "cpanm install Test::More" to restore the pre-93fb39e functionality. > > Good point. So how about like the attached? Fine with me.
Hah ... your backpan link led me to realize the actual problem with Test::More. It got folded into Test::Simple at some point, and evidently cpanm isn't smart enough to handle a request for a back version in such cases. But this works: $ cpanm install Test::Simple@0.87_01 ... $ perl -MTest::More -e 'print $Test::More::VERSION, "\n";' 0.8701 So we oughta recommend that instead. Now I'm wondering what version of IPC::Run to recommend. regards, tom lane
On Sat, Oct 09, 2021 at 04:34:46PM -0400, Tom Lane wrote: > Hah ... your backpan link led me to realize the actual problem with > Test::More. It got folded into Test::Simple at some point, and > evidently cpanm isn't smart enough to handle a request for a back > version in such cases. But this works: > > $ cpanm install Test::Simple@0.87_01 > ... > $ perl -MTest::More -e 'print $Test::More::VERSION, "\n";' > 0.8701 > > So we oughta recommend that instead. Now I'm wondering what > version of IPC::Run to recommend. You mentioned prairiedog uses IPC::Run 0.79. That's from 2005. (Perl 5.8.3 is from 2004, and Test::More 0.87 is from 2009.) I'd just use 0.79 in the README recipe. IPC::Run is easy to upgrade, so if we find cause to rely on a newer version, I'd be fine updating that requirement.
Noah Misch <noah@leadboat.com> writes: > On Sat, Oct 09, 2021 at 04:34:46PM -0400, Tom Lane wrote: >> ... Now I'm wondering what >> version of IPC::Run to recommend. > You mentioned prairiedog uses IPC::Run 0.79. That's from 2005. (Perl 5.8.3 > is from 2004, and Test::More 0.87 is from 2009.) I'd just use 0.79 in the > README recipe. IPC::Run is easy to upgrade, so if we find cause to rely on a > newer version, I'd be fine updating that requirement. Yeah, since we know 0.79 works, there seems no reason to suggest a later version. The only reason to suggest an earlier version would be if some other buildfarm critter is using something older than 0.79. I'm tempted to propose adjusting configure to require IPC::Run >= 0.79, so that we can find out if that's true. If it isn't, that's still a good change to codify what our minimum expectation is. As you say, we can always move that goalpost in future if we find it necessary. However, back to the matter of the recipe. I'm feeling discouraged again because experimentation shows that cpanm insists on updating the ExtUtils suite to current while installing Test::Simple. You can then downgrade that, but it's not a complete fix, because there are some new ExtUtils modules that don't get uninstalled. There's also assorted CPAN infrastructure left behind. The closest I can get to what we want using cpanm is with this recipe: cpanm install Test::Simple@0.87_01 cpanm install IPC::Run@0.79 cpanm install ExtUtils::MakeMaker@6.50 # downgrade (Note: the actual prerequisite of this release of Test::Simple seems to be "> 6.30", but the first such version that actually passes its own tests for me is 6.50. FWIW, prairiedog currently has 6.59.) Attached is the diff of module manifests between a raw perl 5.8.3 installation and what this results in. Probably the added CPAN::Meta modules are mostly harmless, but the forced addition of JSON::PP seems annoying. AFAICT the only way to get to precisely the minimum configuration is to do the extra module installs by hand, without using cpan or cpanm. I'm probably going to go and re-set-up prairiedog that way, but it seems like a bit too much trouble to ask of most developers. Thoughts? regards, tom lane --- perlmodules.raw.583 2021-10-10 12:24:12.700060319 -0400 +++ perlmodules.makemaker650.583 2021-10-10 12:54:29.871522532 -0400 @@ -35,6 +35,16 @@ CGI::Util 1.4 CPAN 1.76_01 CPAN::FirstTime 1.60 +CPAN::Meta 2.143240 +CPAN::Meta::Converter 2.143240 +CPAN::Meta::Feature 2.143240 +CPAN::Meta::History 2.143240 +CPAN::Meta::Merge 2.143240 +CPAN::Meta::Prereqs 2.143240 +CPAN::Meta::Requirements 2.131 +CPAN::Meta::Spec 2.143240 +CPAN::Meta::Validator 2.143240 +CPAN::Meta::YAML 0.011 CPAN::Nox 1.03 Carp 1.01 Carp::Heavy 1.01 @@ -80,37 +90,47 @@ Errno 1.09_00 Exporter 5.57 Exporter::Heavy 5.57 -ExtUtils::Command 1.05 -ExtUtils::Command::MM 0.03 +ExtUtils::Command 7.62 +ExtUtils::Command::MM 6.50 ExtUtils::Constant 0.14 ExtUtils::Embed 1.250601 -ExtUtils::Install 1.32 -ExtUtils::Installed 0.08 -ExtUtils::Liblist 1.01 -ExtUtils::Liblist::Kid 1.3 -ExtUtils::MM 0.04 -ExtUtils::MM_Any 0.07 -ExtUtils::MM_BeOS 1.04 -ExtUtils::MM_Cygwin 1.06 -ExtUtils::MM_DOS 0.02 -ExtUtils::MM_MacOS 1.07 -ExtUtils::MM_NW5 2.06 -ExtUtils::MM_OS2 1.04 -ExtUtils::MM_UWIN 0.02 -ExtUtils::MM_Unix 1.42 -ExtUtils::MM_VMS 5.70 -ExtUtils::MM_Win32 1.09 -ExtUtils::MM_Win95 0.03 -ExtUtils::MY 0.01 -ExtUtils::MakeMaker 6.17 -ExtUtils::MakeMaker::bytes 0.01 -ExtUtils::MakeMaker::vmsish 0.01 -ExtUtils::Manifest 1.42 +ExtUtils::Install 2.06 +ExtUtils::Installed 2.06 +ExtUtils::Liblist 6.50 +ExtUtils::Liblist::Kid 6.5 +ExtUtils::MM 6.50 +ExtUtils::MM_AIX 6.50 +ExtUtils::MM_Any 6.50 +ExtUtils::MM_BeOS 6.50 +ExtUtils::MM_Cygwin 6.50 +ExtUtils::MM_DOS 6.5 +ExtUtils::MM_Darwin 6.50 +ExtUtils::MM_MacOS 6.5 +ExtUtils::MM_NW5 6.50 +ExtUtils::MM_OS2 6.50 +ExtUtils::MM_OS390 7.62 +ExtUtils::MM_QNX 6.50 +ExtUtils::MM_UWIN 6.5 +ExtUtils::MM_Unix 6.50 +ExtUtils::MM_VMS 6.50 +ExtUtils::MM_VOS 6.50 +ExtUtils::MM_Win32 6.50 +ExtUtils::MM_Win95 6.50 +ExtUtils::MY 6.5 +ExtUtils::MakeMaker 6.50 +ExtUtils::MakeMaker::Config 6.50 +ExtUtils::MakeMaker::Locale 7.62 +ExtUtils::MakeMaker::bytes 6.5 +ExtUtils::MakeMaker::version 7.62 +ExtUtils::MakeMaker::version::regex 7.62 +ExtUtils::MakeMaker::version::vpp 7.62 +ExtUtils::MakeMaker::vmsish 6.5 +ExtUtils::Manifest 1.70 ExtUtils::Miniperl undef -ExtUtils::Mkbootstrap 1.15 -ExtUtils::Mksymlists 1.19 -ExtUtils::Packlist 0.04 -ExtUtils::testlib 1.15 +ExtUtils::Mkbootstrap 6.50 +ExtUtils::Mksymlists 6.50 +ExtUtils::Packlist 2.06 +ExtUtils::testlib 6.5 Fatal 1.03 Fcntl 1.05 File::Basename 2.72 @@ -130,7 +150,7 @@ File::Spec::Unix 1.5 File::Spec::VMS 1.4 File::Spec::Win32 1.4 -File::Temp 0.14 +File::Temp 0.22 File::stat 1.00 FileCache 1.03 FileHandle 2.01 @@ -158,8 +178,17 @@ IPC::Msg 1.02 IPC::Open2 1.01 IPC::Open3 1.0105 +IPC::Run 0.79 +IPC::Run::Debug undef +IPC::Run::IO undef +IPC::Run::Timer undef +IPC::Run::Win32Helper undef +IPC::Run::Win32IO undef +IPC::Run::Win32Pump undef IPC::Semaphore 1.02 IPC::SysV 1.04 +JSON::PP 2.27203 +JSON::PP::Boolean undef List::Util 1.13 Locale::Constants 2.01 Locale::Country 2.61 @@ -211,6 +240,7 @@ O 1.00 Opcode 1.05 POSIX 1.07 +Parse::CPAN::Meta 1.4414 PerlIO 1.03 PerlIO::encoding 0.07 PerlIO::scalar 0.02 @@ -263,13 +293,17 @@ Term::Complete 1.401 Term::ReadLine 1.01 Test 1.24 -Test::Builder 0.17 +Test::Builder 0.87_01 +Test::Builder::IO::Scalar 2.110 +Test::Builder::Module 0.87_01 +Test::Builder::Tester 1.18 +Test::Builder::Tester::Color 1.18 Test::Harness 2.40 Test::Harness::Assert 0.02 Test::Harness::Iterator 0.02 Test::Harness::Straps 0.19 -Test::More 0.47 -Test::Simple 0.47 +Test::More 0.87_01 +Test::Simple 0.87_01 Text::Abbrev 1.01 Text::Balanced 1.95 Text::ParseWords 3.21 @@ -317,6 +351,7 @@ fields 2.03 filetest 1.01 if 0.03 +install 0.01 integer 1.00 less 0.01 lib 0.5565
I wrote: > The closest I can get to what we want using cpanm is with this recipe: > cpanm install Test::Simple@0.87_01 > cpanm install IPC::Run@0.79 > cpanm install ExtUtils::MakeMaker@6.50 # downgrade Upon trying to actually use the perlbrew installation, I discovered another oversight in the recipe: at least with old perl versions, you end up with a non-shared libperl, so that --with-perl fails. That leads me to the attached revision... regards, tom lane diff --git a/src/test/perl/README b/src/test/perl/README index bfc7cdcfa3..67a050b9d4 100644 --- a/src/test/perl/README +++ b/src/test/perl/README @@ -70,20 +70,36 @@ perldoc for the test modules, e.g.: perldoc src/test/perl/PostgresNode.pm -Required Perl -------------- +Portability +----------- + +Avoid using any bleeding-edge Perl features. We have buildfarm animals +running Perl versions as old as 5.8.3, so your tests will be expected +to pass on that. -Tests must run on perl 5.8.3 and newer. perlbrew is a good way to obtain such -a Perl; see http://perlbrew.pl . +Also, do not use any non-core Perl modules except IPC::Run. Or, if you +must do so for a particular test, arrange to skip the test when the needed +module isn't present. If unsure, you can consult Module::CoreList to find +out whether a given module is part of the Perl core, and which module +versions shipped with which Perl releases. -Just install and +One way to test for compatibility with old Perl versions is to use +perlbrew; see http://perlbrew.pl . After installing that, do + export PERLBREW_CONFIGURE_FLAGS='-de -Duseshrplib' perlbrew --force install 5.8.3 perlbrew use 5.8.3 perlbrew install-cpanm - cpanm install IPC::Run + cpanm install Test::Simple@0.87_01 + cpanm install IPC::Run@0.79 + cpanm install ExtUtils::MakeMaker@6.50 # downgrade -then re-run configure to ensure the correct Perl is used when running -tests. To verify that the right Perl was found: +Then re-run Postgres' configure to ensure the correct Perl is used when +running tests. To verify that the right Perl was found: grep ^PERL= config.log + +Due to limitations of cpanm, this recipe doesn't exactly duplicate the +module list of older buildfarm animals. The discrepancies should seldom +matter, but if you want to be sure, bypass cpanm and instead manually +install the desired versions of Test::Simple and IPC::Run.
On Sun, Oct 10, 2021 at 01:17:10PM -0400, Tom Lane wrote: > However, back to the matter of the recipe. I'm feeling discouraged > again because experimentation shows that cpanm insists on updating > the ExtUtils suite to current while installing Test::Simple. You > can then downgrade that, but it's not a complete fix, because there > are some new ExtUtils modules that don't get uninstalled. There's > also assorted CPAN infrastructure left behind. > > The closest I can get to what we want using cpanm is with this recipe: > > cpanm install Test::Simple@0.87_01 > cpanm install IPC::Run@0.79 > cpanm install ExtUtils::MakeMaker@6.50 # downgrade > > (Note: the actual prerequisite of this release of Test::Simple seems > to be "> 6.30", but the first such version that actually passes its > own tests for me is 6.50. FWIW, prairiedog currently has 6.59.) While the MakeMaker litter is annoying, I'm not too worried about it. The only other thing I'd consider is doing the MakeMaker 6.50 install before Test::Simple, not after. Then you don't pull in additional dependencies of post-6.50 MakeMaker, if any. On Sun, Oct 10, 2021 at 02:42:11PM -0400, Tom Lane wrote: > Upon trying to actually use the perlbrew installation, I discovered > another oversight in the recipe: at least with old perl versions, > you end up with a non-shared libperl, so that --with-perl fails. > > That leads me to the attached revision... Looks good. Thanks.
Noah Misch <noah@leadboat.com> writes: > On Sun, Oct 10, 2021 at 01:17:10PM -0400, Tom Lane wrote: >> The closest I can get to what we want using cpanm is with this recipe: >> cpanm install Test::Simple@0.87_01 >> cpanm install IPC::Run@0.79 >> cpanm install ExtUtils::MakeMaker@6.50 # downgrade > While the MakeMaker litter is annoying, I'm not too worried about it. The > only other thing I'd consider is doing the MakeMaker 6.50 install before > Test::Simple, not after. Tried that to begin with, doesn't work. There are at least two problems: 1. Before anything else, the first invocation of "cpanm install" wants to pull in "install". That seems to be a dummy module, but it's not without side-effects: it updates ExtUtils to current. If your first request is "cpanm install ExtUtils::MakeMaker@6.50", the version specification is effectively ignored. 2. I then tried doing a second "cpanm install ExtUtils::MakeMaker@6.50", and that did successfully downgrade to 6.50 ... but then the request to update Test::Simple upgraded it again. I'm not really sure why that happened. It looks more like a cpanm bug than anything Test::Simple asked for. I didn't do exhaustive experimentation to see if putting the downgrade before "install IPC::Run" would work. I think we're best off assuming that cpanm will cause that upgrade due to phase-of-the-moon conditions, so putting the downgrade last is the most robust recipe. regards, tom lane
On Sun, Oct 10, 2021 at 04:10:38PM -0400, Tom Lane wrote: > Noah Misch <noah@leadboat.com> writes: > > On Sun, Oct 10, 2021 at 01:17:10PM -0400, Tom Lane wrote: > >> The closest I can get to what we want using cpanm is with this recipe: > >> cpanm install Test::Simple@0.87_01 > >> cpanm install IPC::Run@0.79 > >> cpanm install ExtUtils::MakeMaker@6.50 # downgrade > > > While the MakeMaker litter is annoying, I'm not too worried about it. The > > only other thing I'd consider is doing the MakeMaker 6.50 install before > > Test::Simple, not after. > > Tried that to begin with, doesn't work. There are at least two problems: > > 1. Before anything else, the first invocation of "cpanm install" wants > to pull in "install". That seems to be a dummy module, but it's not > without side-effects: it updates ExtUtils to current. If your first > request is "cpanm install ExtUtils::MakeMaker@6.50", the version > specification is effectively ignored. > > 2. I then tried doing a second "cpanm install ExtUtils::MakeMaker@6.50", > and that did successfully downgrade to 6.50 ... but then the request > to update Test::Simple upgraded it again. I'm not really sure why > that happened. It looks more like a cpanm bug than anything Test::Simple > asked for. > > I didn't do exhaustive experimentation to see if putting the downgrade > before "install IPC::Run" would work. I think we're best off assuming > that cpanm will cause that upgrade due to phase-of-the-moon conditions, > so putting the downgrade last is the most robust recipe. Got it. Good enough!
On 10/9/21 10:25 PM, Noah Misch wrote: > On Sat, Oct 09, 2021 at 04:34:46PM -0400, Tom Lane wrote: >> Hah ... your backpan link led me to realize the actual problem with >> Test::More. It got folded into Test::Simple at some point, and >> evidently cpanm isn't smart enough to handle a request for a back >> version in such cases. But this works: >> >> $ cpanm install Test::Simple@0.87_01 >> ... >> $ perl -MTest::More -e 'print $Test::More::VERSION, "\n";' >> 0.8701 >> >> So we oughta recommend that instead. Now I'm wondering what >> version of IPC::Run to recommend. > You mentioned prairiedog uses IPC::Run 0.79. That's from 2005. (Perl 5.8.3 > is from 2004, and Test::More 0.87 is from 2009.) I'd just use 0.79 in the > README recipe. IPC::Run is easy to upgrade, so if we find cause to rely on a > newer version, I'd be fine updating that requirement. > > Why don't we specify the minimum versions required of these somewhere in the perl code? Perl is pretty good at this. e.g. use IPC::Run 0.79; use Test::More 0.87; It will choke if the supplied version is older. We could even put lines like this in a small script that configure could run. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes: > On 10/9/21 10:25 PM, Noah Misch wrote: >> You mentioned prairiedog uses IPC::Run 0.79. That's from 2005. (Perl 5.8.3 >> is from 2004, and Test::More 0.87 is from 2009.) I'd just use 0.79 in the >> README recipe. IPC::Run is easy to upgrade, so if we find cause to rely on a >> newer version, I'd be fine updating that requirement. > Why don't we specify the minimum versions required of these somewhere in > the perl code? Perl is pretty good at this. configure already checks Test::More's version. I proposed downthread that it should also check IPC::Run, but didn't pull that trigger yet. regards, tom lane
I wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> Why don't we specify the minimum versions required of these somewhere in >> the perl code? Perl is pretty good at this. > configure already checks Test::More's version. I proposed downthread > that it should also check IPC::Run, but didn't pull that trigger yet. Done now. I found an old note indicating that the reason I chose 0.79 for prairiedog back in 2017 is that 0.78 failed its self-test on that machine. 0.78 did pass when I tried it just now on a perlbrew-on-Fedora-34 rig, so I'm not sure what that was about ... but in any case, it discourages me from worrying any further about whether a lower minimum could be sane. regards, tom lane