gn analyze doesn't detect Windows toolchain changes |
|||
Issue descriptionWhen a changed to the toolchain hash is made gn should detect this and request a compilation of the 'all' target. This used to work (as of https://codereview.chromium.org/2780023002) but support was inadvertently deleted by https://chromium-review.googlesource.com/851080. All that needs to happen is that if the hash has changed (originally detected as any change to src\build\vs_toolchain.py) then build the 'all' target. If the change to vs_toolchain.py is something minor (a comment change) then this "ninja all" command will be a NOP, but if the toolchain has actually changed then the compile command lines will all be different so all of the compile commands will be dirty and a full build will happen. So, we just need tools\gn\analyzer.cc to detect changes to vs_toolchain.py and set outputs.status = "Found dependency (all)"; to make this work. Handling this is important because the alternative is flaky or incomplete tests of SDK changes, or using landmines (which are horrible). The immediate need for this is a desire to switch to the April 2018 Windows 10 SDK, needed for some HDR changes.
,
May 11 2018
As I mentioned in the CL, we don't want to hardcode paths into the gn binary itself, so we shouldn't modify analyzer.cc. The way to solve this problem is to add //build/vs_toolchain.py to //testing/buildbot/trybot_analyze_config.json ; doing so will add it to a whitelist that tells the builds to skip analyze and build everything (which has the desired outcome).
,
May 11 2018
Posted https://crrev.com/c/1055869 to fix this.
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a1e408b9a0f3cdec52026b54cee510274e4f7c9 commit 9a1e408b9a0f3cdec52026b54cee510274e4f7c9 Author: Dirk Pranke <dpranke@chromium.org> Date: Mon May 14 16:54:33 2018 Add vs_toolchain.py to analyze exclusions. If //build/vs_toolchain.py is changed (or any file it imports, including gn_helpers.py), we should bypass `analyze` on the trybots because we may need to rebuild everything. (Technically, we only need to do this on Win hosts, but we don't have a good way of filtering that at the moment). This is needed when updating the VS toolchain, including SDK updates. This change makes it so that a full build will be requested. The build will be a NOP unless the toolchain has actually changed, in which case the changed command lines will make all of the compile commands dirty. Bug: 842111 Change-Id: I2222798c7c8fd691383a0eca1414d1e8afdb55f8 Reviewed-on: https://chromium-review.googlesource.com/1055869 Reviewed-by: Bruce Dawson <brucedawson@chromium.org> Reviewed-by: Yuke Liao <liaoyuke@chromium.org> Commit-Queue: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#558342} [modify] https://crrev.com/9a1e408b9a0f3cdec52026b54cee510274e4f7c9/testing/buildbot/trybot_analyze_config.json
,
May 15 2018
Seems to work. I tested with crrev.com/c/1059775 which does nothing but change the toolchain hash and it triggered a full build which seems to be running about 50,000 build steps on Windows, fewer on other platforms (not zero because there are inevitably other code changes being pulled in from ToT). So, I think this is fixed. Thanks!
,
May 23 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by brucedaw...@chromium.org
, May 11 2018