Thread: configure's checks for --enable-tap-tests are insufficient

configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
So I just went through a rather annoying process trying to run
check-world on a Fedora 26 box where I hadn't done it before:

$ ./configure --enable-tap-tests
... fails, whining about IPC::Run
$ sudo yum install perl-IPC-Run
$ ./configure --enable-tap-tests
... fails, whining about prove
... figure out where Fedora hides prove
$ sudo yum install perl-Test-Harness
$ ./configure --enable-tap-tests
... works this time
... make, then
$ make -j check-world
... runs for awhile and then fails
... much excavation finds a complaint about Test::More not being installed
... figure out where Fedora hides Test::More
$ sudo yum install perl-Test-Simple
$ make -j check-world
... runs for awhile and then fails
... much excavation finds a complaint about Time::HiRes not being installed
$ sudo yum install perl-Time-HiRes
$ make -j check-world
... finally, it works!

Now, perhaps this is an indictment of Red Hat's choices about how to
package-ize the Perl world and which packages belong in a default
workstation installation.  But we haven't made it any easier.

I'm thinking that at minimum we ought to make configure --enable-tap-tests
check for Test::More and Time::HiRes as well as IPC::Run.  It might be
useful for somebody to repeat this exercise on a minimal Debian-oid
installation and see if there's a different set of pain points.

            regards, tom lane


Re: configure's checks for --enable-tap-tests are insufficient

From
Craig Ringer
Date:
On 19 March 2018 at 23:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:

$ make -j check-world
... runs for awhile and then fails
... much excavation finds a complaint about Time::HiRes not being installed
$ sudo yum install perl-Time-HiRes

 
That's absurd. Time::HiRes is a core module, as is Test::More. Sigh.

Yes, looks like tests are necessary. I'd argue it's "test for broken Perl" but apparently Fedora think they should split the Perl core dist into tiny pieces.

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Re: configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
Craig Ringer <craig@2ndquadrant.com> writes:
> On 19 March 2018 at 23:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> ... much excavation finds a complaint about Time::HiRes not being installed

> That's absurd. Time::HiRes is a core module, as is Test::More. Sigh.

> Yes, looks like tests are necessary. I'd argue it's "test for broken Perl"
> but apparently Fedora think they should split the Perl core dist into tiny
> pieces.

FWIW, the package boundaries are the same on RHEL6, and probably long
before that.  It's possible that Red Hat have trimmed back which packages
they consider part of a basic Perl installation.  I don't have any data
on whether that's moved over time, but it wouldn't surprise me given
the long-term shifts in Fedora's orientation.

            regards, tom lane


Re: configure's checks for --enable-tap-tests are insufficient

From
Chapman Flack
Date:
On 03/19/18 22:40, Tom Lane wrote:
> Craig Ringer <craig@2ndquadrant.com> writes:
>> Yes, looks like tests are necessary. I'd argue it's "test for broken Perl"
>> but apparently Fedora think they should split the Perl core dist into tiny
>> pieces.
> 
> FWIW, the package boundaries are the same on RHEL6, and probably long
> before that.  It's possible that Red Hat have trimmed back which packages
> they consider part of a basic Perl installation.

Ehh, I'm with Craig. The *perl project* identifies which packages (and
versions) are part of perl core in each perl version ... not Red Hat.

http://cpansearch.perl.org/src/BINGOS/Module-CoreList-5.20180220/lib/Module/CoreList.pm

Red Hat's practice seems super reckless given that there's an actual
module (itself part of core since 5.8.9) that lets you check what
modules you can safely assume are there. I'm sure I'm not the only one
to have written code relying on it....

-Chap


Re: configure's checks for --enable-tap-tests are insufficient

From
Michael Paquier
Date:
On Mon, Mar 19, 2018 at 10:54:59PM -0400, Chapman Flack wrote:
> Ehh, I'm with Craig. The *perl project* identifies which packages (and
> versions) are part of perl core in each perl version ... not Red Hat.
>
> http://cpansearch.perl.org/src/BINGOS/Module-CoreList-5.20180220/lib/Module/CoreList.pm
>
> Red Hat's practice seems super reckless given that there's an actual
> module (itself part of core since 5.8.9) that lets you check what
> modules you can safely assume are there. I'm sure I'm not the only one
> to have written code relying on it....

