[PATCH] Fix PCRE detection on OSX.
mdounin at mdounin.ru
Wed Dec 12 11:04:48 UTC 2012
On Tue, Dec 11, 2012 at 12:19:25AM -0800, Piotr Sikora wrote:
> > + if [ $ngx_found = no -a "$NGX_SYSTEM" = "Darwin" ]; then
> > + ngx_feature="PCRE library"
> > + ngx_feature_path=
> > + ngx_feature_libs="-lpcre"
> > +
> > + . auto/feature
> > + fi
> > +
> > if [ $ngx_found = no ]; then
> > # RedHat RPM, Solaris package
> I like this, it makes much more sense to defer the check instead of
> completely skipping it.
Consider I don't want /usr/local/include to be used at all, e.g.
because I have some incompatible headers there. Right now I can
do this with correct compiler/linker arguments, e.g. like this:
--with-cc-opt="-nostdlibinc -I/usr/include -I/path/to/pcre/include" \
With the patch it will no longer be possible. (The problem is
actually similar to one with the check completely skipped, but
manifests itself in a bit more exotic cases.)
(Moreover, the patch doesn't resolve MacPorts toolchain case as it
checks default include paths before the /opt/local/.)
> > --- a/auto/lib/pcre/conf Mon Nov 26 18:01:49 2012 +0000
> > +++ b/auto/lib/pcre/conf Mon Dec 10 18:21:53 2012 +0400
> > @@ -172,6 +172,7 @@ else
> > ngx_feature="PCRE JIT support"
> > ngx_feature_name="NGX_HAVE_PCRE_JIT"
> > ngx_feature_test="int jit = 0;
> > + pcre_free_study(NULL);
> > pcre_config(PCRE_CONFIG_JIT, &jit);
> > if (jit != 1) return 1;"
> > . auto/feature
> > probably will be good enough. It will prevent build failures
> > by checking if the code we are going to compile will compile.
> > (Yes, I understand that it won't resolve the root cause, i.e. the
> > library vs. headers mismatch. And yes, I understand that it will
> > make PCRE JIT unavailable in some cases. I don't think we care
> > though.)
> While I agree that this is a better test for PCRE JIT presence and
> that it should be included regardless of the OSX issue, I don't think
> that this is solution to the problem at hand, because all it does is
> hide the issue from the end user... Putting JIT capabilities aside, I
> also dislike the fact that we might be throwing away almost 3 years of
> PCRE fixes just because we cannot be bothered to link against newer
> library that's available outside of the default search path... I
> definitely prefer the solution with postponed check for the default
> search path on OSX.
The problem is that the issue isn't specific to PCRE, hence it
can't be fixed by a patch on auto/lib/pcre/conf. It's about
toolchain, and all libraries are equally affected. It's just
happened to be visible with PCRE due to interface change and
resulting build failures. So all patches on auto/lib/pcre/conf is
about hiding the issue from the end user, nothing more.
Correct fix would be to add -L/usr/local/lib in such cases (or
-L/opt/local/lib in case of MacPorts toolchain with MacPorts
installed into /opt/local), but I can't think of an easy way to
correctly detect if we need it.
> Alternatively, we could first try looking for the PCRE library with
> JIT support and fallback to looking for the PCRE library without one:
> but honestly, it seems like a bit of an overkill.
Looks like an overkill for me, too. I'll commit PCRE JIT test
modifications for now.
More information about the nginx-devel