Strip all package types from inner multi-rid tool packages so they don't appear on search-based experiences (#49616)

baronfel · joelverhagen · web-flow · commit 67ab0629537d · 2025-07-01T20:40:30.000-05:00
Co-authored-by: Joel Verhagen &lt;joel.verhagen@gmail.com&gt;
diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets b/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets
@@ -380,12 +380,18 @@ NOTE: This file is imported from the following contexts, so be aware when writin
        PackageType = 'MyDotnetTool'         -> 'DotnetTool;MyDotnetTool'
 
     _PaddedPackageType is used to ensure that the PackageType is semicolon delimited and can be easily checked for an existing DotnetTool package type.
+
+    All of this should only apply for the 'outer' tool package - the 'inner' RID-specific tool packages are always of type DotnetToolRidPackage.
+    This is so that the inner packages do not appear on any tool search results.
     -->
 
-    <AddPackageType CurrentPackageType="$(PackageType)" PackageTypeToAdd="$(_ToolPackageType)">
+    <AddPackageType Condition="$(_ToolPackageType) == 'DotnetTool'" CurrentPackageType="$(PackageType)" PackageTypeToAdd="$(_ToolPackageType)">
       <Output TaskParameter="UpdatedPackageType" PropertyName="PackageType" />
     </AddPackageType>
 
+    <PropertyGroup Condition="$(_ToolPackageType) == 'DotnetToolRidPackage'">
+      <PackageType>DotnetToolRidPackage</PackageType>
+    </PropertyGroup>
   </Target>
 
   <!-- Orchestrator for making the N RID-specific tool packages if this Tool supports that mode.
diff --git a/test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs b/test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs
@@ -1,7 +1,10 @@
 ﻿// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics.CodeAnalysis;
 using System.IO.Compression;
