Summary: | REGRESSION(r254389): High Sierra build is broken | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||
Component: | New Bugs | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||
Status: | RESOLVED DUPLICATE | ||||||||||
Severity: | Normal | CC: | ap, benjamin, cdumez, cmarcelo, dbates, ews-watchlist, glenn, jbedard, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Myles C. Maxfield
2020-01-29 19:48:56 PST
Created attachment 389222 [details]
Patch
Created attachment 389223 [details]
Patch
Comment on attachment 389223 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389223&action=review r- for changing Mojave code path. Please also see my comment in Radar. > Source/WTF/wtf/PlatformHave.h:578 > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS_FAMILY) High Sierra is 10.13 Mojave is 10.14 Catalina is 10.15 Created attachment 389263 [details]
Patch
I didn't verify whether this fix was sufficient. It is unusual that it's enough to ifdef out includes, but no actual implementation code. (In reply to Alexey Proskuryakov from comment #6) > I didn't verify whether this fix was sufficient. It is unusual that it's > enough to ifdef out includes, but no actual implementation code. I think we are fine because the following gets declared after the #endif: @interface NSLocale () + (nonnull NSArray<NSString *> *)minimizedLanguagesFromLanguages:(nonnull NSArray<NSString *> *)languages; @end Comment on attachment 389263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389263&action=review > Source/WTF/wtf/spi/cocoa/NSLocaleSPI.h:30 > +#if HAVE(NSLOCALE_INTERNATIONALSUPPORTEXTENSIONS) I guess we could have used __has_include(<InternationalSupport/NSLocale+InternationalSupportExtensions.h>) too. (In reply to Alexey Proskuryakov from comment #4) > Comment on attachment 389223 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=389223&action=review > > r- for changing Mojave code path. Please also see my comment in Radar. > > > Source/WTF/wtf/PlatformHave.h:578 > > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS_FAMILY) > > High Sierra is 10.13 > Mojave is 10.14 > Catalina is 10.15 This doesn't change the Mojave code path. The relevant code is already hidden behind a respondsToSelector: check. The selector only exists in Catalina. Therefore, previous OSes don't need the #include at all. (In reply to Myles C. Maxfield from comment #9) > (In reply to Alexey Proskuryakov from comment #4) > > Comment on attachment 389223 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=389223&action=review > > > > r- for changing Mojave code path. Please also see my comment in Radar. > > > > > Source/WTF/wtf/PlatformHave.h:578 > > > +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) || PLATFORM(IOS_FAMILY) > > > > High Sierra is 10.13 > > Mojave is 10.14 > > Catalina is 10.15 > > This doesn't change the Mojave code path. The relevant code is already > hidden behind a respondsToSelector: check. As mentioned on your other bug, I cannot find minimizedLanguagesFromLanguages: in my very recent macOS SDK in the <InternationalSupport/NSLocale+InternationalSupportExtensions.h> header. Is this expected? I only have it in my iOS SDK. > The selector only exists in > Catalina. Therefore, previous OSes don't need the #include at all. After more investigation, we determined that a better fix for this was already landed. It does look like the code is wrong for macOS, but it builds now. *** This bug has been marked as a duplicate of bug 206723 *** |