Skip to content
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.

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

Treat old TFMs as unsupported by ILLink analyzers #35837

Merged
merged 4 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@
</PropertyGroup>

<Target Name="_AddKnownILLinkPack" BeforeTargets="ProcessFrameworkReferences">
<PropertyGroup>
<!-- Work around a version check that prevents referencing illink pack from a netstandard project. -->
<_FirstTargetFrameworkVersionToSupportTrimAnalyzer>2.0</_FirstTargetFrameworkVersionToSupportTrimAnalyzer>
</PropertyGroup>
<ItemGroup>
<KnownILLinkPack Include="@(KnownILLinkPack)"
Condition="'%(TargetFramework)' == '$(CurrentTargetFramework)'"
Expand Down
22 changes: 22 additions & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,14 @@ public class ProcessFrameworkReferences : TaskBase

public bool EnableAotAnalyzer { get; set; }

public string FirstTargetFrameworkVersionToSupportAotAnalyzer { get; set; }

public bool PublishTrimmed { get; set; }

public bool IsTrimmable { get; set; }

public string FirstTargetFrameworkVersionToSupportTrimAnalyzer { get; set; }

public bool SilenceIsTrimmableUnsupportedWarning { get; set; }

public string MinNonEolTargetFrameworkForTrimming { get; set; }
Expand All @@ -63,6 +67,8 @@ public class ProcessFrameworkReferences : TaskBase

public bool EnableSingleFileAnalyzer { get; set; }

public string FirstTargetFrameworkVersionToSupportSingleFileAnalyzer { get; set; }

public bool SilenceEnableSingleFileAnalyzerUnsupportedWarning { get; set; }

public string MinNonEolTargetFrameworkForSingleFile { get; set; }
Expand Down Expand Up @@ -828,6 +834,22 @@ private ToolPackSupport AddToolPack(
packagesToDownload.Add(runtimePackToDownload);
}

if (toolPackType is ToolPackType.ILLink)
{
// The ILLink tool pack is available for some TargetFrameworks where we nonetheless consider
// IsTrimmable/IsAotCompatible/EnableSingleFile to be unsupported, because the framework
// was not annotated with the attributes.
var firstTargetFrameworkVersionToSupportAotAnalyzer = NormalizeVersion(new Version(FirstTargetFrameworkVersionToSupportAotAnalyzer));
if ((IsAotCompatible || EnableAotAnalyzer) && normalizedTargetFrameworkVersion < firstTargetFrameworkVersionToSupportAotAnalyzer)
return ToolPackSupport.UnsupportedForTargetFramework;
var firstTargetFrameworkVersionToSupportSingleFileAnalyzer = NormalizeVersion(new Version(FirstTargetFrameworkVersionToSupportSingleFileAnalyzer));
if (EnableSingleFileAnalyzer && normalizedTargetFrameworkVersion < firstTargetFrameworkVersionToSupportSingleFileAnalyzer)
return ToolPackSupport.UnsupportedForTargetFramework;
var firstTargetFrameworkVersionToSupportTrimAnalyzer = NormalizeVersion(new Version(FirstTargetFrameworkVersionToSupportTrimAnalyzer));
if ((IsTrimmable || EnableTrimAnalyzer) && normalizedTargetFrameworkVersion < firstTargetFrameworkVersionToSupportTrimAnalyzer)
return ToolPackSupport.UnsupportedForTargetFramework;
}

// Packs with RID-agnostic build packages that contain MSBuild targets.
if (toolPackType is not ToolPackType.Crossgen2 && EnableRuntimePackDownload)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@ Copyright (c) .NET Foundation. All rights reserved.
<!-- PublishAot depends on PublishTrimmed. This must be set early enough for the KnownILLinkPack to be restored. -->
<PublishTrimmed Condition="'$(PublishTrimmed)' == '' And '$(PublishAot)' == 'true'">true</PublishTrimmed>
<IsTrimmable Condition="'$(IsTrimmable)' == '' and '$(IsAotCompatible)' == 'true'">true</IsTrimmable>

<_FirstTargetFrameworkToSupportTrimming>net6.0</_FirstTargetFrameworkToSupportTrimming>
<_FirstTargetFrameworkToSupportAot>net7.0</_FirstTargetFrameworkToSupportAot>
<_FirstTargetFrameworkToSupportSingleFile>net6.0</_FirstTargetFrameworkToSupportSingleFile>

<_FirstTargetFrameworkVersionToSupportTrimAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportTrimming)'))</_FirstTargetFrameworkVersionToSupportTrimAnalyzer>
<_FirstTargetFrameworkVersionToSupportAotAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportAot)'))</_FirstTargetFrameworkVersionToSupportAotAnalyzer>
<_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportSingleFile)'))</_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>
Comment on lines +26 to +32
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like the first set of properties are used anywhere else. Can we specify the version numbers directly instead of specifying the target frameworks and then parsing out the versions?