At the same time I don't think that there is much we can do in trying to
reduce the split in those distributions.  Even if it were to be changed
in the future, that won't affect existing deployments, past OS base
images and maintenance releases.  So in order to simplify the life of
people running the regression test suites, I would be on board with Tom
in adding more checks at configure level as it simplifies the life of
anybody running the TAP tests on those distributions.
--
Michael

Attachment

Re: configure's checks for --enable-tap-tests are insufficient

From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> Craig Ringer <craig@2ndquadrant.com> writes:
>> On 19 March 2018 at 23:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> ... much excavation finds a complaint about Time::HiRes not being installed
>
>> That's absurd. Time::HiRes is a core module, as is Test::More. Sigh.
>
>> Yes, looks like tests are necessary. I'd argue it's "test for broken Perl"
>> but apparently Fedora think they should split the Perl core dist into tiny
>> pieces.

Note that Fedora aren't the only ones to do this: Debian also splits the
Perl core dist into multiple packages.  The crucial difference is that
on Debian installing the 'perl' package gives you the whole lot, while
on Fedora that's just a subset (similar to Debian's 'perl-base'
package); you have to install 'perl-core' to get everything.

> FWIW, the package boundaries are the same on RHEL6, and probably long
> before that.  It's possible that Red Hat have trimmed back which packages
> they consider part of a basic Perl installation.  I don't have any data
> on whether that's moved over time, but it wouldn't surprise me given
> the long-term shifts in Fedora's orientation.

In Fedora 27 (and thus RHEL 8, whenever that will be) they've finally
rearranged the packages so that installing 'perl' gives you the full
Perl core dist, while the minimal subset has been renamed to
'perl-interpreter'.

https://fedoraproject.org/wiki/Changes/perl_Package_to_Install_Core_Modules

- ilmari
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen


Re: configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Note that Fedora aren't the only ones to do this: Debian also splits the
> Perl core dist into multiple packages.  The crucial difference is that
> on Debian installing the 'perl' package gives you the whole lot, while
> on Fedora that's just a subset (similar to Debian's 'perl-base'
> package); you have to install 'perl-core' to get everything.

Ah-hah.  So it seems to me that it'd be a useful exercise to start
from perl-base and find out what has to be added to get to working
TAP tests.  Anyone?

            regards, tom lane


Re: configure's checks for --enable-tap-tests are insufficient

From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>> Note that Fedora aren't the only ones to do this: Debian also splits the
>> Perl core dist into multiple packages.  The crucial difference is that
>> on Debian installing the 'perl' package gives you the whole lot, while
>> on Fedora that's just a subset (similar to Debian's 'perl-base'
>> package); you have to install 'perl-core' to get everything.
>
> Ah-hah.  So it seems to me that it'd be a useful exercise to start
> from perl-base and find out what has to be added to get to working
> TAP tests.  Anyone?

I disagree.  The set of modules in perl-base is not guaranteed to stay
the same across Debian releases: it's just whatever the Debian base
system needs.  Similarly, the split of modules between the perl and
perl-core (and even indvidual perl-Foo-Bar) packages has varied between
Fedora/RHEL releases.

I think the other way would be better simpler: we require a complete
Perl core installation.  This means that on Fedora < 27 and RHEL < 8 you
need to install perl-core, not perl.

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law


Re: configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> Ah-hah.  So it seems to me that it'd be a useful exercise to start
>> from perl-base and find out what has to be added to get to working
>> TAP tests.  Anyone?

> I think the other way would be better simpler: we require a complete
> Perl core installation.

And you propose to test for that in configure how?

            regards, tom lane


Re: configure's checks for --enable-tap-tests are insufficient

From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>> Tom Lane <tgl@sss.pgh.pa.us> writes:
>>> Ah-hah.  So it seems to me that it'd be a useful exercise to start
>>> from perl-base and find out what has to be added to get to working
>>> TAP tests.  Anyone?
>
>> I think the other way would be better simpler: we require a complete
>> Perl core installation.
>
> And you propose to test for that in configure how?

Good point, I'd overlooked that aspect.  In that case we can't rely on
what Debian or others happen to include in their minimal packages, we
have to explicitly check for _every_ module we use anywhere in the test
suite, core or not (except perhaps pragmas like 'strict' and 'warnings').

