Skip to content

Don't omit Microsoft.NETCore.App from list of frameworks in runtimeconfig #17982

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
merged 2 commits into from
Jun 2, 2021
Merged

Don't omit Microsoft.NETCore.App from list of frameworks in runtimeconfig #17982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c25d638
Don't omit Microsoft.NETCore.App from list of frameworks in runtimeco…
dsplaisted May 31, 2021
6c4881d
Code review feedback
dsplaisted Jun 2, 2021
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
13 changes: 8 additions & 5 deletions src/Tasks/Microsoft.NET.Build.Tasks/GenerateRuntimeConfigurationFiles.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public class GenerateRuntimeConfigurationFiles : TaskBase

public bool GenerateRuntimeConfigDevFile { get; set; }

public bool AlwaysIncludeCoreFramework { get; set; }

List<ITaskItem> _filesWritten = new List<ITaskItem>();

private static readonly string[] RollForwardValues = new string[]
Expand Down Expand Up @@ -206,14 +208,15 @@ private void AddFrameworks(RuntimeOptions runtimeOptions,
HashSet<string> usedFrameworkNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (var platformLibrary in runtimeFrameworks)
{
if (runtimeFrameworks.Length > 1 &&
// In earlier versions of the SDK, we would exclude Microsoft.NETCore.App from the frameworks listed in the runtimeconfig file.
// This was originally a workaround for a bug: https://github.com/dotnet/core-setup/issues/4947
// We would only do this for framework-dependent apps, as the full list was required for self-contained apps.
// As the bug is fixed, we now always include the Microsoft.NETCore.App framework by default for .NET Core 6 and higher
if (!AlwaysIncludeCoreFramework &&
runtimeFrameworks.Length > 1 &&
platformLibrary.Name.Equals("Microsoft.NETCore.App", StringComparison.OrdinalIgnoreCase) &&
isFrameworkDependent)
{
// If there are multiple runtime frameworks, then exclude Microsoft.NETCore.App,
// as a workaround for https://github.com/dotnet/core-setup/issues/4947
// The workaround only applies to normal framework references, included frameworks
// (in self-contained apps) must list all frameworks.
continue;
}

Expand Down
1 change: 1 addition & 0 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.DesignerSupport.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ Copyright (c) .NET Foundation. All rights reserved.
TargetFrameworkMoniker="$(TargetFrameworkMoniker)"
UserRuntimeConfig="$(UserRuntimeConfig)"
WriteAdditionalProbingPathsToMainConfig="true"
AlwaysIncludeCoreFramework="$(AlwaysIncludeCoreFrameworkInRuntimeConfig)"
/>

<ItemGroup>
Expand Down
5 changes: 4 additions & 1 deletion src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ Copyright (c) .NET Foundation. All rights reserved.
<PropertyGroup>
<EnableDynamicLoading Condition="'$(EnableDynamicLoading)' == '' and '$(EnableComHosting)' == 'true'">true</EnableDynamicLoading>
<GenerateRuntimeConfigurationFiles Condition=" '$(GenerateRuntimeConfigurationFiles)' == '' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and ('$(HasRuntimeOutput)' == 'true' or '$(EnableComHosting)' == 'true' or '$(EnableDynamicLoading)' == 'true') ">true</GenerateRuntimeConfigurationFiles>
<AlwaysIncludeCoreFrameworkInRuntimeConfig Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionLessThan('$(TargetFrameworkVersion)', '6.0'))">false</AlwaysIncludeCoreFrameworkInRuntimeConfig>
<AlwaysIncludeCoreFrameworkInRuntimeConfig Condition="'$(AlwaysIncludeCoreFrameworkInRuntimeConfig)' == ''">true</AlwaysIncludeCoreFrameworkInRuntimeConfig>
<UserRuntimeConfig Condition=" '$(UserRuntimeConfig)' == '' ">$(MSBuildProjectDirectory)/runtimeconfig.template.json</UserRuntimeConfig>
<GenerateSatelliteAssembliesForCore Condition=" '$(GenerateSatelliteAssembliesForCore)' == '' and '$(MSBuildRuntimeType)' == 'Core' ">true</GenerateSatelliteAssembliesForCore>
<ComputeNETCoreBuildOutputFiles Condition=" '$(ComputeNETCoreBuildOutputFiles)' == '' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp'">true</ComputeNETCoreBuildOutputFiles>
Expand Down Expand Up @@ -266,7 +268,8 @@ Copyright (c) .NET Foundation. All rights reserved.
AdditionalProbingPaths="@(AdditionalProbingPath)"
IsSelfContained="$(SelfContained)"
WriteIncludedFrameworks="$(_WriteIncludedFrameworks)"
GenerateRuntimeConfigDevFile="$(GenerateRuntimeConfigDevFile)">
GenerateRuntimeConfigDevFile="$(GenerateRuntimeConfigDevFile)"
AlwaysIncludeCoreFramework="$(AlwaysIncludeCoreFrameworkInRuntimeConfig)">

</GenerateRuntimeConfigurationFiles>

Expand Down
33 changes: 19 additions & 14 deletions src/Tests/Microsoft.NET.Build.Tests/GivenFrameworkReferences.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,15 @@ public static void Main(string [] args)
}
}";