Suggested change
<_FirstTargetFrameworkToSupportTrimming>net6.0</_FirstTargetFrameworkToSupportTrimming>
<_FirstTargetFrameworkToSupportAot>net7.0</_FirstTargetFrameworkToSupportAot>
<_FirstTargetFrameworkToSupportSingleFile>net6.0</_FirstTargetFrameworkToSupportSingleFile>
<_FirstTargetFrameworkVersionToSupportTrimAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportTrimming)'))</_FirstTargetFrameworkVersionToSupportTrimAnalyzer>
<_FirstTargetFrameworkVersionToSupportAotAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportAot)'))</_FirstTargetFrameworkVersionToSupportAotAnalyzer>
<_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>$([MSBuild]::GetTargetFrameworkVersion('$(_FirstTargetFrameworkToSupportSingleFile)'))</_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>
<_FirstTargetFrameworkVersionToSupportTrimAnalyzer>6.0</_FirstTargetFrameworkVersionToSupportTrimAnalyzer>
<_FirstTargetFrameworkVersionToSupportAotAnalyzer>7.0</_FirstTargetFrameworkVersionToSupportAotAnalyzer>
<_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>6.0</_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer>

Copy link
Member Author

Choose a reason for hiding this comment

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

They're used by the IsTargetFrameworkCompatible checks added in #35767.

</PropertyGroup>

<ItemDefinitionGroup>
Expand All @@ -43,7 +51,6 @@ Copyright (c) .NET Foundation. All rights reserved.
<_RequiresILLinkPack Condition="'$(_RequiresILLinkPack)' == ''">false</_RequiresILLinkPack>
</PropertyGroup>


<!-- ProcessFrameworkReferences warns when the project settings introduce a dependency on a
tool pack that's not available for the target framework. For trimming, AOT, and single-file, the recommendation
is to multitarget the project to include a more recent TargetFramework. For correctly multitargeted
Expand All @@ -60,9 +67,6 @@ Copyright (c) .NET Foundation. All rights reserved.
and silences the warning if it is. -->

<PropertyGroup>
<_FirstTargetFrameworkToSupportTrimming>net6.0</_FirstTargetFrameworkToSupportTrimming>
<_FirstTargetFrameworkToSupportAot>net7.0</_FirstTargetFrameworkToSupportAot>
<_FirstTargetFrameworkToSupportSingleFile>net6.0</_FirstTargetFrameworkToSupportSingleFile>
<!-- The min non-EOL TFM has already caught up with the first TFM to support trimming/singlefile. No need to compare against it. -->
<_MinNonEolTargetFrameworkForTrimming>$(_MinimumNonEolSupportedNetCoreTargetFramework)</_MinNonEolTargetFrameworkForTrimming>
<_MinNonEolTargetFrameworkForSingleFile>$(_MinimumNonEolSupportedNetCoreTargetFramework)</_MinNonEolTargetFrameworkForSingleFile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,26 @@ Copyright (c) .NET Foundation. All rights reserved.

<!-- Intermediate step to enable ILLink.Analyzers so ILLink, Blazor, Xamarin, AOT, etc. can enable the same flags -->
<EnableSingleFileAnalyzer Condition="'$(EnableSingleFileAnalyzer)' == '' And
('$(PublishSingleFile)' == 'true' Or '$(PublishAot)' == 'true' Or '$(IsAotCompatible)' == 'true')">true</EnableSingleFileAnalyzer>
(
('$(PublishSingleFile)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer)'))) Or
('$(PublishAot)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)'))) Or
'$(IsAotCompatible)' == 'true'
)">true</EnableSingleFileAnalyzer>

<!-- Enable the trim analyzer when any trimming settings are enabled. Warnings may suppressed based on other settings. -->
<EnableTrimAnalyzer Condition="'$(EnableTrimAnalyzer)' == '' And
('$(PublishTrimmed)' == 'true' Or '$(IsTrimmable)' == 'true' Or '$(PublishAot)' == 'true')">true</EnableTrimAnalyzer>
(
('$(PublishTrimmed)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportTrimAnalyzer)'))) Or
('$(PublishAot)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)'))) Or
'$(IsTrimmable)' == 'true'
)">true</EnableTrimAnalyzer>

<!-- Enable the AOT analyzer when AOT is enabled. Warnings may suppressed based on other settings. -->
<EnableAotAnalyzer Condition="'$(EnableAotAnalyzer)' == '' And ('$(PublishAot)' == 'true' or '$(IsAotCompatible)' == 'true')">true</EnableAotAnalyzer>
<EnableAotAnalyzer Condition="'$(EnableAotAnalyzer)' == '' And
(
('$(PublishAot)' == 'true' And '$(TargetFrameworkIdentifier)' == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(_TargetFrameworkVersionWithoutV), '$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)'))) Or
'$(IsAotCompatible)' == 'true'
)">true</EnableAotAnalyzer>

