Makefiles should make webkitdirs aware of the SDK

• Component: Tools / Tests
• Status: NEW ---

[Jonathan Bedard]

Make should work with SDKROOT=iphoneos, for a number of reasons, it does not.

I discovered these issues while fixing https://bugs.webkit.org/show_bug.cgi?id=206618.

[Alexey Proskuryakov]

I'm not sure if we need two ways to build, why is this desirable?

[Jonathan Bedard]

(In reply to Alexey Proskuryakov from comment #1)
> I'm not sure if we need two ways to build, why is this desirable?

Make allows partial builds, you can, for example, just build JavascriptCore. build-webkit (to my knowledge) does not (at least not without a specific script designed to do so)

If we don't intend to support the make workflow for local development, it should fail early and hard. Given that it's so close to working, it seems better to fix it.

[Jonathan Bedard]

Created attachment 388548 [details]
Patch

[Jonathan Bedard]

After exploring this change, I'm quite confused how we've gone this long without this.

This is the reason you have to specify the architecture when running make, after this change, 'make SDKROOT=iphoneos' will work, you won't need 'make SDKROOT=iphoneos ARCHS=arm64'

[Jonathan Bedard]

Created attachment 388573 [details]
Patch

[Jonathan Bedard]

We definitely want some parts of this change (line 371, for example, fixes a bug in build-webkit), but arm64e can't build on our bots at the moment, so this change can't land yet.

[Keith Rollin]

Comment on attachment 388573 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:376
> +            } elsif ($xcodeSDK =~ /^iphoneos/ || /^watchos/ || $xcodeSDK =~ /^appletvos/) {

Does that watchOS regex need to be applied to $xcodeSDK?

[Jonathan Bedard]

Created attachment 388582 [details]
Patch

[Jonathan Bedard]

Created attachment 388717 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 388717 [details]
Patch

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

> Tools/ChangeLog:11
> +        (determineXcodeSDK): Check for SDKROOT=* in passed arguments so that we can
> +        apply the correct arguments for a specific SDK.

I am still skeptical about the idea. Arguments that are passed to make in this way have a simple well defined meaning. Having intermediate scripts intercept them and enable tangentially related magic adds a lot of conceptual complexity. I do not think that we have a practical problem that is worth solving, much less one that is worth solving like this.

[Jonathan Bedard]

Comment on attachment 388717 [details]
Patch

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

>> Tools/ChangeLog:11
>> +        apply the correct arguments for a specific SDK.
> 
> I am still skeptical about the idea. Arguments that are passed to make in this way have a simple well defined meaning. Having intermediate scripts intercept them and enable tangentially related magic adds a lot of conceptual complexity. I do not think that we have a practical problem that is worth solving, much less one that is worth solving like this.

If we don't like the idea of intercepting arguments, then we need to strip webkitdirs.pm of the device specific Xcode options (line 913 through line 922).

The intention of the makefile seems to be to keep our Xcode options unified in a single spot, so that make and build-webkit do mostly the same thing. It seems to me that making Make more like build-webkit is a good thing, but if it's not, then we should make build-webkit more like make.

I also think that build-webkit needs some intermediate interception regardless of what Make is doing. It's clearly not ok for build-webkit to pick an architecture when the user specifies one.

[Jonathan Bedard]

Filed https://bugs.webkit.org/show_bug.cgi?id=207045 for the work in webkitdirs, this bug will just track the work in the Makefile.