Fix gn builds with locally installed VS

Fix gn builds with locally installed VS

gn builds that use a locally installed copy of Visual Studio (that is,
builds with DEPOT_TOOLS_WIN_TOOLCHAIN=0) fail to build because they
use the Windows 8.1 SDK, whereas Chrome now requires the Windows 10
SDK. This change updates the 8.1 references to point to 10.0.10586.0.

BUG=568201
Committed: https://crrev.com/953e376855384defa8c9b127e010c88cb6c56957
Cr-Commit-Position: refs/heads/master@{#370830}

[brucedawson, 2016-01-21 01:31:33]

brucedawson@chromium.org changed reviewers:
+ brettw@chromium.org

[brucedawson, 2016-01-21 01:32:06]

Description was changed from

==========
Tests and fixes for gn builds with locally installed VS
==========

to

==========
Tests and fixes for gn builds with locally installed VS

BUG=568201
==========

[brucedawson, 2016-01-21 01:40:23]

I'm investigating why gn builds of chrome with a locally installed VS 2013
(DEPOT_TOOLS_WIN_TOOLCHAIN=0) are failing. I've found some (maybe all?) of the
problems but I don't know this territory well, so I'm sharing the partial
results.

The change to enum_variant.cc is just so that I can tell if the Windows 10 SDK
is being used more quickly - building base is sufficient.

The change to visual_studio_version.gni doesn't seem to make any difference,
which is odd.

The other changes seem necessary and appropriate, since we're building with the
Windows 10 SDK everywhere else. But I'm not sure if they're sufficient. Can you
take a quick look?

Obviously I need to tidy the code and comments.

[brucedawson, 2016-01-21 01:47:28]

I just realized that this affects VS 2015 also, so some variant of this fix is
definitely needed.

The VS 2015 fix might be much easier - just don't modify INCLUDE at all unless
it is proven necessary, but the VS 2013 fix also works perfectly well for VS
2015.

[Ruud van Asseldonk, 2016-01-21 11:32:59]

I tried this patch locally, it resolves 568201 for me.

[brettw, 2016-01-21 17:37:32]

https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc
File base/win/enum_variant.cc (right):

https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc#ne...
base/win/enum_variant.cc:14: IVirtualDesktopManager* g_vdm;
Is this mixed in from another patch? It doesn't seem to go along with anything
here.

[brucedawson, 2016-01-21 18:44:20]

On 2016/01/21 17:37:32, brettw wrote:
> https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc
> File base/win/enum_variant.cc (right):
> 
>
https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc#ne...
> base/win/enum_variant.cc:14: IVirtualDesktopManager* g_vdm;
> Is this mixed in from another patch? It doesn't seem to go along with anything
> here.

Purely for test purposes. From the first message:

> The change to enum_variant.cc is just so that I can tell if the Windows 10 SDK
< is being used more quickly - building base is sufficient.

Otherwise the failure takes half an hour to hit.

[brettw, 2016-01-21 18:57:57]

Oh, I never read the collapsed comments :)

I have no opinion on whether this is correct. Would scottmg know?

[brucedawson, 2016-01-21 19:01:54]

On 2016/01/21 18:57:57, brettw wrote:
> Oh, I never read the collapsed comments :)
> 
> I have no opinion on whether this is correct. Would scottmg know?

I'll check with scottmg.

[scottmg, 2016-01-21 19:11:30]

On 2016/01/21 19:01:54, brucedawson wrote:
> On 2016/01/21 18:57:57, brettw wrote:
> > Oh, I never read the collapsed comments :)
> > 
> > I have no opinion on whether this is correct. Would scottmg know?
> 
> I'll check with scottmg.

Sure, lgtm I guess. Please make a note to remove the enum_variant code.

(I think we should stop caring about 2013 though)

[brucedawson, 2016-01-21 19:15:59]

> (I think we should stop caring about 2013 though)

Agreed, but this fix is required for VS 2015 as well.

