Bug 218378 – Further lessen reliance on VPATH in WebCore/DerivedSources.make

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Keith Rollin]

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.

[Keith Rollin]

<rdar://problem/70730895>

[Keith Rollin]

Created attachment 412725 [details]
Patch

[Alexey Proskuryakov]

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]

Alexey, your comments make sense. But why didn’t you also set review+?

[Alexey Proskuryakov]

I just don't know makefile syntax enough to r+ anything bu most trivial patches. I have no objections.

[Darin Adler]

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]

Created attachment 412788 [details]
Patch

[Keith Rollin]

Made Darin's suggested change.

[EWS]

Committed r269206: <https://trac.webkit.org/changeset/269206>

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