Bug 199212

Summary: Script which adjusts include paths in ANGLE's copied headers breaks incremental builds
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: ANGLEAssignee: Kenneth Russell <kbr>
Status: RESOLVED FIXED    
Severity: Major CC: darin, dino, ews-watchlist, graouts, kondapallykalyan, krollin, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 169861, 197755    
Bug Blocks: 198948, 212444, 212623    
Attachments:
Description Flags
Patch darin: review+

Kenneth Russell
Reported 2019-06-25 16:47:49 PDT
Per this discussion on webkit-dev: https://lists.webkit.org/pipermail/webkit-dev/2019-June/030707.html and turning on the additional build logging: defaults write com.apple.dt.Xcode ExplainWhyBuildCommandsAreRun -bool YES it's clear that the adjust-angle-include-paths.sh script which was added as a build phase in Issue 197755 is causing ANGLE's headers (specifically ShaderLang.h, but soon others) to be modified every time the build runs, causing excessive recompilation. The necessary fix will be similar to that from Issue 169861.
Attachments
Patch (1.77 KB, patch)
2019-06-25 17:52 PDT, Kenneth Russell 		darin: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Kenneth Russell
Comment 1 2019-06-25 17:52:39 PDT
Created attachment 372886 [details] Patch
EWS Watchlist
Comment 2 2019-06-25 17:54:46 PDT
Note that there are important steps to take when updating ANGLE. See http://trac.webkit.org/wiki/UpdatingANGLE
Kenneth Russell
Comment 3 2019-06-25 17:55:32 PDT
Tested this modification locally. Works correctly on the first and subsequent builds. Note that the fix from bug 169861 didn't really apply. These headers are copied out as part of the "Headers" build phase, and it wasn't feasible to try to prevent running the adjust-angle-include-paths.sh script.
Darin Adler
Comment 4 2019-06-25 18:31:06 PDT
Comment on attachment 372886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372886&action=review > Source/ThirdParty/ANGLE/ChangeLog:11 > + * adjust-angle-include-paths.sh: I think there’s a tab here instead of spaces and the pre-commit Subversion hook will reject this patch as-is.
Dean Jackson
Comment 5 2019-06-25 18:45:07 PDT
Committed r246825: <https://trac.webkit.org/changeset/246825>
Ryan Haddad
Comment 6 2019-06-25 20:57:05 PDT
Reverted r246825 for reason: Breaks internal builds. Committed r246832: <https://trac.webkit.org/changeset/246832>
Ryan Haddad
Comment 7 2019-06-25 20:58:57 PDT
(In reply to Ryan Haddad from comment #6) > Reverted r246825 for reason: > > Breaks internal builds. > > Committed r246832: <https://trac.webkit.org/changeset/246832> Emailed Dean the build failure log.
Kenneth Russell
Comment 8 2019-06-26 10:26:09 PDT
(In reply to Darin Adler from comment #4) > Comment on attachment 372886 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372886&action=review > > > Source/ThirdParty/ANGLE/ChangeLog:11 > > + * adjust-angle-include-paths.sh: > > I think there’s a tab here instead of spaces and the pre-commit Subversion > hook will reject this patch as-is. Sorry about that. I'll fix that up when attempting to reland.
Dean Jackson
Comment 9 2019-06-26 11:28:19 PDT
\<Root>./usr/local/include/ANGLE/angle.timestamp **** buildit [INFO ] 2019-06-25T19:03:34-0700 **** File appears to be empty which is not allowed. **** buildit [INFO ] 2019-06-25T19:03:34-0700 **** **** buildit [INFO ] 2019-06-25T19:03:34-0700 **** <SDKContentRoot>./usr/local/include/ANGLE/angle.timestamp **** buildit [INFO ] 2019-06-25T19:03:34-0700 **** File appears to be empty which is not allowed. Simple fix.
Dean Jackson
Comment 10 2019-06-26 11:28:56 PDT
Ken, I'll make the edit and re-land. Don't worry about submitting a new patch.
Dean Jackson
Comment 11 2019-06-26 11:37:30 PDT
Landed again in r246842
Kenneth Russell
Comment 12 2019-06-26 11:40:19 PDT
Thank you Dean for fixing up this patch and relanding it!
Radar WebKit Bug Importer
Comment 13 2019-06-26 16:40:14 PDT
<rdar://problem/52218406>
Note You need to log in before you can comment on or make changes to this bug.