Grepping the tests (**/t/ src/test/perl/) for 'use' and 'require'
statements and excluding 'strict', 'warnings' and ones defined locally
gives the following list:

Carp
Config
Cwd
Exporter
File::Basename
File::Copy
File::Find
File::Path
File::Spec
File::Temp
IPC::Run
Scalar::Util
Socket
Test::More 0.87
Time::HiRes

I suggest checking for all of those under --enable-tap-tests.

Additionally, plperl uses some modules which we should check for under
--with-perl:

Carp
Getopt::Long
Opcode

Please find attaced separate patches for both of the above.

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law

From 42abdb8cfadda4b50216a9e5130c4ae56a075de5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 20 Mar 2018 16:27:21 +0000
Subject: [PATCH 1/2] Check for all modules used by the TAP tests

Several distributions, notably Fedora-based ones, provide a 'perl'
package which doesn't actually ship all Perl core modules.  Guard
against this by checking for all modules we use, including core
ones (but not pragmata like 'strict' and 'warnings') in configure.
---
 configure    | 4 ++--
 configure.in | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 3943711283..ba736aa262 100755
--- a/configure
+++ b/configure
@@ -16566,7 +16566,7 @@ fi
 
 if test "x$PERL" != x; then
   ax_perl_modules_failed=0
-  for ax_perl_module in 'IPC::Run' ; do
+  for ax_perl_module in 'Carp' 'Config' 'Cwd' 'Exporter' 'File::Basename' 'File::Copy' 'File::Find' 'File::Path'
'File::Spec''File::Temp' 'IPC::Run' 'Scalar::Util' 'Socket' 'Test::More 0.87' 'Time::HiRes' ; do
 
     { $as_echo "$as_me:${as_lineno-$LINENO}: checking for perl module $ax_perl_module" >&5
 $as_echo_n "checking for perl module $ax_perl_module... " >&6; }
 
@@ -16588,7 +16588,7 @@ $as_echo "ok" >&6; };
 
   else
     :
-    as_fn_error $? "Perl module IPC::Run is required to run TAP tests" "$LINENO" 5
+    as_fn_error $? "Perl modules required to run TAP tests not found" "$LINENO" 5
   fi
 else
   { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: could not find perl" >&5
diff --git a/configure.in b/configure.in
index 1babdbb755..1152b7f536 100644
--- a/configure.in
+++ b/configure.in
@@ -2114,8 +2114,10 @@ if test "$enable_tap_tests" = yes; then
   # (prove might be part of a different Perl installation than perl, eg on
   # MSys, so the result of AX_PROG_PERL_MODULES could be irrelevant anyway.)
   if test -z "$PROVE"; then
-    AX_PROG_PERL_MODULES(IPC::Run, ,
-      AC_MSG_ERROR([Perl module IPC::Run is required to run TAP tests]))
+    AX_PROG_PERL_MODULES(Carp Config Cwd Exporter File::Basename File::Copy
+                         File::Find File::Path File::Spec File::Temp IPC::Run
+                         Scalar::Util Socket Test::More=0.87 Time::HiRes, ,
+      AC_MSG_ERROR([Perl modules required to run TAP tests not found]))
   fi
   # Now make sure we know where prove is
   PGAC_PATH_PROGS(PROVE, prove)
-- 
2.16.2

From eb6550a6c5813c4334bab86dbe3d838dc3b18b5e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Tue, 20 Mar 2018 16:40:53 +0000
Subject: [PATCH 2/2] Check for all modules needed by PL/Perl

Several distributions, notably Fedora-based ones, provide a 'perl'
package which doesn't actually ship all Perl core modules.  Guard
against this by checking for all modules we use, including core
ones (but not pragmata like 'strict' and 'warnings') in configure.
---
 configure    | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 configure.in |  2 ++
 2 files changed, 73 insertions(+)

diff --git a/configure b/configure
index ba736aa262..eddbdb8849 100755
--- a/configure
+++ b/configure
@@ -7849,6 +7849,77 @@ else
 $as_echo "$perl_embed_ldflags" >&6; }
 fi
 
