mason icon indicating copy to clipboard operation
mason copied to clipboard

Use -isystem to pass include paths

Open kkaefer opened this issue 7 years ago • 4 comments

We are currently using -I to pass the paths to Mason packages to the compiler. However, we should be using -isystem to prevent warning messages from this third party code.

In CMake, simply adding SYSTEM to target_include_directories suffices. Unfortunately, the Xcode generator doesn't support this yet. This means there are no next actions here until this is fixed in CMake and deployed widely enough in a few years^H^H^Hdecades

/cc @springmeyer

kkaefer avatar Apr 12 '17 13:04 kkaefer

Re-opening - this shouldn't have been closed by https://github.com/mapbox/mapbox-gl-native/issues/8934 😕

brunoabinader avatar May 09 '17 11:05 brunoabinader

Refs https://github.com/mapbox/mapbox-gl-native/issues/9268.

Looks like the upstream cmake issue is fixed for the upcoming 3.9.0 release. Release candidates are available.

jfirebaugh avatar Jun 14 '17 18:06 jfirebaugh

That is great @jfirebaugh. What needs to happen at the mason level to support this? My understanding is:

  • mason.ini store the raw include paths like include_dirs={prefix}/include (e.g. for geometry.hpp 0.9.1) so the decision about using -isystem or -I can be made by the application using mason.
  • Even for packages that currently push both -I and -isystem (oddly) into the result of mason_cflags (like https://github.com/mapbox/mason/blob/master/scripts/variant/1.1.4/script.sh#L25) still end up with only include_dirs={prefix}/include in the mason.ini.
  • So, once we standardize on using mason.ini everywhere the problem is gone.
  • Although it would be ideal to clean up cruft in existing packages to avoid confusion.

Am I missing anything?

springmeyer avatar Jun 14 '17 18:06 springmeyer

Leaving this up to the application using mason sounds good to me. At that level, it looks like we would need to modify the target_add_mason_package macro in mason.cmake to allow supplying the SYSTEM flag to target_include_directories here, and then update the copy of this file in downstream projects like mapbox-gl-native.

jfirebaugh avatar Jun 14 '17 18:06 jfirebaugh