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

[Gilles Khouzam]

Thanks for the feedback Brad,

I have rebased the changes and I think that I have the proper default functionality properly implemented. I've extracted the WINDOWS_TARGET_PLATFORM_VERSION changes into a separate patch.

WINDOWS_TARGET_PLATFORM_VERSION is a target property, for that will specify the WindowsTargetPlatformVersion tag in VS.
WINDOWS_TARGET_PLATFORM_MIN_VERSION is a target property that will specify the WindowsTargetPlatformMinVersion tag in VS.

Now, for the default behavior, if CMAKE_WINDOWS_TARGET_PLATFORM_VERSION is set through a toolchain file or the project, then that will be the default which will initialize the WINDOWS_TARGET_PLATFORM_VERSION for each target through the SetPropertyDefault initialization call. On the other hand, if CMAKE_WINDOWS_TARGET_PLATFORM_VERSION is not set, nothing should happen since this is not a required property other than for Windows 10 Universal (store) apps, the default behavior in that case should be to not have the property.

There is one open issue though. How should we have the value for CMAKE_WINDOWS_TARGET_PLATFORM_VERSION be the latest installed SDK when this is not a Windows Store project? For Windows Store projects this would get set if the property is not defined through the InitializeSystem procedure. How would we handle this for the non Windows Store case? Do this based on the version and no CMAKE_SYSTEM_NAME defined? Or should we force there to be a CMAKE_SYSTEM_NAME to be defined as Windows for example?


-----Original Message-----
From: Brad King [mailto:brad.king at kitware.com] 
Sent: Monday, September 21, 2015 08:09
To: Gilles Khouzam <Gilles.Khouzam at microsoft.com>
Cc: cmake-developers at cmake.org
Subject: Re: [cmake-developers] [Patch] Adding Windows 10 support

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