Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Robert Haas]

On Tue, Feb 16, 2016 at 11:48 AM, Victor Wagner  wrote:
> On Tue, 16 Feb 2016 12:23:56 -0300
> Alvaro Herrera  wrote:
>
>> ... but I agree with the point upthread that this should wait to see
>> what happens with the CMake stuff, since this is not a newly
>> introduced problem.
>
> I doubt, that CMake stuff would be ready for 9.6. There is just one
> commitfest left, and it would be quite a radical change.
>
> And even if would appear in the next release, it cannot be easily
> backpatched to 9.5. So we'll probably live with autoconf until the
> end-of-life of 9.5 series at least.

Yeah, I'm rather doubtful about anything happening with cmake any time soon.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Victor Wagner]

On Tue, 16 Feb 2016 12:23:56 -0300
Alvaro Herrera  wrote:

> ... but I agree with the point upthread that this should wait to see
> what happens with the CMake stuff, since this is not a newly
> introduced problem.

I doubt, that CMake stuff would be ready for 9.6. There is just one
commitfest left, and it would be quite a radical change.

And even if would appear in the next release, it cannot be easily
backpatched to 9.5. So we'll probably live with autoconf until the
end-of-life of 9.5 series at least.  



-- 
   Victor Wagner 


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Alvaro Herrera]

Victor Wagner wrote:

> Not everyone have "standard perl installation" nowadays. Most Linux
> users, for example, use Perl package from the distrubution, and
> distributions love to strip down standard perl installation putting its
> parts into separate packages, some of which might be optional.
> 
> For example, in Centos 6 it is part of perl-Test-Simple package. which
> might be not included into docker images or simular minimal systems for
> container-based deployment.

That's a good point ...

> So, it worth few lines of the configure.in to remind user that "Your
> perl installation is not standard enough".

... but I agree with the point upthread that this should wait to see
what happens with the CMake stuff, since this is not a newly introduced
problem.

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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Victor Wagner]

On Mon, 15 Feb 2016 08:34:11 -0500
Peter Eisentraut  wrote:

> On 2/12/16 8:20 AM, Eugene Kazakov wrote:
> > TAP-tests need two Perl modules: Test::More and IPC::Run.
> > 
> > The patch adds checking of modules in configure.in and configure.  
> 
> I think those modules are part of a standard Perl installation.

Not everyone have "standard perl installation" nowadays. Most Linux
users, for example, use Perl package from the distrubution, and
distributions love to strip down standard perl installation putting its
parts into separate packages, some of which might be optional.

For example, in Centos 6 it is part of perl-Test-Simple package. which
might be not included into docker images or simular minimal systems for
container-based deployment.

So, it worth few lines of the configure.in to remind user that "Your
perl installation is not standard enough".
-- 
 



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Peter Eisentraut]

On 2/15/16 8:57 AM, Teodor Sigaev wrote:
>> On 2/12/16 8:20 AM, Eugene Kazakov wrote:
>>> TAP-tests need two Perl modules: Test::More and IPC::Run.
>> I think those modules are part of a standard Perl installation.
> 
> IPC::Run is not. At least for perl from ports tree in FreeBSD.

Right, that's why we added the configure option.  But Test::More is
standard.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Tom Lane]

Teodor Sigaev  writes:
>> On 2/12/16 8:20 AM, Eugene Kazakov wrote:
>>> TAP-tests need two Perl modules: Test::More and IPC::Run.

>> I think those modules are part of a standard Perl installation.

> IPC::Run is not. At least for perl from ports tree in FreeBSD.

Yeah, I remember having had to install IPC::Run from CPAN, and a couple
of other things too (but I don't remember Test::More specifically), when
setting up some of my buildfarm critters.  It's likely that a lot of
distros bundle these, but I don't think IPC::Run is in a basic
built-from-source Perl.

The real question though is do we need a configure check at all.

Given that investigation into a CMake conversion is actively going
on, I'm hesitant to move the goalposts for it by introducing a brand
new type of configure check.  Maybe we should punt this issue until
that patch is either accepted or decisively rejected.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Michael Paquier]

