Skip to content

Switching to 4 part Windows versions #13272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
3 commits merged into from
Sep 1, 2020
Merged

Switching to 4 part Windows versions #13272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
292d65e
Switching to 4 part Windows versions
sfoslund Aug 31, 2020
82569d2
PR feedback
sfoslund Aug 31, 2020
12839ff
Fixing target platform version test value
sfoslund Sep 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.TargetFrameworkInference.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,17 @@ Copyright (c) .NET Foundation. All rights reserved.
<PropertyGroup Condition="'$(TargetFramework)' != '' and ('$(TargetFrameworkIdentifier)' == '' or '$(TargetFrameworkVersion)' == '')">

<TargetFrameworkIdentifier>$([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)'))</TargetFrameworkIdentifier>
<TargetFrameworkVersion>v$([MSBuild]::GetTargetFrameworkVersion('$(TargetFramework)', 3))</TargetFrameworkVersion>
<_TargetFrameworkVersionLength Condition="$(TargetFrameworkVersion.EndsWith('.0'))" >$([MSBuild]::Subtract($(TargetFrameworkVersion.Length), 2))</_TargetFrameworkVersionLength>
<TargetFrameworkVersion Condition="$(TargetFrameworkVersion.EndsWith('.0'))" >$(TargetFrameworkVersion.Substring(0, $(_TargetFrameworkVersionLength)))</TargetFrameworkVersion>
<TargetFrameworkVersion>v$([MSBuild]::GetTargetFrameworkVersion('$(TargetFramework)', 2))</TargetFrameworkVersion>
</PropertyGroup>

<!--
Parse TargetPlatform properties.
-->
<PropertyGroup Condition="'$(TargetFramework)' != '' and ('$(TargetPlatformIdentifier)' == '' and '$(TargetPlatformVersion)' == '')">
<TargetPlatformIdentifier>$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)'))</TargetPlatformIdentifier>
<TargetPlatformVersion>$([MSBuild]::GetTargetPlatformVersion('$(TargetFramework)', 3))</TargetPlatformVersion>
<TargetPlatformVersion Condition="'$(TargetPlatformVersion)' == '0.0.0'" ></TargetPlatformVersion>
<_TargetPlatformVersionLength Condition="$(TargetPlatformVersion.EndsWith('.0'))" >$([MSBuild]::Subtract($(TargetPlatformVersion.Length), 2))</_TargetPlatformVersionLength>
<TargetPlatformVersion Condition="$(TargetPlatformVersion.EndsWith('.0'))" >$(TargetPlatformVersion.Substring(0, $(_TargetPlatformVersionLength)))</TargetPlatformVersion>
<PropertyGroup Condition="'$(TargetFramework)' != '' and ('$(TargetPlatformIdentifier)' == '' or '$(TargetPlatformVersion)' == '')">
<TargetPlatformIdentifier Condition="'$(TargetPlatformIdentifier)' == ''">$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)'))</TargetPlatformIdentifier>
<TargetPlatformVersion Condition="'$(TargetPlatformIdentifier)' == 'Windows'" >$([MSBuild]::GetTargetPlatformVersion('$(TargetFramework)', 4))</TargetPlatformVersion>
<TargetPlatformVersion Condition="'$(TargetPlatformVersion)' == '' or ('$(TargetPlatformIdentifier)' == 'Windows' and !$([MSBuild]::VersionGreaterThanOrEquals($(TargetPlatformVersion), 10.0)))" >$([MSBuild]::GetTargetPlatformVersion('$(TargetFramework)', 2))</TargetPlatformVersion>
<TargetPlatformVersion Condition="$([MSBuild]::VersionEquals($(TargetPlatformVersion), 0.0))" ></TargetPlatformVersion>
<!-- Normalize casing of windows to Windows -->
<TargetPlatformIdentifier Condition="'$(TargetPlatformIdentifier)' == 'Windows'">Windows</TargetPlatformIdentifier>
</PropertyGroup>
Expand Down
6 changes: 3 additions & 3 deletions .../Microsoft.NET.Build.Tasks/targets/Microsoft.NET.WindowsSdkSupportedTargetPlatforms.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- This file contains a list of the windows target platform versions that are supported by this SDK for .NET. Supported versions are processed in _NormalizeTargetPlatformVersion -->
<Project>
<ItemGroup>
<WindowsSdkSupportedTargetPlatform Include="10.0.19041" />
<WindowsSdkSupportedTargetPlatform Include="10.0.18362" />
<WindowsSdkSupportedTargetPlatform Include="10.0.17763" />
<WindowsSdkSupportedTargetPlatform Include="10.0.19041.0" />
<WindowsSdkSupportedTargetPlatform Include="10.0.18362.0" />
<WindowsSdkSupportedTargetPlatform Include="10.0.17763.0" />
<WindowsSdkSupportedTargetPlatform Include="8.0" />
<WindowsSdkSupportedTargetPlatform Include="7.0" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildALibrary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ public void It_implicitly_defines_compilation_constants_for_the_target_platform(
[Theory]
[InlineData(new[] { "1.0", "1.1" }, "ios", "1.1", new[] { "IOS", "IOS1_1", "IOS1_0" })]
[InlineData(new[] { "11.11", "12.12", "13.13" }, "android", "12.12", new[] { "ANDROID", "ANDROID11_11", "ANDROID12_12" })]
[InlineData(new string[] { /* Use the built in SdkSupportedTargetPlatform items */}, "windows", "10.0.19041", new[] { "WINDOWS", "WINDOWS7_0", "WINDOWS8_0", "WINDOWS10_0_17763", "WINDOWS10_0_18362", "WINDOWS10_0_19041" })]
[InlineData(new string[] { /* Use the built in SdkSupportedTargetPlatform items */}, "windows", "10.0.19041.0", new[] { "WINDOWS", "WINDOWS7_0", "WINDOWS8_0", "WINDOWS10_0_17763_0", "WINDOWS10_0_18362_0", "WINDOWS10_0_19041_0" })]
Copy link
Member

@dsplaisted dsplaisted Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh. This is a side effect I didn't consider. I think we probably want these to be based on the 3 part version if the 4th part is 0.

@terrajobst thoughts?

Copy link

@wli3 wli3 Sep 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we decide on 4 parts, everything should be 4 parts.

public void It_implicitly_defines_compilation_constants_for_the_target_platform_with_backwards_compatibility(string[] sdkSupportedTargetPlatform, string targetPlatformIdentifier, string targetPlatformVersion, string[] expectedDefines)
{
var targetFramework = "net5.0";
Expand Down
17 changes: 11 additions & 6 deletions src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildAWindowsDesktopProject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,22 @@ public void It_fails_if_windows_target_platform_version_is_invalid()
.HaveStdOutContaining("NETSDK1140");
}

[WindowsOnlyFact]
public void It_succeeds_if_windows_target_platform_version_has_trailing_zeros()
[WindowsOnlyTheory]
[InlineData(true)]
[InlineData(false)]
public void It_succeeds_if_windows_target_platform_version_does_not_have_trailing_zeros(bool setInTargetframework)
{
var testProject = new TestProject()
{
Name = "ValidWindowsVersion",
IsSdkProject = true,
TargetFrameworks = "net5.0"
TargetFrameworks = setInTargetframework ? "net5.0-windows10.0.18362" : "net5.0"
};
testProject.AdditionalProperties["TargetPlatformIdentifier"] = "Windows";
testProject.AdditionalProperties["TargetPlatformVersion"] = "10.0.18362.0"; // We must set this manually because if we set it in the TFM we remove the trailing zeroes.
if (!setInTargetframework)
{
testProject.AdditionalProperties["TargetPlatformIdentifier"] = "Windows";
testProject.AdditionalProperties["TargetPlatformVersion"] = "10.0.18362";
}
var testAsset = _testAssetsManager.CreateTestProject(testProject);

var buildCommand = new BuildCommand(testAsset);
Expand All @@ -144,7 +149,7 @@ public void It_succeeds_if_windows_target_platform_version_has_trailing_zeros()
getValuesCommand.Execute()
.Should()
.Pass();
getValuesCommand.GetValues().Should().BeEquivalentTo(new[] { "10.0.18362" });
getValuesCommand.GetValues().Should().BeEquivalentTo(new[] { "10.0.18362.0" });
}

[Fact]
Expand Down
24 changes: 23 additions & 1 deletion src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildWithATargetPlatform.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public GivenThatWeWantToBuildWithATargetPlatform(ITestOutputHelper log) : base(l
[InlineData("net5.0-Windows7.0", ".NETCoreApp", "v5.0", "Windows", "7.0")]
[InlineData("net5.0-WINDOWS7.0", ".NETCoreApp", "v5.0", "Windows", "7.0")]
[InlineData("net5.0-windows", ".NETCoreApp", "v5.0", "Windows", "7.0")]
[InlineData("net5.0-windows10.0.19041", ".NETCoreApp", "v5.0", "Windows", "10.0.19041")]
[InlineData("net5.0-windows10.0.19041.0", ".NETCoreApp", "v5.0", "Windows", "10.0.19041.0")]
public void It_defines_target_platform_from_target_framework(string targetFramework, string expectedTargetFrameworkIdentifier, string expectedTargetFrameworkVersion, string expectedTargetPlatformIdentifier, string expectedTargetPlatformVersion)
{
var testProj = new TestProject()
Expand Down Expand Up @@ -63,6 +63,28 @@ public void It_defines_target_platform_from_target_framework(string targetFramew
assertValue("TargetPlatformDisplayName", $"{expectedTargetPlatformIdentifier} {expectedTargetPlatformVersion}");
}

[WindowsOnlyRequiresMSBuildVersionFact("16.8.0.41402")]
public void It_defines_target_platform_from_target_framework_with_explicit_version()
{
var targetPlatformVersion = "10.0.19041.0";
var targetFramework = "net5.0-windows";
var testProj = new TestProject()
{
Name = "TargetPlatformTests",
IsSdkProject = true,
TargetFrameworks = targetFramework
};
testProj.AdditionalProperties["TargetPlatformVersion"] = targetPlatformVersion;
var testAsset = _testAssetsManager.CreateTestProject(testProj);

var getValuesCommand = new GetValuesCommand(Log, Path.Combine(testAsset.Path, testProj.Name), targetFramework, "TargetPlatformIdentifier");
getValuesCommand
.Execute()
.Should()
.Pass();
getValuesCommand.GetValues().ShouldBeEquivalentTo(new[] { "Windows" });
}

[Fact]
public void It_fails_on_unsupported_os()
{
Expand Down