Fix RID regression by adding a task that calculates the best matching RID for platform #5695
+310
−10
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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> | ||
| <OutputType>Exe</OutputType> | ||
| <TargetFramework>$(DefaultTargetFramework)</TargetFramework> | ||
| <RollForward>Major</RollForward> | ||
| <UsePublicApiAnalyzers>false</UsePublicApiAnalyzers> | ||
| <IsPackable>false</IsPackable> | ||
| <UseAppHost>false</UseAppHost> | ||
| </PropertyGroup> | ||
joperezr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="System.CommandLine" /> | ||
| <PackageReference Include="NuGet.ProjectModel" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> | ||
This file contains hidden or 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,50 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using NuGet.RuntimeModel; | ||
|
|
||
| namespace Aspire.Hosting.Sdk; | ||
|
|
||
| /* | ||
| * These utility methods were copied from the sdk repository to mimic the behavior used when selecting the best matching RID | ||
| * for a given runtime identifier. For more information, please see the original source code at: | ||
| * https://github.com/dotnet/sdk/blob/e6da8ca6de3ec8f392dc87b8529415e1ef59b7ea/src/Tasks/Microsoft.NET.Build.Tasks/NuGetUtils.NuGet.cs#L76-L109 | ||
| */ | ||
|
|
||
| internal static class NuGetUtils | ||
| { | ||
| public static string? GetBestMatchingRid(RuntimeGraph runtimeGraph, string runtimeIdentifier, | ||
| IEnumerable<string> availableRuntimeIdentifiers, out bool wasInGraph) | ||
| { | ||
| return GetBestMatchingRidWithExclusion(runtimeGraph, runtimeIdentifier, | ||
| runtimeIdentifiersToExclude: null, | ||
| availableRuntimeIdentifiers, out wasInGraph); | ||
| } | ||
|
|
||
| public static string? GetBestMatchingRidWithExclusion(RuntimeGraph runtimeGraph, string runtimeIdentifier, | ||
| IEnumerable<string>? runtimeIdentifiersToExclude, | ||
| IEnumerable<string> availableRuntimeIdentifiers, out bool wasInGraph) | ||
| { | ||
| wasInGraph = runtimeGraph.Runtimes.ContainsKey(runtimeIdentifier); | ||
|
|
||
| string? bestMatch = null; | ||
|
|
||
| HashSet<string> availableRids = new(availableRuntimeIdentifiers, StringComparer.Ordinal); | ||
| HashSet<string>? excludedRids = runtimeIdentifiersToExclude switch { null => null, _ => new HashSet<string>(runtimeIdentifiersToExclude, StringComparer.Ordinal) }; | ||
| foreach (var candidateRuntimeIdentifier in runtimeGraph.ExpandRuntime(runtimeIdentifier)) | ||
| { | ||
| if (bestMatch == null && availableRids.Contains(candidateRuntimeIdentifier)) | ||
| { | ||
| bestMatch = candidateRuntimeIdentifier; | ||
| } | ||
|
|
||
| if (excludedRids != null && excludedRids.Contains(candidateRuntimeIdentifier)) | ||
| { | ||
| // Don't treat this as a match | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| return bestMatch; | ||
| } | ||
| } |
This file contains hidden or 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,86 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System.CommandLine; | ||
| using System.CommandLine.Parsing; | ||
| using System.Diagnostics; | ||
| using System.Reflection; | ||
| using Aspire.Hosting.Sdk; | ||
| using NuGet.RuntimeModel; | ||
|
|
||
| namespace Aspire.RuntimeIdentifier.Tool; | ||
|
|
||
| sealed class Program | ||
| { | ||
| static int Main(string[] args) | ||
| { | ||
| CliRootCommand rootCommand = new("Aspire.RuntimeIdentifier.Tool v" + FileVersionInfo.GetVersionInfo(Assembly.GetExecutingAssembly().Location).ProductVersion) | ||
| { | ||
| TreatUnmatchedTokensAsErrors = true | ||
| }; | ||
|
|
||
| CliOption<string?> runtimeGraphPathOption = new("--runtimeGraphPath") | ||
| { | ||
| Description = "Path to runtime graph path to use for RID mapping.", | ||
| Required = true | ||
| }; | ||
|
|
||
| CliOption<string?> netcoreSdkRuntimeIdentifierOption = new("--netcoreSdkRuntimeIdentifier") | ||
| { | ||
| Description = "RID to use for finding the best applicable RID from mapping.", | ||
| Required = true | ||
| }; | ||
|
|
||
| CliOption<string[]> supportedRidsOption = new("--supportedRids") | ||
| { | ||
| Description = "List of RIDs that are supported. Comma-separated.", | ||
| Required = true, | ||
| Arity = ArgumentArity.OneOrMore, | ||
| CustomParser = ParseSupportedRidsArgument | ||
| }; | ||
|
|
||
| rootCommand.Options.Add(runtimeGraphPathOption); | ||
| rootCommand.Options.Add(netcoreSdkRuntimeIdentifierOption); | ||
| rootCommand.Options.Add(supportedRidsOption); | ||
| rootCommand.SetAction((ParseResult parseResult) => | ||
| { | ||
| string rgp = parseResult.GetValue(runtimeGraphPathOption) ?? throw new InvalidOperationException("The --runtimeGraphPath argument is required."); | ||
|
|
||
| if (!File.Exists(rgp)) | ||
| { | ||
| throw new FileNotFoundException("File {0} does not exist. Please ensure the runtime graph path exists.", rgp); | ||
| } | ||
|
|
||
| RuntimeGraph graph = JsonRuntimeFormat.ReadRuntimeGraph(rgp); | ||
|
|
||
| var ridToUse = parseResult.GetValue(netcoreSdkRuntimeIdentifierOption); | ||
|
|
||
| var supportedRids = parseResult.GetValue(supportedRidsOption); | ||
|
|
||
| string? bestRidForPlatform = NuGetUtils.GetBestMatchingRid(graph, ridToUse!, supportedRids!, out bool wasInGraph); | ||
|
|
||
| if (!wasInGraph) | ||
| { | ||
| Console.WriteLine("Unable to find the best rid to use"); | ||
| return -1; | ||
| } | ||
|
|
||
| Console.WriteLine(bestRidForPlatform); | ||
| return 0; | ||
| }); | ||
|
|
||
| return rootCommand.Parse(args).Invoke(); | ||
| } | ||
|
|
||
| private static string[]? ParseSupportedRidsArgument(ArgumentResult result) | ||
|
Member
There was a problem hiding this comment. I'm surprised we need to write custom code for this.
Member
Author
There was a problem hiding this comment. I followed precedent that has been done on other tools: https://github.com/dotnet/sdk/blob/f007eb76461e4fff93c54e3ea2af67ad4eefeb32/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompat.Tool/Program.cs#L362-L371 |
||
| { | ||
| List<string> args = new(); | ||
|
|
||
| foreach (var token in result.Tokens) | ||
| { | ||
| args.AddRange(token.Value.Split(',')); | ||
| } | ||
|
|
||
| return args.ToArray(); | ||
| } | ||
| } | ||
This file contains hidden or 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
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.
Nesting .csprojs under folders that already contain a .csproj is typically not a good idea.
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.
Agreed, though in this case the parent project is a
NoTargetsSDK project meaning it doesn't compile anything, and the child project doesn't have any targets that could be packed by the parent by accident either. Main reason why I made it a child project, is that the parent is the one that packs this. I could change it of course, but this "feels" better. There is also precedent in dotnet/runtime where source generator projects are nested under the package that packs them.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.
For example: https://github.com/dotnet/runtime/tree/main/src/libraries/System.Text.Json/gen
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.
in dotnet/runtime, it is the
srcproject that packs them, which is a sibling, not in a parent directory.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.
Oh -- fair enough. I don't feel strongly about this but I can change it if you want me to.