WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
144747
[CMake] Ignore warnings in system headers
https://bugs.webkit.org/show_bug.cgi?id=144747
Summary
[CMake] Ignore warnings in system headers
Michael Catanzaro
Reported
2015-05-07 09:25:21 PDT
To be able to see any warnings in our code when compiling with Clang, which gives tons of warnings for problems in other projects' header files, we need to mark them as SYSTEM headers. The first parameter to include_directories() should be SYSTEM, then all warnings from headers in those directories will be suppressed, using -isystem instead of -I.
Attachments
Patch
(26.27 KB, patch)
2015-05-13 11:51 PDT
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Patch
(26.26 KB, patch)
2015-05-13 12:25 PDT
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Patch
(25.39 KB, patch)
2015-05-14 13:33 PDT
,
Michael Catanzaro
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Michael Catanzaro
Comment 1
2015-05-13 08:04:37 PDT
The forthcoming patch is not ready for review, due to concerns about a CMake version bump:
https://lists.webkit.org/pipermail/webkit-dev/2015-May/027436.html
I think we might be able to do a WEBKIT_TARGET_INCLUDE_DIRECTORIES compatibility macro. The implementation would not be pretty, though. But I see almost everywhere we need this is in PlatformGTK.cmake, with just one exception. I'm thinking the path of least resistance is to require CMake 2.8.12 only in PlatformGTK.cmake, and guard the one use that affects Mac with a version check.
Michael Catanzaro
Comment 2
2015-05-13 08:07:23 PDT
(In reply to
comment #1
)
> But I > see almost everywhere we need this is in PlatformGTK.cmake, with just one > exception.
Actually, we don't even need to do that. I was inconsistently treating our bundled ANGLE as a system library to suppress warnings from, but it's not a system library, we should show warnings from its headers, it's our responsibility because we bundle it. And that's the only case that matters for Macs.
Michael Catanzaro
Comment 3
2015-05-13 11:47:52 PDT
This is ready for review. I'm having trouble building it fully do to a different bug; let's see what EWS thinks.
Michael Catanzaro
Comment 4
2015-05-13 11:51:28 PDT
Created
attachment 253040
[details]
Patch
WebKit Commit Bot
Comment 5
2015-05-13 11:53:04 PDT
Attachment 253040
[details]
did not pass style-queue: ERROR: Source/WebKit2/PlatformGTK.cmake:491: Alphabetical sorting problem. "PluginProcess/PluginControllerProxy.cpp" should be before "PluginProcess/EntryPoint/unix/PluginProcessMain.cpp". [list/order] [5] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Michael Catanzaro
Comment 6
2015-05-13 12:25:52 PDT
Created
attachment 253047
[details]
Patch
WebKit Commit Bot
Comment 7
2015-05-13 12:28:24 PDT
Attachment 253047
[details]
did not pass style-queue: ERROR: Source/WebKit2/PlatformGTK.cmake:491: Alphabetical sorting problem. "PluginProcess/PluginControllerProxy.cpp" should be before "PluginProcess/EntryPoint/unix/PluginProcessMain.cpp". [list/order] [5] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Michael Catanzaro
Comment 8
2015-05-14 13:33:00 PDT
Created
attachment 253140
[details]
Patch
WebKit Commit Bot
Comment 9
2015-05-14 13:36:06 PDT
Attachment 253140
[details]
did not pass style-queue: ERROR: Source/WebKit2/PlatformGTK.cmake:491: Alphabetical sorting problem. "PluginProcess/PluginControllerProxy.cpp" should be before "PluginProcess/EntryPoint/unix/PluginProcessMain.cpp". [list/order] [5] Total errors found: 1 in 21 files If any of these errors are false positives, please file a bug against check-webkit-style.
Gyuyoung Kim
Comment 10
2015-05-14 18:56:01 PDT
Comment on
attachment 253140
[details]
Patch I think EFL port also needs to use this patch.
Alex Christensen
Comment 11
2015-05-14 18:57:43 PDT
Comment on
attachment 253140
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=253140&action=review
> CMakeLists.txt:1 > -cmake_minimum_required(VERSION 2.8.8) > +cmake_minimum_required(VERSION 2.8.12)
:)
Gyuyoung Kim
Comment 12
2015-05-14 19:08:27 PDT
Comment on
attachment 253140
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=253140&action=review
>> CMakeLists.txt:1 >> +cmake_minimum_required(VERSION 2.8.12) > > :)
I believe you let Mac cmake ports use newer ver. in near future ;)
Michael Catanzaro
Comment 13
2015-05-14 20:36:04 PDT
(In reply to
comment #11
)
> > CMakeLists.txt:1 > > -cmake_minimum_required(VERSION 2.8.8) > > +cmake_minimum_required(VERSION 2.8.12) > > :)
Ah, I should have called that out: I decided to do this unconditionally since you decided it wouldn't hurt your efforts. Seems easiest to have one minimum for the entire build system.
WebKit Commit Bot
Comment 14
2015-05-18 18:36:05 PDT
Comment on
attachment 253140
[details]
Patch Clearing flags on attachment: 253140 Committed
r184536
: <
http://trac.webkit.org/changeset/184536
>
WebKit Commit Bot
Comment 15
2015-05-18 18:36:10 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug