Opened 7 years ago
Last modified 7 years ago
#1465 new defect
configure: use -iquote for $ngx_module_incs
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | other | Version: | 1.13.x |
Keywords: | Cc: | ||
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 | ||
nginx -V: | nginx-1.13.8 |
Description
- nginx-1.13.8
- 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)
see also:
- https://github.com/google/ngx_brotli/issues/64
- https://github.com/phusion/passenger/issues/2017
- https://github.com/nginx/nginx/blob/master/auto/module
./configure --add-module=../ngx_brotli make
---
in ngx_brotli/src/ngx_http_brotli_filter_module.c:
#include <brotli/encode.h>
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:
#include "brotli/encode.h"
But for this to have the desired efect, the CFLAGS have to be changed from:
-I ../ngx_brotli/deps/brotli/include
to
-iquote ../ngx_brotli/deps/brotli/include
as of my first guess the include path flag comes from
ngx_brotli/config:
ngx_module_incs="$brotli/include"
(please excuse, I'm not yet that dig deep in the nginx module config api)
I think this should read something like:
ngx_module_incs_quote="$brotli/include"
which should generate CFLAGS
-iquote ../ngx_brotli/deps/brotli/include
I patched ngx_brotli/src/ngx_http_brotli_filter_module.c
from:
#include <brotli/encode.h>
to (angle brackets replaced by quotes):
#include "brotli/encode.h"
I then manually compiled the file,
fisrt using the CFLAGS as given, aka:
-I ../ngx_brotli/deps/brotli/include
then modifing CFLAGS to -iquote
-iquote ../ngx_brotli/deps/brotli/include
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 , 7 years ago
comment:3 by , 7 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 , 7 years ago
Thanks for your attention,
I fully agree with your reply.
We should really throw this on macports:brotli and maybe ngx_brotli
https://trac.macports.org/ticket/55729
https://github.com/google/ngx_brotli/issues/64
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,
~eike
workaround:
sudo port uninstall brotli