[Buildroot] [PATCH] zsh: don't use host pcre-config

[Buildroot] [PATCH] zsh: don't use host pcre-config

[Baruch Siach]

Commit 5e05faec7b4 (zsh: use the correct target pcre-config) set
ac_cv_prog_PCRECONF to the location of staging pcre-config. Unfortunately zsh
configure script does not actually use this variable when running pcre-config.
Complete the fix with a patch that makes use of ac_cv_prog_PCRECONF.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 .../0001-configure-use-user-set-pcre-config.patch  | 33 ++++++++++++++++++++++
 package/zsh/zsh.mk                                 |  2 ++
 2 files changed, 35 insertions(+)
 create mode 100644 package/zsh/0001-configure-use-user-set-pcre-config.patch

diff --git a/package/zsh/0001-configure-use-user-set-pcre-config.patch b/package/zsh/0001-configure-use-user-set-pcre-config.patch
new file mode 100644
index 000000000000..d4cf59bdddca
--- /dev/null
+++ b/package/zsh/0001-configure-use-user-set-pcre-config.patch
@@ -0,0 +1,33 @@
+From: Baruch Siach <baruch@tkos.co.il>
+Date: Thu, 3 Mar 2016 23:14:39 +0200
+Subject: [PATCH] configure: use user set pcre-config
+
+Setting a non default configuration script location is common practice when
+cross compiling, since the target library might need different flags. zsh
+configure scripts allows the user to set pcre-config location but doesn't
+actually use it. Fix this.
+
+Signed-off-by: Baruch Siach <baruch@tkos.co.il>
+---
+Patch status: sent upstream
+(http://www.zsh.org/mla/workers/2016/msg00619.html)
+
+ configure.ac | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/configure.ac b/configure.ac
+index c3bd713c126a..9947b16066b6 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -925,7 +925,7 @@ fi
+ if test x$enable_pcre = xyes; then
+ dnl pcre-config should probably be employed here
+ dnl AC_SEARCH_LIBS(pcre_compile, pcre)
+-  LIBS="`pcre-config --libs` $LIBS"
++  LIBS="`$ac_cv_prog_PCRECONF --libs` $LIBS"
+ fi
+ 
+ dnl ---------------------
+-- 
+2.7.0
+
diff --git a/package/zsh/zsh.mk b/package/zsh/zsh.mk
index 84dbbde56f92..aa4029f1a296 100644
--- a/package/zsh/zsh.mk
+++ b/package/zsh/zsh.mk
@@ -9,6 +9,8 @@ ZSH_SITE = http://www.zsh.org/pub
 ZSH_SOURCE = zsh-$(ZSH_VERSION).tar.xz
 ZSH_DEPENDENCIES = ncurses
 ZSH_CONF_OPTS = --bindir=/bin
+# Patching configure.ac
+ZSH_AUTORECONF = YES
 ZSH_LICENSE = MIT-like
 ZSH_LICENSE_FILES = LICENCE
 
-- 
2.7.0

[Buildroot] [PATCH] zsh: don't use host pcre-config

[Thomas Petazzoni]

Baruch,

On Thu,  3 Mar 2016 23:46:10 +0200, Baruch Siach wrote:

> +diff --git a/configure.ac b/configure.ac
> +index c3bd713c126a..9947b16066b6 100644
> +--- a/configure.ac
> ++++ b/configure.ac
> +@@ -925,7 +925,7 @@ fi
> + if test x$enable_pcre = xyes; then
> + dnl pcre-config should probably be employed here
> + dnl AC_SEARCH_LIBS(pcre_compile, pcre)
> +-  LIBS="`pcre-config --libs` $LIBS"
> ++  LIBS="`$ac_cv_prog_PCRECONF --libs` $LIBS"

I think it is more correct to use the PRECONF variable rather than
ac_cv_prog_PCRECONF. Indeed, you have:

AC_CHECK_PROG([PCRECONF], pcre-config, pcre-config)

The documentation of AC_CHECK_PROG (at [1]) says:

? Macro: AC_CHECK_PROG (variable, prog-to-check-for, value-if-found, [value-if-not-found], [path = ?$PATH?], [reject])

   Check whether program prog-to-check-for exists in path. If it is
   found, set <variable> to value-if-found, otherwise to
   value-if-not-found, if given. Always pass over reject (an absolute
   file name) even if it is the first found in the search path; in that
   case, set <variable> using the absolute file name of the
   prog-to-check-for found that is not reject. If <variable> was already
   set, do nothing. Calls AC_SUBST for variable. The result of this
   test can be overridden by setting the variable <variable> or the cache
   variable ac_cv_prog_<variable>.

So to me, this means that the expected "output" of this macro is to set
<variable>, which in your case is PRECONF.

Best regards,

Thomas

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Generic-Programs.html

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] zsh: don't use host pcre-config

[Baruch Siach]

Hi Thomas,

