dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.69k stars 4.59k forks source link

OS/arch optional arguments in Bundler and TargetInfo #101548

Open am11 opened 3 months ago

am11 commented 3 months ago

There is a single usage of Bundle ctor in SDK and a few usages in installer tests in runtime repo. Runtime does a better job at detecting the current OS/arch when these arguments are null, while SDK is mostly out-of-sync (today OS is, and a few days ago riscv/loogarch were missing).

I'm not sure about the policy of breaking HostModel public API, but ideally we should just delete these arguments and let hostmodel take care of platform detection in one place. Alternatively, we can just cleanup the SDK code for DRY:

--- a/dotnet/sdk/src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs
+++ b/dotnet/sdk/src/Tasks/Microsoft.NET.Build.Tasks/GenerateBundle.cs
@@ -34,19 +34,6 @@ public class GenerateBundle : TaskBase

         protected override void ExecuteCore()
         {
-            OSPlatform targetOS = RuntimeIdentifier.StartsWith("win") ? OSPlatform.Windows :
-                                  RuntimeIdentifier.StartsWith("osx") ? OSPlatform.OSX : OSPlatform.Linux;
-
-            Architecture targetArch = RuntimeIdentifier.EndsWith("-x64") || RuntimeIdentifier.Contains("-x64-") ? Architecture.X64 :
-                                      RuntimeIdentifier.EndsWith("-x86") || RuntimeIdentifier.Contains("-x86-") ? Architecture.X86 :
-                                      RuntimeIdentifier.EndsWith("-arm64") || RuntimeIdentifier.Contains("-arm64-") ? Architecture.Arm64 :
-                                      RuntimeIdentifier.EndsWith("-arm") || RuntimeIdentifier.Contains("-arm-") ? Architecture.Arm :
-#if !NETFRAMEWORK
-                                      RuntimeIdentifier.EndsWith("-riscv64") || RuntimeIdentifier.Contains("-riscv64-") ? Architecture.RiscV64 :
-                                      RuntimeIdentifier.EndsWith("-loongarch64") || RuntimeIdentifier.Contains("-loongarch64-") ? Architecture.LoongArch64 :
-#endif
-                                      throw new ArgumentException(nameof(RuntimeIdentifier));
-
             BundleOptions options = BundleOptions.None;
             options |= IncludeNativeLibraries ? BundleOptions.BundleNativeBinaries : BundleOptions.None;
             options |= IncludeAllContent ? BundleOptions.BundleAllContent : BundleOptions.None;
@@ -58,10 +45,8 @@ protected override void ExecuteCore()
                 AppHostName,
                 OutputDir,
                 options,
-                targetOS,
-                targetArch,
-                version,
-                ShowDiagnosticOutput);
+                targetFrameworkVersion: version,
+                diagnosticOutput: ShowDiagnosticOutput);

             var fileSpec = new List<FileSpec>(FilesToBundle.Length);
am11 commented 3 months ago

cc @akoeplinger

dotnet-policy-service[bot] commented 3 months ago

Tagging subscribers to this area: @vitek-karas, @agocke See info in area-owners.md if you want to be subscribed.