Thread: Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
[ 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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Daniel Gustafsson
Date:
> 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/




Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Alvaro Herrera
Date:
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)



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Alvaro Herrera
Date:
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/



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Alvaro Herrera
Date:
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)



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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.

Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Daniel Gustafsson
Date:
> 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/




Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Alvaro Herrera
Date:
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)



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Noah Misch
Date:
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.



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Noah Misch
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Daniel Gustafsson
Date:
> 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/




Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Noah Misch
Date:
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.



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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.)

Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Noah Misch
Date:
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.



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Noah Misch
Date:
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.



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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

Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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.

Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Noah Misch
Date:
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.



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Noah Misch
Date:
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!



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Andrew Dunstan
Date:
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




Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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



Re: pgsql: Adjust configure to insist on Perl version >= 5.8.3.

From
Tom Lane
Date:
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