Bug 248202 - [CMake] Use target_precompile_headers for Windows
Summary: [CMake] Use target_precompile_headers for Windows
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CMake (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords:
Depends on: 248246
Blocks:
  Show dependency tree / graph
 
Reported: 2022-11-21 20:28 PST by Fujii Hironori
Modified: 2022-11-27 22:39 PST (History)
3 users (show)

See Also:

Attachments
Patch (26.58 KB, patch)
2022-11-21 20:33 PST, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Patch (25.18 KB, patch)
2022-11-21 20:36 PST, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Patch (26.60 KB, patch)
2022-11-27 19:38 PST, Fujii Hironori 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2022-11-21 20:28:15 PST 
Let's use target_precompile_headers.
Comment 1 Fujii Hironori 2022-11-21 20:33:52 PST 
Created attachment 463654 [details]
Patch
Comment 2 Fujii Hironori 2022-11-21 20:36:41 PST 
Created attachment 463655 [details]
Patch
Comment 3 Don Olmstead 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.
Comment 4 Don Olmstead 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.
Comment 5 Fujii Hironori 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
Comment 6 Fujii Hironori 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.
Comment 7 Fujii Hironori 2022-11-27 17:10:08 PST 
257053@main (bug#248289) bumpded cmake_minimum_required. I'm going to try CMAKE_DISABLE_PRECOMPILE_HEADERS.
Comment 8 Fujii Hironori 2022-11-27 19:38:39 PST 
Created attachment 463747 [details]
Patch
Comment 9 EWS 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].