Bug 248202

Summary: [CMake] Use target_precompile_headers for Windows
Product: WebKit Reporter: Fujii Hironori <fujii.hironori>
Component: CMakeAssignee: Fujii Hironori <fujii.hironori>
Status: RESOLVED FIXED    
Severity: Normal CC: aperez, don.olmstead, ross.kirsling
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=139438
Bug Depends on: 248246    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Fujii Hironori
Reported 2022-11-21 20:28:15 PST
Let's use target_precompile_headers.
Attachments
Patch (26.58 KB, patch)
2022-11-21 20:33 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Patch (25.18 KB, patch)
2022-11-21 20:36 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Patch (26.60 KB, patch)
2022-11-27 19:38 PST, Fujii Hironori 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Fujii Hironori
Comment 1 2022-11-21 20:33:52 PST
Created attachment 463654 [details] Patch
Fujii Hironori
Comment 2 2022-11-21 20:36:41 PST
Created attachment 463655 [details] Patch
Don Olmstead
Comment 3 2022-11-22 10:09:54 PST
Comment on attachment 463655 [details] Patch r=me but I think you should think about adding a check for the CMake version since 3.16 isn't the minimum its 3.12.
Don Olmstead
Comment 4 2022-11-22 10:24:44 PST
Comment on attachment 463655 [details] Patch Actually after thinking about this some more how about we use a variable USE_PRECOMPILED_HEADERS and set it as ON for MSVC and then use that for the check. That way Igalia can experiment with precompiled headers usage with an updated CMake and then its easy for them to turn it on. Also lets just get rid of the WEBKIT_ADD_PRECOMPILED_HEADER macro and just call target_precompile_headers directly.
Fujii Hironori
Comment 5 2022-11-22 12:17:18 PST
Comment on attachment 463655 [details] Patch OK. I will remove WEBKIT_ADD_PRECOMPILED_HEADER macro. And, I will set CMAKE_DISABLE_PRECOMPILE_HEADERS ON unless MSVC. https://cmake.org/cmake/help/latest/variable/CMAKE_DISABLE_PRECOMPILE_HEADERS.html
Fujii Hironori
Comment 6 2022-11-22 12:19:40 PST
No, I can't do that. CMAKE_DISABLE_PRECOMPILE_HEADERS is available since v3.16. I think we should have WEBKIT_ADD_PRECOMPILED_HEADER macro until the minimum version will bump to v3.16.
Fujii Hironori
Comment 7 2022-11-27 17:10:08 PST
257053@main (bug#248289) bumpded cmake_minimum_required. I'm going to try CMAKE_DISABLE_PRECOMPILE_HEADERS.
Fujii Hironori
Comment 8 2022-11-27 19:38:39 PST
Created attachment 463747 [details] Patch
EWS
Comment 9 2022-11-27 22:39:29 PST
Committed 257057@main (48aaac1d1115): <https://commits.webkit.org/257057@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 463747 [details].
Note You need to log in before you can comment on or make changes to this bug.