Bug 182303 - [CMake] Make JavaScriptCore headers copies
Summary: [CMake] Make JavaScriptCore headers copies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Don Olmstead
URL:
Keywords: InRadar
Depends on:
Blocks: 180064
  Show dependency treegraph
 
Reported: 2018-01-30 12:39 PST by Don Olmstead
Modified: 2018-01-31 22:51 PST (History)
11 users (show)

See Also:


Attachments
Patch (24.46 KB, patch)
2018-01-30 14:09 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (24.35 KB, patch)
2018-01-30 14:14 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (25.93 KB, patch)
2018-01-30 17:00 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (24.35 KB, patch)
2018-01-30 17:18 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (28.67 KB, patch)
2018-01-31 14:07 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (30.80 KB, patch)
2018-01-31 15:04 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (32.14 KB, patch)
2018-01-31 17:23 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (31.62 KB, patch)
2018-01-31 19:09 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (32.58 KB, patch)
2018-01-31 19:28 PST, Don Olmstead
no flags Details | Formatted Diff | Diff
Patch (32.99 KB, patch)
2018-01-31 19:40 PST, Don Olmstead
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Don Olmstead 2018-01-30 12:39:40 PST
JavaScriptCore headers should be copied during the build to match the behavior of Apple ports.
Comment 1 Don Olmstead 2018-01-30 14:09:35 PST
Created attachment 332701 [details]
Patch

Trying on the bots
Comment 2 EWS Watchlist 2018-01-30 14:11:19 PST
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.
Comment 3 Don Olmstead 2018-01-30 14:14:30 PST
Created attachment 332703 [details]
Patch

Maybe make GTK happy
Comment 4 EWS Watchlist 2018-01-30 14:19:48 PST
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.
Comment 5 Don Olmstead 2018-01-30 17:00:48 PST
Created attachment 332726 [details]
Patch
Comment 6 Don Olmstead 2018-01-30 17:18:08 PST
Created attachment 332731 [details]
Patch

Removing gtest changes...
Comment 7 Don Olmstead 2018-01-30 18:44:57 PST
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?
Comment 8 Don Olmstead 2018-01-31 14:07:22 PST
Created attachment 332800 [details]
Patch

Come on GTK/WPE bots
Comment 9 Don Olmstead 2018-01-31 15:04:59 PST
Created attachment 332806 [details]
Patch

Trying this again....
Comment 10 Don Olmstead 2018-01-31 15:52:02 PST
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.
Comment 11 Michael Catanzaro 2018-01-31 16:40:04 PST
I'll take a look either later tonight or tomorrow.
Comment 12 Fujii Hironori 2018-01-31 17:06:21 PST
generate-forwarding-headers.pl is invoked in Source/WebKit/PlatformGTK.cmake.
This script overwrites the header files.
Comment 13 Michael Catanzaro 2018-01-31 17:15:23 PST
(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?
Comment 14 Don Olmstead 2018-01-31 17:23:37 PST
Created attachment 332826 [details]
Patch

Remove JavaScriptCore from the generate-forwarding-headers.py script
Comment 15 Don Olmstead 2018-01-31 17:24:29 PST
(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 ;)
Comment 16 Don Olmstead 2018-01-31 19:09:08 PST
Created attachment 332840 [details]
Patch

Missed some forwarding headers in my script so trying again with thi.
Comment 17 EWS Watchlist 2018-01-31 19:11:38 PST
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.
Comment 18 Don Olmstead 2018-01-31 19:28:28 PST
Created attachment 332841 [details]
Patch

Ok I think this will at least get WPE going.
Comment 19 EWS Watchlist 2018-01-31 19:31:06 PST
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.
Comment 20 Don Olmstead 2018-01-31 19:40:58 PST
Created attachment 332842 [details]
Patch
Comment 21 Don Olmstead 2018-01-31 21:32:43 PST
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.
Comment 22 WebKit Commit Bot 2018-01-31 22:50:28 PST
Comment on attachment 332842 [details]
Patch

Clearing flags on attachment: 332842

Committed r227952: <https://trac.webkit.org/changeset/227952>
Comment 23 WebKit Commit Bot 2018-01-31 22:50:30 PST
All reviewed patches have been landed.  Closing bug.
Comment 24 Radar WebKit Bug Importer 2018-01-31 22:51:45 PST
<rdar://problem/37103982>