Bug 168878

Summary:

Add preprocessor guards to Mac specific files that are exposed as private headers on iOS

Product:

WebKit

Reporter:

Alexey Proskuryakov <ap>

Component:

WebKit2

Assignee:

Alexey Proskuryakov <ap>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aakash_jain, commit-queue, joepeck, mitz, sam

Priority:

Version:

Safari 9

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed fix	mitz: review-
proposed patch	none

Alexey Proskuryakov

Reported 2017-02-25 21:42:39 PST

We currently have ifdefs in implementation files, but SPI headers ship as is.

Attachments
proposed fix (1.73 KB, patch) 2017-02-25 21:44 PST, Alexey Proskuryakov	mitz: review-	Details Formatted Diff Diff
proposed patch (1.79 KB, patch) 2017-02-26 18:56 PST, Alexey Proskuryakov	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Alexey Proskuryakov

Comment 1 2017-02-25 21:44:08 PST

Created attachment 302775 [details] proposed fix

mitz

Comment 2 2017-02-26 14:15:48 PST

Comment on attachment 302775 [details] proposed fix We purposefully avoid using EXCLUDED_SOURCE_FILE_NAMES to exclude individual C-family source files in this project (it’s unfortunate that in the mad rush to “upstream” the iOS port, we ended up doing this in other projects and never fully recovered). I suggest adding appropriate preprocessor guards to the files instead.

Alexey Proskuryakov

Comment 3 2017-02-26 15:51:54 PST

Is it actually desirable to ship SPI headers that are fully ifdef'ed out? Using EXCLUDED_SOURCE_FILE_NAMES seems like a better solution to me.

mitz

Comment 4 2017-02-26 16:27:10 PST

(In reply to comment #3) > Is it actually desirable to ship SPI headers that are fully ifdef'ed out? > Using EXCLUDED_SOURCE_FILE_NAMES seems like a better solution to me. EXCLUDED_SOURCE_FILE_NAMES is an out-of-band signal (as opposed to preprocessor macros which are in-band). Its disadvantages include: it is not trivial for people and for the IDE to be aware of it, and it is specified in terms of glob patterns (I suspect many contributors don’t even know what matching rules they obey) that don’t follow the code across renames and refactoring. I believe that those disadvantages outweigh the nicety of excluding the headers from the SDK.

Alexey Proskuryakov

Comment 5 2017-02-26 18:28:37 PST

Normally, I would disagree, as the quality of API/SPI is more important than implementation details. But WebKit2 SPI is in need of much larger cleanup, so this one detail wouldn't make it appreciably better.

Alexey Proskuryakov

Comment 6 2017-02-26 18:56:29 PST

Created attachment 302800 [details] proposed patch

WebKit Commit Bot

Comment 7 2017-02-26 20:32:54 PST

Comment on attachment 302800 [details] proposed patch Clearing flags on attachment: 302800 Committed r213022: <http://trac.webkit.org/changeset/213022>

WebKit Commit Bot

Comment 8 2017-02-26 20:32:58 PST

All reviewed patches have been landed. Closing bug.

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