Bug 184225 - [Xcode] Adopt WK_PLATFORM_NAME for SUPPORTS_TEXT_BASED_API
Summary: [Xcode] Adopt WK_PLATFORM_NAME for SUPPORTS_TEXT_BASED_API
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-04-01 08:18 PDT by mitz
Modified: 2022-02-09 10:14 PST (History)
7 users (show)

See Also:

Attachments
Base SUPPORTS_TEXT_BASED_API on WK_PLATFORM_NAME (2.10 KB, patch)
2018-04-01 08:23 PDT, mitz 		no flags Details | Formatted Diff | Diff
For EWS (2.10 KB, patch)
2018-04-01 14:55 PDT, mitz 		no flags Details | Formatted Diff | Diff
For EWS (2.21 KB, patch)
2018-04-01 14:57 PDT, mitz 		no flags Details | Formatted Diff | Diff
For EWS (8.31 KB, patch)
2018-04-01 15:39 PDT, mitz 		no flags Details | Formatted Diff | Diff
For EWS (8.29 KB, patch)
2018-04-01 15:57 PDT, mitz 		no flags Details | Formatted Diff | Diff
For EWS (8.36 KB, patch)
2018-04-01 18:37 PDT, mitz 		no flags Details | Formatted Diff | Diff
Show Obsolete (4) 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 mitz 2018-04-01 08:18:26 PDT 
<rdar://problem/39079659>
Comment 1 mitz 2018-04-01 08:23:30 PDT 
Created attachment 336956 [details]
Base SUPPORTS_TEXT_BASED_API on WK_PLATFORM_NAME
Comment 2 Alexey Proskuryakov 2018-04-01 10:14:19 PDT 
Comment on attachment 336956 [details]
Base SUPPORTS_TEXT_BASED_API on WK_PLATFORM_NAME

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

> Source/WebKitLegacy/mac/ChangeLog:3
> +        [Xcode] Adopt WK_PLATFORM_NAME for SUPPORTS_TEXT_BASED_API

I’m very much not sure if we want to do this. Will start an e-mail thread to discuss.

> Source/WebKitLegacy/mac/Configurations/WebKitLegacy.xcconfig:137
> +SUPPORTS_TEXT_BASED_API_iphoneos = $(USE_INTERNAL_SDK);

This aspect of existing behavior is not right, we should have it enabled in open source builds too, to have EWS catch mistakes.
Comment 3 mitz 2018-04-01 10:19:00 PDT 
(In reply to Alexey Proskuryakov from comment #2)
> Comment on attachment 336956 [details]
> Base SUPPORTS_TEXT_BASED_API on WK_PLATFORM_NAME
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=336956&action=review
> 
> > Source/WebKitLegacy/mac/ChangeLog:3
> > +        [Xcode] Adopt WK_PLATFORM_NAME for SUPPORTS_TEXT_BASED_API
> 
> I’m very much not sure if we want to do this. Will start an e-mail thread to
> discuss.
> 
> > Source/WebKitLegacy/mac/Configurations/WebKitLegacy.xcconfig:137
> > +SUPPORTS_TEXT_BASED_API_iphoneos = $(USE_INTERNAL_SDK);
> 
> This aspect of existing behavior is not right, we should have it enabled in
> open source builds too, to have EWS catch mistakes.

I agree. The Apple bug referenced in relation to this is <rdar://problem/24582471>, which essentially suggests that we should do this when all versions of Xcode that are supported for the Open Source project include installapi. Do you know if this is now the case? If not, I can also enable it for a subset of Xcode versions, if I get the list of Xcode versions that are supported for Open Source development yet lack installapi.
Comment 4 Alexey Proskuryakov 2018-04-01 14:39:09 PDT 
InstallAPI should be already supported in all Xcode versions we use with iOS. I did not verify that.
Comment 5 mitz 2018-04-01 14:55:42 PDT 
Created attachment 336962 [details]
For EWS
Comment 6 mitz 2018-04-01 14:57:47 PDT 
Created attachment 336963 [details]
For EWS
Comment 7 mitz 2018-04-01 15:39:28 PDT 
Created attachment 336964 [details]
For EWS
Comment 8 mitz 2018-04-01 15:57:13 PDT 
Created attachment 336965 [details]
For EWS
Comment 9 mitz 2018-04-01 18:37:31 PDT 
Created attachment 336967 [details]
For EWS
Comment 10 mitz 2018-04-01 19:34:39 PDT 
(In reply to mitz from comment #9)
> Created attachment 336967 [details]

This demonstrates that installapi can be enabled when building with the public SDK with only minor changes. However, doing so is separate from what this bug is about, and probably warrants a separate bug with a separate patch.