+using NuGet.Packaging;
+using NuGet.Packaging.Core;
 
 namespace Microsoft.DotNet.PackageInstall.Tests
 {
@@ -312,35 +315,116 @@ .. expectedRids.Select(rid => $"{toolSettings.ToolPackageId}.{rid}.{toolSettings
                 .And.HaveStdOutContaining("Hello Tool!");
         }
 
-        private void EnsurePackageIsFdd(string packagePath)
+        [Fact]
+        public void StripsPackageTypesFromInnerToolPackages()
+        {
+            var toolSettings = new TestToolBuilder.TestToolSettings()
+            {
+                RidSpecific = true,
+                AdditionalPackageTypes = ["TestPackageType"]
+            };
+            string toolPackagesPath = ToolBuilder.CreateTestTool(Log, toolSettings, collectBinlogs: true);
+
+            var packages = Directory.GetFiles(toolPackagesPath, "*.nupkg");
+            var packageIdentifier = toolSettings.ToolPackageId;
+            var expectedRids = ToolsetInfo.LatestRuntimeIdentifiers.Split(';');
+
+            packages.Length.Should().Be(expectedRids.Length + 1, "There should be one package for the tool-wrapper and one for each RID");
+            foreach (string rid in expectedRids)
+            {
+                var packageName = $"{toolSettings.ToolPackageId}.{rid}.{toolSettings.ToolPackageVersion}";
+                var package = packages.FirstOrDefault(p => p.EndsWith(packageName + ".nupkg"));
+                package.Should()
+                    .NotBeNull($"Package {packageName} should be present in the tool packages directory")
+                    .And.Satisfy<string>(EnsurePackageIsAnExecutable)
+                    .And.Satisfy<string>(EnsurePackageOnlyHasToolRidPackageType);
+            }
+
+            // top-level package should declare all of the rids
+            var topLevelPackage = packages.First(p => p.EndsWith($"{packageIdentifier}.{toolSettings.ToolPackageVersion}.nupkg"));
+            topLevelPackage.Should().NotBeNull($"Package {packageIdentifier}.{toolSettings.ToolPackageVersion}.nupkg should be present in the tool packages directory")
+                .And.Satisfy<string>(EnsurePackageHasNoRunner)
+                .And.Satisfy<string>(EnsurePackageHasToolPackageTypeAnd(toolSettings.AdditionalPackageTypes!));
+            var foundRids = GetRidsInSettingsFile(topLevelPackage);
+            foundRids.Should().BeEquivalentTo(expectedRids, "The top-level package should declare all of the RIDs for the tools it contains");
+        }
+
+        private Action<string> EnsurePackageHasToolPackageTypeAnd(string[] additionalPackageTypes) => (string packagePath) =>
+        {
+            var nuspec = GetPackageNuspec(packagePath);
+            var packageTypes = nuspec.GetPackageTypes();
+            PackageType[] expectedPackageTypes = [new PackageType("DotnetTool", PackageType.EmptyVersion), .. additionalPackageTypes.Select(t => new PackageType(t, PackageType.EmptyVersion))];
+            packageTypes.Should().NotBeNull("The PackageType element should not be null.")
+                .And.HaveCount(1 + additionalPackageTypes.Length, "The package should have a PackageType element for each additional type.")
+                .And.BeEquivalentTo(expectedPackageTypes, "The PackageType should be 'DotnetTool'.");
+        };
+
+        static void EnsurePackageOnlyHasToolRidPackageType(string packagePath)
+        {
+            var nuspec = GetPackageNuspec(packagePath);
+            var packageTypes = nuspec.GetPackageTypes();
+            packageTypes.Should().NotBeNull("The PackageType element should not be null.")
+                .And.HaveCount(1, "The package should only have a single PackageType element.")
+                .And.Contain(new PackageType("DotnetToolRidPackage", PackageType.EmptyVersion), "The PackageType should be 'DotnetToolRidPackage'.");
+        }
+
+        private static NuspecReader GetPackageNuspec(string packagePath)
+        {
+            using var zipArchive = ZipFile.OpenRead(packagePath);
+            var nuspecEntry = zipArchive.Entries.First(e => e.Name.EndsWith("nuspec")!);
+            var stream = nuspecEntry.Open();
+            return new NuspecReader(stream);
+        }
+
+        static void EnsurePackageIsFdd(string packagePath)
         {
             var settingsXml = GetToolSettingsFile(packagePath);
             var runner = GetRunnerFromSettingsFile(settingsXml);
             runner.Should().Be("dotnet", "The tool should be packaged as a framework-dependent executable (FDD) with a 'dotnet' runner.");
         }
 
-        private void EnsurePackageIsAnExecutable(string packagePath)
+        static void EnsurePackageHasNoRunner(string packagePath)
+        {
+            var settingsXml = GetToolSettingsFile(packagePath);
+            if (TryGetRunnerFromSettingsFile(settingsXml, out _))
+            {
+                throw new Exception("The tool settings file should  not contain a 'Runner' attribute.");
+            }
+        }
+
+        static void EnsurePackageIsAnExecutable(string packagePath)
         {
             var settingsXml = GetToolSettingsFile(packagePath);
             var runner = GetRunnerFromSettingsFile(settingsXml);
             runner.Should().Be("executable", "The tool should be packaged as a executable with an 'executable' runner.");
         }
 
-        private object GetRunnerFromSettingsFile(XElement settingsXml)
+        static string GetRunnerFromSettingsFile(XElement settingsXml)
+        {
+            if (TryGetRunnerFromSettingsFile(settingsXml, out string? runner))
+            {
+                return runner;
+            } else
+            {
+                throw new InvalidOperationException("The tool settings file does not contain a 'Runner' attribute.");
+            }
+        }
+        static bool TryGetRunnerFromSettingsFile(XElement settingsXml, [NotNullWhen(true)] out string? runner)
         {
-            return settingsXml.Elements("Commands").First().Elements("Command").First().Attribute("Runner")?.Value
-                   ?? throw new InvalidOperationException("The tool settings file does not contain a 'Runner' attribute.");
+            var commandNode = settingsXml.Elements("Commands").First().Elements("Command").First();
+            runner = commandNode.Attributes().FirstOrDefault(a => a.Name == "Runner")?.Value;
+            return runner is not null;
         }
 
-        private string[] GetRidsInSettingsFile(string packagePath)
+        static string[] GetRidsInSettingsFile(string packagePath)
         {
             var settingsXml = GetToolSettingsFile(packagePath);
             var rids = GetRidsInSettingsFile(settingsXml);
             rids.Should().NotBeEmpty("The tool settings file should contain at least one RuntimeIdentifierPackage element.");
             return rids;
         }
 
-        private string[] GetRidsInSettingsFile(XElement settingsXml)
+        static string[] GetRidsInSettingsFile(XElement settingsXml)
         {
             var nodes = (settingsXml.Nodes()
                     .First(n => n is XElement e && e.Name == "RuntimeIdentifierPackages") as XElement)!.Nodes()
@@ -350,7 +434,7 @@ private string[] GetRidsInSettingsFile(XElement settingsXml)
             return nodes;
         }
 
-        private XElement GetToolSettingsFile(string packagePath)
+        static XElement GetToolSettingsFile(string packagePath)
         {
             using var zipArchive = ZipFile.OpenRead(packagePath);
             var nuspecEntry = zipArchive.Entries.First(e => e.Name == "DotnetToolSettings.xml")!;
@@ -363,7 +447,7 @@ private XElement GetToolSettingsFile(string packagePath)
         /// <summary>
         /// Opens the nupkg and verifies that it does not contain a dependency on the given dll.
         /// </summary>
-        private void EnsurePackageLacksTrimmedDependency(string packagePath, string dll)
+        static void EnsurePackageLacksTrimmedDependency(string packagePath, string dll)
         {
             using var zipArchive = ZipFile.OpenRead(packagePath);
             zipArchive.Entries.Should().NotContain(
diff --git a/test/Microsoft.DotNet.PackageInstall.Tests/TestToolBuilder.cs b/test/Microsoft.DotNet.PackageInstall.Tests/TestToolBuilder.cs
@@ -27,6 +27,7 @@ public class TestToolSettings
             public string ToolPackageId { get; set; } = "TestTool";
             public string ToolPackageVersion { get; set; } = "1.0.0";
             public string ToolCommandName { get; set; } = "TestTool";
+            public string[]? AdditionalPackageTypes { get; set; } = null;
 
             public bool NativeAOT { get; set { field = value; this.RidSpecific = value; } } = false;
             public bool SelfContained { get; set { field = value; this.RidSpecific = value; } } = false;
@@ -35,15 +36,56 @@ public class TestToolSettings
             public bool RidSpecific { get; set; } = false;
             public bool IncludeCurrentRid { get; set; } = true;
 
-            public string GetIdentifier() => $"{ToolPackageId}-{ToolPackageVersion}-{ToolCommandName}-{(NativeAOT ? "nativeaot" : SelfContained ? "selfcontained" : Trimmed ? "trimmed" : "managed")}{(RidSpecific ? "-specific" : "")}{(IncludeAnyRid ? "-anyrid" : "")}{(IncludeCurrentRid ? "" : "-no-current-rid")}";
+            public string GetIdentifier() {
+                var builder = new StringBuilder();
+                builder.Append(ToolPackageId.ToLowerInvariant());
+                builder.Append('-');
+                builder.Append(ToolPackageVersion.ToLowerInvariant());
+                builder.Append('-');
+                builder.Append(ToolCommandName.ToLowerInvariant());
+                if (NativeAOT)
+                {
+                    builder.Append("-nativeaot");
+                }
+                else if (SelfContained)
+                {
+                    builder.Append("-selfcontained");
+                }
+                else if (Trimmed)
+                {
+                    builder.Append("-trimmed");
+                }
+                else
+                {
+                    builder.Append("-managed");
+                }
+                if (RidSpecific)
+                {
+                    builder.Append("-specific");
+                }
+                if (IncludeAnyRid)
+                {
+                    builder.Append("-anyrid");
+                }
+                if (!IncludeCurrentRid)
+                {
+                    builder.Append("-no-current-rid");
+                }
+                if (AdditionalPackageTypes is not null && AdditionalPackageTypes.Length > 0)
+                {
+                    builder.Append('-');
+                    builder.Append(string.Join("-", AdditionalPackageTypes.Select(p => p.ToLowerInvariant())));
+                }
+
+                return builder.ToString();
+            }
         }
 
 
         public string CreateTestTool(ITestOutputHelper log, TestToolSettings toolSettings, bool collectBinlogs = false)
         {
             var targetDirectory = Path.Combine(TestContext.Current.TestExecutionDirectory, "TestTools", toolSettings.GetIdentifier());
 
-
             var testProject = new TestProject(toolSettings.ToolPackageId)
             {
                 TargetFrameworks = ToolsetInfo.CurrentTargetFramework,
@@ -82,6 +124,11 @@ public string CreateTestTool(ITestOutputHelper log, TestToolSettings toolSetting
                 testProject.AdditionalProperties["PublishTrimmed"] = "true";
             }
 
+            if (toolSettings.AdditionalPackageTypes is not null && toolSettings.AdditionalPackageTypes.Length > 0)
+            {
+                testProject.AdditionalProperties["PackageType"] = string.Join(";", toolSettings.AdditionalPackageTypes);
+            }
+
             testProject.SourceFiles.Add("Program.cs", "Console.WriteLine(\"Hello Tool!\");");
 
             var testAssetManager = new TestAssetsManager(log);
