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

On 7 Oct 2021, at 20:26, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Previously it only checked for version >= 5.8.0, although the
documentation has said that the minimum version is 5.8.3 since
commit dea6ba939.

src/test/perl/README still claims "5.8.0 and newer", not sure how important
that is to fix but it seems a bit inconsistent now.

--
Daniel Gustafsson https://vmware.com/

Daniel Gustafsson <daniel@yesql.se> writes:

On 7 Oct 2021, at 20:26, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Previously it only checked for version >= 5.8.0, although the
documentation has said that the minimum version is 5.8.3 since
commit dea6ba939.

src/test/perl/README still claims "5.8.0 and newer", not sure how important
that is to fix but it seems a bit inconsistent now.

Ah, done. I grepped for other possible references to 5.8.x, and
found

src/tools/msvc/gendef.pl:use 5.8.0;

but I don't think we need to change that.

regards, tom lane

I wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

src/test/perl/README still claims "5.8.0 and newer", not sure how important
that is to fix but it seems a bit inconsistent now.

Ah, done.

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.

(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.

So maybe this advice is helpful, but I'm not very convinced.

regards, tom lane

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.

--
Daniel Gustafsson https://vmware.com/

[ 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

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