On Mon, Feb 15, 2016 at 10:57 PM, Teodor Sigaev  wrote:
>> On 2/12/16 8:20 AM, Eugene Kazakov wrote:
>>>
>>> TAP-tests need two Perl modules: Test::More and IPC::Run.
>>
>> I think those modules are part of a standard Perl installation.
>
> IPC::Run is not. At least for perl from ports tree in FreeBSD.

On OSX and on Windows as well they are now shipped AFAIK.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Teodor Sigaev]

On 2/12/16 8:20 AM, Eugene Kazakov wrote:

TAP-tests need two Perl modules: Test::More and IPC::Run.

I think those modules are part of a standard Perl installation.


IPC::Run is not. At least for perl from ports tree in FreeBSD.


--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Peter Eisentraut]

On 2/12/16 8:20 AM, Eugene Kazakov wrote:
> TAP-tests need two Perl modules: Test::More and IPC::Run.
> 
> The patch adds checking of modules in configure.in and configure.

I think those modules are part of a standard Perl installation.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Eugene Kazakov]

13.02.2016 16:04, Michael Paquier :

On Sat, Feb 13, 2016 at 1:47 PM, Robert Haas  wrote:

On Fri, Feb 12, 2016 at 8:20 AM, Eugene Kazakov
 wrote:

TAP-tests need two Perl modules: Test::More and IPC::Run.

The patch adds checking of modules in configure.in and configure.

Why would we want that?

I was doubtful at the beginning, but it doesn't hurt to have those
sanity checks in configure actually. The idea is that when
--enable-tap-tests is used now we simply error with "Can't locate
IPC/Run.pm in @INC" when kicking the tests, this check would allow one
to know if his environment is adapted to run the tests or not before
compiling anything.

And with this patch, we would fail now with that:
configure: error: Need Perl IPC::Run module

By the way, the patch given upthread by Eugene is incorrect. To begin
with, AX_PROG_PERL_MODULES has not been compiled by autoconf and I
can't believe that it is available on all platforms, for example on
OSX 10.8 I could not see it. And it is actually here:
https://www.gnu.org/software/autoconf-archive/ax_prog_perl_modules.html

I would recommend grabbing a copy of this file, and change the error
message as follows:
Perl module IPC::Run is required to run TAP tests

See the patch attached as reference, we could simplify the macro of
this m4 file and remove the check for perl, which is here:
+# Make sure we have perl
+if test -z "$PERL"; then
+AC_CHECK_PROG(PERL,perl,perl)
+fi
Though I kept the original script as-is in the patch attached.
Regards,

Michael,

Thank you. You are right, of course. I missed the 
m4_ax_prog_perl_modules. Please, see the fixed version of patch in the 
attach. I added m4_ax_prog_perl_modules and change the error messages.


The best regards,
Eugene Kazakov,
Postgres Professional
diff --git a/aclocal.m4 b/aclocal.m4
index 6f930b6..e6b2aaf 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -10,3 +10,4 @@ m4_include([config/perl.m4])
 m4_include([config/programs.m4])
 m4_include([config/python.m4])
 m4_include([config/tcl.m4])
