New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove replacing -I with -isystem #1126
Remove replacing -I with -isystem #1126
Conversation
Building OLA with a GCC 6 cross-toolchain fails: ``` /usr/bin/arm-linux-g++ -DHAVE_CONFIG_H -I. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -I./include -I./include -Wall -Wformat -W -isystem /usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include -pthread -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -pthread -c -o libs/acn/e131_transmit_test.o libs/acn/e131_transmit_test.cpp /usr/bin/arm-linux-g++ -DHAVE_CONFIG_H -I. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -I./include -I./include -Wall -Wformat -W -isystem /usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include -pthread -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -pthread -c -o libs/acn/E131TestFramework.o libs/acn/E131TestFramework.cpp In file included from /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/ext/string_conversions.h:41:0, from /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/bits/basic_string.h:5402, from /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/string:52, from ./tools/ola_trigger/config.ypp:2: /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/cstdlib:75:25: fatal error: stdlib.h: No such file or directory #include_next <stdlib.h> ^ compilation terminated. ``` The C++ library in GCC 6 now provides its own `<stdlib.h>` header that wraps the C library header of the same name, so in `<cstdlib>` the header include ``` ``` has become ``` ``` `#include_next` is sensitive to the order of directories in the preprocessor's search path, so if that order is changed with `-isystem` then the compiler can't find the right header: ``` [1] /usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include [2] /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0 [..] End of search list. ``` `<cstdlib>` is located in [2] whereas `<stdlib.h>` (C library header) is in [1]. In this case, the `#include_next <stdlib.h>` statement in `<cstdlib>`, located in [2], is evaluated **after** the search path [1], so the compiler does not find the right system header. The problem is that the OLA build system replaces the `-I` in the CFLAGS from libprotobuf with `-isystem` to fix some warnings treated as errors in the libprotobuf header files. `-isystem` should be used to suppress warnings in system headers only and the libprotobuf header files are not system files. The correct fix is to compile with less restrictions and remove `-Werror` for the build. As using `-isystem` is reordering GCCs search path and using `-isystem` is really not necessary, remove the faulty replacement of `-I`. Closes: #1125
Fixes: http://autobuild.buildroot.net/results/2d5/2d55b5d88a06c7b8e6baeb96973009a451e992d9/ http://autobuild.buildroot.net/results/899/89922e61c583cf1d11bd0bafdd5586c35d8f6e15/ http://autobuild.buildroot.net/results/d5b/d5b8fe66ff8d9ea91e87ef6fbe8274f5e24aa7b0/ http://autobuild.buildroot.net/results/89b/89b136e6dced6ca9842a1f23141b0cb999f783da/ .. and many more. Building OLA with a GCC 6 cross-toolchain fails: ``` /usr/bin/arm-linux-g++ -DHAVE_CONFIG_H -I. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -I./include -I./include -Wall -Wformat -W -isystem /usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include -pthread -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -pthread -c -o libs/acn/e131_transmit_test.o libs/acn/e131_transmit_test.cpp /usr/bin/arm-linux-g++ -DHAVE_CONFIG_H -I. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -I./include -I./include -Wall -Wformat -W -isystem /usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include -pthread -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -pthread -c -o libs/acn/E131TestFramework.o libs/acn/E131TestFramework.cpp In file included from /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/ext/string_conversions.h:41:0, from /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/bits/basic_string.h:5402, from /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/string:52, from ./tools/ola_trigger/config.ypp:2: /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0/cstdlib:75:25: fatal error: stdlib.h: No such file or directory #include_next <stdlib.h> ^ compilation terminated. ``` The C++ library in GCC 6 now provides its own `<stdlib.h>` header that wraps the C library header of the same name, so in `<cstdlib>` the header include ``` #include <stdlib.h> ``` has become ``` #include_next <stdlib.h> ``` `#include_next` is sensitive to the order of directories in the preprocessor's search path, so if that order is changed with `-isystem` then the compiler can't find the right header: ``` [1] /usr/arm-buildroot-linux-gnueabihf/sysroot/usr/include [2] /opt/ext-toolchain/arm-buildroot-linux-gnueabihf/include/c++/6.1.0 [..] End of search list. ``` `<cstdlib>` is located in [2] whereas `<stdlib.h>` (C library header) is in [1]. In this case, the `#include_next <stdlib.h>` statement in `<cstdlib>`, located in [2], is evaluated **after** the search path [1], so the compiler does not find the right system header. The problem here is that the OLA build system replaces the `-I` in the CFLAGS from libprotobuf with `-isystem` to fix some warnings treated as errors in the libprotobuf header files. `-isystem` should be used to suppress warnings in system headers only and the libprotobuf header files are not system files. The correct fix is to compile with less restrictions and remove `-Werror` for the build. This is already done by passing `--disable-fatal-warnings` to OLA. Fix the reordering of the GCC search paths by removing the replacement of `-I` with `-isystem`. Upstream status: OpenLightingProject/ola#1126 Signed-off-by: Jörg Krause <joerg.krause@embedded.rocks> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Seems legit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @joerg-krause thanks for this.
Firstly can you resync, as we've just merged some more changes into master. Better yet, please can you target our 0.10 branch, so we can get this into 0.10.3 if it's affecting current builds.
You'll also need to get Travis to pass, ignore the lint fails, as that is already fixed in 0.10 (and the will appear in master soon). You need to get the two Mac builds to compile at least (there's currently an issue with one of the Mac checks that I still need to fix).
They're currently failing with:
'''
/usr/local/bin/protoc --cpp_out common/protocol --proto_path ../../common/protocol ../../common/protocol/Ola.proto
[libprotobuf WARNING google/protobuf/compiler/parser.cc:546] No syntax specified for the proto file: Ola.proto. Please use 'syntax = "proto2";' or 'syntax = "proto3";' to specify a syntax version. (Defaulted to proto2 syntax.)
depbase=echo protoc/CppFileGenerator.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'
;
g++ -DHAVE_CONFIG_H -I. -I../.. -I../../include -I./include -Wall -Wformat -W -fvisibility-inlines-hidden -D_THREAD_SAFE -I/usr/local/Cellar/protobuf/3.1.0/include -Werror -g -O2 -std=gnu++98 -D_THREAD_SAFE -MT protoc/CppFileGenerator.o -MD -MP -MF $depbase.Tpo -c -o protoc/CppFileGenerator.o ../../protoc/CppFileGenerator.cpp &&
mv -f $depbase.Tpo $depbase.Po
In file included from ../../protoc/CppFileGenerator.cpp:37:
In file included from /usr/local/Cellar/protobuf/3.1.0/include/google/protobuf/descriptor.h:59:
/usr/local/Cellar/protobuf/3.1.0/include/google/protobuf/stubs/shared_ptr.h:441:58: error: unused parameter 'other' [-Werror,-Wunused-parameter]
enable_shared_from_this(const enable_shared_from_this& other) { }
^
/usr/local/Cellar/protobuf/3.1.0/include/google/protobuf/stubs/shared_ptr.h:442:69: error: unused parameter 'other' [-Werror,-Wunused-parameter]
enable_shared_from_this& operator=(const enable_shared_from_this& other) {
^
'''
We worked around some similar issues here so you can fix it in a similar way:
https://github.com/OpenLightingProject/ola/pull/1111/files
I opened a new PR #1130 which targets the 0.10.2 branch. |
Replaced by #1130. |
Building OLA with a GCC 6 cross-toolchain fails:
The C++ library in GCC 6 now provides its own
<stdlib.h>
header thatwraps the C library header of the same name, so in
<cstdlib>
theheader include
has become
#include_next
is sensitive to the order of directories in thepreprocessor's search path, so if that order is changed with
-isystem
then the compiler can't find the right header:
<cstdlib>
is located in [2] whereas<stdlib.h>
(C library header) isin [1]. In this case, the
#include_next <stdlib.h>
statement in<cstdlib>
, located in [2], is evaluated after the search path [1],so the compiler does not find the right system header.
The problem is that the OLA build system replaces the
-I
in the CFLAGSfrom libprotobuf with
-isystem
to fix some warnings treated as errorsin the libprotobuf header files.
-isystem
should be used to suppress warnings in system headers onlyand the libprotobuf header files are not system files.
The correct fix is to compile with less restrictions and remove
-Werror
for the build.As using
-isystem
is reordering GCCs search path and using-isystem
is really not necessary, remove the faulty replacement of
-I
.Closes: #1125