Bug 240297 - [CMake][VS] PrivateHeaders/WebCore/WebCoreJSBuiltinInternals.h is not updated in incremental build
Summary: [CMake][VS] PrivateHeaders/WebCore/WebCoreJSBuiltinInternals.h is not updated...
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:
Blocks:
 
Reported: 2022-05-10 23:24 PDT by Fujii Hironori
Modified: 2022-05-16 15:57 PDT (History)
7 users (show)

See Also:


Attachments
WIP patch (608 bytes, patch)
2022-05-10 23:28 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (2.75 KB, patch)
2022-05-12 19:27 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2022-05-10 23:24:33 PDT
[CMake][VS] PrivateHeaders/WebCore/WebCoreJSBuiltinInternals.h is not updated in incremental build

1. Build WinCairo with CMake Visual Studio generator
   perl Tools\Scripts\build-webkit --wincairo --debug --no-ninja

2. Edit builtins_generate_internals_wrapper_header.py to append a blank line

diff --git a/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py b/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py
index 8858460a0982..3faa59d20a70 100644
--- a/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py
+++ b/Source/JavaScriptCore/Scripts/wkbuiltins/builtins_generate_internals_wrapper_header.py
@@ -48,6 +48,7 @@ class BuiltinsInternalsWrapperHeaderGenerator(BuiltinsGenerator):
 
         sections = []
         sections.append(self.generate_license())
+        sections.append('')
         sections.append(Template(Templates.DoNotEditWarning).substitute(args))
         sections.append(Template(Templates.HeaderIncludeGuard).substitute(args))
         sections.append(self.generate_secondary_header_includes())

3. Incremental-build WinCairo
   perl Tools\Scripts\build-webkit --wincairo --debug --no-ninja

4. Check WebCoreJSBuiltinInternals.h files

$ ls -l WebKitBuild/Debug/WebCore/**/WebCoreJSBuiltinInternals.h
-rwxrwxrwx 1 fujii fujii 3541 May 11 15:12 WebKitBuild/Debug/WebCore/DerivedSources/WebCoreJSBuiltinInternals.h*
-rwxrwxrwx 1 fujii fujii 3537 May 11 14:30 WebKitBuild/Debug/WebCore/PrivateHeaders/WebCore/WebCoreJSBuiltinInternals.h*

PrivateHeaders/WebCore/WebCoreJSBuiltinInternals.h is not updated.
Comment 1 Fujii Hironori 2022-05-10 23:28:05 PDT
Created attachment 459144 [details]
WIP patch
Comment 2 Fujii Hironori 2022-05-11 23:18:30 PDT
1.
${WebCore_DERIVED_SOURCES_DIR}/WebCoreJSBuiltinInternals.h is generated by this add_custom_command.
https://github.com/WebKit/WebKit/blob/992f5b028f19101857ea6590c3f75bb41f543a65/Source/WebCore/CMakeLists.txt#L2375-L2383

2.
${WebCore_DERIVED_SOURCES_DIR}/WebCoreJSBuiltinInternals.h is added to WebCore_SOURCES.
https://github.com/WebKit/WebKit/blob/992f5b028f19101857ea6590c3f75bb41f543a65/Source/WebCore/CMakeLists.txt#L2388

3.
${WebCore_DERIVED_SOURCES_DIR}/WebCoreJSBuiltinInternals.h is added to WebCore_PRIVATE_FRAMEWORK_HEADERS.
https://github.com/WebKit/WebKit/blob/992f5b028f19101857ea6590c3f75bb41f543a65/Source/WebCore/Headers.cmake#L2116

4.
${WebCore_PRIVATE_FRAMEWORK_HEADERS} are copied to ${WebCore_PRIVATE_FRAMEWORK_HEADERS_DIR}/WebCore directory in WebCore_CopyPrivateHeaders target by this WEBKIT_COPY_FILES.
https://github.com/WebKit/WebKit/blob/992f5b028f19101857ea6590c3f75bb41f543a65/Source/WebCore/CMakeLists.txt#L2438-L2442

5.
WebCore_CopyPrivateHeaders target has a dependency to WebCore target.
https://github.com/WebKit/WebKit/blob/992f5b028f19101857ea6590c3f75bb41f543a65/Source/WebCore/CMakeLists.txt#L2449

6.
WEBKIT_COPY_FILES uses add_custom_command to copy files. It specifies the file as MAIN_DEPENDENCY.
https://github.com/WebKit/WebKit/blob/992f5b028f19101857ea6590c3f75bb41f543a65/Source/cmake/WebKitMacros.cmake#L473

7.
MAIN_DEPENDENCY suggests to Visual Studio generators where to hang the custom command.
https://cmake.org/cmake/help/latest/command/add_custom_command.html

8.
In this case, copying command of ${WebCore_DERIVED_SOURCES_DIR}/WebCoreJSBuiltinInternals.h is attached to the file itself.

9.
${WebCore_DERIVED_SOURCES_DIR}/WebCoreJSBuiltinInternals.h is included into both WebCore.vcxproj and WebCore_CopyPrivateHeaders.vcxproj due to (2) and (3).

10.
If a source file attached with a custom command is belong to multiple vcxproj, CMake VS generator generates the build rule only in the least-dependent vcxproj.
https://gitlab.kitware.com/cmake/cmake/-/commit/f59c33a763ba1483129f0e721bc2394bb7442876

11.
In this case, WebCore.vcxproj is the least-dependent target due to (5).
So, the copying command is in WebCore.vcxproj.

12.
If both generate-js-builtins.py command and the copying command are in WebCore.vcxproj, they don't work as expected.
I don't know the reason.

In the WIP patch, ${WebCore_DERIVED_SOURCES_DIR}/WebCoreJSBuiltinInternals.h isn't added to WebCore_SOURCES.
Then, the copying command is generated in WebCore_CopyPrivateHeaders.vcxproj. It works.
Comment 3 Fujii Hironori 2022-05-12 19:27:53 PDT
Created attachment 459271 [details]
Patch
Comment 4 EWS 2022-05-16 15:57:46 PDT
Committed r294273 (250619@main): <https://commits.webkit.org/250619@main>

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