Bug 167608 - Don't migrate WebKit DOM headers in MigrateHeaders.make
Summary: Don't migrate WebKit DOM headers in MigrateHeaders.make
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-30 15:08 PST by Aakash Jain
Modified: 2017-02-08 22:09 PST (History)
6 users (show)

See Also:

Attachments
Proposed patch (7.97 KB, patch)
2017-01-30 15:35 PST, Aakash Jain 		ap: review-
ap: commit-queue-
Details | Formatted Diff | Diff
proposed patch (152.87 KB, patch)
2017-02-07 17:16 PST, Alexey Proskuryakov 		no flags Details | Formatted Diff | Diff
proposed patch (154.23 KB, patch)
2017-02-08 00:05 PST, Alexey Proskuryakov 		darin: review+
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 Aakash Jain 2017-01-30 15:08:58 PST 
Source/WebKit/mac/MigrateHeaders.make copies the headers from WebCore to WebKit. However the file is out of date.

As part of https://bugs.webkit.org/show_bug.cgi?id=160654 , many DOM headers were moved from WebCore to WebKit. We should cleanup these files from MigrateHeaders.make as well.
Comment 1 Aakash Jain 2017-01-30 15:35:32 PST 
Created attachment 300152 [details]
Proposed patch
Comment 2 Alexey Proskuryakov 2017-01-30 16:57:29 PST 
The current version of this patch this breaks the build.
Comment 3 Alexey Proskuryakov 2017-02-07 17:14:07 PST 
More changes are needed here, taking this.
Comment 4 Alexey Proskuryakov 2017-02-07 17:16:10 PST 
Created attachment 300863 [details]
proposed patch
Comment 5 WebKit Commit Bot 2017-02-07 17:18:58 PST 
Attachment 300863 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/DOM/DOMTokenList.mm:28:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/mac/DOM/DOMTokenList.mm:32:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/WebKit/mac/DOM/DOMImplementation.mm:27:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 3 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Alexey Proskuryakov 2017-02-07 17:55:20 PST 
Forgot to make deleting iOS headers conditional. Will fix.
Comment 7 mitz 2017-02-07 20:29:50 PST 
Comment on attachment 300863 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300863&action=review

> Source/WebKit/mac/ChangeLog:45
> +        * postprocess-headers.sh: There are two SPI headers that are only exposed on iOS.
> +        This is the cleanest way I found to avoid exposing them on Mac (we don't migrate
> +        them, so it would be wrong to keep the logic in MigrateHeaders.make).

Have you considered naming the two headers in the EXCLUDED_SOURCE_FILES build setting when targeting macOS? If it works, then it’s somewhat cleaner in that the files do not get copied, then deleted, in each incremental build of the framework.
Comment 8 Alexey Proskuryakov 2017-02-08 00:05:38 PST 
Created attachment 300882 [details]
proposed patch

It works, thank you Dan!
Comment 9 WebKit Commit Bot 2017-02-08 00:07:47 PST 
Attachment 300882 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/DOM/DOMTokenList.mm:28:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WebKit/mac/DOM/DOMTokenList.mm:32:  Bad include order. Mixing system and custom headers.  [build/include_order] [4]
ERROR: Source/WebKit/mac/DOM/DOMImplementation.mm:27:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 3 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Alexey Proskuryakov 2017-02-08 22:09:04 PST 
Committed http://trac.webkit.org/r211931