Bug 210132

Summary: [CMake] Use WEBKIT_EXECUTABLE in WebKitTestRunner
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: CMakeAssignee: Don Olmstead <don.olmstead>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, aperez, benjamin, cdumez, cmarcelo, ews-watchlist, gyuyoung.kim, Hironori.Fujii, keith_miller, mark.lam, mcatanzaro, msaboff, ross.kirsling, ryuan.choi, saam, sergio, tzagallo
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
Patch
aperez: review+
Patch
none
Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
Patch none

Description Don Olmstead 2020-04-07 10:41:31 PDT 
...
Comment 1 Don Olmstead 2020-04-07 10:42:22 PDT Comment hidden (obsolete)
Comment 2 Don Olmstead 2020-04-07 10:57:51 PDT Comment hidden (obsolete)
Comment 3 Don Olmstead 2020-04-07 11:43:29 PDT 
Created attachment 395715 [details]
Patch
Comment 4 Adrian Perez 2020-04-07 14:00:25 PDT 
Comment on attachment 395715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395715&action=review

Simplifications are always welcome :]

> Tools/WebKitTestRunner/CMakeLists.txt:34
> +    ${CMAKE_BINARY_DIR}

Just wondering… it seems quite common to add ${CMAKE_BINARY_DIR} as
one of the include directories, so maybe it would make sense to always
add it in the _WEBKIT_TARGET() macro. WDYT?
Comment 5 Fujii Hironori 2020-04-07 14:11:05 PDT 
Comment on attachment 395715 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=395715&action=review

> Tools/WebKitTestRunner/CMakeLists.txt:16
> +    CyclicRedundancyCheck.cpp

You replaced ${WEBKIT_TESTRUNNER_DIR}/CyclicRedundancyCheck.cpp with CyclicRedundancyCheck.cpp by removing ${WEBKIT_TESTRUNNER_DIR}. This change looks good. You should do that more.

> Tools/WebKitTestRunner/CMakeLists.txt:60
> +    ${WebKitTestRunner_INJECTEDBUNDLE_DIR}/TextInputController.cpp

Replace ${WebKitTestRunner_INJECTEDBUNDLE_DIR}/TextInputController.cpp with InjectedBundle/TextInputController.cpp.
Comment 6 Don Olmstead 2020-04-08 10:47:04 PDT 
Created attachment 395833 [details]
Patch
Comment 7 Don Olmstead 2020-04-08 10:53:01 PDT 
Created attachment 395835 [details]
Patch
Comment 8 Don Olmstead 2020-04-10 21:53:33 PDT 
Created attachment 396149 [details]
WIP Patch
Comment 9 Adrian Perez 2020-04-15 07:08:46 PDT 
The failures from the GTK/EWS bots seem to be caused by the compiler
not being able to find the “cmakeconfig.h” file , so we would need to
add ${CMAKE_BINARY_DIR} to the list of include directories for the
different targets.

Would it make sense to have the _WEBKIT_TARGET() always add implicitly
${CMAKE_BINARY_DIR} as an include directory for the targets it creates?
Comment 10 Don Olmstead 2020-04-22 13:22:34 PDT 
Created attachment 397245 [details]
WIP Patch
Comment 11 Don Olmstead 2020-04-22 14:41:32 PDT 
Created attachment 397263 [details]
WIP Patch
Comment 12 Don Olmstead 2020-04-22 14:54:09 PDT 
Created attachment 397266 [details]
WIP Patch
Comment 13 Don Olmstead 2020-04-22 15:26:54 PDT 
Created attachment 397275 [details]
Patch
Comment 14 EWS 2020-04-22 16:41:42 PDT 
Committed r260539: <https://trac.webkit.org/changeset/260539>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 397275 [details].