Add GetCompatibilePackageTargetFrameworks Task to the new packageValidation package #7057
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using NuGet.Frameworks; | ||
| using System.Collections.Generic; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.DotNet.PackageValidation.Tests | ||
| { | ||
| public class GetCompatibilePackageTargetFrameworksTests | ||
| { | ||
| public GetCompatibilePackageTargetFrameworksTests() | ||
| { | ||
| GetCompatiblePackageTargetFrameworks.Initialize(); | ||
| } | ||
|
|
||
| public static IEnumerable<object[]> PackageTFMData => new List<object[]> | ||
| { | ||
| // single target framework in package | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard20}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard20, FrameworkConstants.CommonFrameworks.NetCoreApp20, FrameworkConstants.CommonFrameworks.Net463, FrameworkConstants.CommonFrameworks.Net461, FrameworkConstants.CommonFrameworks.Net462} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp20}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp20} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp21}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp21} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.Net461}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.Net461} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.Net45}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.Net45} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp30}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp30} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp31}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp31} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard21}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard21, FrameworkConstants.CommonFrameworks.NetCoreApp30 } }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard12}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard12, FrameworkConstants.CommonFrameworks.Net451 } }, | ||
|
|
||
| // two target frameworks in package | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard20, FrameworkConstants.CommonFrameworks.Net461}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard20, FrameworkConstants.CommonFrameworks.NetCoreApp20, FrameworkConstants.CommonFrameworks.Net463, FrameworkConstants.CommonFrameworks.Net461, FrameworkConstants.CommonFrameworks.Net462} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard20, FrameworkConstants.CommonFrameworks.NetCoreApp30}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetStandard20, FrameworkConstants.CommonFrameworks.NetCoreApp30, FrameworkConstants.CommonFrameworks.NetCoreApp20, FrameworkConstants.CommonFrameworks.Net463, FrameworkConstants.CommonFrameworks.Net461, FrameworkConstants.CommonFrameworks.Net462} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp30, FrameworkConstants.CommonFrameworks.Net461}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp30, FrameworkConstants.CommonFrameworks.Net461} }, | ||
| new object[] { new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp30, FrameworkConstants.CommonFrameworks.Net50}, new List<NuGetFramework> { FrameworkConstants.CommonFrameworks.NetCoreApp30, FrameworkConstants.CommonFrameworks.Net50} }, | ||
| }; | ||
|
|
||
| [Theory] | ||
| [MemberData(nameof(PackageTFMData))] | ||
| public void GetCompatibleFrameworks(List<NuGetFramework> packageFrameworks, List<NuGetFramework> expectedTestFrameworks) | ||
| { | ||
| List<NuGetFramework> actualTestFrameworks = GetCompatiblePackageTargetFrameworks.GetTestFrameworks(packageFrameworks); | ||
| CollectionsEqual(expectedTestFrameworks, actualTestFrameworks); | ||
| } | ||
|
|
||
| private static void CollectionsEqual<T>(IEnumerable<T> T1, IEnumerable<T> T2) | ||
| { | ||
| foreach (var item in T1) | ||
| { | ||
| Assert.Contains(item, T2); | ||
| } | ||
| foreach (var item in T2) | ||
| { | ||
| Assert.Contains(item, T1); | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>netcoreapp3.1</TargetFramework> | ||
| <IsPackable>false</IsPackable> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Build.Utilities.Core" Version="$(MicrosoftBuildUtilitiesCoreVersion)" /> | ||
| <PackageReference Include="NuGet.Frameworks" Version="$(NuGetVersion)" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <ProjectReference Include="..\Microsoft.DotNet.PackageValidation\Microsoft.DotNet.PackageValidation.csproj" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using Microsoft.Build.Framework; | ||
| using Microsoft.Build.Utilities; | ||
| using Microsoft.DotNet.Build.Tasks; | ||
| using NuGet.Frameworks; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
|
|
||
| namespace Microsoft.DotNet.PackageValidation | ||
| { | ||
| public class GetCompatiblePackageTargetFrameworks : BuildTask | ||
| { | ||
| private static List<NuGetFramework> allTargetFrameworks = allTargetFrameworks = new(); | ||
| private static Dictionary<NuGetFramework, HashSet<NuGetFramework>> packageTfmMapping = new(); | ||
|
|
||
| [Required] | ||
| public string[] PackagePaths { get; set; } | ||
|
|
||
| [Output] | ||
| public ITaskItem[] TestProjects { get; set; } | ||
|
|
||
| public override bool Execute() | ||
| { | ||
| bool result = true; | ||
| List<ITaskItem> testProjects = new List<ITaskItem>(); | ||
|
|
||
| try | ||
| { | ||
| Initialize(); | ||
| foreach (var packagePath in PackagePaths) | ||
| { | ||
| Package package = NupkgParser.CreatePackageObject(packagePath); | ||
| List<NuGetFramework> packageTargetFrameworks = package.PackageAssets.Where(t => t.AssetType != AssetType.RuntimeAsset).Select(t => t.TargetFramework).Distinct().ToList(); | ||
|
|
||
| List<NuGetFramework> frameworksToTest = GetTestFrameworks(packageTargetFrameworks); | ||
| testProjects.AddRange(CreateItemFromTestFramework(package.Title, package.Version, frameworksToTest, GetRidsFromPackage(package))); | ||
| } | ||
|
|
||
| // Removing empty items. | ||
| TestProjects = testProjects.Where(tfm => tfm.ItemSpec != "").ToArray(); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| Log.LogErrorFromException(e, showStackTrace: false); | ||
| } | ||
|
|
||
| return result && !Log.HasLoggedErrors; | ||
| } | ||
|
|
||
| public static List<NuGetFramework> GetTestFrameworks(List<NuGetFramework> packageTargetFrameworks) | ||
|
There was a problem hiding this comment. nit: this can be IEnumerable and you can avoid calls ToList |
||
| { | ||
| List<NuGetFramework> frameworksToTest = new List<NuGetFramework>(); | ||
|
|
||
| // Testing the package installation on all tfms linked with package targetframeworks. | ||
| foreach (var item in packageTargetFrameworks) | ||
| { | ||
| if (packageTfmMapping.ContainsKey(item)) | ||
| frameworksToTest.AddRange(packageTfmMapping[item].ToList()); | ||
| } | ||
|
|
||
| // Pruning the test matrix by removing the frameworks we dont want to test. | ||
| frameworksToTest = frameworksToTest.Where(tfm => allTargetFrameworks.Contains(tfm)).ToList(); | ||
|
|
||
| // Adding the frameworks in the packages to the test matrix; | ||
| frameworksToTest.AddRange(packageTargetFrameworks); | ||
| frameworksToTest = frameworksToTest.Distinct().ToList(); | ||
| return frameworksToTest; | ||
| } | ||
|
|
||
| public static void Initialize() | ||
| { | ||
| // Defining the set of known frameworks that we care to test | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetCoreApp20); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetCoreApp21); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetCoreApp30); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetCoreApp31); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net50); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net45); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net451); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net452); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net46); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net461); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net462); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.Net463); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard10); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard11); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard12); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard13); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard14); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard15); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard16); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard17); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard20); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.NetStandard21); | ||
| allTargetFrameworks.Add(FrameworkConstants.CommonFrameworks.UAP10); | ||
|
There was a problem hiding this comment. Agree with what others have said about this, better to have this data in props. If it were easy to calculate it from NuGet that would be even better, but we found that wasn't the case so better to just define in props so it can more easily be controlled by caller. |
||
|
|
||
| // creating a map framework in package => frameworks to test based on default compatibilty mapping. | ||
| foreach (var item in DefaultFrameworkMappings.Instance.CompatibilityMappings) | ||
| { | ||
| NuGetFramework forwardTfm = item.SupportedFrameworkRange.Max; | ||
| NuGetFramework reverseTfm = item.TargetFrameworkRange.Min; | ||
| if (packageTfmMapping.ContainsKey(forwardTfm)) | ||
| { | ||
| packageTfmMapping[forwardTfm].Add(reverseTfm); | ||
| } | ||
| else | ||
| { | ||
| packageTfmMapping.Add(forwardTfm, new HashSet<NuGetFramework> { reverseTfm }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| public List<ITaskItem> CreateItemFromTestFramework(string title, string version, List<NuGetFramework> testFrameworks, string rids) | ||
|
There was a problem hiding this comment. Similarly List vs IEnumerable. Try to avoid using concrete collections unless you actually need to. It allocates unnecessary arrays. General rules: |
||
| { | ||
| List<ITaskItem> testprojects = new List<ITaskItem>(); | ||
| foreach (var framework in testFrameworks) | ||
| { | ||
| var supportedPackage = new TaskItem(title); | ||
|
There was a problem hiding this comment. title here is confusing. Shouldn't this be packageId? |
||
| supportedPackage.SetMetadata("Version", version); | ||
| supportedPackage.SetMetadata("TargetFramework", framework.ToString()); | ||
| supportedPackage.SetMetadata("TargetFrameworkShort", framework.GetShortFolderName()); | ||
|
|
||
| if (!String.IsNullOrEmpty(rids)) | ||
| { | ||
| supportedPackage.SetMetadata("RuntimeIdentifiers", rids); | ||
| } | ||
| testprojects.Add(supportedPackage); | ||
| } | ||
|
|
||
| return testprojects; | ||
| } | ||
|
|
||
| public string GetRidsFromPackage(Package package) | ||
| { | ||
| List<string> rids = new List<string>(); | ||
|
There was a problem hiding this comment. You can avoid a list allocation here by making this method return an enumerable and yield returning the items |
||
| foreach (var item in package.PackageAssets) | ||
| { | ||
| if (item.AssetType == AssetType.RuntimeAsset) | ||
| { | ||
| if (!rids.Contains(item.Rid + "-x64")) | ||
| rids.Add(item.Rid + "-x64"); | ||
| } | ||
| } | ||
| return string.Join(";", rids); | ||
|
There was a problem hiding this comment. Why are you joining here when other methods deal with lists/enumerables? Consider moving the join to the CreateItemFromTestFramework method where this is used. |
||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFramework>netcoreapp3.1</TargetFramework> | ||
| <ExcludeFromSourceBuild>false</ExcludeFromSourceBuild> | ||
| <PackageType>MSBuildSdk</PackageType> | ||
| <IncludeBuildOutput>false</IncludeBuildOutput> | ||
| <IsPackable>true</IsPackable> | ||
| </PropertyGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Build" Version="$(MicrosoftBuildVersion)" Publish="false" /> | ||
| <PackageReference Include="Microsoft.Build.Tasks.Core" Version="$(MicrosoftBuildTasksCoreVersion)" Publish="false" /> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MsbuildTaskMicrosoftCodeAnalysisCSharpVersion)" ExcludeAssets="analyzers" /> | ||
| <PackageReference Include="NuGet.Frameworks" Version="$(NuGetVersion)" /> | ||
| <PackageReference Include="NuGet.Packaging" Version="$(NuGetVersion)" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Compile Include="..\Common\Internal\BuildTask.cs" /> | ||
| </ItemGroup> | ||
|
|
||
| <Import Project="$(RepoRoot)eng\BuildTask.targets" /> | ||
|
|
||
| </Project> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to document your inputs and outputs. What's the expected identity and metadata and what is it used for. This is the tasks's public API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming here is off, consider TestPackages