Skip to content

Support runtime packs with differing runtime IDs #12199

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 1 commit into from
Jun 25, 2020
Merged

Support runtime packs with differing runtime IDs #12199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
103 changes: 56 additions & 47 deletions src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Text.RegularExpressions;
using Microsoft.Build.Framework;
using Microsoft.Build.Utilities;
using Newtonsoft.Json;
Expand Down Expand Up @@ -230,58 +229,74 @@ protected override void ExecuteCore()
isTrimmable = selectedRuntimePack?.IsTrimmable;
}

// Only add runtime packs where the framework reference name matches the RuntimeFrameworkName
// Framework references for "profiles" will use the runtime pack from the corresponding non-profile framework
bool includeInPackageDownload;
KnownRuntimePack runtimePackForRuntimeIDProcessing;
if (knownFrameworkReference.Name.Equals(knownFrameworkReference.RuntimeFrameworkName, StringComparison.OrdinalIgnoreCase))
{
bool processedPrimaryRuntimeIdentifier = false;
// Only add runtime packs where the framework reference name matches the RuntimeFrameworkName
Copy link

Choose a reason for hiding this comment

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

Ideally these comments should be able to express in code. But meanwhile this class should really split into 4-5 class to make things clear. I will try to refactor that. (you should try too if you have time. reference http://arlobelshee.com/the-core-6-refactorings/)

// Framework references for "profiles" will use the runtime pack from the corresponding non-profile framework
runtimePackForRuntimeIDProcessing = selectedRuntimePack.Value;
includeInPackageDownload = true;
}
else if (!knownFrameworkReference.RuntimePackRuntimeIdentifiers.Equals(selectedRuntimePack?.RuntimePackRuntimeIdentifiers))
{
// If the profile has a different set of runtime identifiers than the runtime pack, use the profile.
runtimePackForRuntimeIDProcessing = knownFrameworkReference.ToKnownRuntimePack();
includeInPackageDownload = true;
}
else
Copy link

Choose a reason for hiding this comment

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

this "else" I am not too sure. Is it the same logic before #11824 ?

{
// For the remaining profiles, don't include them in package download but add them to unavaliable if necessary.
runtimePackForRuntimeIDProcessing = knownFrameworkReference.ToKnownRuntimePack();
includeInPackageDownload = false;
}

if ((SelfContained || ReadyToRunEnabled) &&
!string.IsNullOrEmpty(RuntimeIdentifier) &&
selectedRuntimePack != null &&
!string.IsNullOrEmpty(selectedRuntimePack?.RuntimePackNamePatterns))
{
bool processedPrimaryRuntimeIdentifier = false;

// Find other KnownFrameworkReferences that map to the same runtime pack, if any
List<string> additionalFrameworkReferencesForRuntimePack = null;
foreach (var additionalKnownFrameworkReference in knownFrameworkReferencesForTargetFramework)
if ((SelfContained || ReadyToRunEnabled) &&
!string.IsNullOrEmpty(RuntimeIdentifier) &&
selectedRuntimePack != null &&
!string.IsNullOrEmpty(selectedRuntimePack?.RuntimePackNamePatterns))
{

// Find other KnownFrameworkReferences that map to the same runtime pack, if any
List<string> additionalFrameworkReferencesForRuntimePack = null;
foreach (var additionalKnownFrameworkReference in knownFrameworkReferencesForTargetFramework)
{
if (additionalKnownFrameworkReference.RuntimeFrameworkName.Equals(knownFrameworkReference.RuntimeFrameworkName, StringComparison.OrdinalIgnoreCase) &&
!additionalKnownFrameworkReference.RuntimeFrameworkName.Equals(additionalKnownFrameworkReference.Name, StringComparison.OrdinalIgnoreCase))
{
if (additionalKnownFrameworkReference.RuntimeFrameworkName.Equals(knownFrameworkReference.RuntimeFrameworkName, StringComparison.OrdinalIgnoreCase) &&
!additionalKnownFrameworkReference.RuntimeFrameworkName.Equals(additionalKnownFrameworkReference.Name, StringComparison.OrdinalIgnoreCase))
if (additionalFrameworkReferencesForRuntimePack == null)
{
if (additionalFrameworkReferencesForRuntimePack == null)
{
additionalFrameworkReferencesForRuntimePack = new List<string>();
}
additionalFrameworkReferencesForRuntimePack.Add(additionalKnownFrameworkReference.Name);
additionalFrameworkReferencesForRuntimePack = new List<string>();
}
additionalFrameworkReferencesForRuntimePack.Add(additionalKnownFrameworkReference.Name);
}
}

ProcessRuntimeIdentifier(RuntimeIdentifier, selectedRuntimePack.Value, runtimePackVersion, additionalFrameworkReferencesForRuntimePack,
unrecognizedRuntimeIdentifiers, unavailableRuntimePacks, runtimePacks, packagesToDownload, isTrimmable);
ProcessRuntimeIdentifier(RuntimeIdentifier, runtimePackForRuntimeIDProcessing, runtimePackVersion, additionalFrameworkReferencesForRuntimePack,
unrecognizedRuntimeIdentifiers, unavailableRuntimePacks, runtimePacks, packagesToDownload, isTrimmable, includeInPackageDownload);

processedPrimaryRuntimeIdentifier = true;
}
processedPrimaryRuntimeIdentifier = true;
}

if (RuntimeIdentifiers != null)
if (RuntimeIdentifiers != null)
{
foreach (var runtimeIdentifier in RuntimeIdentifiers)
{
foreach (var runtimeIdentifier in RuntimeIdentifiers)
if (processedPrimaryRuntimeIdentifier && runtimeIdentifier == this.RuntimeIdentifier)
{
if (processedPrimaryRuntimeIdentifier && runtimeIdentifier == this.RuntimeIdentifier)
{
// We've already processed this RID
continue;
}

// Pass in null for the runtimePacks list, as for these runtime identifiers we only want to
// download the runtime packs, but not use the assets from them
ProcessRuntimeIdentifier(runtimeIdentifier, selectedRuntimePack.Value, runtimePackVersion, additionalFrameworkReferencesForRuntimePack: null,
unrecognizedRuntimeIdentifiers, unavailableRuntimePacks, runtimePacks: null, packagesToDownload, isTrimmable);
// We've already processed this RID
continue;
}

// Pass in null for the runtimePacks list, as for these runtime identifiers we only want to
// download the runtime packs, but not use the assets from them
ProcessRuntimeIdentifier(runtimeIdentifier, runtimePackForRuntimeIDProcessing, runtimePackVersion, additionalFrameworkReferencesForRuntimePack: null,
unrecognizedRuntimeIdentifiers, unavailableRuntimePacks, runtimePacks: null, packagesToDownload, isTrimmable, includeInPackageDownload);
}
}


if (!string.IsNullOrEmpty(knownFrameworkReference.RuntimeFrameworkName))
{
TaskItem runtimeFramework = new TaskItem(knownFrameworkReference.RuntimeFrameworkName);
Expand Down Expand Up @@ -370,7 +385,7 @@ protected override void ExecuteCore()
{
string runtimePackDescriptionForErrorMessage = knownFrameworkReference.RuntimeFrameworkName +
(requiredLabelsMetadata == string.Empty ? string.Empty : ":" + requiredLabelsMetadata);

Log.LogError(Strings.ConflictingRuntimePackInformation, runtimePackDescriptionForErrorMessage,
string.Join(Environment.NewLine, matchingRuntimePacks.Select(rp => rp.RuntimePackNamePatterns)));

Expand All @@ -387,7 +402,8 @@ private void ProcessRuntimeIdentifier(
List<ITaskItem> unavailableRuntimePacks,
List<ITaskItem> runtimePacks,
List<ITaskItem> packagesToDownload,
string isTrimmable)
string isTrimmable,
bool addToPackageDownload)
{
var runtimeGraph = new RuntimeGraphCache(this).GetRuntimeGraph(RuntimeGraphPath);
var knownFrameworkReferenceRuntimePackRuntimeIdentifiers = selectedRuntimePack.RuntimePackRuntimeIdentifiers.Split(';');
Expand Down Expand Up @@ -418,7 +434,7 @@ private void ProcessRuntimeIdentifier(
unrecognizedRuntimeIdentifiers.Add(runtimeIdentifier);
}
}
else
else if (addToPackageDownload)
{
foreach (var runtimePackNamePattern in selectedRuntimePack.RuntimePackNamePatterns.Split(';'))
{
Expand Down Expand Up @@ -620,18 +636,13 @@ public KnownFrameworkReference(ITaskItem item)
// The framework name to write to the runtimeconfig file (and the name of the folder under dotnet/shared)
public string RuntimeFrameworkName => _item.GetMetadata(MetadataKeys.RuntimeFrameworkName);
public string DefaultRuntimeFrameworkVersion => _item.GetMetadata("DefaultRuntimeFrameworkVersion");
//public string LatestRuntimeFrameworkVersion => _item.GetMetadata("LatestRuntimeFrameworkVersion");

// The ID of the targeting pack NuGet package to reference
public string TargetingPackName => _item.GetMetadata("TargetingPackName");
public string TargetingPackVersion => _item.GetMetadata("TargetingPackVersion");
public string TargetingPackFormat => _item.GetMetadata("TargetingPackFormat");

//public string RuntimePackNamePatterns => _item.GetMetadata("RuntimePackNamePatterns");

//public string RuntimePackRuntimeIdentifiers => _item.GetMetadata(MetadataKeys.RuntimePackRuntimeIdentifiers);

//public string IsTrimmable => _item.GetMetadata(MetadataKeys.IsTrimmable);
public string RuntimePackRuntimeIdentifiers => _item.GetMetadata(MetadataKeys.RuntimePackRuntimeIdentifiers);

public bool IsWindowsOnly => _item.HasMetadataValue("IsWindowsOnly", "true");

Expand Down Expand Up @@ -668,8 +679,6 @@ public KnownRuntimePack(ITaskItem item)
public string Name => _item.ItemSpec;

//// The framework name to write to the runtimeconfig file (and the name of the folder under dotnet/shared)
//public string RuntimeFrameworkName => _item.GetMetadata(MetadataKeys.RuntimeFrameworkName);
//public string DefaultRuntimeFrameworkVersion => _item.GetMetadata("DefaultRuntimeFrameworkVersion");
public string LatestRuntimeFrameworkVersion => _item.GetMetadata("LatestRuntimeFrameworkVersion");

public string RuntimePackNamePatterns => _item.GetMetadata("RuntimePackNamePatterns");
Expand Down
42 changes: 38 additions & 4 deletions src/Tests/Microsoft.NET.Build.Tests/KnownRuntimePackTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Xml.Linq;
using FluentAssertions;
using Microsoft.NET.TestFramework;
Expand Down Expand Up @@ -139,5 +136,42 @@ private void AddItem(TestAsset testAsset, XElement item)
itemGroup.Add(item);
});
}

[WindowsOnlyFact]
public void ItCanPublishArm64Winforms()
Copy link

Choose a reason for hiding this comment

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

You can write unit test for this task like src\Tasks\Microsoft.NET.Build.Tasks.UnitTests\ProcessFrameworkReferencesTests.cs. It will be at least better for documenting this long class

{
var testDirectory = _testAssetsManager.CreateTestDirectory().Path;

var newCommand = new DotnetCommand(Log, "new", "winforms", "--no-restore");
newCommand.WorkingDirectory = testDirectory;
newCommand.Execute().Should().Pass();

new PublishCommand(Log, testDirectory)
.Execute("/p:RuntimeIdentifier=win-arm64")
.Should()
.Pass();

var selfContainedPublishDir = new DirectoryInfo(testDirectory)
.Sub("bin").Sub("Debug").GetDirectories().FirstOrDefault()
.Sub("win-arm64").Sub("publish");

selfContainedPublishDir.Should().HaveFilesMatching("System.Windows.Forms.dll", SearchOption.TopDirectoryOnly);
selfContainedPublishDir.Should().HaveFilesMatching($"{new DirectoryInfo(testDirectory).Name}.dll", SearchOption.TopDirectoryOnly);
}

[WindowsOnlyFact]
public void ItCantPublishArm64Wpf()
{
var testDirectory = _testAssetsManager.CreateTestDirectory().Path;

var newCommand = new DotnetCommand(Log, "new", "wpf", "--no-restore");
newCommand.WorkingDirectory = testDirectory;
newCommand.Execute().Should().Pass();

new PublishCommand(Log, testDirectory)
.Execute("/p:RuntimeIdentifier=win-arm64")
.Should()
.Fail();
}
}
}