+# Make sure we have perl
+if test -z "$PERL"; then
+# Extract the first word of "perl", so it can be a program name with args.
+set dummy perl; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_PERL+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test -n "$PERL"; then
+  ac_cv_prog_PERL="$PERL" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_PERL="perl"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+fi
+fi
+PERL=$ac_cv_prog_PERL
+if test -n "$PERL"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $PERL" >&5
+$as_echo "$PERL" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+fi
+
+if test "x$PERL" != x; then
+  ax_perl_modules_failed=0
+  for ax_perl_module in 'Carp' 'Getopt::Long' 'Opcode' ; do
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for perl module $ax_perl_module" >&5
+$as_echo_n "checking for perl module $ax_perl_module... " >&6; }
+
+    # Would be nice to log result here, but can't rely on autoconf internals
+    $PERL -e "use $ax_perl_module; exit" > /dev/null 2>&1
+    if test $? -ne 0; then
+      { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; };
+      ax_perl_modules_failed=1
+   else
+      { $as_echo "$as_me:${as_lineno-$LINENO}: result: ok" >&5
+$as_echo "ok" >&6; };
+    fi
+  done
+
+  # Run optional shell commands
+  if test "$ax_perl_modules_failed" = 0; then
+    :
+
+  else
+    :
+    as_fn_error $? "Perl needed to build PL/Perl not found" "$LINENO" 5
+  fi
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: could not find perl" >&5
+$as_echo "$as_me: WARNING: could not find perl" >&2;}
+fi
 fi
 
 if test "$with_python" = yes; then
diff --git a/configure.in b/configure.in
index 1152b7f536..3cbaa71481 100644
--- a/configure.in
+++ b/configure.in
@@ -912,6 +912,8 @@ PL/Perl.])
   fi
   PGAC_CHECK_PERL_EMBED_CCFLAGS
   PGAC_CHECK_PERL_EMBED_LDFLAGS
+  AX_PROG_PERL_MODULES(Carp Getopt::Long Opcode, ,
+      AC_MSG_ERROR([Perl needed to build PL/Perl not found]))
 fi
 
 if test "$with_python" = yes; then
-- 
2.16.2


Re: configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>>> I think the other way would be better simpler: we require a complete
>>> Perl core installation.

>> And you propose to test for that in configure how?

> Good point, I'd overlooked that aspect.  In that case we can't rely on
> what Debian or others happen to include in their minimal packages, we
> have to explicitly check for _every_ module we use anywhere in the test
> suite, core or not (except perhaps pragmas like 'strict' and 'warnings').

That seems like expensive overkill to me, especially since it seems
unlikely that we'd keep the list up-to-date over time.  I just want to
test for modules that we know are likely to be omitted on popular
platforms.  I've proposed testing two that would improve the user
experience on Red Hat; what's the equivalent minimum set for Debian?

