Reland of Change default Windows compiler to VS 2015

Reland of Change default Windows compiler to VS 2015

The change to get_landmines.py is there because modifying this file
affects analyze behavior so that all tests run. Changing the printed
message is purely a side effect.

This change also removes some redundant INCLUDE paths. These are
unnecessary when building with VS 2015 (because it defaults to the
Windows 10 SDK) and actively harmful (they make the INCLUDE path
problematically long).

This change was redone in order to fix merge conflicts and because after
a few weeks a fresh set of approvals seems reasonable.

The original change was landed as crrev.com/1598493004

BUG=440500, 584782
Committed: https://crrev.com/d4dcbd342dd54f55383daf8bc44b2c9d97fe0d0b
Cr-Commit-Position: refs/heads/master@{#380382}

[brucedawson, 2016-03-08 22:18:44]

Description was changed from

==========
Reland of Change default Windows compiler to VS 2015

The change to get_landmines.py is there because modifying this file
affects analyze behavior so that all tests run. Changing the printed
message is purely a side effect.

This change also removes some redundant INCLUDE paths. These are
unnecessary when building with VS 2015 (because it defaults to the
Windows 10 SDK) and actively harmful (they make the INCLUDE path
problematically long).

This change was redone in order to fix merge conflicts and because after
a few weeks a fresh set of approvals seems reasonable.

BUG=440500, 584782
==========

to

==========
Reland of Change default Windows compiler to VS 2015

The change to get_landmines.py is there because modifying this file
affects analyze behavior so that all tests run. Changing the printed
message is purely a side effect.

This change also removes some redundant INCLUDE paths. These are
unnecessary when building with VS 2015 (because it defaults to the
Windows 10 SDK) and actively harmful (they make the INCLUDE path
problematically long).

This change was redone in order to fix merge conflicts and because after
a few weeks a fresh set of approvals seems reasonable.

The original change was landed as crrev.com/1598493004

BUG=440500, 584782
==========

[brucedawson, 2016-03-08 22:18:44]

brucedawson@chromium.org changed reviewers:
+ dpranke@chromium.org, scottmg@chromium.org

[brucedawson, 2016-03-08 22:20:33]

Pending the landing of two changes (a toolchain update and some warning fixes
that the toolchain update requires) I believe we are ready to change the default
compiler again.

I could do it tonight or tomorrow night, depending on how much additional
testing is desired. Let me know if you have any thoughts on that.

[scottmg, 2016-03-08 22:23:37]

lgtm

(Did this get the stuff that Nico added to package_from_installed recently?)

[brucedawson, 2016-03-08 22:29:17]

On 2016/03/08 22:23:37, scottmg wrote:
> lgtm
> 
> (Did this get the stuff that Nico added to package_from_installed recently?)

It did not. I'll rebuild the toolchain and test it.

[Nico, 2016-03-08 22:39:45]

Does this work with the released version of 2015? If not, what's the failure
mode for that? Should we wait for a public release of update 2 for e.g. the
Opera folks?

I added a win_clang_dbg bot to your try set -- please let me know if it comes
back red, then I'll try to fix that quickly.

[brucedawson, 2016-03-08 22:46:17]

On 2016/03/08 22:39:45, Nico wrote:
> Does this work with the released version of 2015? If not, what's the failure
> mode for that? Should we wait for a public release of update 2 for e.g. the
> Opera folks?
> 
> I added a win_clang_dbg bot to your try set -- please let me know if it comes
> back red, then I'll try to fix that quickly.

I have discussed with Opera. The remaining failures for those using VS 2015
Update 1 are a code-gen bug when using PGO, occasional linker failures in debug
builds, and some code-gen bugs that we are working around. All were deemed
acceptable for the brief period.

[Dirk Pranke, 2016-03-08 22:49:20]

Can we avoid the change to get_landmines by doing something like what we do for
clang and embedding an msvs version into the command lines (e.g., #defining
CR_CLANG_REVISION)?

[Dirk Pranke, 2016-03-08 22:49:29]

lgtm otherwise, though.

[brucedawson, 2016-03-09 01:39:28]

On 2016/03/08 22:49:20, Dirk Pranke wrote:
> Can we avoid the change to get_landmines by doing something like what we do
for
> clang and embedding an msvs version into the command lines (e.g., #defining
> CR_CLANG_REVISION)?

I'd rather not. Partially because I don't want to restart testing (I know the
the landmines change triggers a clobber and triggers all tests) and also because
leaving an unnecessary define on the command-line would be a bit of a wart. The
compiler version is already available as a pre-defined macro.

[Dirk Pranke, 2016-03-09 02:03:15]

On 2016/03/09 01:39:28, brucedawson wrote:
> On 2016/03/08 22:49:20, Dirk Pranke wrote:
> > Can we avoid the change to get_landmines by doing something like what we do
> for
> > clang and embedding an msvs version into the command lines (e.g., #defining
> > CR_CLANG_REVISION)?
> 
> I'd rather not. Partially because I don't want to restart testing (I know the
> the landmines change triggers a clobber and triggers all tests) and also
because
> leaving an unnecessary define on the command-line would be a bit of a wart.
The
> compiler version is already available as a pre-defined macro.

I'm fine for punting the idea to a separate change (i.e., not now as part of
this), 
but I think it's a good idea to switch to a command line arg rather than relying
on landmines, 
and it's probably an even better idea to at least have msvs do things the same
way 
clang does, even if it is a wart; the alternative is to make sure that we always
remember to
update landmines, which IMO is way more of a wart (and more error prone).

However, if we don't use landmines we need to make sure that analyze is skipped
some other
way; we should make sure that changes to build/vs_toolchain.py do bypass analyze

