Added Windows 8.1/Windows Phone 8.1 minimal support for creating projects that compile modules that…

I need this change to create Windows 8.1/Windows Phone 8.1 projects that compile modules that can be consumed by Windows Store Applications (WinRT).

This change is required as a prerequisite to adding proper Windows Store (8.1 only) support to Google's ANGLE codebase.

Patch from coopp@microsoft.com.

R=scottmg@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=1981

[coop, 2014-09-18 18:01:35]

coopp@microsoft.com changed reviewers:
+ bradnelson@chromium.org, scottmg@chromium.org

[coop, 2014-09-18 18:04:16]

In order to un-fork the MS Open Tech version of Angle, we need to add support
for creating Windows 8.1 and Windows Phone 8.1 WinRT DLLs to the GYP tool.

ANGLE requires proper .GYP files to be authored to produce the required
projects/solutions for compilation.  This small change enables standard .dll
windows 8.1/phone 8.1 projects to be created.

[scottmg, 2014-09-18 18:18:01]

Can you write a test that exercises this somehow? Something that generates a
module and confirms that it has reasonable flags. There's similar stuff in
test/win/linker-flags, etc.

(Does building with ninja actually work? That's what msvs_emulation.py is for.)

https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py
File pylib/gyp/msvs_emulation.py (right):

https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py#...
pylib/gyp/msvs_emulation.py:479: lib('TargetMachine', map={'1': 'X86', '17':
'X64', '3': 'ARM'}, prefix='/MACHINE:')
nit; wrap to 80 col

[coop, 2014-09-18 18:23:06]

On 2014/09/18 18:18:01, scottmg wrote:
> Can you write a test that exercises this somehow? Something that generates a
> module and confirms that it has reasonable flags. There's similar stuff in
> test/win/linker-flags, etc.
> 
> (Does building with ninja actually work? That's what msvs_emulation.py is
for.)
> 
> https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py
> File pylib/gyp/msvs_emulation.py (right):
> 
>
https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py#...
> pylib/gyp/msvs_emulation.py:479: lib('TargetMachine', map={'1': 'X86', '17':
> 'X64', '3': 'ARM'}, prefix='/MACHINE:')
> nit; wrap to 80 col

I will add a test for this.  
I was hoping that you would come back with a pointer to where I could look for
an example of the type of test I should write.
And you did!! Thanks!

I ran the windows GYP tests with my changes and didn't see any regressions.

[scottmg, 2014-09-18 18:27:17]

On 2014/09/18 18:23:06, coop wrote:
> On 2014/09/18 18:18:01, scottmg wrote:
> > Can you write a test that exercises this somehow? Something that generates a
> > module and confirms that it has reasonable flags. There's similar stuff in
> > test/win/linker-flags, etc.
> > 
> > (Does building with ninja actually work? That's what msvs_emulation.py is
> for.)
> > 
> > https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py
> > File pylib/gyp/msvs_emulation.py (right):
> > 
> >
>
https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py#...
> > pylib/gyp/msvs_emulation.py:479: lib('TargetMachine', map={'1': 'X86', '17':
> > 'X64', '3': 'ARM'}, prefix='/MACHINE:')
> > nit; wrap to 80 col
> 
> I will add a test for this.  
> I was hoping that you would come back with a pointer to where I could look for
> an example of the type of test I should write.
> And you did!! Thanks!
> 
> I ran the windows GYP tests with my changes and didn't see any regressions.

OK, I'm not sure what toolchains are installed on the bots
http://build.chromium.org/f/client/gyp/, I think they're still on vs2012 for
some reason. Hopefully they include the ARM toolchain.

[coop, 2014-09-19 14:37:48]

On 2014/09/18 18:27:17, scottmg wrote:
> On 2014/09/18 18:23:06, coop wrote:
> > On 2014/09/18 18:18:01, scottmg wrote:
> > > Can you write a test that exercises this somehow? Something that generates
a
> > > module and confirms that it has reasonable flags. There's similar stuff in
> > > test/win/linker-flags, etc.
> > > 
> > > (Does building with ninja actually work? That's what msvs_emulation.py is
> > for.)
> > > 
> > >
https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py
> > > File pylib/gyp/msvs_emulation.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/521693002/diff/1/pylib/gyp/msvs_emulation.py#...
> > > pylib/gyp/msvs_emulation.py:479: lib('TargetMachine', map={'1': 'X86',
'17':
> > > 'X64', '3': 'ARM'}, prefix='/MACHINE:')
> > > nit; wrap to 80 col
> > 
> > I will add a test for this.  
> > I was hoping that you would come back with a pointer to where I could look
for
> > an example of the type of test I should write.
> > And you did!! Thanks!
> > 
> > I ran the windows GYP tests with my changes and didn't see any regressions.
> 
> OK, I'm not sure what toolchains are installed on the bots
> http://build.chromium.org/f/client/gyp/, I think they're still on vs2012 for
> some reason. Hopefully they include the ARM toolchain.

I fixed the 80 column requirement in my code review and added a target machine
test for attempting to link arm.
ARM is an interesting beast for msvc because native ARM .exe projects are not
supported.  Only WinRT ARM exes are allowed.

I am working on crafting an additional test for the other changes that will
attempt to verify that the toolset and other project properties are correctly
configured.

[coop, 2014-09-22 23:54:21]

I just added a simple test that uses the new setting and generates a .dll
project that is compiled and checked for success.  This test creates a windows
and windows phone compatible .dll.

[scottmg, 2014-09-23 00:01:13]

https://codereview.chromium.org/521693002/diff/40001/test/win/enable-winrt/dl...
File test/win/enable-winrt/dllmain.cc (right):

https://codereview.chromium.org/521693002/diff/40001/test/win/enable-winrt/dl...
test/win/enable-winrt/dllmain.cc:15: bool TryToUseSomeWinRT()
nit; please reformat this to the standard style (primarily: { on same line, 2
space indents)

https://codereview.chromium.org/521693002/diff/40001/test/win/enable-winrt/dl...
test/win/enable-winrt/dllmain.cc:17: ComPtr<IDisplayPropertiesStatics> dp;
This code isn't being run in the test, afaict. Does its existence confirm
something at link time?

https://codereview.chromium.org/521693002/diff/40001/test/win/gyptest-link-en...
File test/win/gyptest-link-enable-winrt.py (right):

https://codereview.chromium.org/521693002/diff/40001/test/win/gyptest-link-en...
test/win/gyptest-link-enable-winrt.py:20: test =
TestGyp.TestGyp(formats=['msvs'])
since this test doesn't run on ninja (that's fine, if someone wants WinPhone
support on ninja they can add it), please remove the change to
msvs_emulation.py, as that's not used for regular msvs solution generation.

[coop, 2014-09-23 14:47:39]

I updated my changes based on the code review feedback.  

Answers to a code review comment:

I have the WinRT code in the test module to force linking against a WinRT
specific export which verified that the generated project is configured
properly.

[scottmg, 2014-09-23 16:24:33]

On 2014/09/23 14:47:39, coop wrote:
> I updated my changes based on the code review feedback.  
> 
> Answers to a code review comment:
> 
> I have the WinRT code in the test module to force linking against a WinRT
> specific export which verified that the generated project is configured
> properly.

OK, thanks that's what I was wondering. (In general, you can click on the
comment and click "Reply" in the "Side-by-side diffs" view, so that the comments
appear threaded with each other.

[scottmg, 2014-09-23 16:25:09]

With the minor fiddling below, LGTM. Once you upload another patch with those
changes, I can land it for you.

Thanks!

https://codereview.chromium.org/521693002/diff/60001/pylib/gyp/msvs_emulation.py
File pylib/gyp/msvs_emulation.py (right):

https://codereview.chromium.org/521693002/diff/60001/pylib/gyp/msvs_emulation...
pylib/gyp/msvs_emulation.py:479: lib('TargetMachine', map={'1': 'X86', '17':
'X64'},
please revert this file

https://codereview.chromium.org/521693002/diff/60001/test/win/enable-winrt/dl...
File test/win/enable-winrt/dllmain.cc (right):

https://codereview.chromium.org/521693002/diff/60001/test/win/enable-winrt/dl...
test/win/enable-winrt/dllmain.cc:15: bool TryToUseSomeWinRT()
nit; { goes on previous line (sorry, I know this is annoying when you're not
used to it)

https://codereview.chromium.org/521693002/diff/60001/test/win/enable-winrt/dl...
test/win/enable-winrt/dllmain.cc:23: return true;
nit; too much indent

[scottmg, 2014-09-23 16:28:32]

Oh, and if there's an ANGLE bug regarding this, please add a BUG=xyz to the CL
description.

(Just to confirm the CLA signatories, I'm assuming based on your email address
you're Cooper Partin, right? The document doesn't have email addresses
unfortunately.)

[coop, 2014-09-23 16:55:34]

On 2014/09/23 16:28:32, scottmg wrote:
> Oh, and if there's an ANGLE bug regarding this, please add a BUG=xyz to the CL
> description.
> 
> (Just to confirm the CLA signatories, I'm assuming based on your email address
> you're Cooper Partin, right? The document doesn't have email addresses
> unfortunately.)

Yes, I am Cooper Partin :).  I do not have an official ANGLE bug for this.
This is prep work for an incoming ANGLE additions that I have already sent 
mail to Shannon about.

Thank you so much for jumping on this review.

[scottmg, 2014-09-23 17:04:15]

Committed patchset #5 (id:80001) manually as 1981 (presubmit successful).

[scottmg, 2014-09-23 17:04:25]

https://codereview.chromium.org/521693002/diff/80001/pylib/gyp/generator/msvs.py
File pylib/gyp/generator/msvs.py (right):

https://codereview.chromium.org/521693002/diff/80001/pylib/gyp/generator/msvs...
pylib/gyp/generator/msvs.py:2601: properties = {}
(missed this, deleted this line on landing.)