| Message ID | 0f3d133b0082f11b75b9fad170d1b81211352dd6.1457041570.git.baruch@tkos.co.il |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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
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
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
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
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
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