[cmake-developers] [Patch] Adding Windows 10 support

[Brad King]

On 09/23/2015 06:48 PM, Gilles Khouzam wrote:
> This adds only the WINDOWS_TARGET_PLATFORM_VERSION property as it
> currently only supports the desktop scenario and is extracted
> from the rest of the Windows 10 Store support.

Thanks.  While reviewing this much simpler patch I realized that
the WindowsTargetPlatformVersion is more like PlatformToolset than
I previously thought.  This led me to another design, perhaps closer
to one of your earlier patches, in which the VS 2015+ generators
select the WindowsTargetPlatformVersion up front based either on
CMAKE_WINDOWS_TARGET_PLATFORM_VERSION or on a computed default.
Either way we should set CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION
to report the selection to CMakeDetermineCompilerId for use in
"CompilerId/VS-10.vcxproj.in".  For now I decided to leave out
support for per-target WINDOWS_TARGET_PLATFORM_VERSION properties.

While testing these changes I found a bug that I've now fixed:

 cmCoreTryCompile: Fix internal argument vector construction
 http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=710bde43

Please try out the attached patches based on that version.  Then
provide fixup patches based on the comments below if needed.

> If CMAKE_SYSTEM_VERSION is set to 10.0 then

After your change to add a third component to CMAKE_HOST_SYSTEM_VERSION
the value of CMAKE_SYSTEM_VERSION on a Windows 10 host may have
a third component.  Therefore we should check that the version
starts with "10.0" rather than is exactly this version.  Actually
perhaps we should use cmSystemTools::VersionCompare to do actual
integer version component checks.

> the latest Windows 10 SDK but not more recent than the current
> build of Windows.

Rather than using VerifyVersionInfo for this, shouldn't the check
simply compare the SDK version to the targeted CMAKE_SYSTEM_VERSION?
If we are cross-compiling or otherwise specifying a specific target
version of Windows then we do not want the SDK to exceed that
regardless of what the host is running.

Also, the sorting logic in GetWindows10SDKVersion appears to use
lexicographic string ordering rather than a component-wise integer
comparison.  This will not always produce the correct result.  This
is another candidate for using cmSystemTools::VersionCompare.

> There is one thing that I've changed that I want to make sure is
> the right thing. As it stands, CMAKE_SYSTEM_VERSION is only valid
> when cross-compiling, I've changed the CMakeDetermineSystem.cmake
> file to not use the HOST_SYSTEM_VERSION when CMAKE_SYSTEM_VERSION
> is set. Otherwise, we can only use this feature through
> CMAKE_SYSTEM_VERSION on cross-compiling scenarios.

That makes sense.  I've split that out into its own commit and
explained the motivation in the commit message.  See attached
patch.

> I'm not sure what the best way to test this feature, it can be
> added to any desktop project on Windows 10 and it should work
> properly. I've tried it with CMake itself and it's working fine
> and building against the Win10 SDK.

We could have the test suite detect when it is building on a
Windows 10 host and then add a test that sets
CMAKE_WINDOWS_TARGET_PLATFORM_VERSION and verifies that the
value ends up in a generated project file.  We already have
some tests that parse the generated .sln file to check for
specific content.

Also just simply by running on a Win 10 host then the entire test
suite should build with CMAKE_SYSTEM_VERSION set automatically high
enough to enable default SDK selection.  Please look at setting up
nightly testing submissions to the dashboard from such a host once
the changes are integrated.

Thanks,
-Brad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Allow-CMAKE_SYSTEM_VERSION-to-be-set-without-CMAKE_S.patch
Type: text/x-diff
Size: 1323 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20150924/1e44814c/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-VS-Add-hook-to-initialize-Windows-platform-settings.patch
Type: text/x-diff
Size: 3270 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20150924/1e44814c/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-VS-Add-support-for-selecting-the-Windows-10-SDK.patch
Type: text/x-diff
Size: 13967 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20150924/1e44814c/attachment-0002.patch>