OS/arch optional arguments in Bundler and TargetInfo

[am11]

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]

cc @akoeplinger

[dotnet-policy-service]

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

[elinor-fung]

The target runtime identifier is not necessarily the same as the current one, so we can't remove the OS/arch arguments entirely from Bundler. Maybe the SDK could do something like explicitly check if (target) RuntimeIdentifier is the same as RuntimeInformation.RuntimeIdentifier and only pass OS and arch if they are different.

[am11]

Ah I see. We can make it non-optional argument and delete auto inference from ctor.

runtime/src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs

Lines 42 to 43 in c02569b

	OSPlatform? targetOS = null,
	Architecture? targetArch = null,

(null is only passed from unit tests)