Bug 182980

Summary:

[CMake] Split declaration of JSC headers into public and private

Product:

WebKit

Reporter:

Don Olmstead <don.olmstead>

Component:

Tools / Tests

Assignee:

Don Olmstead <don.olmstead>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, annulen, commit-queue, ews-watchlist, keith_miller, lforschler, mcatanzaro, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	mcatanzaro: review-
Patch	don.olmstead: review-
Patch	none
Patch	none

Don Olmstead

Reported 2018-02-20 11:57:31 PST

JavaScriptCore headers in Xcode are listed as both private and public. The CMake declarations should reflect that.

Attachments
Patch (4.87 KB, patch) 2018-02-20 11:58 PST, Don Olmstead	mcatanzaro: review-	Details Formatted Diff Diff
Patch (5.87 KB, patch) 2018-02-20 13:08 PST, Don Olmstead	don.olmstead: review-	Details Formatted Diff Diff
Patch (6.63 KB, patch) 2018-02-20 13:47 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Patch (6.66 KB, patch) 2018-02-20 16:19 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Don Olmstead

Comment 1 2018-02-20 11:58:31 PST

Created attachment 334291 [details] Patch Splits everything into PUBLIC_FRAMEWORK_HEADERS and PRIVATE_FRAMEWORK_HEADERS.

Michael Catanzaro

Comment 2 2018-02-20 12:25:56 PST

Comment on attachment 334291 [details] Patch Problem is the public headers are different for different ports. GTK and WPE hardcode the list of non-Objective C headers in PlatformGTK.cmake and PlatformWPE.cmake. Best to clean this up now: add two separate lists of public framework headers, one for the C API, and one for Objective C. And get rid of the WPE and GTK-specific lists.

Don Olmstead

Comment 3 2018-02-20 13:08:12 PST

Created attachment 334297 [details] Patch Fixing based on review comments

Don Olmstead

Comment 4 2018-02-20 13:42:23 PST

Comment on attachment 334297 [details] Patch Ugh a windows file didn't end up here... Another patch it is...

Don Olmstead

Comment 5 2018-02-20 13:47:35 PST

Created attachment 334300 [details] Patch

EWS Watchlist

Comment 6 2018-02-20 13:48:48 PST

Attachment 334300 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/PlatformMac.cmake:20: Alphabetical sorting problem. "API/JSContext.h" should be before "API/JavaScriptCore.h". [list/order] [5] Total errors found: 1 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.

Michael Catanzaro

Comment 7 2018-02-20 15:45:41 PST

Comment on attachment 334300 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=334300&action=review Windows EWS is still sad. > Source/JavaScriptCore/CMakeLists.txt:343 > + API/JavaScript.h I guess it makes sense to placate style checker and move it down below JSValueRef.h. > Source/JavaScriptCore/PlatformMac.cmake:18 > +list(APPEND JavaScriptCore_PUBLIC_FRAMEWORK_HEADERS Good idea!

Don Olmstead

Comment 8 2018-02-20 16:19:45 PST

Created attachment 334315 [details] Patch Hopefully making style checker and winders happy

Don Olmstead

Comment 9 2018-02-21 11:26:26 PST

Michael we good now?

Michael Catanzaro

Comment 10 2018-02-21 11:40:43 PST

Comment on attachment 334315 [details] Patch I think so!

Don Olmstead

Comment 11 2018-02-21 11:41:49 PST

Comment on attachment 334315 [details] Patch Thanks!

WebKit Commit Bot

Comment 12 2018-02-21 14:02:08 PST

Comment on attachment 334315 [details] Patch Clearing flags on attachment: 334315 Committed r228898: <https://trac.webkit.org/changeset/228898>

WebKit Commit Bot

Comment 13 2018-02-21 14:02:10 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 14 2018-02-21 14:03:56 PST

<rdar://problem/37760307>

Note You need to log in before you can comment on or make changes to this bug.