Bug 217371

Summary: Update post-processing rules for headers to not unnecessarily change timestamps
Product: WebKit Reporter: Keith Rollin <krollin>
Component: Tools / TestsAssignee: Keith Rollin <krollin>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, darin, dino, eric.carlson, ews-watchlist, graouts, kbr, keith_miller, kondapallykalyan, mark.lam, msaboff, saam, sam, tzagallo, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Keith Rollin
Reported 2020-10-06 02:34:22 PDT
Under XCBuild, the scripts employed in custom build rules can be invoked in innocuous situations. A common example is when the user is building from the command-line and they change the `make` output from stdout to a file, or vice-versa. Changing the output changes the setting of the COLOR_DIAGNOSTICS environment variable, which is enough to cause XCBuild to think something is different and that the custom build rule needs to be invoked. For the script's part, nothing significant has changed, yet it post-processes the header files, causing their modification dates to change, causing downstream rebuilds to occur. Fix this problem by adopting an approach that doesn't modify the post-processed header files unless their contents actually change.
Attachments
Patch (18.25 KB, patch)
2020-10-06 20:01 PDT, Keith Rollin 		no flags
DetailsFormatted DiffDiff
Patch (18.70 KB, patch)
2020-10-07 16:17 PDT, Keith Rollin 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-10-06 02:34:35 PDT
<rdar://problem/69992230>
Keith Rollin
Comment 2 2020-10-06 02:51:35 PDT
*** Bug 212444 has been marked as a duplicate of this bug. ***
Keith Rollin
Comment 3 2020-10-06 20:01:26 PDT
Created attachment 410726 [details] Patch
EWS Watchlist
Comment 4 2020-10-06 20:02:31 PDT
Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE
Darin Adler
Comment 5 2020-10-07 08:30:18 PDT
Comment on attachment 410726 [details] Patch When I do something like this in a make file I end up creating a situation where there is even more rebuilding because there is no record the work was done. No risk of that here?
Keith Rollin
Comment 6 2020-10-07 10:57:25 PDT
I believe there is a small risk of what you describe. I think we can get into the situation you describe if the upstream/source files change their modification dates but not their actual contents. If that were to happen, these scripts would process those upstream files but not change the destination files. On the next build, the scripts would again get invoked, do their stuff, and not actually change anything. The destination's modification date would never be brought up-to-date with respect to the source files. I'll address this by touching the destination files if the source files are newer, regardless of any actual content change. Doing this will cause an unnecessary rebuild, but it will happen only once, and the conditions under which this will happen should be extremely rare (why would the source file have its modification date changed without having its contents changed)?
Keith Rollin
Comment 7 2020-10-07 16:17:12 PDT
Created attachment 410796 [details] Patch
EWS
Comment 8 2020-10-07 17:23:58 PDT
Committed r268163: <https://trac.webkit.org/changeset/268163> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410796 [details].
Note You need to log in before you can comment on or make changes to this bug.