Bug 206982 – REGRESSION(r254389): High Sierra build is broken

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: New Bugs

[Myles C. Maxfield]

REGRESSION(r254389): High Sierra build is broken

[Myles C. Maxfield]

Created attachment 389222 [details]
Patch

[Myles C. Maxfield]

<rdar://problem/59005412>

[Myles C. Maxfield]

Created attachment 389223 [details]
Patch

[Alexey Proskuryakov]

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

[Chris Dumez]

Created attachment 389263 [details]
Patch

[Alexey Proskuryakov]

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.

[Chris Dumez]

(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

[Chris Dumez]

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.

[Myles C. Maxfield]

(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.

[Chris Dumez]

(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.

[Alexey Proskuryakov]

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 ***