I think the documentation can reasonably say that we expect perl-core
(plus IPC::Run, if that's not core).  But configure's task is just to
make a reasonable attempt to notice if you're missing something.  If
we have to tweak it again in the future, that's fine with me.

            regards, tom lane


Re: configure's checks for --enable-tap-tests are insufficient

From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Tom Lane <tgl@sss.pgh.pa.us> writes:

> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>> Tom Lane <tgl@sss.pgh.pa.us> writes:
>>> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
>>>> I think the other way would be better simpler: we require a complete
>>>> Perl core installation.
>
>>> And you propose to test for that in configure how?
>
>> Good point, I'd overlooked that aspect.  In that case we can't rely on
>> what Debian or others happen to include in their minimal packages, we
>> have to explicitly check for _every_ module we use anywhere in the test
>> suite, core or not (except perhaps pragmas like 'strict' and 'warnings').
>
> That seems like expensive overkill to me, especially since it seems
> unlikely that we'd keep the list up-to-date over time. 

The patches uptread add about 0.2s to a 6.9s configure run here, I don't
consider that very expensive.  We don't add new modules very often, so I
don't think it's much of a maintenance overhead either.

> I just want to test for modules that we know are likely to be omitted
> on popular platforms.  I've proposed testing two that would improve
> the user experience on Red Hat; what's the equivalent minimum set for
> Debian?

On Debian (I've checked the previous and current stable releases, as
well as unstable) the only package required by the TAP tests not in
perl-base is Time::HiRes.  Of the modules required by PL/Perl, Opcode is
not in perl-base either.

BTW, should include the version in the Test::More check, since we only
requie Perl 5.8, but that only shipped Test::More 0.47, while we require
0.97.

- ilmari
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen


Re: configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> I just want to test for modules that we know are likely to be omitted
>> on popular platforms.  I've proposed testing two that would improve
>> the user experience on Red Hat; what's the equivalent minimum set for
>> Debian?

> On Debian (I've checked the previous and current stable releases, as
> well as unstable) the only package required by the TAP tests not in
> perl-base is Time::HiRes.  Of the modules required by PL/Perl, Opcode is
> not in perl-base either.

Ah, thanks.  Not sure we need to make an explicit test for Opcode; we've
not heard of anyone not having that.

> BTW, should include the version in the Test::More check, since we only
> requie Perl 5.8, but that only shipped Test::More 0.47, while we require
> 0.97.

Good point, though it looks to me like we're requiring 0.87 not 0.97?

Anyway, I propose the attached.  It produces output like

checking for perl module IPC::Run... ok
checking for perl module Test::More 0.87... no
checking for perl module Time::HiRes... ok
configure: error: Additional Perl modules are required to run TAP tests

which seems a better approach to me than making the user find out one
at a time which modules are needed.

            regards, tom lane

diff --git a/configure b/configure
index 3943711..1be8a1a 100755
*** a/configure
--- b/configure
*************** if test "$enable_tap_tests" = yes; then
*** 16517,16522 ****
--- 16517,16528 ----
    # (prove might be part of a different Perl installation than perl, eg on
    # MSys, so the result of AX_PROG_PERL_MODULES could be irrelevant anyway.)
    if test -z "$PROVE"; then
+     # Test::More and Time::HiRes are supposed to be part of core Perl,
+     # but some distros omit them in a minimal installation.
+
+
+
+



*************** fi
*** 16566,16572 ****

  if test "x$PERL" != x; then
    ax_perl_modules_failed=0
!   for ax_perl_module in 'IPC::Run' ; do
      { $as_echo "$as_me:${as_lineno-$LINENO}: checking for perl module $ax_perl_module" >&5
  $as_echo_n "checking for perl module $ax_perl_module... " >&6; }

--- 16572,16578 ----

  if test "x$PERL" != x; then
    ax_perl_modules_failed=0
!   for ax_perl_module in 'IPC::Run' 'Test::More 0.87' 'Time::HiRes' ; do
      { $as_echo "$as_me:${as_lineno-$LINENO}: checking for perl module $ax_perl_module" >&5
  $as_echo_n "checking for perl module $ax_perl_module... " >&6; }

*************** $as_echo "ok" >&6; };
*** 16588,16594 ****

    else
      :
