Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
On 23.07.2014 02:25, Guenter Knauf wrote: Hi Rainer, On 22.07.2014 23:01, Rainer Jung wrote: documenting the requirement PCRE = 6.7 and dropping the check (and error message) for PCRE_DUPNAMES from server/util_pcre.c. -1. Please think of non-configure builds; it doesnt hurt if the code errors out when the requirements do not met. Good point. But in this case even after dropping the check, it would mean the build would error out because PCRE_DUPNAMES isn't known. So then you would have to check the (new) info in the docs, that minimum PCRE version is 6.7. Of course an #error message would help because it is more explicit, but I think that's not how the code currently is structured in general. If there is a dependency for a library version, we don't check in the sources whether the right version is available. Regards, Rainer
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Hi Rainer, On 23.07.2014 12:18, Rainer Jung wrote: Good point. But in this case even after dropping the check, it would mean the build would error out because PCRE_DUPNAMES isn't known. So then you would have to check the (new) info in the docs, that minimum PCRE version is 6.7. I meant to keep this check (which you removed with 1612921) ... Of course an #error message would help because it is more explicit, but I think that's not how the code currently is structured in general. If there is a dependency for a library version, we don't check in the sources whether the right version is available. not true - we do; f.e. from mod_ssl_ct.c: #include apr_version.h #if !APR_VERSION_AT_LEAST(1,5,0) #error mod_ssl_ct requires APR 1.5.0 or later! (for apr_escape.h) #endif and: #if OPENSSL_VERSION_NUMBER 0x10002001L #error mod_ssl_ct requires OpenSSL 1.0.2-beta1 or later #endif and I'm sure there are many more similar places ... but I think the check should directly happen after pcre.h is included and not in the middle of code; see 1612945. Günter.
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Hi, shouldn't the #error just a few lines below be updated as well, to be more explicit than too old ? CJ Le 22/07/2014 21:29, rj...@apache.org a écrit : Author: rjung Date: Tue Jul 22 19:29:08 2014 New Revision: 1612653 URL: http://svn.apache.org/r1612653 Log: Clarify comment. Modified: httpd/httpd/trunk/server/util_pcre.c Modified: httpd/httpd/trunk/server/util_pcre.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1612653r1=1612652r2=1612653view=diff == --- httpd/httpd/trunk/server/util_pcre.c (original) +++ httpd/httpd/trunk/server/util_pcre.c Tue Jul 22 19:29:08 2014 @@ -125,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * const char *errorptr; int erroffset; int errcode = 0; -/* PCRE_DUPNAMES is only present in more recent versions of PCRE */ +/* PCRE_DUPNAMES is only present since version 6.7 of PCRE */ #ifdef PCRE_DUPNAMES int options = PCRE_DUPNAMES; #else
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
On 22.07.2014 22:20, Christophe JAILLET wrote: Hi, shouldn't the #error just a few lines below be updated as well, to be more explicit than too old ? You are right. But what about instead changing the configure pcre version test: Index: configure.in === --- configure.in(revision 1611600) +++ configure.in(working copy) @@ -236,7 +236,9 @@ fi case `$PCRE_CONFIG --version` in [[1-5].*]) -AC_MSG_ERROR([Need at least pcre version 6.0]) +AC_MSG_ERROR([Need at least pcre version 6.7]) + [6.[0-6]*]) +AC_MSG_ERROR([Need at least pcre version 6.7]) ;; esac AC_MSG_NOTICE([Using external PCRE library from $PCRE_CONFIG]) documenting the requirement PCRE = 6.7 and dropping the check (and error message) for PCRE_DUPNAMES from server/util_pcre.c. I got the version number from looking and the old PCRE tags. Regards, Rainer Le 22/07/2014 21:29, rj...@apache.org a écrit : Author: rjung Date: Tue Jul 22 19:29:08 2014 New Revision: 1612653 URL: http://svn.apache.org/r1612653 Log: Clarify comment. Modified: httpd/httpd/trunk/server/util_pcre.c Modified: httpd/httpd/trunk/server/util_pcre.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1612653r1=1612652r2=1612653view=diff == --- httpd/httpd/trunk/server/util_pcre.c (original) +++ httpd/httpd/trunk/server/util_pcre.c Tue Jul 22 19:29:08 2014 @@ -125,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * const char *errorptr; int erroffset; int errcode = 0; -/* PCRE_DUPNAMES is only present in more recent versions of PCRE */ +/* PCRE_DUPNAMES is only present since version 6.7 of PCRE */ #ifdef PCRE_DUPNAMES int options = PCRE_DUPNAMES; #else
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
+1 Le 22/07/2014 23:01, Rainer Jung a écrit : On 22.07.2014 22:20, Christophe JAILLET wrote: Hi, shouldn't the #error just a few lines below be updated as well, to be more explicit than too old ? You are right. But what about instead changing the configure pcre version test: Index: configure.in === --- configure.in(revision 1611600) +++ configure.in(working copy) @@ -236,7 +236,9 @@ fi case `$PCRE_CONFIG --version` in [[1-5].*]) -AC_MSG_ERROR([Need at least pcre version 6.0]) +AC_MSG_ERROR([Need at least pcre version 6.7]) + [6.[0-6]*]) +AC_MSG_ERROR([Need at least pcre version 6.7]) ;; esac AC_MSG_NOTICE([Using external PCRE library from $PCRE_CONFIG]) documenting the requirement PCRE = 6.7 and dropping the check (and error message) for PCRE_DUPNAMES from server/util_pcre.c. I got the version number from looking and the old PCRE tags. Regards, Rainer Le 22/07/2014 21:29, rj...@apache.org a écrit : Author: rjung Date: Tue Jul 22 19:29:08 2014 New Revision: 1612653 URL: http://svn.apache.org/r1612653 Log: Clarify comment. Modified: httpd/httpd/trunk/server/util_pcre.c Modified: httpd/httpd/trunk/server/util_pcre.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1612653r1=1612652r2=1612653view=diff == --- httpd/httpd/trunk/server/util_pcre.c (original) +++ httpd/httpd/trunk/server/util_pcre.c Tue Jul 22 19:29:08 2014 @@ -125,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * const char *errorptr; int erroffset; int errcode = 0; -/* PCRE_DUPNAMES is only present in more recent versions of PCRE */ +/* PCRE_DUPNAMES is only present since version 6.7 of PCRE */ #ifdef PCRE_DUPNAMES int options = PCRE_DUPNAMES; #else
Re: svn commit: r1612653 - /httpd/httpd/trunk/server/util_pcre.c
Hi Rainer, On 22.07.2014 23:01, Rainer Jung wrote: documenting the requirement PCRE = 6.7 and dropping the check (and error message) for PCRE_DUPNAMES from server/util_pcre.c. -1. Please think of non-configure builds; it doesnt hurt if the code errors out when the requirements do not met. Gün.
