| Summary: | [CMake] Avoid building WebCore with ANGLE's OpenGL/EGL headers | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Zan Dobersek <zan> |
| Component: | New Bugs | Assignee: | Zan Dobersek <zan> |
| Status: | RESOLVED FIXED | ||
| Severity: | Normal | CC: | buildbot, bunhere, clopez, commit-queue, gyuyoung.kim, mrobinson, rakuco, rniwa, sergio |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Attachments: | |||
|
Description
Zan Dobersek
2014-07-22 11:53:25 PDT
Created attachment 235304 [details]
Patch
Comment on attachment 235304 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235304&action=review > Source/WebCore/CMakeLists.txt:3608 > - list(APPEND WebCore_LIBRARIES ANGLESupport) > + target_include_directories(ANGLESupport PRIVATE "${THIRDPARTY_DIR}/ANGLE/include") > + target_link_libraries(WebCore ANGLESupport) Why use target_link_libraries directly here instead of WebCore_LIBRARIES? Comment on attachment 235304 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235304&action=review >> Source/WebCore/CMakeLists.txt:3608 >> + target_link_libraries(WebCore ANGLESupport) > > Why use target_link_libraries directly here instead of WebCore_LIBRARIES? In ChangeLog I mentioned this readds the ANGLESupport's include directories to WebCore. I retested it now and that might not be true. I'll re-check it and adjust accordingly before landing. Created attachment 235347 [details]
Patch for landing
Testing through the EWS.
Comment on attachment 235347 [details] Patch for landing Attachment 235347 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6539658175447040 New failing tests: media/W3C/video/networkState/networkState_during_loadstart.html Created attachment 235349 [details]
Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 235347 [details] Patch for landing Attachment 235347 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6734099934871552 New failing tests: media/W3C/video/networkState/networkState_during_loadstart.html Created attachment 235350 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
(In reply to comment #7) > (From update of attachment 235347 [details]) > Attachment 235347 [details] did not pass mac-wk2-ews (mac-wk2): > Output: http://webkit-queues.appspot.com/results/6734099934871552 > > New failing tests: > media/W3C/video/networkState/networkState_during_loadstart.html The patch doesn't affect this. Comment on attachment 235347 [details] Patch for landing Clearing flags on attachment: 235347 Committed r171475: <http://trac.webkit.org/changeset/171475> All reviewed patches have been landed. Closing bug. Comment on attachment 235347 [details] Patch for landing View in context: https://bugs.webkit.org/attachment.cgi?id=235347&action=review > Source/WebCore/CMakeLists.txt:3607 > + target_include_directories(ANGLESupport PRIVATE "${THIRDPARTY_DIR}/ANGLE/include") target_include_directories is introduced at cmake 2.8.11 ( http://www.kitware.com/blog/home/post/492 ) And minimum required version is 2.8.3. Should we bump the cmake version for this? (In reply to comment #12) > (From update of attachment 235347 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235347&action=review > > > Source/WebCore/CMakeLists.txt:3607 > > + target_include_directories(ANGLESupport PRIVATE "${THIRDPARTY_DIR}/ANGLE/include") > > target_include_directories is introduced at cmake 2.8.11 ( http://www.kitware.com/blog/home/post/492 ) > And minimum required version is 2.8.3. > > Should we bump the cmake version for this? Thanks for the reminder. Bumping the required version in bug #135382. |