Bug 182303 – [CMake] Make JavaScriptCore headers copies

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Don Olmstead]

JavaScriptCore headers should be copied during the build to match the behavior of Apple ports.

[Don Olmstead]

Created attachment 332701 [details]
Patch

Trying on the bots

[EWS Watchlist]

Attachment 332701 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/CMakeLists.txt:818:  Alphabetical sorting problem. "runtime/MemoryStatistics.h" should be before "runtime/Microtask.h".  [list/order] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 332703 [details]
Patch

Maybe make GTK happy

[EWS Watchlist]

Attachment 332703 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/CMakeLists.txt:819:  Alphabetical sorting problem. "runtime/MemoryStatistics.h" should be before "runtime/Microtask.h".  [list/order] [5]
Total errors found: 1 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 332726 [details]
Patch

[Don Olmstead]

Created attachment 332731 [details]
Patch

Removing gtest changes...

[Don Olmstead]

Ok so I think I need a bit more background here to figure out how to make this all work.

So there are some instances of includes like

// From CommonVM.cpp
#include <runtime/VM.h>

But there are also other instances like

// From InbandTextTrackPrivateAVCF.cpp
#include <JavaScriptCore/VM.h>

Are apple builds installing certain headers in certain places rather than it being completely flat?

[Don Olmstead]

Created attachment 332800 [details]
Patch

Come on GTK/WPE bots

[Don Olmstead]

Created attachment 332806 [details]
Patch

Trying this again....

[Don Olmstead]

I'm not sure what is up with the GTK/WPE bots.

In file included from ../../Source/WebCore/config.h:37:0,
                 from DerivedSources/WebCore/JSXMLSerializer.cpp:21,
                 from /home/ews/igalia-wpe-ews/WebKit/WebKitBuild/Release/DerivedSources/WebCore/unified-sources/UnifiedSource105.cpp:1:
DerivedSources/ForwardingHeaders/JavaScriptCore/JSExportMacros.h:1:51: fatal error: JavaScriptCore/runtime/JSExportMacros.h: No such file or directory
 #include "JavaScriptCore/runtime/JSExportMacros.h"
                                                   ^

It looks like they're not using copies. mcatanzaro if you can assist in debugging that would be greatly appreciated as I don't have a GTK environment immediately handy.

[Michael Catanzaro]

I'll take a look either later tonight or tomorrow.

[Fujii Hironori]

generate-forwarding-headers.pl is invoked in Source/WebKit/PlatformGTK.cmake.
This script overwrites the header files.

[Michael Catanzaro]

(In reply to Fujii Hironori from comment #12)
> generate-forwarding-headers.pl is invoked in Source/WebKit/PlatformGTK.cmake.
> This script overwrites the header files.

Fujii :)

Good now? Still need me to look at it?

[Don Olmstead]

Created attachment 332826 [details]
Patch

Remove JavaScriptCore from the generate-forwarding-headers.py script

[Don Olmstead]

(In reply to Fujii Hironori from comment #12)
> generate-forwarding-headers.pl is invoked in Source/WebKit/PlatformGTK.cmake.
> This script overwrites the header files.

Good catch! Forgot about that script...

(In reply to Michael Catanzaro from comment #13)
> (In reply to Fujii Hironori from comment #12)
> > generate-forwarding-headers.pl is invoked in Source/WebKit/PlatformGTK.cmake.
> > This script overwrites the header files.
> 
> Fujii :)
> 
> Good now? Still need me to look at it?

Depends if this patch works now ;)

[Don Olmstead]

Created attachment 332840 [details]
Patch

Missed some forwarding headers in my script so trying again with thi.

[EWS Watchlist]

Attachment 332840 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/CMakeLists.txt:573:  There should be exactly one empty line instead of 0 between "inspector/remote/RemoteInspector.h" and "inspector/remote/cocoa/RemoteInspectorXPCConnection.h".  [list/emptyline] [5]
ERROR: Source/JavaScriptCore/CMakeLists.txt:574:  There should be exactly one empty line instead of 0 between "inspector/remote/cocoa/RemoteInspectorXPCConnection.h" and "inspector/remote/glib/RemoteInspectorServer.h".  [list/emptyline] [5]
ERROR: Source/JavaScriptCore/CMakeLists.txt:624:  There should be exactly one empty line instead of 0 between "parser/VariableEnvironment.h" and "profiler/ProfilerBytecode.h".  [list/emptyline] [5]
Total errors found: 3 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 332841 [details]
Patch

Ok I think this will at least get WPE going.

[EWS Watchlist]

Attachment 332841 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Created attachment 332842 [details]
Patch

[Don Olmstead]

Comment on attachment 332842 [details]
Patch

Ok looks like GTK and WPE are good now.

So this patch enumerates all the headers within JSC and installs them into DerivedSources/ForwardingHeaders. The generate-forwarding-headers.pl script is modified to remove creation of headers within JSC.

It also makes it so all the cmake files are only adding additional includes that are port specific.

Just a warning Michael the EWS bots seemed to have trouble with this patch so when it lands the GTK/WPE bots might need to do a clean build to get rid of the old forwarding headers.

[WebKit Commit Bot]

Comment on attachment 332842 [details]
Patch

Clearing flags on attachment: 332842

Committed r227952: <https://trac.webkit.org/changeset/227952>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37103982>