This was discovered when I tried to enable nocompile tests on Windows. Running nocompile tests on Windows might not be useful, but having all the bots running supported VC versions sounds reasonable.

The related CL is:
https://codereview.chromium.org/2912193002/

I'm quite confused. The error path that you list implies an installed version of Visual Studio 12.0 (VS 2013). First, we haven't built with that version of the compiler for almost a year, and for most of that time it has been impossible to build Chrome with VS 2013 - we very quickly gained dependencies on VS 2015.

Also, we *never* build using an installed version of VS on the bots. We either never have or at least haven't for many years. So, I don't know what is going on.

On more recent try jobs I see errors like this:

FAILED: obj/base/base_nocompile_tests/histogram_unittest_nc.obj 
ninja -t msvc -e environment.x86 -- E:\b\c\goma_client/gomacc.exe "E:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616\VC\bin\amd64_x86/cl.exe" /nologo /showIncludes /FC @obj/base/base_nocompile_tests/histogram_unittest_nc.obj.rsp /c gen/base/histogram_unittest_nc.cc /Foobj/base/base_nocompile_tests/histogram_unittest_nc.obj /Fd"obj/base/base_nocompile_tests_cc.pdb"
e:\b\c\b\win\src\out\release\gen\base\histogram_unittest_nc.cc(14): fatal error C1189: #error:  "NCTEST_ENUM_MAX_OUT_OF_RANGE Failed: Expectations [r'static_assert failed "\|boundary\| is out of range of HistogramBase::Sample"'] did not match output."

Note the "E:\b\depot_tools\win_toolchain\vs_files\d3cb0e37bdd120ad0ac4650b674b09e81be45616" path which is from VS 2015, extracted to the depot_tools\win_toolchain directory. This is normal and expected.


In short, I don't know what's going on. The Windows trybots definitely do *not* use an outdated VC version. But, on the other hand, I can't explain this behavior.

I don't entirely understand what the nocompile tests are. If it's helpful I can try reproducing the behavior on my local machine.

BTW, the CL which switched all Windows builders (try bots, official builders, etc.) to VS 2015 was https://codereview.chromium.org/1783773004. The version of VS 2015 was later updated in https://codereview.chromium.org/2106203002. These were both quite a long time ago and they affect *all* builders.

Before the bots were updated to use pinned toolchain in depot_tools, the system image might have installed version of Visual Studio. After the migration, we might not have purged the legacy versions from the system and environment variables, and the usage of headers are mixed now. Could that be the case?

In my local (well, GCE) windows machine, I also tried to reproduce it. In a combination of installing local version of VS12, VS14, depot_tools toolchains, and skipping "gn gen", I've seen the same error, but I can no longer reproduce it after another "gn gen".

I could certainly believe that there could be an old installed copy of VS 2013 on some machines, but I've never seen an installed copy be accidentally used.

There were some changes to the toolchain deployment over the last week so it is possible that you did a build in the middle of one of those, and somehow triggered a bad code-path.

I think this bug is ephemeral. I'd recommend reverting your workaround and running try jobs again. If the VS 2013 version of std::max is still being used then I'll investigate, but I'm guessing that it won't be.

Thanks for your suggestion! I'll give it a try and update here how it goes.

It still repro just now on windows bots:
https://codereview.chromium.org/2912193002/#ps160001

We might have been using the wrong version of headers without knowing it.

We definitely haven't been using the wrong versions of the headers - we would have noticed that a long time ago, I'm sure. That particular line from histogram_samples.h has been compiling fine on all of the try bots for a long time:

  static_assert(std::numeric_limits<HistogramBase::Sample>::max() <
                    std::numeric_limits<int64_t>::max(),
                "Get() |max| must be able to hold Histogram::Sample max + 1");

So, something about the NC configuration is changing the build and exposing a previously hidden toolchain configuration bug. I know nothing about the NC compile builds which is making it hard for me to diagnose this. When I do local builds of the CL I get different failures, but I don't have VS 2013 installed.

I'm changing the title of the bug. Any more references to explain what the NC build is trying to do could be helpful.

The nocompile configuration can easily diverge with our main config, so it's fragile in its current form. I think the road blocker is that there's no way (that I know of) to extract the final value of variables set in GN configs, like cflags, defines, and such, so nocompile configuration just copy the minimal set of necessary arguments.