<!-- EnforceCodeStyleInBuild Allows code style analyzers to be disabled in bulk via msbuild if the user wants to -->
<EnforceCodeStyleInBuild Condition="'$(EnforceCodeStyleInBuild)' == ''">false</EnforceCodeStyleInBuild>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@ Copyright (c) .NET Foundation. All rights reserved.
SilenceIsAotCompatibleUnsupportedWarning="$(_SilenceIsAotCompatibleUnsupportedWarning)"
MinNonEolTargetFrameworkForAot="$(_MinNonEolTargetFrameworkForAot)"
EnableAotAnalyzer="$(EnableAotAnalyzer)"
FirstTargetFrameworkVersionToSupportAotAnalyzer="$(_FirstTargetFrameworkVersionToSupportAotAnalyzer)"
PublishTrimmed="$(PublishTrimmed)"
IsTrimmable="$(IsTrimmable)"
FirstTargetFrameworkVersionToSupportTrimAnalyzer="$(_FirstTargetFrameworkVersionToSupportTrimAnalyzer)"
SilenceIsTrimmableUnsupportedWarning="$(_SilenceIsTrimmableUnsupportedWarning)"
MinNonEolTargetFrameworkForTrimming="$(_MinNonEolTargetFrameworkForTrimming)"
EnableTrimAnalyzer="$(EnableTrimAnalyzer)"
EnableSingleFileAnalyzer="$(EnableSingleFileAnalyzer)"
FirstTargetFrameworkVersionToSupportSingleFileAnalyzer="$(_FirstTargetFrameworkVersionToSupportSingleFileAnalyzer)"
SilenceEnableSingleFileAnalyzerUnsupportedWarning="$(_SilenceEnableSingleFileAnalyzerUnsupportedWarning)"
MinNonEolTargetFrameworkForSingleFile="$(_MinNonEolTargetFrameworkForSingleFile)"
AotUseKnownRuntimePackForTarget="$(PublishAotUsingRuntimePack)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,45 @@ public void ILLink_analyzer_warnings_are_produced_using_EnableSingleFileAnalyzer
.And.HaveStdOutContaining("(10,13): warning IL3001");
}


[RequiresMSBuildVersionTheory("17.0.0.32901")]
[InlineData("netcoreapp2.1", true)]
[InlineData("netcoreapp3.0", false)]
[InlineData("netcoreapp3.1", false)]
[InlineData("net5.0", false)]
[InlineData("net6.0", false)]
[InlineData("net7.0", false)]
public void PublishSingleFile_fails_for_unsupported_target_framework(string targetFramework, bool shouldFail)
{
var testProject = new TestProject()
{
Name = "HelloWorld",
IsExe = true,
TargetFrameworks = targetFramework
};
testProject.AdditionalProperties["PublishSingleFile"] = "true";
testProject.AdditionalProperties["SelfContained"] = "true";
testProject.AdditionalProperties["NoWarn"] = "NETSDK1138"; // Silence warning about targeting EOL TFMs
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework);

var publishCommand = new PublishCommand(testAsset);
var result = publishCommand.Execute(RuntimeIdentifier);
if (shouldFail)
{
result.Should().Fail()
.And.HaveStdOutContaining(Strings.PublishSingleFileRequiresVersion30);
}
else
{
result.Should().Pass()
.And.NotHaveStdOutContaining("warning");
}
}

[RequiresMSBuildVersionTheory("17.8.0")]
[InlineData("netstandard2.0", true)]
[InlineData("net5.0", true)]
[InlineData("net6.0", false)]
[InlineData("netstandard2.0;net5.0", true)] // None of these TFMs are supported for single-file
[InlineData("netstandard2.0;net6.0", false)] // Net6.0 is the min TFM supported for single-file and targeting.
[InlineData("netstandard2.0;net8.0", true)] // Net8.0 is supported for single-file, but leaves a "gap" for the supported net6./net7.0 TFMs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Reflection.PortableExecutable;
using Microsoft.DotNet.Cli.Utils;
using Microsoft.NET.Build.Tasks;
using Newtonsoft.Json.Linq;
using static Microsoft.NET.Publish.Tests.PublishTestUtils;

Expand Down Expand Up @@ -590,13 +591,49 @@ public void IsAotCompatible_implies_enable_analyzers(string targetFramework)
.And.NotHaveStdOutContaining("warning IL3002");
}

[RequiresMSBuildVersionTheory("17.0.0.32901")]
[InlineData("net5.0", true)]
[InlineData("net6.0", true)]
[InlineData("net7.0", false)]
public void PublishAot_fails_for_unsupported_target_framework(string targetFramework, bool shouldFail)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
// OSX wasn't supported before net8
return;
}