[brettw, 2016-01-21 19:24:17]

https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc
File base/win/enum_variant.cc (right):

https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc#ne...
base/win/enum_variant.cc:14: IVirtualDesktopManager* g_vdm;
You're going to remove this before checkin, right?

[brucedawson, 2016-01-21 19:38:48]

https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc
File base/win/enum_variant.cc (right):

https://codereview.chromium.org/1615603002/diff/1/base/win/enum_variant.cc#ne...
base/win/enum_variant.cc:14: IVirtualDesktopManager* g_vdm;
On 2016/01/21 19:24:17, brettw wrote:
> You're going to remove this before checkin, right?

Yes. Definitely. I need to test a few more variants also.

[brucedawson, 2016-01-21 21:41:01]

Description was changed from

==========
Tests and fixes for gn builds with locally installed VS

BUG=568201
==========

to

==========
Fix gn builds with locally installed VS

gn builds that use a locally installed copy of Visual Studio (that is,
builds with DEPOT_TOOLS_WIN_TOOLCHAIN=0) fail to build because they
use the Windows 8.1 SDK, whereas Chrome now requires the Windows 10
SDK. This change updates the 8.1 references to point to 10.0.10586.0.

BUG=568201
==========

[brucedawson, 2016-01-21 21:42:01]

Test code removed and comments updated. I'll commit after the try jobs have had
some time to run.

Let me know if you have any feedback on the final version.

[brettw, 2016-01-21 21:44:20]

lgtm

[brucedawson, 2016-01-21 21:51:51]

The CQ bit was checked by brucedawson@chromium.org

[brucedawson, 2016-01-21 21:51:51]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/1615603002/#ps20001
(title: "Tweaked comments and removed test code")

[commit-bot: I haz the power, 2016-01-21 21:52:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1615603002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1615603002/20001

[commit-bot: I haz the power, 2016-01-21 23:35:42]

Description was changed from

==========
Fix gn builds with locally installed VS

gn builds that use a locally installed copy of Visual Studio (that is,
builds with DEPOT_TOOLS_WIN_TOOLCHAIN=0) fail to build because they
use the Windows 8.1 SDK, whereas Chrome now requires the Windows 10
SDK. This change updates the 8.1 references to point to 10.0.10586.0.

BUG=568201
==========

to

==========
Fix gn builds with locally installed VS

gn builds that use a locally installed copy of Visual Studio (that is,
builds with DEPOT_TOOLS_WIN_TOOLCHAIN=0) fail to build because they
use the Windows 8.1 SDK, whereas Chrome now requires the Windows 10
SDK. This change updates the 8.1 references to point to 10.0.10586.0.

BUG=568201
==========

[commit-bot: I haz the power, 2016-01-21 23:35:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-21 23:38:06]

Description was changed from

==========
Fix gn builds with locally installed VS

gn builds that use a locally installed copy of Visual Studio (that is,
builds with DEPOT_TOOLS_WIN_TOOLCHAIN=0) fail to build because they
use the Windows 8.1 SDK, whereas Chrome now requires the Windows 10
SDK. This change updates the 8.1 references to point to 10.0.10586.0.

BUG=568201
==========

to

==========
Fix gn builds with locally installed VS

gn builds that use a locally installed copy of Visual Studio (that is,
builds with DEPOT_TOOLS_WIN_TOOLCHAIN=0) fail to build because they
use the Windows 8.1 SDK, whereas Chrome now requires the Windows 10
SDK. This change updates the 8.1 references to point to 10.0.10586.0.

BUG=568201

Committed: https://crrev.com/953e376855384defa8c9b127e010c88cb6c56957
Cr-Commit-Position: refs/heads/master@{#370830}
==========

[commit-bot: I haz the power, 2016-01-21 23:38:07]

Patchset 2 (id:??) landed as
https://crrev.com/953e376855384defa8c9b127e010c88cb6c56957
Cr-Commit-Position: refs/heads/master@{#370830}