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

project_loader: use -isystem instead of -I for system include paths #2974

Merged
merged 2 commits into from
Mar 17, 2020

Conversation

cjp256
Copy link
Contributor

@cjp256 cjp256 commented Mar 13, 2020

project_loader: use -isystem instead of -I for system include paths

Warnings are ignored for sytem includes. However, when a system
include gets installed to staging directory, the user may be
issued warnings (errors if -Werror) because snapcraft is adding
include paths thats include these system headers.

Use -isystem rather than -I to add system include paths to
the default compiler flags. The other option is to use -idirafter,
but is more likely to affect current builds by changing the order
of the search paths.

For example:

In file included from /root/parts/workspace/install/usr/include/x86_64-linux-gnu/bits/fcntl.h:61:0,
from /root/parts/workspace/install/usr/include/fcntl.h:35,
from /root/parts/workspace/build/benchmark_catkin/benchmark_src-prefix/src/benchmark_src/src/sysinfo.cc:23:
/root/parts/workspace/install/usr/include/x86_64-linux-gnu/bits/fcntl-linux.h:355:27: error: ISO C++ forbids zero-size array ‘f_handle’ [-Wpedantic]
unsigned char f_handle[0];

See:

[1] https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html

The header files declaring interfaces to the operating system and runtime libraries often cannot be written in strictly conforming C. Therefore, GCC gives code found in system headers special treatment. All warnings, other than those generated by ‘#warning’ (see Diagnostics), are suppressed while GCC is processing a system header.

[2] https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html

The -isystem and -idirafter options also mark the directory as a system directory, so that it gets the same special treatment that is applied to the standard system directories.

Signed-off-by: Chris Patterson chris.patterson@canonical.com

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

Warnings are ignored for sytem includes.  However, when a system
include gets installed to staging directory, the user may be
issued warnings (errors if -Werror) because snapcraft is adding
include paths thats include these system headers.

Use `-isystem` rather than `-I` to add system include paths to
the default compiler flags.  The other option is to use `-idirafter`,
but is more likely to affect current builds by changing the order
of the search paths.

For example:
```
In file included from /root/parts/workspace/install/usr/include/x86_64-linux-gnu/bits/fcntl.h:61:0,
from /root/parts/workspace/install/usr/include/fcntl.h:35,
from /root/parts/workspace/build/benchmark_catkin/benchmark_src-prefix/src/benchmark_src/src/sysinfo.cc:23:
/root/parts/workspace/install/usr/include/x86_64-linux-gnu/bits/fcntl-linux.h:355:27: error: ISO C++ forbids zero-size array ‘f_handle’ [-Wpedantic]
unsigned char f_handle[0];
```

See:

[1] https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html

> The header files declaring interfaces to the operating system and runtime libraries often cannot be written in strictly conforming C. Therefore, GCC gives code found in system headers special treatment. All warnings, other than those generated by ‘#warning’ (see Diagnostics), are suppressed while GCC is processing a system header.

[2] https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html

> The -isystem and -idirafter options also mark the directory as a system directory, so that it gets the same special treatment that is applied to the standard system directories.

Signed-off-by: Chris Patterson <chris.patterson@canonical.com>
@codecov-io
Copy link

Codecov Report

Merging #2974 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2974   +/-   ##
=======================================
  Coverage   88.17%   88.17%           
=======================================
  Files         229      229           
  Lines       16575    16575           
  Branches     2559     2559           
=======================================
  Hits        14615    14615           
  Misses       1424     1424           
  Partials      536      536
Impacted Files Coverage Δ
snapcraft/internal/project_loader/_env.py 95.74% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f89aef...9d9f23a. Read the comment docs.

Copy link
Contributor

@kyrofa kyrofa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find @cjp256! Makes sense and looks good to me.

@sergiusens sergiusens merged commit d3cd37c into master Mar 17, 2020
@sergiusens sergiusens deleted the fix-isystem branch March 17, 2020 13:20
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

4 participants