Bug 206620 - Makefiles should make webkitdirs aware of the SDK
Summary: Makefiles should make webkitdirs aware of the SDK
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-22 15:37 PST by Jonathan Bedard
Modified: 2020-01-31 07:50 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.95 KB, patch)
2020-01-23 08:15 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (3.88 KB, patch)
2020-01-23 12:01 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (3.89 KB, patch)
2020-01-23 13:00 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (3.88 KB, patch)
2020-01-24 12:17 PST, Jonathan Bedard
jbedard: review?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2020-01-22 15:37:46 PST
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.
Comment 1 Alexey Proskuryakov 2020-01-22 17:13:59 PST
I'm not sure if we need two ways to build, why is this desirable?
Comment 2 Jonathan Bedard 2020-01-22 17:40:36 PST
(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.
Comment 3 Jonathan Bedard 2020-01-23 08:15:33 PST
Created attachment 388548 [details]
Patch
Comment 4 Jonathan Bedard 2020-01-23 08:17:29 PST
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'
Comment 5 Jonathan Bedard 2020-01-23 12:01:30 PST
Created attachment 388573 [details]
Patch
Comment 6 Jonathan Bedard 2020-01-23 12:09:39 PST
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.
Comment 7 Keith Rollin 2020-01-23 12:43:54 PST
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?
Comment 8 Jonathan Bedard 2020-01-23 13:00:28 PST
Created attachment 388582 [details]
Patch
Comment 9 Jonathan Bedard 2020-01-24 12:17:54 PST
Created attachment 388717 [details]
Patch
Comment 10 Alexey Proskuryakov 2020-01-24 13:55:52 PST
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.
Comment 11 Jonathan Bedard 2020-01-24 14:19:24 PST
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.
Comment 12 Jonathan Bedard 2020-01-31 07:50:23 PST
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.