Bug 209317 – Unable to build WebKit with iOS 13.4 SDK

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

[Ryan Haddad]

Unable to build WebKit with iOS 13.4 SDK

[Radar WebKit Bug Importer]

<rdar://problem/60656534>

[Tim Horton]

Created attachment 394051 [details]
Patch

[EWS]

Committed r258751: <https://trac.webkit.org/changeset/258751>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394051 [details].

[Tim Horton]

Reopening for one follow-up

[Tim Horton]

Created attachment 394552 [details]
Patch

[Tim Horton]

I disagree with all of the stylebot complaints in this case, so I'm gonna ignore it.

[Alexey Proskuryakov]

Hmm, how is a bunch of numeric version checks throughout the code better than a HAVE macro with a descriptive name?

[Tim Horton]

(In reply to Alexey Proskuryakov from comment #7)
> Hmm, how is a bunch of numeric version checks throughout the code better
> than a HAVE macro with a descriptive name?

Because as we currently use it, the HAVE() macro would also be true before? We had it! It just was SPI! Now it's not! Maybe we'd want a bunch of them with names like HAVE(WHATEVER_AS_API)? I think that this is unprecedented, but could go either way.

[Tim Horton]

(In reply to Tim Horton from comment #8)
> (In reply to Alexey Proskuryakov from comment #7)
> > Hmm, how is a bunch of numeric version checks throughout the code better
> > than a HAVE macro with a descriptive name?
> 
> Because as we currently use it, the HAVE() macro would also be true before?
> We had it! It just was SPI! Now it's not! Maybe we'd want a bunch of them
> with names like HAVE(WHATEVER_AS_API)? I think that this is unprecedented,
> but could go either way.

Also, I feel like SPI headers are a very special case, not "throughout the code"...

[Alexey Proskuryakov]

> Also, I feel like SPI headers are a very special case, not "throughout the code"...

There is some truth to that...

[EWS]

Committed r259025: <https://trac.webkit.org/changeset/259025>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394552 [details].

Comment on attachment 394552 [details]
Patch

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

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:233
> +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 130400

This should me MAX_ALLOWED, since the presence of the definitions in the SDK depends on the SDK version used at build time, not the deployment target.

(In reply to Tim Horton from comment #9)

> Also, I feel like SPI headers are a very special case, not "throughout the
> code"...

Yes, since those headers mimic SDK content, as a rule they should be using SDK version conditionals.

[Tim Horton]

(In reply to mitz from comment #13)
> (In reply to Tim Horton from comment #9)
> 
> > Also, I feel like SPI headers are a very special case, not "throughout the
> > code"...
> 
> Yes, since those headers mimic SDK content, as a rule they should be using
> SDK version conditionals.

Haha.

(In reply to mitz from comment #12)
> Comment on attachment 394552 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=394552&action=review
> 
> > Source/WebKit/Platform/spi/ios/UIKitSPI.h:233
> > +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 130400
> 
> This should me MAX_ALLOWED, since the presence of the definitions in the SDK
> depends on the SDK version used at build time, not the deployment target.

Yes, good point! I will fix.

[Tim Horton]

(In reply to Tim Horton from comment #14)
> (In reply to mitz from comment #13)
> > > Source/WebKit/Platform/spi/ios/UIKitSPI.h:233
> > > +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 130400
> > 
> > This should me MAX_ALLOWED, since the presence of the definitions in the SDK
> > depends on the SDK version used at build time, not the deployment target.
> 
> Yes, good point! I will fix.

Hmm, this is actually not working...

[Tim Horton]

(In reply to Tim Horton from comment #15)
> (In reply to Tim Horton from comment #14)
> > (In reply to mitz from comment #13)
> > > > Source/WebKit/Platform/spi/ios/UIKitSPI.h:233
> > > > +#if PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 130400
> > > 
> > > This should me MAX_ALLOWED, since the presence of the definitions in the SDK
> > > depends on the SDK version used at build time, not the deployment target.
> > 
> > Yes, good point! I will fix.
> 
> Hmm, this is actually not working...

Looks like MAX_ALLOWED remains at 130000

[Tim Horton]

Reopening to attach new patch.

[Tim Horton]

Created attachment 394737 [details]
Patch

[EWS]

Committed r259133: <https://trac.webkit.org/changeset/259133>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394737 [details].