Bug 209890 - Enable the use of XCBuild by default in Apple builds
Summary: Enable the use of XCBuild by default in Apple builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Rollin
URL:
Keywords: InRadar
Depends on: 209946
Blocks: 212444 212469
  Show dependency treegraph
 
Reported: 2020-04-01 19:06 PDT by Keith Rollin
Modified: 2020-05-29 18:06 PDT (History)
21 users (show)

See Also:


Attachments
Patch (214.30 KB, patch)
2020-04-01 19:37 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff
Patch (37.40 KB, patch)
2020-04-02 15:54 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff
This is the same patch, but it's been rebased on ToT. (37.28 KB, patch)
2020-04-08 00:57 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff
Patch (37.29 KB, patch)
2020-05-25 22:44 PDT, Keith Rollin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Rollin 2020-04-01 19:06:12 PDT
Switch from the "legacy" Xcode build system to the "new" build system (also known as "XCBuild"). Switching to the new system speeds up builds by a small percentage, better validates projects for build-related issues (such as dependency cycles), lets WebKit benefit from future improvements in XCBuild such as those coming from the underlying llbuild open source project, and prepares us for any other tools built for this new ecosystem.
Comment 1 Keith Rollin 2020-04-01 19:15:58 PDT
<rdar://problem/44182078>

Also: <rdar://problem/59321872>, <rdar://problem/59321906>, and <rdar://problem/59321973>, which cover discrete tasks that were folded into this one.
Comment 2 Keith Rollin 2020-04-01 19:37:53 PDT
Created attachment 395236 [details]
Patch
Comment 3 EWS Watchlist 2020-04-01 19:38:36 PDT
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Comment 4 Frédéric Wang (:fredw) 2020-04-02 07:52:50 PDT
This is what I get when trying to launch build-webkit and build-webkit --ios-simulator in parallel:

note: Using new build system
note: Planning build
note: Using build description from disk
Build system information
error: unable to attach DB: error: accessing build database "/Users/fred/WebKit/WebKitBuild/XCBuildData/build.db": database is locked Possibly there are two concurrent builds running in the same filesystem location.
Comment 5 Jonathan Bedard 2020-04-02 08:27:41 PDT
(In reply to Frédéric Wang (:fredw) from comment #4)
> This is what I get when trying to launch build-webkit and build-webkit
> --ios-simulator in parallel:
> 
> note: Using new build system
> note: Planning build
> note: Using build description from disk
> Build system information
> error: unable to attach DB: error: accessing build database
> "/Users/fred/WebKit/WebKitBuild/XCBuildData/build.db": database is locked
> Possibly there are two concurrent builds running in the same filesystem
> location.

Would not expect running two build-webkits in parallel to work...there is probably considerable work that would need to be done to make this possible, not sure it's even desirable.
Comment 6 Darin Adler 2020-04-02 09:29:35 PDT
I think with the old build system build-webkit in parallel did work.

Seems likely it could still work as long as they aren't both pointing to the same build target directory.
Comment 7 Alex Christensen 2020-04-02 10:27:17 PDT
Comment on attachment 395236 [details]
Patch

Could we sort the libwebrtc project in a separate patch?
Comment 8 Darin Adler 2020-04-02 11:04:38 PDT
(In reply to Alex Christensen from comment #7)
> Could we sort the libwebrtc project in a separate patch?

That’s not just sorting. The UUIDs are all changing too. Something else is going on here.
Comment 9 Keith Rollin 2020-04-02 11:26:32 PDT
I'm not sorting it. I think that's part of webkit-patch.
Comment 10 Keith Rollin 2020-04-02 11:30:11 PDT
Oh, crud. I had a bigger response to post, but it got lost. Quickly:

* Not being able to perform parallel builds is intrinsic to the new build system. There's nothing we can directly do about it.

* As Darin says, using WEBKIT_OUTPUT_DIR to set the build directory when calling build-webkit would be a good workaround.

* One can also pass --no-xcbuild to build-webkit, but if the old build system were to go away, that option would no longer work.

* I'm surprised there's benefit to running build-webkit in parallel.
Comment 11 Darin Adler 2020-04-02 11:42:10 PDT
(In reply to Keith Rollin from comment #9)
> I'm not sorting it. I think that's part of webkit-patch.

Pretty sure that’s not accurate.
Comment 12 Darin Adler 2020-04-02 11:45:52 PDT
OK, I’m wrong. The UUIDs are not changing. I think Alex is correct that we can re-sort libwebrtc first, land that, then rebase this so the patch is smaller.

We should.

Maybe I’m also wrong about what did the sorting. Maybe webkit-patch did do it. Sorry, Keith.
Comment 13 Keith Rollin 2020-04-02 11:46:51 PDT
Yeah, it's in webkit-patch:

class Upload(AbstractPatchUploadingCommand):
    name = "upload"
    help_text = "Automates the process of uploading a patch for review"
    argument_names = "[BUGID]"
    show_in_main_help = True
    steps = [
        steps.ValidateChangeLogs,
        steps.CheckStyle,
        steps.PromptForBugOrTitle,
        steps.CreateBug,
        steps.SortXcodeProjectFiles,
...
Comment 14 Keith Rollin 2020-04-02 11:49:28 PDT
The project was resorted as a side-effect of my having modified it. I don't know how to get it sorted without first modifying it. So I can't create a pre-patch that just sorts the files.

I guess I could use git to split the current patch. I'll see if that works.
Comment 15 Frédéric Wang (:fredw) 2020-04-02 11:50:14 PDT
(In reply to EWS Watchlist from comment #3)
> Note that there are important steps to take when updating ANGLE. See
> http://trac.webkit.org/wiki/UpdatingANGLE

Note sure that's related but I get the following build error with build-webkit --ios-simulator:

> note: Using new build system
> note: Planning build
> note: Constructing build description
> error: OpenGL is not available when building for iOS Simulator. (in target 'ANGLE' from project 'ANGLE')
>
> ** BUILD FAILED **
Comment 16 Darin Adler 2020-04-02 11:52:42 PDT
(In reply to Keith Rollin from comment #14)
> I don't know how to get it sorted without first modifying it.

% sort-Xcode-project-file <project-file-path>
Comment 17 Keith Rollin 2020-04-02 11:55:42 PDT
(In reply to Darin Adler from comment #16)
> (In reply to Keith Rollin from comment #14)
> > I don't know how to get it sorted without first modifying it.
> 
> % sort-Xcode-project-file <project-file-path>

Thanks. That will make splitting this apart much simpler.
Comment 18 Keith Rollin 2020-04-02 11:56:11 PDT
(In reply to Frédéric Wang (:fredw) from comment #15)
> (In reply to EWS Watchlist from comment #3)
> > Note that there are important steps to take when updating ANGLE. See
> > http://trac.webkit.org/wiki/UpdatingANGLE
> 
> Note sure that's related but I get the following build error with
> build-webkit --ios-simulator:
> 
> > note: Using new build system
> > note: Planning build
> > note: Constructing build description
> > error: OpenGL is not available when building for iOS Simulator. (in target 'ANGLE' from project 'ANGLE')
> >
> > ** BUILD FAILED **

I'm pinging Dean to see if he knows what's happening here. If not, I'll look into it.
Comment 19 Dean Jackson 2020-04-02 12:47:22 PDT
(In reply to Keith Rollin from comment #18)

> > > error: OpenGL is not available when building for iOS Simulator. (in target 'ANGLE' from project 'ANGLE')
> > >
> > > ** BUILD FAILED **
> 
> I'm pinging Dean to see if he knows what's happening here. If not, I'll look
> into it.

It seems like the build is trying to use OpenGL rather than OpenGL ES when compiling for the simulator.
Comment 20 Dean Jackson 2020-04-02 12:55:34 PDT
(In reply to Dean Jackson from comment #19)
> (In reply to Keith Rollin from comment #18)
> 
> > > > error: OpenGL is not available when building for iOS Simulator. (in target 'ANGLE' from project 'ANGLE')
> > > >
> > > > ** BUILD FAILED **
> > 
> > I'm pinging Dean to see if he knows what's happening here. If not, I'll look
> > into it.
> 
> It seems like the build is trying to use OpenGL rather than OpenGL ES when
> compiling for the simulator.

Note that ANGLE doesn't link with OpenGL or OpenGLES. I believe this error comes from trying to #include <OpenGL/OpenGL.h>

But that is guarded by:

#if defined(ANGLE_PLATFORM_MACOS) || defined(ANGLE_PLATFORM_MACCATALYST)
Comment 21 Dean Jackson 2020-04-02 13:07:25 PDT
#ifdef ANGLE_PLATFORM_APPLE
#    include <TargetConditionals.h>
#    if TARGET_OS_OSX
#        define ANGLE_PLATFORM_MACOS 1
#    elif TARGET_OS_IPHONE
#        define ANGLE_PLATFORM_IOS 1
#        define GLES_SILENCE_DEPRECATION
#        if TARGET_OS_SIMULATOR
#            define ANGLE_PLATFORM_IOS_SIMULATOR 1
#        endif
#        if TARGET_OS_MACCATALYST
#            define ANGLE_PLATFORM_MACCATALYST
#        endif
#    endif
#endif
Comment 22 Keith Rollin 2020-04-02 15:54:49 PDT
Created attachment 395313 [details]
Patch
Comment 23 Keith Rollin 2020-04-02 15:55:23 PDT
Landed patch that only sort libwebrtc. This patch is rebased on top of that.
Comment 24 Darin Adler 2020-04-02 16:05:28 PDT
Comment on attachment 395313 [details]
Patch

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

> Source/ThirdParty/ANGLE/ANGLE.xcodeproj/project.pbxproj:2994
> -				6E28B8742294DD8000717E69 /* ShellScript */,
> +				6E28B8742294DD8000717E69 /* Adjust ANGLE Paths */,

Does seem like a better name for this step.
Comment 25 Keith Rollin 2020-04-02 16:05:58 PDT
(In reply to Dean Jackson from comment #20)
> 
> Note that ANGLE doesn't link with OpenGL or OpenGLES. I believe this error
> comes from trying to #include <OpenGL/OpenGL.h>

Dean, the error seems to be printed before compilation starts. So I don't think it's related to an import/include statement.

You say ANGLE doesn't link with OpenGL. But when I open the project and check the "Link Binary With Libraries" step, I see OpenGL.framework on the list. So it seems like it *is* linking with OpenGL?

That aside, I've been building iOS-sim for months with xcbuild. I have no idea why this error is cropping up for Frederic. I'll do some local testing.
Comment 26 Keith Rollin 2020-04-02 16:25:33 PDT
OK, I'm seeing the same issue. Seems to only affect external builds. I'll see if I can track down the difference between that and the builds I've been doing for the last few months.
Comment 27 Keith Rollin 2020-04-02 16:54:06 PDT
Building for macOS works. Probably because the macOS SDK has an OpenGL.framework and none of the other SDKs do. But the same could be said of the internal builds that have been succeeding.

When building for macOS with the external Xcode, I do at least see:

warning: OpenGL is deprecated. Consider migrating to Metal instead. (in target 'ANGLE' from project 'ANGLE')

I also see that macOS compiles with -DANGLE_ENABLE_OPENGL, which seems to indicate that OpenGL is being used. And the Libtool statement is indeed pulling in -framework OpenGL.

I need to switch back to our internal SDKs and see what they have to say about this.
Comment 28 Keith Rollin 2020-04-02 17:12:27 PDT
It looks like we're given some leeway when using it internally:

note: Using new build system
note: Planning build
note: Constructing build description
note: Build description constructed in 0.305542401s
note: Using build description '812a8b603661f37cd4cdc0f99d3273d6'
Build system information
note: Using eager compilation

warning: OpenGL is not available when building for iOS Simulator. (in target 'ANGLE' from project 'ANGLE')
Comment 29 Keith Rollin 2020-04-02 17:24:43 PDT
Dean says "Note that ANGLE doesn't link with OpenGL or OpenGLES." I can believe that that's true for non-macOS platforms, since the OpenGL framework doesn't ship in those SDKs. So I'll see what happens if I adjust the project to link with OpenGL only for macOS platforms, this approach seeming to be most compatible with what's currently actually happening.
Comment 30 Keith Rollin 2020-04-02 18:28:42 PDT
I unwittingly removed OpenGL from *all* configurations, including macOS, and it worked (to the extent that it linked). So the ultimate patch (which I'll do separate from this one) will likely take that form.
Comment 31 Keith Rollin 2020-04-02 19:20:12 PDT
ANGLE patch posted. I'll land this one once that one has.
Comment 32 Frédéric Wang (:fredw) 2020-04-03 08:28:00 PDT
Hi,

Just confirming that I can build and run safari with on iOS/macOS with the new build system and the public SDK (not internal build) with the extra ANGLE patch.

I can also confirm that the legacy system allowed parallel builds of the iOS and macOS port, that's basically what I've done in the past each time I rebase my local WebKit copy. Probably not useful to speed up the build, but at least I can easily run the two things in parallel and watch for any build error (unfortunately this happens because the bots use the internal build). So I think it's a regression, I can personally live with that, at worse using two WebKitBuild directories as Darin suggested. But I guess that's something to warn people about...
Comment 33 Jonathan Bedard 2020-04-03 08:41:50 PDT
(In reply to Frédéric Wang (:fredw) from comment #32)
> Hi,
> 
> Just confirming that I can build and run safari with on iOS/macOS with the
> new build system and the public SDK (not internal build) with the extra
> ANGLE patch.
> 
> I can also confirm that the legacy system allowed parallel builds of the iOS
> and macOS port, that's basically what I've done in the past each time I
> rebase my local WebKit copy. Probably not useful to speed up the build, but
> at least I can easily run the two things in parallel and watch for any build
> error (unfortunately this happens because the bots use the internal build).

For what it's worth, EWS and build.webkit.org infrastructure uses the public build, not the internal one.

> So I think it's a regression, I can personally live with that, at worse
> using two WebKitBuild directories as Darin suggested. But I guess that's
> something to warn people about...
Comment 34 Frédéric Wang (:fredw) 2020-04-03 09:33:58 PDT
(In reply to Jonathan Bedard from comment #33)
> (In reply to Frédéric Wang (:fredw) from comment #32)
> > Hi,
> > 
> > Just confirming that I can build and run safari with on iOS/macOS with the
> > new build system and the public SDK (not internal build) with the extra
> > ANGLE patch.
> > 
> > I can also confirm that the legacy system allowed parallel builds of the iOS
> > and macOS port, that's basically what I've done in the past each time I
> > rebase my local WebKit copy. Probably not useful to speed up the build, but
> > at least I can easily run the two things in parallel and watch for any build
> > error (unfortunately this happens because the bots use the internal build).
> 
> For what it's worth, EWS and build.webkit.org infrastructure uses the public
> build, not the internal one.
> 

OK I stand corrected. I was confused with the breakage of run-safari, which is not tested by the build.webkit.org infrastructure...
Comment 35 Keith Rollin 2020-04-08 00:57:32 PDT
Created attachment 395780 [details]
This is the same patch, but it's been rebased on ToT.
Comment 36 EWS 2020-04-08 02:12:14 PDT
Committed r259708: <https://trac.webkit.org/changeset/259708>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395780 [details].
Comment 38 Truitt Savell 2020-04-08 10:25:31 PDT
Reverted r259708 for reason:

Broke the iOS device Build

Committed r259726: <https://trac.webkit.org/changeset/259726>
Comment 39 Daniel Bates 2020-04-08 10:42:54 PDT
Clarifying question: Is using Legacy WebKit build system still supported? Or is no longer a supported configuration?

I'm seeing a spew of "Processing header files with build rules is not supported by the legacy build system." errors building JavaScriptCore using the legacy build system and the build fails now. I still build some Apple Internal projects that don't build with the new build system so it would be convenient for me to keep building with legacy so that I can have these projects in one Xcode workspace.
Comment 40 Keith Rollin 2020-04-08 12:06:32 PDT
Yes, it's conditionally supported. If you build in the IDE, you'll have to reset the workspace/project option, though this will change the projects and you'll have to be careful about checking them in or not. If you build with `make`, use the following:

make ... ARGS='-UseNewBuildSystem=NO USE_NEW_BUILD_SYSTEM=NO'
Comment 41 Keith Rollin 2020-04-08 12:09:12 PDT
When building with the public Xcode for iOS device, we also get the following error:

error: An empty identity is not valid when signing a binary for the product type 'Command-line Tool'. (in target 'yasm' from project 'libwebrtc')
error: An empty identity is not valid when signing a binary for the product type 'Dynamic Library'. (in target 'libwebrtc' from project 'libwebrtc')
Comment 42 Darin Adler 2020-04-08 12:24:35 PDT
(In reply to Keith Rollin from comment #41)
> When building with the public Xcode for iOS device, we also get the
> following error:
> 
> error: An empty identity is not valid when signing a binary for the product
> type 'Command-line Tool'. (in target 'yasm' from project 'libwebrtc')
> error: An empty identity is not valid when signing a binary for the product
> type 'Dynamic Library'. (in target 'libwebrtc' from project 'libwebrtc')

Is someone going to fix that, or are you saying we should all know we’ll get the error and we’re all supposed to ignore it?
Comment 43 Alexey Proskuryakov 2020-04-08 13:30:04 PDT
> Clarifying question: Is using Legacy WebKit build system still supported? Or is no longer a supported configuration?

I would add to Keith's answer. We still use the legacy build system with older Xcode versions (when building for macOS Mojave or Catalina), so it is fully supported in those configurations.

It's possible to use it with Xcode 11.4, as Keith described. But there are no bots running this configuration, so there is no expectation that it will continue to work, unless someone makes it their ongoing task to keep it working, and unless we agree to accept patches with fixes.
Comment 44 Alexey Proskuryakov 2020-04-08 13:30:26 PDT
> Is someone going to fix that, or are you saying we should all know we’ll get the error and we’re all supposed to ignore it?

The change was reverted, and shouldn't be landed until this is fixed. It is a bit of an edge case though, as it only affected open source iOS device builds.

This wasn't caught by EWS because it was upgraded to Xcode 11.4 right after this patch processing completed.
Comment 45 Keith Rollin 2020-04-08 14:15:52 PDT
>> Is someone going to fix that, or are you saying we should all know we’ll get the error and we’re all supposed to ignore it?
>
> The change was reverted, and shouldn't be landed until this is fixed. It is a bit of an edge case though, as it only affected open source iOS device builds.

Right, that's what I was saying. I'm working on a fix now. I'll land that and address the other reported issues (such as the one Dan mentioned here) before attempting to throw the XCBuild switch again.
Comment 46 Keith Rollin 2020-04-08 14:33:21 PDT
I think we need to wait until <rdar://60397030> has been fixed before we can try to reland this.
Comment 47 Keith Rollin 2020-05-25 22:44:15 PDT
Created attachment 400231 [details]
Patch
Comment 48 Keith Rollin 2020-05-25 22:47:34 PDT
<rdar://60397030> has been fixed, and all our bots have been updated to account for it. All prerequisites for landing this patch have been addressed. I've rebased it and am staging it for landing tomorrow (Tuesday, 2020-23-05).
Comment 49 EWS 2020-05-26 09:17:49 PDT
Committed r262147: <https://trac.webkit.org/changeset/262147>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 400231 [details].
Comment 50 Keith Rollin 2020-05-27 00:40:45 PDT
Committed r262182: <https://trac.webkit.org/changeset/262182>