+m4_include([config/m4_ax_prog_perl_modules.m4])
diff --git a/config/m4_ax_prog_perl_modules.m4 
b/config/m4_ax_prog_perl_modules.m4
new file mode 100644
index 000..ed5dd30
--- /dev/null
+++ b/config/m4_ax_prog_perl_modules.m4
@@ -0,0 +1,72 @@
+# ===
+#   http://www.gnu.org/software/autoconf-archive/ax_prog_perl_modules.html
+# ===
+#
+# SYNOPSIS
+#
+#   AX_PROG_PERL_MODULES([MODULES], [ACTION-IF-TRUE], [ACTION-IF-FALSE])
+#
+# DESCRIPTION
+#
+#   Checks to see if the given perl modules are available. If true the shell
+#   commands in ACTION-IF-TRUE are executed. If not the shell commands in
+#   ACTION-IF-FALSE are run. Note if $PERL is not set (for example by
+#   calling AC_CHECK_PROG, or AC_PATH_PROG), AC_CHECK_PROG(PERL, perl, perl)
+#   will be run.
+#
+#   MODULES is a space separated list of module names. To check for a
+#   minimum version of a module, append the version number to the module
+#   name, separated by an equals sign.
+#
+#   Example:
+#
+# AX_PROG_PERL_MODULES( Text::Wrap Net::LDAP=1.0.3, ,
+#   AC_MSG_WARN(Need some Perl modules)
+#
+# LICENSE
+#
+#   Copyright (c) 2009 Dean Povey 
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 7
+
+AU_ALIAS([AC_PROG_PERL_MODULES], [AX_PROG_PERL_MODULES])
+AC_DEFUN([AX_PROG_PERL_MODULES],[dnl
+
+m4_define([ax_perl_modules])
+m4_foreach([ax_perl_module], m4_split(m4_normalize([$1])),
+ [
+  m4_append([ax_perl_modules],
+[']m4_bpatsubst(ax_perl_module,=,[ ])[' ])
+  ])
+
+if test "x$PERL" != x; then
+  ax_perl_modules_failed=0
+  for ax_perl_module in ax_perl_modules; do
+AC_MSG_CHECKING(for perl module $ax_perl_module)
+
+# 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
+  AC_MSG_RESULT(no);
+  ax_perl_modules_failed=1
+   else
+  AC_MSG_RESULT(ok);
+fi
+  done
+
+  # Run optional shell commands
+  if test "$ax_perl_modules_failed" = 0; then
+:
+$2
+  else
+:
+$3
+  fi
+else
+  AC_MSG_WARN(could not find perl)
+fi])dnl
diff --git a/configure b/configure
index 1903815..2d80202 100755
--- a/configure
+++ b/configure
@@ -15554,6 +15554,79 @@ done
   if test -z "$PERL"; then
 as_fn_error $? "Perl not found" "$LINENO" 5
   fi
+
+
+
+
+
+
+
+if test "x$PERL" != x; then
+  ax_perl_modules_failed=0
+  for ax_perl_module in 'Test::More' ; do
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for perl module 
$ax_perl_module" >&5

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Michael Paquier]

On Sat, Feb 13, 2016 at 1:47 PM, Robert Haas  wrote:
> On Fri, Feb 12, 2016 at 8:20 AM, Eugene Kazakov
>  wrote:
>> TAP-tests need two Perl modules: Test::More and IPC::Run.
>>
>> The patch adds checking of modules in configure.in and configure.
>
> Why would we want that?

I was doubtful at the beginning, but it doesn't hurt to have those
sanity checks in configure actually. The idea is that when
--enable-tap-tests is used now we simply error with "Can't locate
IPC/Run.pm in @INC" when kicking the tests, this check would allow one
to know if his environment is adapted to run the tests or not before
compiling anything.

And with this patch, we would fail now with that:
configure: error: Need Perl IPC::Run module

By the way, the patch given upthread by Eugene is incorrect. To begin
with, AX_PROG_PERL_MODULES has not been compiled by autoconf and I
can't believe that it is available on all platforms, for example on
OSX 10.8 I could not see it. And it is actually here:
https://www.gnu.org/software/autoconf-archive/ax_prog_perl_modules.html

I would recommend grabbing a copy of this file, and change the error
message as follows:
Perl module IPC::Run is required to run TAP tests

See the patch attached as reference, we could simplify the macro of
this m4 file and remove the check for perl, which is here:
+# Make sure we have perl
+if test -z "$PERL"; then
+AC_CHECK_PROG(PERL,perl,perl)
+fi
Though I kept the original script as-is in the patch attached.
Regards,
-- 
Michael
diff --git a/aclocal.m4 b/aclocal.m4
index 6f930b6..db11141 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -1,5 +1,6 @@
 dnl aclocal.m4
 m4_include([config/ac_func_accept_argtypes.m4])
+m4_include([config/ax_prog_perl_modules.m4])
 m4_include([config/ax_pthread.m4])
 m4_include([config/c-compiler.m4])
 m4_include([config/c-library.m4])
diff --git a/config/ax_prog_perl_modules.m4 b/config/ax_prog_perl_modules.m4
new file mode 100644
index 000..11a326c
--- /dev/null
+++ b/config/ax_prog_perl_modules.m4
@@ -0,0 +1,77 @@
+# ===
+#   http://www.gnu.org/software/autoconf-archive/ax_prog_perl_modules.html
+# ===
+#
+# SYNOPSIS
+#
+#   AX_PROG_PERL_MODULES([MODULES], [ACTION-IF-TRUE], [ACTION-IF-FALSE])
+#
+# DESCRIPTION
+#
+#   Checks to see if the given perl modules are available. If true the shell
+#   commands in ACTION-IF-TRUE are executed. If not the shell commands in
+#   ACTION-IF-FALSE are run. Note if $PERL is not set (for example by
+#   calling AC_CHECK_PROG, or AC_PATH_PROG), AC_CHECK_PROG(PERL, perl, perl)
+#   will be run.
+#
+#   MODULES is a space separated list of module names. To check for a
+#   minimum version of a module, append the version number to the module
+#   name, separated by an equals sign.
+#
+#   Example:
+#
+# AX_PROG_PERL_MODULES( Text::Wrap Net::LDAP=1.0.3, ,
+#   AC_MSG_WARN(Need some Perl modules)
+#
+# LICENSE
+#
+#   Copyright (c) 2009 Dean Povey 
+#
+#   Copying and distribution of this file, with or without modification, are
+#   permitted in any medium without royalty provided the copyright notice
+#   and this notice are preserved. This file is offered as-is, without any
+#   warranty.
+
+#serial 7
+
+AU_ALIAS([AC_PROG_PERL_MODULES], [AX_PROG_PERL_MODULES])
+AC_DEFUN([AX_PROG_PERL_MODULES],[dnl
+
+m4_define([ax_perl_modules])
+m4_foreach([ax_perl_module], m4_split(m4_normalize([$1])),
+	  [
+	   m4_append([ax_perl_modules],
+		 [']m4_bpatsubst(ax_perl_module,=,[ ])[' ])
+  ])
+
+# Make sure we have perl
+if test -z "$PERL"; then
+AC_CHECK_PROG(PERL,perl,perl)
+fi
+
+if test "x$PERL" != x; then
+  ax_perl_modules_failed=0
+  for ax_perl_module in ax_perl_modules; do
+AC_MSG_CHECKING(for perl module $ax_perl_module)
+
+# 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
+  AC_MSG_RESULT(no);
+  ax_perl_modules_failed=1
+   else
+  AC_MSG_RESULT(ok);
+fi
+  done
+
+  # Run optional shell commands
+  if test "$ax_perl_modules_failed" = 0; then
+:
+$2
+  else
+:
+$3
+  fi
+else
+  AC_MSG_WARN(could not find perl)
+fi])dnl
diff --git a/configure b/configure
index b3f3abe..3613fd6 100755
--- a/configure
+++ b/configure
@@ -15554,6 +15554,161 @@ done
   if test -z "$PERL"; then
 as_fn_error $? "Perl not found" "$LINENO" 5
   fi
+  # Check for necessary modules
+
+
+
+
+
+
+# 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
+ 

Re: [HACKERS] Small PATCH: check of 2 Perl modules

[Robert Haas]

On Fri, Feb 12, 2016 at 8:20 AM, Eugene Kazakov
 wrote:
> TAP-tests need two Perl modules: Test::More and IPC::Run.
>
> The patch adds checking of modules in configure.in and configure.

Why would we want that?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers