Bug 237115 - [XCBuild] Don't automatically use the ../Internal/WebKit workspace
Summary: [XCBuild] Don't automatically use the ../Internal/WebKit workspace
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: Elliott Williams
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-23 14:45 PST by Elliott Williams
Modified: 2022-02-23 16:48 PST (History)
2 users (show)

See Also:


Attachments
Patch (8.28 KB, patch)
2022-02-23 15:19 PST, Elliott Williams
no flags Details | Formatted Diff | Diff
Patch (1.76 KB, patch)
2022-02-23 16:00 PST, Elliott Williams
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Elliott Williams 2022-02-23 14:45:32 PST
When the USE_WORKSPACE=YES Make flag was added in https://commits.webkit.org/246232@main, we configured Make to automatically use the workspace at ../Internal/WebKit/WebKit.xcworkspace, if it was available. The idea was to make it easier for workspace builds to automatically detect dependencies needed by folks at Apple, like WebKitAdditions.

In practice, this has made workspace builds silently rely on code from outside of the WebKit repo, and it's unintuitive since non-workspace Make builds do not do this. Perhaps we can revisit this in the future when we're fully migrated to workspace builds, but for now I propose we reverse this.
Comment 1 Elliott Williams 2022-02-23 15:19:53 PST
Created attachment 453034 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2022-02-23 15:20:46 PST
<rdar://problem/89381493>
Comment 3 Alexey Proskuryakov 2022-02-23 15:43:23 PST
Comment on attachment 453034 [details]
Patch

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

High level question tangentially related to this patch: will workspace builds allow for changing a file in WebCore, and rebuilding just WebCore `make -C Source/WebCore` without linking all dependent projects? Often, engineers know that there is no API change, and relinking everything would waste time.

> Makefile.shared:-9
> -	$(dir $(lastword $(MAKEFILE_LIST)))../Internal/WebKit/WebKit.xcworkspace \

I'm curious why this patch touches so many files, as opposed to just removing this line.
Comment 4 Elliott Williams 2022-02-23 15:52:20 PST
(In reply to Alexey Proskuryakov from comment #3)
> High level question tangentially related to this patch: will workspace
> builds allow for changing a file in WebCore, and rebuilding just WebCore
> `make -C Source/WebCore` without linking all dependent projects? Often,
> engineers know that there is no API change, and relinking everything would
> waste time.

`make -C Source/WebCore` will build everything up to WebCore, so in your scenario the only thing that would be re-linked is WebCore itself. If I'm understanding correctly, that's the behavior engineers would expect.
 
> > Makefile.shared:-9
> > -	$(dir $(lastword $(MAKEFILE_LIST)))../Internal/WebKit/WebKit.xcworkspace \
> 
> I'm curious why this patch touches so many files, as opposed to just
> removing this line.

I did this so that any Makefile could set the workspace path before including `Makefile.shared`. But on second thought, I don't think that's actually necessary. Internal projects won't call into this `Makefile.shared` at all, so they are free to point to their own workspace as part of <rdar://88844770>.

I'll make this change and merge w/o more review.
Comment 5 Elliott Williams 2022-02-23 16:00:56 PST
Created attachment 453041 [details]
Patch
Comment 6 EWS 2022-02-23 16:48:20 PST
Committed r290400 (247712@main): <https://commits.webkit.org/247712@main>

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