Bug 204480 – Allow for partially-built SafariServices when building WebKit

• Component: WebKit Misc.
• Status: NEW

[Keith Rollin]

WebKit imports <SafariServices/SSReadingList.h> in a couple of places. Normally, this framework header can be found in the iOS SDK. However, in some build configurations, a local, partially-built version of SafariServices will exist. This partially-built framework will have an empty Headers directory. This empty directory will hide the SDK directory, causing the WebKit build to fail because it can't find the SSReadingList.h header. Address this by copying the bits we need from SSReadingList.h into WebKit as a fallback for the times the real header can't be found.

[Keith Rollin]

<rdar://problem/57371344>

[Keith Rollin]

Created attachment 384102 [details]
Patch

[Keith Rollin]

Adding some more people who might review this.

[Jonathan Bedard]

Comment on attachment 384102 [details]
Patch

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

> Source/WebKit/UIProcess/ios/WKActionSheetAssistant.mm:56
> +#if __has_include(<SafariServices/SSReadingList.h>)

Seems like it would be better to move this check inside of SSReadingListSPI.h

[Keith Rollin]

OK, will change.

[Keith Rollin]

Created attachment 384739 [details]
Patch

[Sam Weinig]

What causes this condition to exist? What has changed to make this start happening now?

[Tim Horton]

(In reply to Sam Weinig from comment #7)
> What causes this condition to exist? What has changed to make this start
> happening now?

This is not new (not even sort of!). It happens when the SafariServices build fails or is cancelled after creating the framework but before copying the headers, which is not uncommon.

[Sam Weinig]

(In reply to Tim Horton from comment #8)
> (In reply to Sam Weinig from comment #7)
> > What causes this condition to exist? What has changed to make this start
> > happening now?
> 
> This is not new (not even sort of!). It happens when the SafariServices
> build fails or is cancelled after creating the framework but before copying
> the headers, which is not uncommon.

Duplicating a header seems like an unfortunate workaround, and one that won't scale. 

Were any alternative solutions considered? For instance, and this may be worse, running a script at the beginning of the build which removes any partially constructed frameworks from the build products? Or getting Xcode to stop doing this?

[Keith Rollin]

> What causes this condition to exist?

This condition can exist when building with XCBuild and when using a workspace-based scheme that allows all projects to be examined and built in parallel. SafariServices will start to build and will create its (empty) directory structure, but will then suspend until WebKit is done building.

> What has changed to make this start happening now?

The attempt to enable XCBuild for WebKit builds.

> Duplicating a header seems like an unfortunate workaround, and one that won't scale. 

Yeah, I don't particularly fancy it. But it was approachable with a minimum of fuss. And duplicating the bits of the header that we needed seemed feasible given that the source header doesn't change much, and was inline with the approach in our our SPI files. That said, I'm open to other suggestions. (The one you give doesn't really work given the original motivating task of supporting XCBuild.)

[Keith Rollin]

Even if r+'d, I'm going to hold off checking this in. XCBuild may have a change that can help us in Radar 57371055.

[Jonathan Bedard]

(In reply to Sam Weinig from comment #9)
> (In reply to Tim Horton from comment #8)
> > (In reply to Sam Weinig from comment #7)
> > > What causes this condition to exist? What has changed to make this start
> > > happening now?
> > 
> > This is not new (not even sort of!). It happens when the SafariServices
> > build fails or is cancelled after creating the framework but before copying
> > the headers, which is not uncommon.
> 
> Duplicating a header seems like an unfortunate workaround, and one that
> won't scale. 
> 
> Were any alternative solutions considered? For instance, and this may be
> worse, running a script at the beginning of the build which removes any
> partially constructed frameworks from the build products? Or getting Xcode
> to stop doing this?

We do this sort of header duplication pretty frequently with system headers, the only difference here is that we're dealing with a system header we own. It's a bit unfortunate, but header duplication is already a cost we seem to have accepted.

[Tim Horton]

(In reply to Jonathan Bedard from comment #12)
> (In reply to Sam Weinig from comment #9)
> > (In reply to Tim Horton from comment #8)
> > > (In reply to Sam Weinig from comment #7)
> > > > What causes this condition to exist? What has changed to make this start
> > > > happening now?
> > > 
> > > This is not new (not even sort of!). It happens when the SafariServices
> > > build fails or is cancelled after creating the framework but before copying
> > > the headers, which is not uncommon.
> > 
> > Duplicating a header seems like an unfortunate workaround, and one that
> > won't scale. 
> > 
> > Were any alternative solutions considered? For instance, and this may be
> > worse, running a script at the beginning of the build which removes any
> > partially constructed frameworks from the build products? Or getting Xcode
> > to stop doing this?
> 
> We do this sort of header duplication pretty frequently with system headers,
> the only difference here is that we're dealing with a system header we own.
> It's a bit unfortunate, but header duplication is already a cost we seem to
> have accepted.

Not the same thing. This is a public header, so the usual reasons for having a copy in the tree do not apply.