GNU bug report logs - #16757
[PATCH 1/2] search for pcre via pkg-config

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16757 in the body.
You can then email your comments to 16757 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: bug-grep <at> gnu.org
Subject: [PATCH 1/2] search for pcre via pkg-config
Date: Fri, 14 Feb 2014 17:09:31 -0500

Since pcre ships a pkg-config module now, let's default to using that.
If the system lacks pkg-config support, we still fall back to the old
logic of probing the headers/libs directly.

* configure.ac: Call m4_pattern_forbid on the PKG_ namespace.
* m4/pcre.m4: Change serial to 2.
(gl_FUNC_PCRE): Require PKG_PROG_PKG_CONFIG. Change LIB_PCRE to PCRE_LIBS.
Set up PCRE_CFLAGS like PCRE_LIBS. Call PKG_CHECK_MODULES and move existing
logic to the 4th arg.
* src/Makefile.am (grep_LDADD): Change LIB_PCRE to PCRE_LIBS.
(AM_CPPFLAGS): Add $(PCRE_CFLAGS).
---
 configure.ac    |  2 ++
 m4/pcre.m4      | 42 ++++++++++++++++++++++++++----------------
 src/Makefile.am |  4 ++--
 3 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/configure.ac b/configure.ac
index 886449b..a579317 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,8 @@ AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.
 
 AC_CONFIG_HEADERS([config.h:config.hin])
 
+m4_pattern_forbid([^PKG_])
+
 dnl Checks for programs.
 AC_CANONICAL_HOST
 AC_PROG_AWK
diff --git a/m4/pcre.m4 b/m4/pcre.m4
index a0b2b68..c424443 100644
--- a/m4/pcre.m4
+++ b/m4/pcre.m4
@@ -1,5 +1,5 @@
 # pcre.m4 - check for libpcre support
-# serial 1
+# serial 2
 
 # Copyright (C) 2010-2014 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
