View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0013476CMakeModulespublic2012-08-16 03:092013-05-06 09:32
ReporterBenjamin Kloster 
Assigned ToClinton Stimpson 
PrioritynormalSeverityminorReproducibilityN/A
StatusclosedResolutionfixed 
PlatformLinuxOSUbuntuOS Version12.04
Product VersionCMake 2.8.7 
Target VersionCMake 2.8.11Fixed in Version 
Summary0013476: FindQt4: Provide option to include libraries as system headers
DescriptionIncluding Qt headers with a high level of warnings enabled (most notably -Weffc++) causes a plethora of warnings that the user can't fix. These warnings drown out the warnings in the user's code and prevent the usage of the -Werror flag to enforce warning free compilation.

CMake provides the SYSTEM option for the INCLUDE_DIRECTORIES command, instructing GCC (and presumably other compilers as well) to include the directory as "system header", thus ignoring warnings caused within those headers.

It would be of great help if the FindQt4 module provided an option to include the Qt headers as system headers. Attached is a patch for UseQt4.cmake that is included by FindQt4. This patch adds the control flag "QT_INCLUDE_AS_SYSTEM_HEADERS" that can be set before the "find_package(Qt4)", just like QT_USE_XYZ. If set to TRUE, the Qt headers will be included as system headers, otherwise the old behaviour still applies.
TagsFindQt4.cmake
Attached Files? file icon FindQt4.cmake.system_headers_patch [^] (1,488 bytes) 2012-08-16 03:09
patch file icon v2.8.9_FindUseQt4.cmake_system_include.patch [^] (17,788 bytes) 2012-09-19 17:44 [Show Content]
patch file icon v2.8.9_FindUseQt4.cmake_system_include2.patch [^] (2,366 bytes) 2012-09-19 17:49 [Show Content]

 Relationships

  Notes
(0031067)
Scott Bailey (reporter)
2012-09-19 17:44

I think most users will prefer setting the directories to SYSTEM includes, thus a preferable variable is 'QT_INCLUDE_DIRS_NO_SYSTEM'.

Find/UsewxWidgets.cmake follows this convention.

I am attaching a patch with changes modeled after the wxWidgets files.
(0031068)
Scott Bailey (reporter)
2012-09-19 17:51

Please see v2.8.9_FindUseQt4.cmake_system_include2.patch. (Sorry: I forgot to disable delete-trailing-whitespaces from mey editor.)
(0031774)
Thomas Sondergaard (reporter)
2012-11-30 05:09

I'd like to see this patch go in.
(0031775)
Thomas Sondergaard (reporter)
2012-11-30 05:18

I tested v2.8.9_FindUseQt4.cmake_system_include2.patch and it works fine for me
(0031776)
David Cole (manager)
2012-11-30 06:41

Clinton, does the attached patch seem reasonable to you? It is a behavior change, strictly speaking, except on Apple and OpenBSD, but I don't think it would be a harmful one.
(0031777)
Thomas Sondergaard (reporter)
2012-11-30 06:47

If the behaviour change is a concern, the original suggestion can be used, ie require QT_INCLUDE_AS_SYSTEM_HEADERS to be set to include as system headers.
(0031780)
David Cole (manager)
2012-11-30 06:59

Thanks, Thomas. I agree. Although, if most people want the Qt headers as system headers, and there is no serious downside to the slight behavior change, then I wouldn't object to this...

But: If it could cause serious problems for some users, then we should do it in the safe / backwards compatible way...
(0031782)
Clinton Stimpson (developer)
2012-11-30 10:30

My first question is why does it have this?

IF(APPLE OR CMAKE_CXX_PLATFORM_ID MATCHES "OpenBSD")
  SET(QT_INCLUDE_DIRS_NO_SYSTEM 1)
ENDIF()

Is there a problem using include_directories(SYSTEM ...) on Apple and OpenBSD? If so, should we fix include_directories()? Or is this a problem with Qt on those platforms?

There is also a typo
"This variable has o effect for Mac or OpenBSD."
You probably meant to say
"This variable has no effect for Mac or OpenBSD."
(0031793)
Scott Bailey (reporter)
2012-12-01 02:21

Regarding the typo: yes, I can say with authority that is the fix!

Regarding "behavior change": most people, in my limited polling experience, neither increase compiler warnings nor set them as errors. Thus few people will notice the change. Furthermore, I don't think Qt library development--those reliant on the warnings--are using CMake; not for developing the Qt libraries, anyway.


Regarding Apple/OpenBSD and SYSTEM: this was a direct copy from 'UsewxWidgets.cmake'. See defect 0013219 for more info; however, also see defect 0010837. I don't have a readily available Apple to experiment with. Maybe in a few weeks. Neither do I have an OpenBSD install.

Regardless, the problem is related to -isystem for older versions of Apple's flavor of gcc.

OpenBSD is a different story: a quick google search seems to indicate -isystem is not supported... It should probably be disabled in Modules/Compiler/GNU.cmake. But that is a different defect...


I think this fix should NOT include Apple/OpenBSD workarounds...
(0031822)
Clinton Stimpson (developer)
2012-12-03 15:29

691ac05 Qt4: Add SYSTEM option to include_directories.

Thanks for the patch.
(0032981)
Robert Maynard (manager)
2013-05-06 09:32

Closing resolved issues that have not been updated in more than 4 months.

 Issue History
Date Modified Username Field Change
2012-08-16 03:09 Benjamin Kloster New Issue
2012-08-16 03:09 Benjamin Kloster File Added: FindQt4.cmake.system_headers_patch
2012-08-16 03:10 Benjamin Kloster Tag Attached: FindQt4.cmake
2012-09-19 17:44 Scott Bailey Note Added: 0031067
2012-09-19 17:44 Scott Bailey File Added: v2.8.9_FindUseQt4.cmake_system_include.patch
2012-09-19 17:49 Scott Bailey File Added: v2.8.9_FindUseQt4.cmake_system_include2.patch
2012-09-19 17:51 Scott Bailey Note Added: 0031068
2012-11-30 05:09 Thomas Sondergaard Note Added: 0031774
2012-11-30 05:18 Thomas Sondergaard Note Added: 0031775
2012-11-30 06:40 David Cole Target Version => CMake 2.8.11
2012-11-30 06:40 David Cole Assigned To => Clinton Stimpson
2012-11-30 06:40 David Cole Status new => assigned
2012-11-30 06:41 David Cole Note Added: 0031776
2012-11-30 06:47 Thomas Sondergaard Note Added: 0031777
2012-11-30 06:59 David Cole Note Added: 0031780
2012-11-30 10:30 Clinton Stimpson Note Added: 0031782
2012-12-01 02:21 Scott Bailey Note Added: 0031793
2012-12-03 15:29 Clinton Stimpson Note Added: 0031822
2012-12-03 15:29 Clinton Stimpson Status assigned => resolved
2012-12-03 15:29 Clinton Stimpson Resolution open => fixed
2013-05-06 09:32 Robert Maynard Note Added: 0032981
2013-05-06 09:32 Robert Maynard Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team