[WindowsOnlyFact]
public void Multiple_frameworks_are_written_to_runtimeconfig_when_there_are_multiple_FrameworkReferences()
[WindowsOnlyTheory]
[InlineData("net6.0", true)]
[InlineData("netcoreapp3.1", false)]
public void Multiple_frameworks_are_written_to_runtimeconfig_when_there_are_multiple_FrameworkReferences(string targetFramework, bool shouldIncludeBaseFramework)
{
var testProject = new TestProject()
{
Name = "MultipleFrameworkReferenceTest",
Copy link

Choose a reason for hiding this comment

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

test for lower TFM still having the Microsoft.NETCore.App?

TargetFrameworks = "netcoreapp3.0",
TargetFrameworks = targetFramework,
IsExe = true
};

Expand All @@ -49,7 +51,7 @@ public void Multiple_frameworks_are_written_to_runtimeconfig_when_there_are_mult

testProject.SourceFiles.Add("Program.cs", FrameworkReferenceEmptyProgramSource);

var testAsset = _testAssetsManager.CreateTestProject(testProject);
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFramework);

var buildCommand = new BuildCommand(testAsset);

Expand All @@ -63,9 +65,14 @@ public void Multiple_frameworks_are_written_to_runtimeconfig_when_there_are_mult
string runtimeConfigFile = Path.Combine(outputDirectory.FullName, testProject.Name + ".runtimeconfig.json");
var runtimeFrameworkNames = GetRuntimeFrameworks(runtimeConfigFile);

// When we remove the workaround for https://github.com/dotnet/core-setup/issues/4947 in GenerateRuntimeConfigurationFiles,
// Microsoft.NETCore.App will need to be added to this list
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.AspNetCore.App", "Microsoft.WindowsDesktop.App");
if (shouldIncludeBaseFramework)
{
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.AspNetCore.App", "Microsoft.WindowsDesktop.App", "Microsoft.NETCore.App");
}
else
{
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.AspNetCore.App", "Microsoft.WindowsDesktop.App");
}
}

[Theory]
Expand Down Expand Up @@ -148,7 +155,7 @@ public void DuplicateFrameworksAreNotWrittenToRuntimeConfigWhenThereAreDifferent
var testProject = new TestProject()
{
Name = "MultipleProfileFrameworkReferenceTest",
TargetFrameworks = "netcoreapp3.0",
TargetFrameworks = "net6.0",
IsExe = true
};

Expand All @@ -171,9 +178,7 @@ public void DuplicateFrameworksAreNotWrittenToRuntimeConfigWhenThereAreDifferent
string runtimeConfigFile = Path.Combine(outputDirectory.FullName, testProject.Name + ".runtimeconfig.json");
var runtimeFrameworkNames = GetRuntimeFrameworks(runtimeConfigFile);

// When we remove the workaround for https://github.com/dotnet/core-setup/issues/4947 in GenerateRuntimeConfigurationFiles,
// Microsoft.NETCore.App will need to be added to this list
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.WindowsDesktop.App");
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.WindowsDesktop.App", "Microsoft.NETCore.App");
}

[Fact]
Expand Down Expand Up @@ -677,14 +682,14 @@ public void TransitiveFrameworkReferenceFromProjectReference()
var testProject = new TestProject()
{
Name = "TransitiveFrameworkReference",
TargetFrameworks = "netcoreapp3.0",
TargetFrameworks = "net6.0",
IsExe = true
};

var referencedProject = new TestProject()
{
Name = "ReferencedProject",
TargetFrameworks = "netcoreapp3.0",
TargetFrameworks = "net6.0",
};

referencedProject.FrameworkReferences.Add("Microsoft.ASPNETCORE.App");
Expand All @@ -707,7 +712,7 @@ public void TransitiveFrameworkReferenceFromProjectReference()

// When we remove the workaround for https://github.com/dotnet/core-setup/issues/4947 in GenerateRuntimeConfigurationFiles,
// Microsoft.NETCore.App will need to be added to this list
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.AspNetCore.App");
runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.AspNetCore.App", "Microsoft.NETCore.App");
}

[Fact]
Expand Down