On Sun, Mar 06, 2016 at 01:23:13PM +0100, Thomas Petazzoni wrote:
> On Thu,  3 Mar 2016 23:46:10 +0200, Baruch Siach wrote:
> > +diff --git a/configure.ac b/configure.ac
> > +index c3bd713c126a..9947b16066b6 100644
> > +--- a/configure.ac
> > ++++ b/configure.ac
> > +@@ -925,7 +925,7 @@ fi
> > + if test x$enable_pcre = xyes; then
> > + dnl pcre-config should probably be employed here
> > + dnl AC_SEARCH_LIBS(pcre_compile, pcre)
> > +-  LIBS="`pcre-config --libs` $LIBS"
> > ++  LIBS="`$ac_cv_prog_PCRECONF --libs` $LIBS"
> 
> I think it is more correct to use the PRECONF variable rather than
> ac_cv_prog_PCRECONF. Indeed, you have:
> 
> AC_CHECK_PROG([PCRECONF], pcre-config, pcre-config)
> 
> The documentation of AC_CHECK_PROG (at [1]) says:
> 
> ? Macro: AC_CHECK_PROG (variable, prog-to-check-for, value-if-found, [value-if-not-found], [path = ?$PATH?], [reject])
> 
>    Check whether program prog-to-check-for exists in path. If it is
>    found, set <variable> to value-if-found, otherwise to
>    value-if-not-found, if given. Always pass over reject (an absolute
>    file name) even if it is the first found in the search path; in that
>    case, set <variable> using the absolute file name of the
>    prog-to-check-for found that is not reject. If <variable> was already
>    set, do nothing. Calls AC_SUBST for variable. The result of this
>    test can be overridden by setting the variable <variable> or the cache
>    variable ac_cv_prog_<variable>.
> 
> So to me, this means that the expected "output" of this macro is to set
> <variable>, which in your case is PRECONF.

Thanks for the information.

This patch is now upstream, so I'll send another one upstream changing 
$ac_cv_prog_PCRECONF to $PCRECONF. If upstream accepts that I'll send v2 with 
a combined patch.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH] zsh: don't use host pcre-config

[Thomas Petazzoni]

Baruch,

On Sun, 6 Mar 2016 15:15:19 +0200, Baruch Siach wrote:

> > So to me, this means that the expected "output" of this macro is to set
> > <variable>, which in your case is PRECONF.
> 
> Thanks for the information.

Note that the above was my own interpretation of the autoconf rules,
and what I generally see in most configure.ac. People knowing autoconf
better might disagree.

> This patch is now upstream, so I'll send another one upstream changing 
> $ac_cv_prog_PCRECONF to $PCRECONF. If upstream accepts that I'll send v2 with 
> a combined patch.

Notice that there is another place in the configure.ac where
ac_cv_prog_PRECONF is already used.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] zsh: don't use host pcre-config

[Baruch Siach]

Hi Thomas,

On Sun, Mar 06, 2016 at 02:30:29PM +0100, Thomas Petazzoni wrote:
> On Sun, 6 Mar 2016 15:15:19 +0200, Baruch Siach wrote:
> 
> > > So to me, this means that the expected "output" of this macro is to set
> > > <variable>, which in your case is PRECONF.
> > 
> > Thanks for the information.
> 
> Note that the above was my own interpretation of the autoconf rules,
> and what I generally see in most configure.ac. People knowing autoconf
> better might disagree.
> 
> > This patch is now upstream, so I'll send another one upstream changing 
> > $ac_cv_prog_PCRECONF to $PCRECONF. If upstream accepts that I'll send v2 with 
> > a combined patch.
> 
> Notice that there is another place in the configure.ac where
> ac_cv_prog_PRECONF is already used.

Right. Provided that other ac_cv_ variables are used throughout zsh 
configure.ac do you think is is worth the trouble to clean up 
ac_cv_prog_PCRECONF use?

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH] zsh: don't use host pcre-config

[Thomas Petazzoni]

Hello,

On Sun, 6 Mar 2016 19:50:05 +0200, Baruch Siach wrote:

> > Notice that there is another place in the configure.ac where
> > ac_cv_prog_PRECONF is already used.
> 
> Right. Provided that other ac_cv_ variables are used throughout zsh 
> configure.ac do you think is is worth the trouble to clean up 
> ac_cv_prog_PCRECONF use?

Probably not. I'll just apply your patch as-is.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] zsh: don't use host pcre-config

[Thomas Petazzoni]

Dear Baruch Siach,

On Thu,  3 Mar 2016 23:46:10 +0200, Baruch Siach wrote:
> Commit 5e05faec7b4 (zsh: use the correct target pcre-config) set
> ac_cv_prog_PCRECONF to the location of staging pcre-config. Unfortunately zsh
> configure script does not actually use this variable when running pcre-config.
> Complete the fix with a patch that makes use of ac_cv_prog_PCRECONF.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  .../0001-configure-use-user-set-pcre-config.patch  | 33 ++++++++++++++++++++++
>  package/zsh/zsh.mk                                 |  2 ++
>  2 files changed, 35 insertions(+)
>  create mode 100644 package/zsh/0001-configure-use-user-set-pcre-config.patch

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com