(they don't currently).

[brucedawson, 2016-03-10 06:02:18]

Okay, going for it.

The try job with the warning suppressions embedded (crrev.com/1779713003,
warning suppressions landed as a separate CL) passed its try jobs except for a
timeout, so this should land.

[brucedawson, 2016-03-10 06:02:32]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-03-10 06:02:48]

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

[commit-bot: I haz the power, 2016-03-10 09:20:47]

Description was changed from

==========
Reland of Change default Windows compiler to VS 2015

The change to get_landmines.py is there because modifying this file
affects analyze behavior so that all tests run. Changing the printed
message is purely a side effect.

This change also removes some redundant INCLUDE paths. These are
unnecessary when building with VS 2015 (because it defaults to the
Windows 10 SDK) and actively harmful (they make the INCLUDE path
problematically long).

This change was redone in order to fix merge conflicts and because after
a few weeks a fresh set of approvals seems reasonable.

The original change was landed as crrev.com/1598493004

BUG=440500, 584782
==========

to

==========
Reland of Change default Windows compiler to VS 2015

The change to get_landmines.py is there because modifying this file
affects analyze behavior so that all tests run. Changing the printed
message is purely a side effect.

This change also removes some redundant INCLUDE paths. These are
unnecessary when building with VS 2015 (because it defaults to the
Windows 10 SDK) and actively harmful (they make the INCLUDE path
problematically long).

This change was redone in order to fix merge conflicts and because after
a few weeks a fresh set of approvals seems reasonable.

The original change was landed as crrev.com/1598493004

BUG=440500, 584782
==========

[commit-bot: I haz the power, 2016-03-10 09:20:48]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-10 09:22:59]

Description was changed from

==========
Reland of Change default Windows compiler to VS 2015

The change to get_landmines.py is there because modifying this file
affects analyze behavior so that all tests run. Changing the printed
message is purely a side effect.

This change also removes some redundant INCLUDE paths. These are
unnecessary when building with VS 2015 (because it defaults to the
Windows 10 SDK) and actively harmful (they make the INCLUDE path
problematically long).

This change was redone in order to fix merge conflicts and because after
a few weeks a fresh set of approvals seems reasonable.

The original change was landed as crrev.com/1598493004

BUG=440500, 584782
==========

to

==========
Reland of Change default Windows compiler to VS 2015

The change to get_landmines.py is there because modifying this file
affects analyze behavior so that all tests run. Changing the printed
message is purely a side effect.

This change also removes some redundant INCLUDE paths. These are
unnecessary when building with VS 2015 (because it defaults to the
Windows 10 SDK) and actively harmful (they make the INCLUDE path
problematically long).

This change was redone in order to fix merge conflicts and because after
a few weeks a fresh set of approvals seems reasonable.

The original change was landed as crrev.com/1598493004

BUG=440500, 584782

Committed: https://crrev.com/d4dcbd342dd54f55383daf8bc44b2c9d97fe0d0b
Cr-Commit-Position: refs/heads/master@{#380382}
==========

[commit-bot: I haz the power, 2016-03-10 09:23:00]

Patchset 1 (id:??) landed as
https://crrev.com/d4dcbd342dd54f55383daf8bc44b2c9d97fe0d0b
Cr-Commit-Position: refs/heads/master@{#380382}

[vasilii, 2016-03-10 12:39:46]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1778343002/ by vasilii@chromium.org.

The reason for reverting is: Broke isolate tests on Win8 GN (dbg) and Win x64 GN
(dbg).
https://build.chromium.org/p/chromium.win/builders/Win8%20GN%20%28dbg%29/buil...

[found] [hashed/size/to hash] [looked up/to lookup] [uploaded/size/to
upload/size]
04:10:31.512494 Root: E:\b\build\slave\Win8_GN__dbg_\build\src
04:10:31.513479 Root: E:\b\build\slave\Win8_GN__dbg_\build\src

events_unittests  GetFileAttributesEx
E:\b\build\slave\Win8_GN__dbg_\build\src\out\Debug\msvcp120d.dll: The system
cannot find the file specified.
04:10:31.988532
PushDirectory(E:\b\build\slave\Win8_GN__dbg_\build\src\content\test\data\) =
1574 files
04:10:32.060523
PushDirectory(E:\b\build\slave\Win8_GN__dbg_\build\src\media\test\data\) = 233
files
04:10:32.435549
PushDirectory(E:\b\build\slave\Win8_GN__dbg_\build\src\net\data\) = 974 files
04:10:32.464549
PushDirectory(E:\b\build\slave\Win8_GN__dbg_\build\src\net\tools\testserver\) =
11 files
[2843] [6/29.0Mib/39] [0/6] [0/0b/0/0b] 1s
04:10:32.505555
PushDirectory(E:\b\build\slave\Win8_GN__dbg_\build\src\third_party\pyftpdlib\) =
37 files
04:10:32.539557
PushDirectory(E:\b\build\slave\Win8_GN__dbg_\build\src\third_party\pywebsocket\)
= 99 files
04:10:32.578571
PushDirectory(E:\b\build\slave\Win8_GN__dbg_\build\src\third_party\tlslite\) =
105 files


content_browsertests  GetFileAttributesEx
E:\b\build\slave\Win8_GN__dbg_\build\src\out\Debug\msvcp120d.dll: The system
cannot find the file specified.
Hits    :     0 (0b)
Misses  :     0 (0b)
Duration: 1.301s
isolate: GetFileAttributesEx
E:\b\build\slave\Win8_GN__dbg_\build\src\out\Debug\msvcp120d.dll: The system
cannot find the file specified.
step returned non-zero exit code: 1
.