Bug 199212 - Script which adjusts include paths in ANGLE's copied headers breaks incremental builds
Summary: Script which adjusts include paths in ANGLE's copied headers breaks increment...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: ANGLE (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Major
Assignee: Kenneth Russell
URL:
Keywords: InRadar
Depends on: 169861 197755
Blocks: 198948
  Show dependency treegraph
 
Reported: 2019-06-25 16:47 PDT by Kenneth Russell
Modified: 2019-06-26 16:40 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.77 KB, patch)
2019-06-25 17:52 PDT, Kenneth Russell
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Russell 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.
Comment 1 Kenneth Russell 2019-06-25 17:52:39 PDT
Created attachment 372886 [details]
Patch
Comment 2 Build Bot 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
Comment 3 Kenneth Russell 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.
Comment 4 Darin Adler 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.
Comment 5 Dean Jackson 2019-06-25 18:45:07 PDT
Committed r246825: <https://trac.webkit.org/changeset/246825>
Comment 6 Ryan Haddad 2019-06-25 20:57:05 PDT
Reverted r246825 for reason:

Breaks internal builds.

Committed r246832: <https://trac.webkit.org/changeset/246832>
Comment 7 Ryan Haddad 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.
Comment 8 Kenneth Russell 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.
Comment 9 Dean Jackson 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.
Comment 10 Dean Jackson 2019-06-26 11:28:56 PDT
Ken, I'll make the edit and re-land. Don't worry about submitting a new patch.
Comment 11 Dean Jackson 2019-06-26 11:37:30 PDT
Landed again in r246842
Comment 12 Kenneth Russell 2019-06-26 11:40:19 PDT
Thank you Dean for fixing up this patch and relanding it!
Comment 13 Radar WebKit Bug Importer 2019-06-26 16:40:14 PDT
<rdar://problem/52218406>