Bug 168878 - Add preprocessor guards to Mac specific files that are exposed as private headers on iOS
Summary: Add preprocessor guards to Mac specific files that are exposed as private hea...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Safari 9
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-25 21:42 PST by Alexey Proskuryakov
Modified: 2017-02-26 20:32 PST (History)
5 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2017-02-25 21:42:39 PST
We currently have ifdefs in implementation files, but SPI headers ship as is.
Comment 1 Alexey Proskuryakov 2017-02-25 21:44:08 PST
Created attachment 302775 [details]
proposed fix
Comment 2 mitz 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.
Comment 3 Alexey Proskuryakov 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.
Comment 4 mitz 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.
Comment 5 Alexey Proskuryakov 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.
Comment 6 Alexey Proskuryakov 2017-02-26 18:56:29 PST
Created attachment 302800 [details]
proposed patch
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2017-02-26 20:32:58 PST
All reviewed patches have been landed.  Closing bug.