Created attachment 10348 [details] build log excerpt The build system is putting flags from environment before own flags. This leads in an error on Mac OS X when building with MacPorts that is providing a own version of gssapi.h header. I'm attaching a relevant excerpt of the build log. This ticket should be assigned to Ralph Böhme <slow@samba.org>.
Indeed, manually fixing the compilation command by putting -I/opt/include at the end works, see attachment.
Created attachment 10349 [details] Manual compilation with CPPFLAGS from env at the end
This is what automake can teach us on the subject of user flags versus project flags: <http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html> "Automake always uses two of these variables when compiling C sources files. When compiling an object file for the mumble target, the first variable will be mumble_CPPFLAGS if it is defined, or AM_CPPFLAGS otherwise. The second variable is always CPPFLAGS."
Created attachment 10350 [details] Put CPPFLAGS behind internal INCLUDES Urks, this is probably worse then anticipated. The canonical environment variable for include paths is CPPFLAGS and this is where Macports correctly inserts it's path (CPPFLAGS=-I/opt/local/include). Otoh, waf when building the list of include path at configure time from config checks puts it's includes in a different variables (afaict interenally variables suffixed by _CCINCFLAGS), which is just fine. At some point, when driving the compiler, all needed flags are put together in a specific order, which is defined, afaict, in buildtools/wafadmin/Tools/cc.py: cc_str = '${CC} ${CCFLAGS} ${CPPFLAGS} ${_CCINCFLAGS} ${_CCDEFFLAGS} ${CC_SRC_F}${SRC} ${CC_TGT_F}${TGT}' CPPFLAGS is put before _CCINCFLAGS, which is probably not correct. What should be used instead is cc_str = '${CC} ${CCFLAGS} ${_CCINCFLAGS} ${CPPFLAGS} ${_CCDEFFLAGS} ${CC_SRC_F}${SRC} ${CC_TGT_F}${TGT}' Attached patch for 4.1.2 fixes this and makes the build pass in Macports. But this can be applied to Samba in general as it's chaning an upsteam waf file, we'd need a more isolated patch for buildtools/wafsamba/ that changes the upstream default. I think this needs discussion on samba-technical and waf-users. I may be completely wrong with my analysis of the situation, hell, I can't even program python! :)
I confirm being able to build 4.1.12 on 10.9.5 with MacPorts by patching buildtools/ as suggested.
Here's a testcase that demonstrates the issue. Expected ouput is foo: FOO in either case, but due do the wrong ordering if CPPFLAGS is defined the actual output is foo: BAR [ralph@fedora20 waftests]$ tree . ├── external_include │ └── myheader.h ├── internal_include │ └── myheader.h ├── main.c └── wscript 2 directories, 4 files [ralph@fedora20 waftests]$ find . -type f | xargs -t -n 1 cat cat ./main.c #include <stdio.h> #include "myheader.h" int main(int argc, char **argv) { printf("foo: %s\n", FOO); return 0; } cat ./external_include/myheader.h #ifndef MYHEADER_H #define MYHEADER_H #define FOO "BAR" #endif cat ./internal_include/myheader.h #ifndef MYHEADER_H #define MYHEADER_H #define FOO "FOO" #endif cat ./wscript #!/usr/bin/env python top = '.' out = 'build' def options(opt): opt.load('compiler_c') def configure(conf): conf.load('compiler_c') def build(bld): bld.program(source='main.c',target='app',includes = 'internal_include') [ralph@fedora20 waftests]$ waf distclean && waf configure build -v && ./build/app 'distclean' finished successfully (0.000s) Setting top to : /home/ralph/waftests Setting out to : /home/ralph/waftests/build Checking for 'c_bgxlc' (c compiler) : not found Checking for 'gcc' (c compiler) : /usr/bin/gcc 'configure' finished successfully (0.047s) Waf: Entering directory `/home/ralph/waftests/build' [1/2] c: main.c -> build/main.c.1.o 11:38:12 runner ['/usr/bin/gcc', '-I/home/ralph/waftests/build/internal_include', '-I/home/ralph/waftests/internal_include', '../main.c', '-c', '-o', 'main.c.1.o'] [2/2] cprogram: build/main.c.1.o -> build/app 11:38:12 runner ['/usr/bin/gcc', 'main.c.1.o', '-o', '/home/ralph/waftests/build/app', '-Wl,-Bstatic', '-Wl,-Bdynamic'] Waf: Leaving directory `/home/ralph/waftests/build' 'build' finished successfully (0.073s) foo: FOO [ralph@fedora20 waftests]$ waf distclean && CPPFLAGS=-I$PWD/external_include waf configure build -v && ./build/app 'distclean' finished successfully (0.001s) Setting top to : /home/ralph/waftests Setting out to : /home/ralph/waftests/build Checking for 'c_bgxlc' (c compiler) : not found Checking for 'gcc' (c compiler) : /usr/bin/gcc 'configure' finished successfully (0.048s) Waf: Entering directory `/home/ralph/waftests/build' [1/2] c: main.c -> build/main.c.1.o 11:38:26 runner ['/usr/bin/gcc', '-I/home/ralph/waftests/external_include', '-I/home/ralph/waftests/build/internal_include', '-I/home/ralph/waftests/internal_include', '../main.c', '-c', '-o', 'main.c.1.o'] [2/2] cprogram: build/main.c.1.o -> build/app 11:38:26 runner ['/usr/bin/gcc', 'main.c.1.o', '-o', '/home/ralph/waftests/build/app', '-Wl,-Bstatic', '-Wl,-Bdynamic'] Waf: Leaving directory `/home/ralph/waftests/build' 'build' finished successfully (0.072s) foo: BAR [ralph@fedora20 waftests]$
Created attachment 10354 [details] testcase as automake project Same testcase with automake: [ralph@fedora20 automaketest]$ make clean && make && ./app test -z "app" || rm -f app rm -f *.o gcc -DPACKAGE_NAME=\"app\" -DPACKAGE_TARNAME=\"app\" -DPACKAGE_VERSION=\"1.0.0\" -DPACKAGE_STRING=\"app\ 1.0.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"app\" -DVERSION=\"1.0.0\" -I. -I./internal_include/ -g -O2 -MT app-main.o -MD -MP -MF .deps/app-main.Tpo -c -o app-main.o `test -f 'main.c' || echo './'`main.c mv -f .deps/app-main.Tpo .deps/app-main.Po gcc -g -O2 -o app app-main.o foo: FOO [ralph@fedora20 automaketest]$ make clean && CPPFLAGS=$PWD/external_include make && ./app test -z "app" || rm -f app rm -f *.o gcc -DPACKAGE_NAME=\"app\" -DPACKAGE_TARNAME=\"app\" -DPACKAGE_VERSION=\"1.0.0\" -DPACKAGE_STRING=\"app\ 1.0.0\" -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"app\" -DVERSION=\"1.0.0\" -I. -I./internal_include/ -g -O2 -MT app-main.o -MD -MP -MF .deps/app-main.Tpo -c -o app-main.o `test -f 'main.c' || echo './'`main.c mv -f .deps/app-main.Tpo .deps/app-main.Po gcc -g -O2 -o app app-main.o foo: FOO
Created attachment 10553 [details] Patch for master Patch for master pair-programmed with metze that ensures CPPFLAGS and LDFLAGS are always appended at the end of the compiler/linker invocation.
Created attachment 10554 [details] Patch for master Patch for master pair-programmed with metze that ensures CPPFLAGS and LDFLAGS are always appended at the end of the compiler/linker invocation.
Adding LDFLAGS to the end is a problem. It doesn't make sense to add -L after -l. With this patch, if a user tells Samba to search for custom libs in some path /foo because they want to override the default system library, then it won't work because the -L appears at the end with this patch. Note the same applies to -I, depending on the compiler. Consider the following: $ cat z.c #include <zlib.h> $ cc -V cc: Sun C 5.12 SunOS_sparc Patch 148917-06 2013/06/11 $ cc -E z.c | grep zlib.h # 1 "/usr/include/zlib.h" # 35 "/usr/include/zlib.h" $ cc -I/opt/fsw/libz12/include -E z.c | grep zlib.h # 1 "/opt/fsw/libz12/include/zlib.h" # 35 "/opt/fsw/libz12/include/zlib.h" $ cc -I/usr/include -I/opt/fsw/libz12/include -E z.c | grep zlib.h # 1 "/usr/include/zlib.h" # 35 "/usr/include/zlib.h" Note that with the Sun C compiler, the path to zlib.h depends on which -I appears first. Now, let's look at gcc-4.4.6: $ gcc -E z.c | grep zlib.h # 1 "/usr/include/zlib.h" 1 3 4 ... $ gcc -I/opt/fsw/libz12/include -E z.c | grep zlib.h # 1 "/opt/fsw/libz12/include/zlib.h" 1 ... $ gcc -I/usr/include -I/opt/fsw/libz12/include -E z.c | grep zlib.h # 1 "/opt/fsw/libz12/include/zlib.h" 1 ... So, with GCC, the -I path that appears last "wins". With the Sun C compiler, the -I path that appears first "wins". So, while this patch "fixes" things with GCC, it breaks compilation on Solaris.
Adding LDFLAGS to the end is indeed a problem. Unfortunately this breaks the use of -Wl,--as-needed as used in Debian. -Wl,--as-needed is applied to any libraries specified *after* it on the command-line.
(In reply to Albert Chin from comment #10) > Adding LDFLAGS to the end is a problem. It doesn't make sense to add -L > after -l. You and Jelmer are absolutely right! This was not intended and I'm sorry that this escaped me. The intention was to put the user env LDFLAGS behind our internal -L search paths, but not behind LIBS. > So, with GCC, the -I path that appears last "wins". With the Sun C > compiler, the -I path that appears first "wins". So, while this patch > "fixes" things with GCC, it breaks compilation on Solaris. I guess then we're screwed, not sure what autotools does wrt this. Do they adapt the commandline order depending on the compiler? I don't think so. The problem this patch was trying to address, is that the paths the user env LDFLAGS -L and CPPFLAGS -I point at may contain Samba headers and libraries that may get picked up instead of the in-tree versions. This is wrong. We have to ensure our build correctly picks up in-tree versions of libraries. I'm working on an correct fix addressing this, as long we should indeed revert my broken commit. Sorry for the hassle!
Created attachment 10991 [details] WIP patch for search path ordering Can you review and/or test this version. Thanks!
Created attachment 11083 [details] Patch for master
Created attachment 11084 [details] Patch for 4.2
Parsing user (C|LD)FLAGS into distinct variables for library search paths (-I|L) and compiler/linker flags (like --as-needed) may work.
(In reply to Ralph Böhme from comment #16) Do you know why you did this? commit ec085fe8b3d4f54294194f87a2875d75c4d92c20 Author: Ralph Boehme <slow@samba.org> AuthorDate: Sat Apr 25 03:38:48 2015 +0200 Commit: Ralph Böhme <slow@samba.org> CommitDate: Sun Apr 26 18:40:39 2015 +0200 Revert "wafsamba: flags from enviroment are put before our own internal versions" This reverts commit b2bb6aeb8057ac725f6ad12378344b201c3a3ba2. Signed-off-by: Ralph Boehme <slow@samba.org> Reviewed-by: Jelmer Vernooij <jelmer@samba.org> Autobuild-User(master): Ralph Böhme <slow@samba.org> Autobuild-Date(master): Sun Apr 26 18:40:39 CEST 2015 on sn-devel-104
(In reply to Stefan Metzmacher from comment #17) Ah, it's in comment #11
(In reply to Stefan Metzmacher from comment #18) I think we need to split -I from CCFLAGS into CPPPATH after cc_add_flags() and -L from LINKFLAGS into LIBPATH after link_add_flags(). We need to do something similar for ADDITIONAL_CFLAGS and ADDITIONAL_LDFLAGS.