On 2013-07-25 13:25, Stephen Kelly wrote:
Brad King wrote:
On 07/25/2013 12:22 PM, Stephen Kelly wrote:
library A should have a unit test which attempts to compile all
of its headers with all warnings enabled. In Qt every module has a
'headersclean' unit test which does exactly that.
While this is a good idea we're not going to assume every project has
such discipline. Also some consumers may include headers with a
different preprocessing context that exposes the warning.
Right. Even if all headers are used by the upstream itself, this still
applies. However, I still think IMPORTED=SYSTEM by *default* is a good idea,
and let the edge case switch it back. I'm not seeing support for it though,
so I guess I'll drop it.
Somewhat echoing Brad's reply here, but it's not that I'm opposed to the
idea, merely "concerned" that it is possible to get non-system includes
when that is desired.
Perhaps that is a misreading on my part, but I was getting the
impression this change would make it so that imported targets can only
ever be SYSTEM.
Then why not make INTERFACE_SYSTEM_INCLUDE_DIRECTORIES the default in
the Qt imported targets and have a switch to turn them off? The code
set(QT_INCLUDE_DIRS_NO_SYSTEM 1)
find_package(Qt5Core)
is not so bad in the edge case that needs it.
I don't like it though :). I'd prefer not to have any variables that change
the behavior of a find_package call, so that only one find_package is ever
needed, not multiple in different directories for scope.
FWIW, I agree; variables controlling find_package (except search paths)
are almost always an inferior solution :-).
That said... what about having a SYSTEM (and/or NO_SYSTEM) flag for
find_package? This could, for config-based packages with imported
targets, control the default behavior for imported include directories.
Either way, the tll() SYSTEM option could still be useful.
I'll add that tomorrow.
Again FWIW, a nice advantage of this is the ability to specify SYSTEM
includes per target. (I'm not sure why you'd want to build one target
with imported includes as SYSTEM and another not, but you could...)
--
Matthew
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
