Opened 5 years ago
Last modified 5 years ago
#1465 new defect
configure: use -iquote for $ngx_module_incs
|Reported by:||Owned by:|
|uname -a:||Darwin pro.local 16.7.0 Darwin Kernel Version 16.7.0: Mon Nov 13 21:56:25 PST 2017; root:xnu-3789.72.11~1/RELEASE_X86_64 x86_64|
- Darwin pro.local 16.7.0 Darwin Kernel Version 16.7.0: Mon Nov 13 21:56:25 PST 2017; root:xnu-3789.72.11~1/RELEASE_X86_64 x86_64
I found a problem when trying to compile ngx_brotli on macOS/OSX
which might be rooted in nginx/configure (or at least needs some help there)
./configure --add-module=../ngx_brotli make
pulls in </opt/local/include/brotli/encode.h>
instead of "../ngx_brotli/deps/brotli/include/brotli/encode.h"
(I have installed "brotli" from macports which installs an incompatible version of brotli/encode.h)
This clearly is a bug in ngx_brotli, which should read:
But for this to have the desired efect, the CFLAGS have to be changed from:
as of my first guess the include path flag comes from
(please excuse, I'm not yet that dig deep in the nginx module config api)
I think this should read something like:
which should generate CFLAGS
I patched ngx_brotli/src/ngx_http_brotli_filter_module.c
to (angle brackets replaced by quotes):
I then manually compiled the file,
fisrt using the CFLAGS as given, aka:
then modifing CFLAGS to -iquote
first compile fails:
nginx-1.13.8 $ sh -x c-I + /usr/bin/cc -c -pipe -O -Wall -Wextra -Wpointer-arith -Wconditional-uninitialized -Wno-unused-parameter -Wno-deprecated-declarations -Werror -g -Wno-error -Wno-deprecated-declarations -I src/core -I src/event -I src/event/modules -I src/os/unix -I /opt/local/include -I /usr/local/src/openssl-1.1.0f/.openssl/include -I objs -I src/http -I src/http/modules -I src/http/v2 -I ../ngx_brotli/deps/brotli/include -I /opt/local/lib/ruby2.5/gems/2.5.0/gems/passenger-5.1.12/src -o objs/addon/src/ngx_http_brotli_filter_module.o ../ngx_brotli/src/ngx_http_brotli_filter_module.c ../ngx_brotli/src/ngx_http_brotli_filter_module.c:272:28: warning: implicit declaration of function 'BrotliEncoderInputBlockSize' is invalid in C99 [-Wimplicit-function-declaration] ctx->brotli_ring = BrotliEncoderInputBlockSize(ctx->encoder);
my manually modified compile works (replaced -I by -iquote)
nginx-1.13.8 $ sh -x c-iquote + /usr/bin/cc -c -pipe -O -Wall -Wextra -Wpointer-arith -Wconditional-uninitialized -Wno-unused-parameter -Wno-deprecated-declarations -Werror -g -Wno-error -Wno-deprecated-declarations -I src/core -I src/event -I src/event/modules -I src/os/unix -I /opt/local/include -I /usr/local/src/openssl-1.1.0f/.openssl/include -I objs -I src/http -I src/http/modules -I src/http/v2 -iquote ../ngx_brotli/deps/brotli/include -I /opt/local/lib/ruby2.5/gems/2.5.0/gems/passenger-5.1.12/src -o objs/addon/src/ngx_http_brotli_filter_module.o ../ngx_brotli/src/ngx_http_brotli_filter_module.c
I did set this to major priority (you might even want to raise this to critical)
because pulling in wrong header files can lead to really bad and hard to diagnose bugs.
Change History (4)
comment:1 by , 5 years ago
comment:3 by , 5 years ago
|Priority:||major → minor|
In nginx, we intentionally do not use quoted includes, as this will result in inconsistent handling for files located in different parts of the source tree. Instead, we use the
-I compiler flag and angular includes, which is defined by POSIX and behaves as follows:
... For headers whose names are enclosed in angle brackets ( "<>" ), the header shall be searched for only in directories named in -I options and then in the usual places. ...
The problem you are facing is due to the fact that there is
-I /opt/local/include, likely for the PCRE library, before the ngx_brotli module include directories. This causes
/opt/local/include to be searched before the include directories in ngx_brotli sources.
Unfortunately, it is not something trivial to resolve. Switching to using quoted includes and the non-standard
-iquote option is certainly not something we are going to do, as this will cause problems on other platforms and compilers.
Most trivial solution is to avoid conflicts in your build environment. In particular, nginx itself uses the
ngx_... prefix for files to avoid potential conflicts with various other files.
A potential improvement might be to move include directories pulled by various libraries to the end of the list of include directories. This might have other unexpected effects though, and also not really trival to do.
comment:4 by , 5 years ago
Thanks for your attention,
I fully agree with your reply.
We should really throw this on macports:brotli and maybe ngx_brotli
But there is still an issue that I did not get resolved, yet:
I could not yet find out, how that /opt/local/include got into my -I path of ./configure.
Because when running [abbreviated, see above]
env CC=/usr/bin/cc [...] ./configure
then that /opt/local paths should not creep into the ./configure.
Sorry for making so much fuzz about that,
but nginx is the front facing component on my web server,
and therefor I want to have fine control, of what get's in there.
So having a wrong include of brotli in there, raised my alarms.
The nignx compile should not be attackable by bad include paths.
Maybe we should teach autoconf about this.
This is out of the reach of the nignx compile team.
Thanks for your response,
sudo port uninstall brotli