RESOLVED FIXED 168878
Add preprocessor guards to Mac specific files that are exposed as private headers on iOS 
https://bugs.webkit.org/show_bug.cgi?id=168878
Summary Add preprocessor guards to Mac specific files that are exposed as private hea...
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-
DetailsFormatted DiffDiff
proposed patch (1.79 KB, patch)
2017-02-26 18:56 PST, Alexey Proskuryakov 		no flags
DetailsFormatted DiffDiff
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.