Note that this is not Android only - it definitely happens on Linux and Windows and I assume it happens everywhere.

I have a tentative CL (crrev.com/2780983004) that will reduce the impact by detecting when about_credits.html has not actually changed and avoiding writing it in that case.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5acc4c7976d45ace977c14019593c9413d0841d

commit b5acc4c7976d45ace977c14019593c9413d0841d
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Mar 30 17:16:52 2017

Reduce overbuilding due to about_credits.html

Efficiently determining when to build about_credits.html is hard.
Currently it is marked as dirty anytime the root .ninja file is
modified, so after every "gn gen" or modification of any BUILD.gn file
it gets rewritten, triggering 106 steps on Windows, 58 steps on Linux.

This change just makes the script smart enough to not write the file if
its contents have not changed. Thus, 105 of the 106 steps on Windows,
including linking of chrome.exe, can be skipped.

In addition to slightly improving build times this change makes it more
obvious what is going on. It makes it (fairly) clear that
about_credits.html is being regenerated but has not actually changed.

R=agrieve@chromium.org
BUG=692601

Review-Url: https://codereview.chromium.org/2780983004
Cr-Commit-Position: refs/heads/master@{#460807}

[modify] https://crrev.com/b5acc4c7976d45ace977c14019593c9413d0841d/tools/licenses.py

Hmm, I think it was somewhat nicer when a "ninja; touch BUILD.gn; ninja" ended up with a "nothing to do" much like a "ninja; ninja".

How about making the license regeneration dep on DEPS and the git sha instead?

Yeah, certainly this is still not a great solution, I don't think DEPS or git shaw is workable though:
ninja uses timestamps only, not sure how you'd implement the sha1 idea.
ninja shouldn't depend on anything above the gn root (which is src/). DEPS is in the parent of that.

hmm? looks like //DEPS to me: https://cs.chromium.org/chromium/src/DEPS

As for the git sha, there's .git/HEAD that can be made more reliable by having a helper script do "git rev-parse --git-dir" and then dep on $git_dir/HEAD. Its better not to hardcode .git since that'd break on .git dir-links like git-worktree uses, for example.

(we have a dep-on-git-head exec_script in our fork elsewhere, and it seems to be fine (tm))

Well that's embarrassing :P. I was thinking about .gclient :/. Still though, you don't know which DEPS to use without looking at .gclient (e.g. Android downstream src/DEPS is not the root one.

More on this though - if you depended only on DEPS or git sha, you could change the thrid_party deps locally, then do a build, and it would not notice that the license file should be rebuilt. I'd never want there to be a way where ninja builds incorrect things.

I now find since potentially https://codereview.chromium.org/2780983004, after a clean build, if I run "gn gen" to generate a solution file, this marks part of my build dirty and running ninja causes several build files to have to be rebuilt:

c:\src\chromium\src>ninja -C out\goma32 chrome -j100
ninja: Entering directory `out\goma32'
ninja: no work to do.

c:\src\chromium\src>gn gen --sln=base_unittests --ide=vs2015 --filters=//base:base_unittests out\goma32
Generating Visual Studio projects took 36ms
Done. Made 5073 targets from 1237 files in 3845ms

c:\src\chromium\src>ninja -C out\goma32 chrome -j100
ninja: Entering directory `out\goma32'
[0/107] ACTION //components/resources:about_credits(//build/toolchain/win:clang_x86)

Re comment #7, crrev.com/2780983004 didn't cause the extra build steps. It allows short-circuiting them. That is, prior to that CL all ~107 steps would run. Now ninja *says* it will run 107 steps but stops after the first one or two. If you are seeing otherwise then let me know.

Not having any action after regenerating .ninja files would be ideal, but that is a separate issue that predates my change.

For reference the overbuilding after a gn gen appears to have started after https://codereview.chromium.org/2670833004

Comment 0 sounds similar to a problem we had with .java compilations in gyp. What we did there was to write the collected list of files to a file (only if it changed), so that that list file's timestamp would get updated both when files got added or removed. Could that approach work here? (https://bugs.chromium.org/p/chromium/issues/detail?id=177552#c18)

I agree that `gn gen` making the build dirty is probably a cure that's worse than the disease.

The disease was that bots had incorrect builds because they were not rebuilding the file when they should have been. Running "ninja" twice in a row still results in "no steps" (aside from some android bots *cough bug 646165*).

Another way to fix this would be to have gn not write the file if the contents have not changed. 

I think I'm somewhat still missing why this is important enough to spend time on?

`gn gen` used to not cause rebuilds, now it does. How is this not a good invariant to have?

I don't disagree that it wouldn't be good to fix.
I am saying that it's less bad than bug 646165, and also less bad than incorrect builds.

Notice files are most interesting on official builds, which do clobber bots. So this makes development worse for devs on all platforms without a big advantage in return.

(I'm not saying bug 646165 isn't important too, but it's not a regression.)

The original bug motivating the change was because the stale license files caused a sawtooth to occur on perf bots. Some slaves noticed to rebuild and some did not.

I am still getting a forced rebuild of files when I do a gn gen - should I file another bug for this or is this bug the best one to comment on?

I think this is the right bug for that.

 Issue 724550  has been merged into this issue.