boostorg / cmake

CMake support infrastructure Boost submodule
94 stars 27 forks source link

Shouldn't boost headers be marked as system in CMake config? #21

Closed ixSci closed 4 months ago

ixSci commented 2 years ago

Right now every boost target provides its headers by setting the following property: INTERFACE_INCLUDE_DIRECTORIES. And there is another property: INTERFACE_SYSTEM_INCLUDE_DIRECTORIES which, I think, should be used as well. I don't know if that should be default or not but I believe it should at least be supported so users can opt-in on boost library being treated as system.

kkaefer commented 1 year ago

Please don’t do this. System headers are treated as immutable by compilers and are exempt from dependency/mtime checks. That means an update of your Boost header won’t trigger an automatic rebuild in all cases and you’ll spend a few hours or days tracking down why your build fails even though all the files are checked out correctly.

jzakrzewski commented 1 year ago

Compilers do not care about the timestamps. For other tool (like ninja) I don't believe it analyzes whether one passes -I/path/to/headers or -isystem /path/to/headers...

tastyherring commented 4 months ago

Marking boosts header as system will allow users to increase the compiler warning level for their projects code and not get flood with warnings cause by boost system headers. This would be very useful.

pdimov commented 4 months ago

IMPORTED targets always use SYSTEM for their includes by default, even for the INTERFACE_INCLUDE_DIRECTORIES property.

See https://cmake.org/cmake/help/latest/prop_tgt/NO_SYSTEM_FROM_IMPORTED.html

ixSci commented 4 months ago

IMPORTED targets always use SYSTEM for their includes by default, even for the INTERFACE_INCLUDE_DIRECTORIES property.

See https://cmake.org/cmake/help/latest/prop_tgt/NO_SYSTEM_FROM_IMPORTED.html

Very good find, thank you! So this issue makes no sense and should be closed, then. Nice of CMake to have this information buried somewhere you can't easily find but what can you do.