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

[Brad King]

On 09/10/2015 09:31 PM, Gilles Khouzam wrote:
> Here is the patch for Windows 10 Support which would be issue 15686.
[snip]
> This change requires the change for issue 0015674 for determining the
> version of the OS.

That change is now done as posted here:

 [PATCH] [CMake 0015674]: Windows: Correctly determine Windows version
 http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/14327/focus=14478

Please rebase the rest of your Win 10 patch on that and revise according
to the comments below.

> New Properties Added:
> VS_TARGET_PLATFORM_VERSION: Target property. Specifies that the SDK being
> used to compile the project. For Windows 10 RTM, that will be 10.0.10240.0.
> For Store apps, this property is required by Visual Studio. If the property
> is not specified, the system will be queried for the available Windows 10
> SDKs installed and the most recent but less than or equal version than the
> host system version will be set as a default (in
> CMAKE_VS_TARGET_PLATFORM_VERSION) and used.

Please split this part out into its own (preceding) patch, much like was
done for the windows version detection.  Also, currently the patch does
not implement [CMAKE_]VS_TARGET_PLATFORM_VERSION as previously discussed:

 http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/14201/focus=14246

For example:

> +  if (this->SystemVersion == "10.0")
> +    {
> +    // Find the default version of the Windows 10 SDK and set
> +    // a default CMAKE_VS_TARGET_PLATFORM_VERSION
> +    std::string sdkVersion = GetWindows10SDKVersion();
> +    if(sdkVersion.empty())
> +      {
> +      e << "Could not find an appropriate version of the Windows 10 SDK"
> +        << "installed on this machine";
> +      mf->IssueMessage(cmake::FATAL_ERROR, e.str());
> +      return false;
> +      }
> +    mf->AddDefinition("CMAKE_VS_TARGET_PLATFORM_VERSION",
> +      sdkVersion.c_str());
> +    }

Logic like this should be used to select a default SDK when neither the
property nor the variable is set.  I think we've had some confusion
between the user-settable target property/variable and the generator-
reported selection result used by the compiler id.  The latter is
used here in your patch:

> +        set(id_WindowsTargetPlatformVersion "<WindowsTargetPlatformVersion>${CMAKE_VS_TARGET_PLATFORM_VERSION}</WindowsTargetPlatformVersion>")

Currently the CMAKE_VS_<VAR> convention is used by the VS generators
to report information about what the've decided.  I think this makes
sense so a variable called CMAKE_VS_WINDOWS_TARGET_PLATFORM_VERSION
could serve this purpose (reporting to project code what VS decided
to use).

What we need separately is an interface for users and/or project code
to select a specific WindowsTargetPlatformVersion.  The target property
currently called VS_TARGET_PLATFORM_VERSION will work well for project
code to set the value for specific targets.  We still need another
setting for users or toolchain files to set to make it a project-wide
default.  Perhaps we should have a separate variable called

  CMAKE_WINDOWS_TARGET_PLATFORM_VERSION

and rename the target property to just

  WINDOWS_TARGET_PLATFORM_VERSION

The property should be initialized via SetPropertyDefault to copy
the variable setting into each target.

-Brad