RESOLVED FIXED218378
Further lessen reliance on VPATH in WebCore/DerivedSources.make 
https://bugs.webkit.org/show_bug.cgi?id=218378
Summary Further lessen reliance on VPATH in WebCore/DerivedSources.make
Keith Rollin
Reported 2020-10-30 02:19:47 PDT
Bug 217696 updated WebCore/DerivedSources.make to rely less on VPATH and make more use of explicit partial or full paths. The solution there, however, did not go far enough, and led to failures when building WebKit for Safari Tech Preview and using old SDKs that contains files that have since been "upstreamed" into WebKit. Address this problem by taking further control of how DerivedSources.make finds needed files instead of using the VPATH mechanism.
Attachments
Patch (13.25 KB, patch)
2020-10-30 02:22 PDT, Keith Rollin 		no flags
DetailsFormatted DiffDiff
Patch (13.18 KB, patch)
2020-10-30 12:36 PDT, Keith Rollin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Keith Rollin
Comment 1 2020-10-30 02:19:57 PDT
<rdar://problem/70730895>
Keith Rollin
Comment 2 2020-10-30 02:22:08 PDT
Created attachment 412725 [details] Patch
Alexey Proskuryakov
Comment 3 2020-10-30 10:13:54 PDT
Comment on attachment 412725 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412725&action=review > Source/WebCore/ChangeLog:17 > + No new tests -- this is a build fix. I think that this is the right approach to addressing the regression causing a build failure. However, it makes me concerned about general case. The old SDK can have old copies of other files, not just IDLs. For example, we cannot defend against the compiler finding headers in a wrong directory. In a build system that modifies the SDK as it goes, the comprehensive solution would be to clean the SDK before building, and that's already done in some situations - I'm actually somewhat surprised that we ran into this where we did. But I'm not certain on how any potential solutions for local engineering builds could look like.
Darin Adler
Comment 4 2020-10-30 11:16:40 PDT
Alexey, your comments make sense. But why didn’t you also set review+?
Alexey Proskuryakov
Comment 5 2020-10-30 11:53:18 PDT
I just don't know makefile syntax enough to r+ anything bu most trivial patches. I have no objections.
Darin Adler
Comment 6 2020-10-30 12:26:13 PDT
Comment on attachment 412725 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412725&action=review > Source/WebCore/DerivedSources.make:1315 > + $(sort \ > + $(foreach \ I think putting sort and foreach on separate lines is perhaps overdoing it a bit. I’m OK with this sort of vertical formatting if it makes things clearer but I’m not sure it’s having that effect here, and especially not for the sort line vs. the foreach lines. It might be hard to see the structure in one super-long line, but we should explore other ways to do that. I think this one might be *easier* to read as one line.
Keith Rollin
Comment 7 2020-10-30 12:36:34 PDT
Created attachment 412788 [details] Patch
Keith Rollin
Comment 8 2020-10-30 12:37:52 PDT
Made Darin's suggested change.
EWS
Comment 9 2020-10-30 13:34:34 PDT
Committed r269206: <https://trac.webkit.org/changeset/269206> All reviewed patches have been landed. Closing bug and clearing flags on attachment 412788 [details].
Note You need to log in before you can comment on or make changes to this bug.