New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 612310 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

VS toolchain changes don't trigger rebuild of .ninja files

Project Member Reported by brucedaw...@chromium.org, May 16 2016

Issue description

gn users expect their .ninja files to be rebuilt whenever it is necessary, so any situation that doesn't trigger that rebuilding will cause confusion. In particular, if the Windows toolchain hash in build\vs_toolchain.py is changed and "gclient runhooks" is run then gn doesn't notice anything and will say that nothing needs to be rebuilt.

If "gn gen" is run on a directory then the next run of ninja will rebuild the affected targets.

This is a rare problem and could be addressed using landmines, but if there is some elegant way of having ninja detect that the .ninja files need regenerating, as it does with other changes, that would be consistent and ideal.

See crrev.com/1959413002 for an example.
 
Cc: brettw@chromium.org thakis@chromium.org scottmg@chromium.org
Components: Build
Status: Available (was: Untriaged)
The way we do this for clang rolls is to set a #define on the command line that contains the clang revision:

https://code.google.com/p/chromium/codesearch?q=clang_revision#chromium/src/build/config/compiler/BUILD.gn&l=105

We could presumably do something similar for msvs. I don't remember why we don't, and I'm sure we've talked about this before. Maybe Scott or Nico remembers ...

Comment 2 by thakis@chromium.org, May 16 2016

I somehow thought we had a bug for this already (but can't find it now).

For clang-cl, I fixed this by moving the /I flags for the toolchain includes from the environment.foo file to the command line (in https://codereview.chromium.org/1724533002 /  bug 588831 ) -- so on toolchain update, the new hash changes the command line, and ninja's command line tracking causes a rebuild. The exact same approach would work for 64-bit cl.exe too.

Adding a dummy define like Dirk says is another possible approach.
Blocking: 354261
Labels: -Build-Tools-GN Proj-GN-Migration
I'm guessing we should probably fix this before dropping GYP support ...
Or at least before we change the toolchain again. The last few toolchain changes did this using landmines, but that's not ideal. VS 2015 Update 3 may be landing soon and we'll have to evaluate whether we want it.
Blocking: -354261
As suggested by laforge@ and a conversation w/ the monorail folks, I'm going to try tracking GN-Migration related issues by *just* using the Proj-GN-Migration label, and not using blocking/rollup bugs, so that we can use blocking for just tasks that truly need to be completed before other tasks can make progress.
Labels: -Proj-GN-Migration
Clearing the Proj-GN-Migration label since it didn't block the GN migration (I'm trying to figure out what, if any GYP/GN-related tasks might be left).
Owner: brucedaw...@chromium.org
Status: Fixed (was: Available)
Most aspects of this should be fixed. On the try bots this change ensures that "gn analyze" will trigger a build of all instead of nothing when vs_toolchain.py changes:

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

commit b62c0d449adee39905c8e0b7cd262f22502ae67a
Author: brucedawson <brucedawson@chromium.org>
Date: Thu Mar 30 21:35:17 2017

Get gn analyze to respect changes to vs_toolchain.py

When changing the default compiler toolchain, either when changing
_GetDesiredVsToolchainHashes() or CURRENT_DEFAULT_TOOLCHAIN_VERSION,
it is important that gn analyze knows that this means that everything
needs rebuilding. This avoids having to use the blunt-hammer of
landmines.

R=dpranke@chromium.org
BUG= 683729 , 555273 

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

[modify] https://crrev.com/b62c0d449adee39905c8e0b7cd262f22502ae67a/tools/gn/analyzer.cc


A previous change to put each toolchain in a separate directory ensures that the individual compile and link commands will be dirty because the command-line will have changed.

The only thing that doesn't happen is that the .ninja files on local developer's machines are not forcibly regenerated. However this can easily be addressed by ensuring that a small BUILD.gn change is committed together with any change to the toolchain. Or not, as it is rare/unlikely that .ninja files won't need regenerating after syncing.

I'm going to close this as fixed.

Sign in to add a comment