Bug 235094

Summary: [XCBuild] Add "product dependencies" which influence workspace build order
Product: WebKit Reporter: Elliott Williams <emw>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, ews-watchlist, hi, jbedard, keith_miller, mark.lam, msaboff, pangle, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Description Elliott Williams 2022-01-11 20:25:05 PST
[XCBuild] Add "product dependencies" which influence workspace build order
Comment 1 Elliott Williams 2022-01-11 21:48:20 PST
Created attachment 448905 [details]
Patch
Comment 2 Elliott Williams 2022-01-11 21:49:25 PST
rdar://86871388
Comment 3 Radar WebKit Bug Importer 2022-01-11 21:50:43 PST
<rdar://problem/87436702>
Comment 4 Elliott Williams 2022-01-11 22:54:58 PST
Created attachment 448908 [details]
Patch
Comment 5 Alexey Proskuryakov 2022-01-12 09:18:15 PST
Comment on attachment 448908 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:8
> +        Some ancillary targets (e.g. script-only targets like Derived Sources) do not have implicit
> +        dependencies visible to Xcode. In workspace builds, we need to give XCBuild additional
> +        information to ensure that they always run after their dependencies.

Is this a temporary measure before scripts in these targets have inputs/outputs defined, or is it necessary even when those are present?
Comment 6 Elliott Williams 2022-01-12 10:19:32 PST
(In reply to Alexey Proskuryakov from comment #5)
> Is this a temporary measure before scripts in these targets have
> inputs/outputs defined, or is it necessary even when those are present?

I thought it would always be necessary, but checking my work right now it looks like these dependencies could be elided in the future if we get more precise inputs/outputs.

Script inputs/outputs will influence whether a script runs, and what order it runs relative to other tasks (citation: https://help.apple.com/xcode/mac/11.4/#/devc8c930575). A missing script input will not _cause_ another target to build, meaning script inputs are not inferred as _target dependencies_ the same way linked frameworks or copied products are. But that's okay for us, because the main targets of each project have the necessary target dependency.

For example: JavaScriptCore (the target) links against libWTF.a, which forms a target dependency on WTF. JSC's "Generate Unified Sources" is an explicit target dependency of JavaScriptCore. So when we build JavaScriptCore, both WTF and Generate Unified Sources are in the build graph, and script phase inputs/outputs would be enough to show the relationship between Generate Unified Sources and the headers it uses.

So, while I don't want to commit to deleting all of these product dependencies yet, I think they could be deleted later on to improve performance.
Comment 7 Alexey Proskuryakov 2022-01-12 10:34:19 PST
Comment on attachment 448908 [details]
Patch

Sounds good! EWS is of course red, marking r- to get this out of review queue for now.
Comment 8 Elliott Williams 2022-01-12 14:56:51 PST
Created attachment 449000 [details]
Patch
Comment 9 EWS 2022-01-13 17:02:50 PST
Committed r287999 (246026@main): <https://commits.webkit.org/246026@main>

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