[ 
https://issues.apache.org/jira/browse/THRIFT-5018?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Allen Cronce updated THRIFT-5018:
---------------------------------
    Description: 
We saw build errors with Thrift 0.13.0 like the following when compiling with a 
proxy compiler (effectively a script that sits in front of clang) under Linux:
{code:java}
stdlib.h: No such file or directory #include_next <stdlib.h>{code}
Compiling the same Thrift sources with clang 6.0.0 directly on the same system 
worked fine.

We traced it down to the the following compile command line being added in the 
proxy compiler case:
{code:java}
 -isystem /usr/include 
{code}
When we manually removed the above option from a failing command line it 
succeeded.

Searching online indicates that this is a known issue with the above option and 
certain versions of clang and gcc. The typical work around for CMake generated 
projects is to include the following CMake option:
{code:java}
set (CMAKE_NO_SYSTEM_FROM_IMPORTED ON){code}
We import Thrift via cmake using the ExternalProject_Add command. Attempting to 
pass the above option in the usual CMAKE_ARGS to the external project did not 
work. It was too bad because we really didn’t want to change the Thrift sources.

Eventually we found this thread, where the poster was seeing the same problem 
with Thrift:

[https://patchwork.openembedded.org/patch/154233/]

In the end what we did to work around this was to remove the SYSTEM option from 
every include_directories call in the Thrift CMake sources. For example, the 
following:
{code:java}
include_directories(SYSTEM "${Boost_INCLUDE_DIRS}”){code}
Was changed to:
{code:java}
include_directories("${Boost_INCLUDE_DIRS}”){code}
After making this change everywhere in the Thrift cmake sources, the build 
succeeded.

It’s unclear to us the exact mechanism whereby the errant -isystem command is 
being included. Perhaps this is a CMake bug. In our case we were using CMake 
3.14.3.

But there does seem to be some consensus online that using -isystem might cause 
more harm than good. For example, see this thread:

[https://stackoverflow.com/questions/37218953/isystem-on-a-system-include-directory-causes-errors]

And specifically this quote:
{quote}To summarize, be cautious about using -isystem for its error-silencing 
side-effects; if the directory being included is already on the search path, 
the order of the path may be modified and GCC may report errors.
{quote}
If there’s any interest, we can provide a patch to 0.13.0 that removes the 
SYSTEM option. Although it’s just as easy to find them all and remove them if 
you agree that this should be fixed.

  was:
We saw build errors with Thrift 0.13.0 like the following when compiling with a 
proxy compiler (effectively a script that sits in front of clang) under Linux:

 
{code:java}
stdlib.h: No such file or directory #include_next <stdlib.h>{code}
 

Compiling the same Thrift sources with clang 6.0.0 directly on the same system 
worked fine.

We traced it down to the the following compile command line being added in the 
proxy compiler case:

 
{code:java}
 -isystem /usr/include 
{code}
When we manually removed the above option from a failing command line it 
succeeded.

Searching online indicates that this is a known issue with the above option and 
certain versions of clang and gcc. The typical work around for CMake generated 
projects is to include the following CMake option:

 
{code:java}
set (CMAKE_NO_SYSTEM_FROM_IMPORTED ON){code}
 

We import Thrift via cmake using the ExternalProject_Add command. Attempting to 
pass the above option in the usual CMAKE_ARGS to the external project did not 
work. It was too bad because we really didn’t want to change the Thrift sources.

Eventually we found this thread, where the poster was seeing the same problem 
with Thrift:

[https://patchwork.openembedded.org/patch/154233/]

In the end what we did to work around this was to remove the SYSTEM option from 
every include_directories call in the Thrift CMake sources. For example, the 
following:
{code:java}
include_directories(SYSTEM "${Boost_INCLUDE_DIRS}”){code}
Was changed to:
{code:java}
include_directories("${Boost_INCLUDE_DIRS}”){code}
After making this change everywhere in the Thrift cmake sources, the build 
succeeded.

It’s unclear to us the exact mechanism whereby the errant -isystem command is 
being included. Perhaps this is a CMake bug. In our case we were using CMake 
3.14.3.

But there does seem to be some consensus online that using -isystem might cause 
more harm than good. For example, see this thread:

[https://stackoverflow.com/questions/37218953/isystem-on-a-system-include-directory-causes-errors]

And specifically this quote:
{quote}To summarize, be cautious about using -isystem for its error-silencing 
side-effects; if the directory being included is already on the search path, 
the order of the path may be modified and GCC may report errors.
{quote}
If there’s any interest, we can provide a patch to 0.13.0 that removes the 
SYSTEM option. Although it’s just as easy to find them all and remove them if 
you agree that this should be fixed.


> Build failure with clang proxy compiler under Linux due to cmake 
> include_directories SYSTEM option
> --------------------------------------------------------------------------------------------------
>
>                 Key: THRIFT-5018
>                 URL: https://issues.apache.org/jira/browse/THRIFT-5018
>             Project: Thrift
>          Issue Type: Bug
>          Components: Build Process
>    Affects Versions: 0.13.0
>            Reporter: Allen Cronce
>            Priority: Major
>
> We saw build errors with Thrift 0.13.0 like the following when compiling with 
> a proxy compiler (effectively a script that sits in front of clang) under 
> Linux:
> {code:java}
> stdlib.h: No such file or directory #include_next <stdlib.h>{code}
> Compiling the same Thrift sources with clang 6.0.0 directly on the same 
> system worked fine.
> We traced it down to the the following compile command line being added in 
> the proxy compiler case:
> {code:java}
>  -isystem /usr/include 
> {code}
> When we manually removed the above option from a failing command line it 
> succeeded.
> Searching online indicates that this is a known issue with the above option 
> and certain versions of clang and gcc. The typical work around for CMake 
> generated projects is to include the following CMake option:
> {code:java}
> set (CMAKE_NO_SYSTEM_FROM_IMPORTED ON){code}
> We import Thrift via cmake using the ExternalProject_Add command. Attempting 
> to pass the above option in the usual CMAKE_ARGS to the external project did 
> not work. It was too bad because we really didn’t want to change the Thrift 
> sources.
> Eventually we found this thread, where the poster was seeing the same problem 
> with Thrift:
> [https://patchwork.openembedded.org/patch/154233/]
> In the end what we did to work around this was to remove the SYSTEM option 
> from every include_directories call in the Thrift CMake sources. For example, 
> the following:
> {code:java}
> include_directories(SYSTEM "${Boost_INCLUDE_DIRS}”){code}
> Was changed to:
> {code:java}
> include_directories("${Boost_INCLUDE_DIRS}”){code}
> After making this change everywhere in the Thrift cmake sources, the build 
> succeeded.
> It’s unclear to us the exact mechanism whereby the errant -isystem command is 
> being included. Perhaps this is a CMake bug. In our case we were using CMake 
> 3.14.3.
> But there does seem to be some consensus online that using -isystem might 
> cause more harm than good. For example, see this thread:
> [https://stackoverflow.com/questions/37218953/isystem-on-a-system-include-directory-causes-errors]
> And specifically this quote:
> {quote}To summarize, be cautious about using -isystem for its error-silencing 
> side-effects; if the directory being included is already on the search path, 
> the order of the path may be modified and GCC may report errors.
> {quote}
> If there’s any interest, we can provide a patch to 0.13.0 that removes the 
> SYSTEM option. Although it’s just as easy to find them all and remove them if 
> you agree that this should be fixed.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to