@@ -8,6 +8,8 @@
 
 AC_DEFUN([gl_FUNC_PCRE],
 [
+  AC_REQUIRE([PKG_PROG_PKG_CONFIG])
+
   AC_ARG_ENABLE([perl-regexp],
     AC_HELP_STRING([--disable-perl-regexp],
                    [disable perl-regexp (pcre) support]),
@@ -17,25 +19,33 @@ AC_DEFUN([gl_FUNC_PCRE],
      esac],
     [test_pcre=yes])
 
-  LIB_PCRE=
-  AC_SUBST([LIB_PCRE])
+  PCRE_LIBS=
+  AC_SUBST([PCRE_LIBS])
+  PCRE_CFLAGS=
+  AC_SUBST([PCRE_CFLAGS])
   use_pcre=no
 
   if test x"$test_pcre" = x"yes"; then
-    AC_CHECK_HEADERS([pcre.h])
-    AC_CHECK_HEADERS([pcre/pcre.h])
-    if test $ac_cv_header_pcre_h = yes \
-        || test $ac_cv_header_pcre_pcre_h = yes; then
-      pcre_saved_LIBS=$LIBS
-      AC_SEARCH_LIBS([pcre_compile], [pcre],
-        [test "$ac_cv_search_pcre_compile" = "none required" ||
-         LIB_PCRE=$ac_cv_search_pcre_compile])
-      AC_CHECK_FUNCS([pcre_compile])
-      LIBS=$pcre_saved_LIBS
-      if test $ac_cv_func_pcre_compile = yes; then
-        use_pcre=yes
+    PKG_CHECK_MODULES([PCRE], [libpcre], [dnl
+      # Found everything we needed via pkg-config.
+      use_pcre=yes
+      ], [dnl
+      # Fall back when pkg-config is not available.
+      AC_CHECK_HEADERS([pcre.h])
+      AC_CHECK_HEADERS([pcre/pcre.h])
+      if test $ac_cv_header_pcre_h = yes \
+          || test $ac_cv_header_pcre_pcre_h = yes; then
+        pcre_saved_LIBS=$LIBS
+        AC_SEARCH_LIBS([pcre_compile], [pcre],
+          [test "$ac_cv_search_pcre_compile" = "none required" ||
+           PCRE_LIBS=$ac_cv_search_pcre_compile])
+        AC_CHECK_FUNCS([pcre_compile])
+        LIBS=$pcre_saved_LIBS
+        if test $ac_cv_func_pcre_compile = yes; then
+          use_pcre=yes
+        fi
       fi
-    fi
+    ])
     if test $use_pcre = no; then
       AC_MSG_WARN([libpcre development library was not found or not usable.])
       AC_MSG_WARN([AC_PACKAGE_NAME will be built without pcre support.])
diff --git a/src/Makefile.am b/src/Makefile.am
index 9283d1a..5b2f2b9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -41,8 +41,8 @@ LDADD = \
   ../lib/libgreputils.a $(LIBINTL) ../lib/libgreputils.a $(LIBICONV) \
   $(LIBTHREAD)
 
-grep_LDADD = $(LDADD) $(LIB_PCRE)
+grep_LDADD = $(LDADD) $(PCRE_LIBS)
 localedir = $(datadir)/locale
-AM_CPPFLAGS = -I$(top_builddir)/lib -I$(top_srcdir)/lib
+AM_CPPFLAGS = -I$(top_builddir)/lib -I$(top_srcdir)/lib $(PCRE_CFLAGS)
 
 EXTRA_DIST = dosbuf.c
-- 
1.8.5.4

Message #8 received at 16757 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Mike Frysinger <vapier <at> gentoo.org>
Cc: 16757 <at> debbugs.gnu.org
Subject: Re: bug#16757: [PATCH 1/2] search for pcre via pkg-config
Date: Sat, 15 Feb 2014 20:20:35 -0800

[Message part 1 (text/plain, inline)]

On Fri, Feb 14, 2014 at 2:09 PM, Mike Frysinger <vapier <at> gentoo.org> wrote:
> Since pcre ships a pkg-config module now, let's default to using that.
> If the system lacks pkg-config support, we still fall back to the old
> logic of probing the headers/libs directly.
>
> * configure.ac: Call m4_pattern_forbid on the PKG_ namespace.
> * m4/pcre.m4: Change serial to 2.
> (gl_FUNC_PCRE): Require PKG_PROG_PKG_CONFIG. Change LIB_PCRE to PCRE_LIBS.
> Set up PCRE_CFLAGS like PCRE_LIBS. Call PKG_CHECK_MODULES and move existing
> logic to the 4th arg.
> * src/Makefile.am (grep_LDADD): Change LIB_PCRE to PCRE_LIBS.
> (AM_CPPFLAGS): Add $(PCRE_CFLAGS).

Thanks, Mike, but when I try to build with that on debian unstable, I
get the attached, so I'd rather defer this one until after the
release.

[makerr-pcre-problem (application/octet-stream, attachment)]

Message #11 received at 16757 <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: Jim Meyering <jim <at> meyering.net>
Cc: 16757 <at> debbugs.gnu.org
Subject: Re: bug#16757: [PATCH 1/2] search for pcre via pkg-config
Date: Sun, 16 Feb 2014 01:04:05 -0500

[Message part 1 (text/plain, inline)]

On Saturday, February 15, 2014 20:20:35 Jim Meyering wrote:
> On Fri, Feb 14, 2014 at 2:09 PM, Mike Frysinger wrote:
> > Since pcre ships a pkg-config module now, let's default to using that.
> > If the system lacks pkg-config support, we still fall back to the old
> > logic of probing the headers/libs directly.
> > 
> > * configure.ac: Call m4_pattern_forbid on the PKG_ namespace.
> > * m4/pcre.m4: Change serial to 2.
> > (gl_FUNC_PCRE): Require PKG_PROG_PKG_CONFIG. Change LIB_PCRE to PCRE_LIBS.
> > Set up PCRE_CFLAGS like PCRE_LIBS. Call PKG_CHECK_MODULES and move
> > existing
> > logic to the 4th arg.
> > * src/Makefile.am (grep_LDADD): Change LIB_PCRE to PCRE_LIBS.
> > (AM_CPPFLAGS): Add $(PCRE_CFLAGS).
> 
> Thanks, Mike, but when I try to build with that on debian unstable, I
> get the attached, so I'd rather defer this one until after the
> release.

looks like need to define HAVE_PCRE_H manually.  no idea why it built on my 
system.  must have had stale files or something.  updated patch attached.

sure, np letting it bake in master
-mike

[signature.asc (application/pgp-signature, inline)]

[0001-search-for-pcre-via-pkg-config.patch (text/x-patch, inline)]

From a54bc1e4428388f5a8968e223bd7e1f9f2fbf41a Mon Sep 17 00:00:00 2001
From: Mike Frysinger <vapier <at> gentoo.org>
Date: Fri, 14 Feb 2014 17:06:03 -0500
Subject: [PATCH] search for pcre via pkg-config

Since pcre ships a pkg-config module now, let's default to using that.
If the system lacks pkg-config support, we still fall back to the old
logic of probing the headers/libs directly.

* configure.ac: Call m4_pattern_forbid on the PKG_ namespace.
* m4/pcre.m4: Change serial to 2.
(gl_FUNC_PCRE): Require PKG_PROG_PKG_CONFIG. Change LIB_PCRE to PCRE_LIBS.
Set up PCRE_CFLAGS like PCRE_LIBS. Call PKG_CHECK_MODULES and move existing
logic to the 4th arg.
* src/Makefile.am (grep_LDADD): Change LIB_PCRE to PCRE_LIBS.
(AM_CPPFLAGS): Add $(PCRE_CFLAGS).
---
 configure.ac    |  2 ++
 m4/pcre.m4      | 43 +++++++++++++++++++++++++++----------------
 src/Makefile.am |  4 ++--
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/configure.ac b/configure.ac
index 886449b..a579317 100644
--- a/configure.ac
+++ b/configure.ac
@@ -75,6 +75,8 @@ AM_SILENT_RULES([yes]) # make --enable-silent-rules the default.
 
 AC_CONFIG_HEADERS([config.h:config.hin])
 
+m4_pattern_forbid([^PKG_])
+
 dnl Checks for programs.
 AC_CANONICAL_HOST
 AC_PROG_AWK
diff --git a/m4/pcre.m4 b/m4/pcre.m4
index a0b2b68..0be75f3 100644
--- a/m4/pcre.m4
+++ b/m4/pcre.m4
@@ -1,5 +1,5 @@
 # pcre.m4 - check for libpcre support
-# serial 1
+# serial 2
 
 # Copyright (C) 2010-2014 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
@@ -8,6 +8,8 @@
 
 AC_DEFUN([gl_FUNC_PCRE],
 [
+  AC_REQUIRE([PKG_PROG_PKG_CONFIG])
+
   AC_ARG_ENABLE([perl-regexp],
     AC_HELP_STRING([--disable-perl-regexp],
                    [disable perl-regexp (pcre) support]),
@@ -17,25 +19,34 @@ AC_DEFUN([gl_FUNC_PCRE],
      esac],
     [test_pcre=yes])
 
-  LIB_PCRE=
-  AC_SUBST([LIB_PCRE])
+  PCRE_LIBS=
+  AC_SUBST([PCRE_LIBS])
+  PCRE_CFLAGS=
+  AC_SUBST([PCRE_CFLAGS])
   use_pcre=no
 
   if test x"$test_pcre" = x"yes"; then
-    AC_CHECK_HEADERS([pcre.h])
-    AC_CHECK_HEADERS([pcre/pcre.h])
-    if test $ac_cv_header_pcre_h = yes \
-        || test $ac_cv_header_pcre_pcre_h = yes; then
-      pcre_saved_LIBS=$LIBS
-      AC_SEARCH_LIBS([pcre_compile], [pcre],
-        [test "$ac_cv_search_pcre_compile" = "none required" ||
-         LIB_PCRE=$ac_cv_search_pcre_compile])
-      AC_CHECK_FUNCS([pcre_compile])
-      LIBS=$pcre_saved_LIBS
-      if test $ac_cv_func_pcre_compile = yes; then
-        use_pcre=yes
+    PKG_CHECK_MODULES([PCRE], [libpcre], [dnl
+      # Found everything we needed via pkg-config.
+      use_pcre=yes
+      AC_DEFINE([HAVE_PCRE_H], [1], [pcre.h is available])
+      ], [dnl
+      # Fall back when pkg-config is not available.
+      AC_CHECK_HEADERS([pcre.h])
+      AC_CHECK_HEADERS([pcre/pcre.h])
+      if test $ac_cv_header_pcre_h = yes \
+          || test $ac_cv_header_pcre_pcre_h = yes; then
+        pcre_saved_LIBS=$LIBS
+        AC_SEARCH_LIBS([pcre_compile], [pcre],
+          [test "$ac_cv_search_pcre_compile" = "none required" ||
+           PCRE_LIBS=$ac_cv_search_pcre_compile])
+        AC_CHECK_FUNCS([pcre_compile])
+        LIBS=$pcre_saved_LIBS
+        if test $ac_cv_func_pcre_compile = yes; then
+          use_pcre=yes
+        fi
       fi
-    fi
+    ])
     if test $use_pcre = no; then
       AC_MSG_WARN([libpcre development library was not found or not usable.])
       AC_MSG_WARN([AC_PACKAGE_NAME will be built without pcre support.])
diff --git a/src/Makefile.am b/src/Makefile.am
index 9283d1a..5b2f2b9 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -41,8 +41,8 @@ LDADD = \
   ../lib/libgreputils.a $(LIBINTL) ../lib/libgreputils.a $(LIBICONV) \
   $(LIBTHREAD)
 
-grep_LDADD = $(LDADD) $(LIB_PCRE)
+grep_LDADD = $(LDADD) $(PCRE_LIBS)
 localedir = $(datadir)/locale
-AM_CPPFLAGS = -I$(top_builddir)/lib -I$(top_srcdir)/lib
+AM_CPPFLAGS = -I$(top_builddir)/lib -I$(top_srcdir)/lib $(PCRE_CFLAGS)
 
 EXTRA_DIST = dosbuf.c
-- 
1.8.5.5

Message #16 received at 16757 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Mike Frysinger <vapier <at> gentoo.org>
Cc: 16757 <at> debbugs.gnu.org
Subject: Re:  [PATCH 1/2] search for pcre via pkg-config
Date: Mon, 21 Apr 2014 17:29:22 -0700

Thanks for the patch in <http://bugs.gnu.org/16757>.  In looking it over 
I see a couple of things.  First, it looks big enough that we'll need 
copyright papers for it.  Can you please get the ball rolling on that? 
I assume you know the drill.

Second, doesn't this patch create a dependency on pkg-config, in the 
sense that if one checks out 'grep' from git, one needs pkg-config to 
build grep?  If so, that should be added to the README-prereq file.

Message #21 received at 16757 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Mike Frysinger <vapier <at> gentoo.org>
Cc: 16757 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] search for pcre via pkg-config
Date: Sun, 25 May 2014 01:14:01 -0700

[Message part 1 (text/plain, inline)]

I wrote a different patch to search for libpcre via pkg-config, stealing 
much of the code from Emacs, which needs to do something similar and 
which has been debugged on multiple platforms.  This patch should 
address the issues raised so far in this bug report.  It's attached.  If 
there's no objection I'd like to install it into the grep master.

[0001-build-use-pkg-config-if-available-to-configure-libpc.patch (text/plain, attachment)]

Message #26 received at 16757 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 16757 <at> debbugs.gnu.org
Subject: Re:  [PATCH 1/2] search for pcre via pkg-config
Date: Sun, 25 May 2014 13:46:17 -0700

[Message part 1 (text/plain, inline)]

Holger Bruenjes pointed out a typo in my patch; attached is a fix, and 
an combined patch incorporating the fix.

[0001-build-remove-typo-and-don-t-bother-with-usr-include-.patch (text/plain, attachment)]

[combined.patch (text/plain, attachment)]

Message #29 received at 16757 <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 16757 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] search for pcre via pkg-config
Date: Mon, 6 Apr 2015 03:14:20 -0400

[Message part 1 (text/plain, inline)]

On 25 May 2014 01:14, Paul Eggert wrote:
> --- a/m4/pcre.m4
> +++ b/m4/pcre.m4
>
> +  if test $test_pcre = yes; then

the previous code quoted test_pcre ... i guess previously it was normalized into 
"yes" and "no", but seems pretty cheap & easy to be resilient ?

> +    PKG_CHECK_MODULES([PCRE], [libpcre], [],
> +      [if test -r /usr/include/pcre/pcre.h &&
> +          test ! -r /usr/include/pcre.h
> +       then

ugh, but i guess i don't really care since this is the default and it can go 
poking around in whatever terrible way it wants.

> +         PCRE_CFLAGS=-I/usr/include/libpng

png eh ?

> +    if test "$pcre_cv_have_pcre_compile" = yes; then
> +      use_pcre=yes
> +    else
>        AC_MSG_WARN([AC_PACKAGE_NAME will be built without pcre support.])
>      fi

ideally this would error out if the user passed --enable-perl-regexp.  the 
test_pcre setup would have to pick like "auto" though in the default case.

> +  if test $use_pcre = yes; then

quote it ?
-mike

[signature.asc (application/pgp-signature, inline)]

Message #32 received at 16757 <at> debbugs.gnu.org (full text, mbox):

From: Mike Frysinger <vapier <at> gentoo.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 16757 <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] search for pcre via pkg-config
Date: Mon, 6 Apr 2015 03:15:18 -0400

[Message part 1 (text/plain, inline)]

On 21 Apr 2014 17:29, Paul Eggert wrote:
> Thanks for the patch in <http://bugs.gnu.org/16757>.  In looking it over 
> I see a couple of things.  First, it looks big enough that we'll need 
> copyright papers for it.  Can you please get the ball rolling on that? 
> I assume you know the drill.

fwiw, this should be all set

> Second, doesn't this patch create a dependency on pkg-config, in the 
> sense that if one checks out 'grep' from git, one needs pkg-config to 
> build grep?  If so, that should be added to the README-prereq file.

it does
-mike

[signature.asc (application/pgp-signature, inline)]

Message #37 received at 16757-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 16757-done <at> debbugs.gnu.org
Subject: Re: [PATCH 1/2] search for pcre via pkg-config
Date: Mon, 06 Apr 2015 16:12:59 -0700

[Message part 1 (text/plain, inline)]

On 04/06/2015 12:14 AM, Mike Frysinger wrote:
> On 25 May 2014 01:14, Paul Eggert wrote:
>> --- a/m4/pcre.m4
>> +++ b/m4/pcre.m4
>>
>> +  if test $test_pcre = yes; then
> the previous code quoted test_pcre ... i guess previously it was normalized into
> "yes" and "no", but seems pretty cheap & easy to be resilient ?

For gnulib we tend to prefer omitting the quotes when they're not 
needed, to make the code *less* resilient (i.e., to find bugs 
better....).  A style thing, admittedly.


>> +    PKG_CHECK_MODULES([PCRE], [libpcre], [],
>> +      [if test -r /usr/include/pcre/pcre.h &&
>> +          test ! -r /usr/include/pcre.h
>> +       then
> ugh, but i guess i don't really care since this is the default and it can go
> poking around in whatever terrible way it wants.

That was fixed in <http://bugs.gnu.org/16757#26>.

>
>> +         PCRE_CFLAGS=-I/usr/include/libpng
> png eh ?

This was also fixed.

>> +    if test "$pcre_cv_have_pcre_compile" = yes; then
>> +      use_pcre=yes
>> +    else
>>         AC_MSG_WARN([AC_PACKAGE_NAME will be built without pcre support.])
>>       fi
> ideally this would error out if the user passed --enable-perl-regexp.  the
> test_pcre setup would have to pick like "auto" though in the default case.

Thanks, fixed by the attached further patch.  I've installed this and 
the earlier patches in this sequence, and am marking this as done.

[0001-build-improve-pkg-config-doc-and-error-handling.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.