Bug 237115

Summary: [XCBuild] Don't automatically use the ../Internal/WebKit workspace
Product: WebKit Reporter: Elliott Williams <emw>
Component: Tools / TestsAssignee: Elliott Williams <emw>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

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].