var rid = EnvironmentInfo.GetCompatibleRid(targetFramework);

var testProject = new TestProject()
{
Name = "HelloWorld",
TargetFrameworks = targetFramework
};
testProject.AdditionalProperties["PublishAot"] = "true";
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework);

var publishCommand = new PublishCommand(testAsset);
var result = publishCommand.Execute($"/p:RuntimeIdentifier={rid}");
if (shouldFail) {
result.Should().Fail()
.And.HaveStdOutContaining(Strings.AotUnsupportedTargetFramework);
} else {
result.Should().Pass()
.And.NotHaveStdOutContaining("warning");
}
}

[RequiresMSBuildVersionTheory("17.8.0")]
[InlineData("netstandard2.0", true)]
[InlineData("net6.0", true)]
[InlineData("net7.0", false)]
[InlineData("netstandard2.0;net5.0", true)] // None of these TFMs are supported for AOT
[InlineData("netstandard2.0;net7.0", false)] // Net7.0 is the min TFM supported for AOT and targeting.
[InlineData("netstandard2.0;net8.0", true)] // Net8.0 is supported for AOT, but leaves a "gap" for the supported net7.0 TFMs.
[InlineData("alias-ns2", true)]
[InlineData("alias-n6", false)]
[InlineData("alias-n6", true)]
[InlineData("alias-n7", false)]
[InlineData("alias-n7;alias-n8", false)] // If all TFMs are supported, there's no warning even though the project uses aliases.
[InlineData("alias-ns2;alias-n7", true)] // This is correctly multi-targeted, but the logic can't detect this due to the alias so it still warns.
public void IsAotCompatible_warns_when_expected_for_not_correctly_multitarget_libraries(string targetFrameworks, bool shouldWarn)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,25 +148,39 @@ public void PublishTrimmed_fails_when_no_matching_pack_is_found(string targetFra
}

[RequiresMSBuildVersionTheory("17.0.0.32901")]
[InlineData("netcoreapp2.0")]
[InlineData("netcoreapp2.1")]
[InlineData("netstandard2.1")]
public void PublishTrimmed_fails_for_unsupported_target_framework(string targetFramework)
[InlineData("netcoreapp2.0", true)]
[InlineData("netcoreapp2.1", true)]
[InlineData("netstandard2.1", true)]
[InlineData("netcoreapp3.0", false)]
[InlineData("netcoreapp3.1", false)]
[InlineData("net5.0", false)]
[InlineData("net6.0", false)]
public void PublishTrimmed_fails_for_unsupported_target_framework(string targetFramework, bool shouldFail)
{
var projectName = "HelloWorld";
var rid = EnvironmentInfo.GetCompatibleRid(targetFramework);

var testProject = CreateTestProjectForILLinkTesting(targetFramework, projectName);
testProject.AdditionalProperties["PublishTrimmed"] = "true";
testProject.AdditionalProperties["NoWarn"] = "NETSDK1138"; // Silence warning about targeting EOL TFMs
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework);
var publishCommand = new PublishCommand(testAsset);
publishCommand.Execute($"/p:RuntimeIdentifier={rid}", "/p:PublishTrimmed=true")
.Should().Fail()
.And.HaveStdOutContaining($"error {Strings.PublishTrimmedRequiresVersion30}");
var result = publishCommand.Execute($"/p:RuntimeIdentifier={rid}");
if (shouldFail) {
result.Should().Fail()
.And.HaveStdOutContaining($"error {Strings.PublishTrimmedRequiresVersion30}");
} else {
result.Should().Pass()
.And.NotHaveStdOutContaining("warning");
}
}

[RequiresMSBuildVersionTheory("17.8.0")]
[InlineData("netstandard2.0", true)]
[InlineData("netstandard2.1", true)]
[InlineData("netcoreapp3.1", true)]
[InlineData("net5.0", true)]
[InlineData("net6.0", false)]
[InlineData("netstandard2.0;net5.0", true)] // None of these TFMs are supported for trimming
[InlineData("netstandard2.0;net6.0", false)] // Net6.0 is the min TFM supported for trimming and targeting.
[InlineData("netstandard2.0;net8.0", true)] // Net8.0 is supported for trimming, but leaves a "gap" for the supported net6.0/net7.0 TFMs.
Expand All @@ -181,6 +195,7 @@ public void IsTrimmable_warns_when_expected_for_not_correctly_multitargeted_libr

var testProject = CreateTestProjectForILLinkTesting(targetFrameworks, projectName);
testProject.AdditionalProperties["IsTrimmable"] = "true";
testProject.AdditionalProperties["NoWarn"] = "NETSDK1138"; // Silence warning about targeting EOL TFMs
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFrameworks)
.WithProjectChanges(AddTargetFrameworkAliases);

Expand Down