!     as_fn_error $? "Perl module IPC::Run is required to run TAP tests" "$LINENO" 5
    fi
  else
    { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: could not find perl" >&5
--- 16594,16600 ----

    else
      :
!     as_fn_error $? "Additional Perl modules are required to run TAP tests" "$LINENO" 5
    fi
  else
    { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: could not find perl" >&5
diff --git a/configure.in b/configure.in
index 1babdbb..9a6f4b1 100644
*** a/configure.in
--- b/configure.in
*************** if test "$enable_tap_tests" = yes; then
*** 2114,2121 ****
    # (prove might be part of a different Perl installation than perl, eg on
    # MSys, so the result of AX_PROG_PERL_MODULES could be irrelevant anyway.)
    if test -z "$PROVE"; then
!     AX_PROG_PERL_MODULES(IPC::Run, ,
!       AC_MSG_ERROR([Perl module IPC::Run is required to run TAP tests]))
    fi
    # Now make sure we know where prove is
    PGAC_PATH_PROGS(PROVE, prove)
--- 2114,2123 ----
    # (prove might be part of a different Perl installation than perl, eg on
    # MSys, so the result of AX_PROG_PERL_MODULES could be irrelevant anyway.)
    if test -z "$PROVE"; then
!     # Test::More and Time::HiRes are supposed to be part of core Perl,
!     # but some distros omit them in a minimal installation.
!     AX_PROG_PERL_MODULES([IPC::Run Test::More=0.87 Time::HiRes], ,
!       [AC_MSG_ERROR([Additional Perl modules are required to run TAP tests])])
    fi
    # Now make sure we know where prove is
    PGAC_PATH_PROGS(PROVE, prove)

Re: configure's checks for --enable-tap-tests are insufficient

From
Alvaro Herrera
Date:
Tom Lane wrote:
> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> > Tom Lane <tgl@sss.pgh.pa.us> writes:
> >> I just want to test for modules that we know are likely to be omitted
> >> on popular platforms.  I've proposed testing two that would improve
> >> the user experience on Red Hat; what's the equivalent minimum set for
> >> Debian?
> 
> > On Debian (I've checked the previous and current stable releases, as
> > well as unstable) the only package required by the TAP tests not in
> > perl-base is Time::HiRes.  Of the modules required by PL/Perl, Opcode is
> > not in perl-base either.
> 
> Ah, thanks.  Not sure we need to make an explicit test for Opcode; we've
> not heard of anyone not having that.

In Debian stable, Opcode is in package libperl5.24.  This probably
explains why nobody runs into trouble with it, since libperl is already
checked by configure for other reasons.  Not sure about the RH world.

> Anyway, I propose the attached.  It produces output like
> 
> checking for perl module IPC::Run... ok
> checking for perl module Test::More 0.87... no
> checking for perl module Time::HiRes... ok
> configure: error: Additional Perl modules are required to run TAP tests

This seems good to me.

Time::HiRes is also in libperl5.24, so as far Debian goes, that one
would be unnecessary; but IPC::Run and Test::More are in separate
packages (libipc-run-perl and libtest-base-perl, respectively.)

As for the others Dagfinn mentioned, most seem to be in perl-base, and a
few in perl-modules-5.24, which is depended upon by libperl5.24, so I
think the proposed set is good enough for now.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Re: configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> Tom Lane wrote:
>> Ah, thanks.  Not sure we need to make an explicit test for Opcode; we've
>> not heard of anyone not having that.

> In Debian stable, Opcode is in package libperl5.24.  This probably
> explains why nobody runs into trouble with it, since libperl is already
> checked by configure for other reasons.  Not sure about the RH world.

[ pokes around ]  Opcode is in the base perl package on RHEL6;
on Fedora 28, it's in perl-interpreter.  Either way, it's in the same
package as /usr/bin/perl, so ain't nobody not gonna have it.

> As for the others Dagfinn mentioned, most seem to be in perl-base, and a
> few in perl-modules-5.24, which is depended upon by libperl5.24, so I
> think the proposed set is good enough for now.

Sounds good.  We can always tweak the list some more if the situation
changes, or if we hear from someone using a different distro that slices
things up yet differently.  But I don't feel a need to expend configure
cycles on purely hypothetical configurations.  I imagine that most
off-the-beaten-track cases would involve hand-built Perl installations,
and those will almost certainly have all of "Perl core".  So I think
as long as we cover what the major Linux distros are doing, that's
probably good enough.

            regards, tom lane


Re: configure's checks for --enable-tap-tests are insufficient

From
ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

> Tom Lane wrote:
>
>> Ah, thanks.  Not sure we need to make an explicit test for Opcode; we've
>> not heard of anyone not having that.
>
> In Debian stable, Opcode is in package libperl5.24.  This probably
> explains why nobody runs into trouble with it, since libperl is already
> checked by configure for other reasons.  Not sure about the RH world.

Yes, libperl5.xx is pulled in by libperl-dev, which is needed for
PL/Perl anyway.

>> Anyway, I propose the attached.  It produces output like
>> 
>> checking for perl module IPC::Run... ok
>> checking for perl module Test::More 0.87... no
>> checking for perl module Time::HiRes... ok
>> configure: error: Additional Perl modules are required to run TAP tests
>
> This seems good to me.
>
> Time::HiRes is also in libperl5.24, so as far Debian goes, that one
> would be unnecessary;

libperl5.xx is not pulled in if you only have perl-base, you need to
install 'perl' to get that. But Debian isn't really the problem, because
when we say we require perl, people do 'apt install perl' which gives
them the whole shebang.

> but IPC::Run and Test::More are in separate packages (libipc-run-perl
> and libtest-base-perl, respectively.)

Test::More is in perl-modules-5.xx, which is pulled in by 'perl'.

> As for the others Dagfinn mentioned, most seem to be in perl-base, and a
> few in perl-modules-5.24, which is depended upon by libperl5.24, so I
> think the proposed set is good enough for now.

I agree.

- ilmari
-- 
"A disappointingly low fraction of the human race is,
 at any given time, on fire." - Stig Sandbeck Mathisen


Re: configure's checks for --enable-tap-tests are insufficient

From
Andres Freund
Date:
On 2018-03-20 17:23:26 +0000, Dagfinn Ilmari Mannsåker wrote:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
> > That seems like expensive overkill to me, especially since it seems
> > unlikely that we'd keep the list up-to-date over time. 
> 
> The patches uptread add about 0.2s to a 6.9s configure run here, I don't
> consider that very expensive.  We don't add new modules very often, so I
> don't think it's much of a maintenance overhead either.

Not huge, but also not nothing. But I think if we care we could make
this cheaper fairly easily? I don't think there's any need to invoke
perl one-by-one for each module. Should be trivial to have perl report
back all the modules it's missing from a list of arguments.

It's also certainly annoying that AX_PROG_PERL_MODULES doesn't support
caching. I'm not sure we're benefiting much from using the AX* script
here.

Greetings,

Andres Freund


Re: configure's checks for --enable-tap-tests are insufficient

From
Tom Lane
Date:
Andres Freund <andres@anarazel.de> writes:
> It's also certainly annoying that AX_PROG_PERL_MODULES doesn't support
> caching.

Yeah, that was the main reason I didn't want to add wholesale module
probing.  Improving that script is no doubt possible, but it's beyond
what I care to do about the issue.

            regards, tom lane


Re: configure's checks for --enable-tap-tests are insufficient

From
Christoph Berg
Date:
Re: Alvaro Herrera 2018-03-20 <20180320185038.r3hjvfio4elnaqku@alvherre.pgsql>
> Time::HiRes is also in libperl5.24, so as far Debian goes, that one
> would be unnecessary; but IPC::Run and Test::More are in separate
> packages (libipc-run-perl and libtest-base-perl, respectively.)

The Debian package build-depends on these:

 libipc-run-perl,
 libperl-dev,
 perl (>= 5.8),

Test::More is provided by several packages:

perl-modules-5.24: /usr/share/perl/5.24.1/Test/More.pm
perl-modules-5.26: /usr/share/perl/5.26.1/Test/More.pm
libtest-simple-perl: /usr/share/perl5/Test/More.pm

... so depending on "perl" is enough, which pulls in
perl-modules-$current.

> As for the others Dagfinn mentioned, most seem to be in perl-base, and a
> few in perl-modules-5.24, which is depended upon by libperl5.24, so I
> think the proposed set is good enough for now.

Christoph


Re: configure's checks for --enable-tap-tests are insufficient

From
Alvaro Herrera
Date:
Christoph Berg wrote:
> Re: Alvaro Herrera 2018-03-20 <20180320185038.r3hjvfio4elnaqku@alvherre.pgsql>
> > Time::HiRes is also in libperl5.24, so as far Debian goes, that one
> > would be unnecessary; but IPC::Run and Test::More are in separate
> > packages (libipc-run-perl and libtest-base-perl, respectively.)
> 
> The Debian package build-depends on these:
> 
>  libipc-run-perl,
>  libperl-dev,
>  perl (>= 5.8),
> 
> Test::More is provided by several packages:
> 
> perl-modules-5.24: /usr/share/perl/5.24.1/Test/More.pm
> perl-modules-5.26: /usr/share/perl/5.26.1/Test/More.pm
> libtest-simple-perl: /usr/share/perl5/Test/More.pm
> 
> ... so depending on "perl" is enough, which pulls in
> perl-modules-$current.

Actually, the whole point of this exercise is to ensure that configure
complains properly if somebody installs perl-base but not perl, that is,
what happens if perl-modules-5.24 is not there.

I think you need libperl5.xx in order to use --with-perl; and if you
install that one, you're going to get perl-modules-5.xx as a side
effect, so that scenario is covered.  I don't have a minimal Debian
installation to try
  configure --without-perl --enable-tap-tests

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services