Skip to content
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 #1130

Closed
wants to merge 1 commit into from
Closed

Remove replacing -I with -isystem #1130

wants to merge 1 commit into from

Conversation

joerg-krause
Copy link
Contributor

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 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

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
@peternewman
Copy link
Member

Sorry @joerg-krause please can you target our 0.10 branch; 0.10.2 is just branched off and NEWS updated just before we release, where as 0.10 will get a branch that becomes 0.10.3.

@joerg-krause joerg-krause deleted the branch-0.10.2 branch September 27, 2016 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants