Keep Publish* properties across Restore so that we can gather all required implicit packages (#49501)

Author: marcpopMSFT

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.PackTool.targets

@@ -42,13 +42,15 @@ NOTE: This file is imported from the following contexts, so be aware when writin
     <IncludeBuildOutput>false</IncludeBuildOutput>
 
     <!-- the publish* properties _can_ be set, but only for the 'inner' RID-specific builds. We need to make sure that for the outer, agnostic build they are unset  -->
-    <PublishSelfContained Condition="'$(RuntimeIdentifier)' == ''">false</PublishSelfContained>
+    <!-- RID information is also stripped during Restore, so we need to make sure user
+         decisions are preserved when Restoring, so that publishing-related packages are implicitly included.  -->
+    <PublishSelfContained Condition="'$(RuntimeIdentifier)' == '' and '$(MSBuildIsRestoring)' != 'true'">false</PublishSelfContained>
     <!-- Have to set SelfContained similarly because PackTool targets are imported _after_ RuntimeIdentifierInference targets, where the Publish* properties are
          forwarded to the 'base' properties. -->
-    <SelfContained Condition="'$(RuntimeIdentifier)' == ''">false</SelfContained>
-    <PublishTrimmed Condition="'$(RuntimeIdentifier)' == ''">false</PublishTrimmed>
-    <PublishReadyToRun Condition="'$(RuntimeIdentifier)' == ''">false</PublishReadyToRun>
-    <PublishSingleFile Condition="'$(RuntimeIdentifier)' == ''">false</PublishSingleFile>
+    <SelfContained Condition="'$(RuntimeIdentifier)' == '' and '$(MSBuildIsRestoring)' != 'true'">false</SelfContained>
+    <PublishTrimmed Condition="'$(RuntimeIdentifier)' == '' and '$(MSBuildIsRestoring)' != 'true'">false</PublishTrimmed>
+    <PublishReadyToRun Condition="'$(RuntimeIdentifier)' == '' and '$(MSBuildIsRestoring)' != 'true'">false</PublishReadyToRun>
+    <PublishSingleFile Condition="'$(RuntimeIdentifier)' == '' and '$(MSBuildIsRestoring)' != 'true'">false</PublishSingleFile>
 
     <!-- We need to know if the inner builds are _intended_ to be AOT even if we then explicitly disable AOT for the outer builds.
          Knowing this lets us correctly decide to create the RID-specific inner tools or not when packaging the outer tool. -->

test/Microsoft.DotNet.PackageInstall.Tests/EndToEndToolTests.cs

@@ -154,7 +154,7 @@ public void PackagesMultipleToolsWithASingleInvocation()
             {
                 var packageName = $"{toolSettings.ToolPackageId}.{rid}.{toolSettings.ToolPackageVersion}";
                 var package = packages.FirstOrDefault(p => p.EndsWith(packageName + ".nupkg"));
-                packages.Should().NotBeNull($"Package {packageName} should be present in the tool packages directory");
+                package.Should().NotBeNull($"Package {packageName} should be present in the tool packages directory");
             }
 
             // top-level package should declare all of the rids
@@ -170,6 +170,53 @@ public void PackagesMultipleToolsWithASingleInvocation()
                     .Select(e => (e as XElement)!.Attributes().First(a => a.Name == "RuntimeIdentifier").Value);
             packageNodes.Should().BeEquivalentTo(expectedRids, "The top-level package should declare all of the RIDs for the tools it contains");
         }
+
+        [Fact]
+        public void PackagesMultipleTrimmedToolsWithASingleInvocation()
+        {
+            var toolSettings = new TestToolBuilder.TestToolSettings()
+            {
+                Trimmed = true
+            };
+            string toolPackagesPath = ToolBuilder.CreateTestTool(Log, toolSettings);
+
+            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");
+                EnsurePackageLacksTrimmedDependency(package!, "System.Xml.dll");
+            }
+
+            // top-level package should declare all of the rids
+            var topLevelPackage = packages.First(p => p.EndsWith($"{packageIdentifier}.{toolSettings.ToolPackageVersion}.nupkg"));
+            using var zipArchive = ZipFile.OpenRead(topLevelPackage);
+            var nuspecEntry = zipArchive.GetEntry($"tools/{ToolsetInfo.CurrentTargetFramework}/any/DotnetToolSettings.xml")!;
+            var stream = nuspecEntry.Open();
+            var xml = XDocument.Load(stream, LoadOptions.None);
+            var packageNodes =
+                (xml.Root!.Nodes()
+                    .First(n => n is XElement e && e.Name == "RuntimeIdentifierPackages") as XElement)!.Nodes()
+                    .Where(n => (n as XElement)!.Name == "RuntimeIdentifierPackage")
+                    .Select(e => (e as XElement)!.Attributes().First(a => a.Name == "RuntimeIdentifier").Value);
+            packageNodes.Should().BeEquivalentTo(expectedRids, "The top-level package should declare all of the RIDs for the tools it contains");
+        }
+
+        /// <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)
+        {
+            using var zipArchive = ZipFile.OpenRead(packagePath);
+            zipArchive.Entries.Should().NotContain(
+                e => e.FullName.EndsWith(dll, StringComparison.OrdinalIgnoreCase),
+                $"The package {Path.GetFileName(packagePath)} should not contain a dependency on {dll}.");
+        }
     }
 
     static class EndToEndToolTestExtensions

test/Microsoft.DotNet.PackageInstall.Tests/TestToolBuilder.cs

@@ -34,8 +34,9 @@ public class TestToolSettings
 
             public bool NativeAOT { get; set; } = false;
             public bool SelfContained { get; set; } = false;
+            public bool Trimmed { get; set; } = false;
 
-            public string GetIdentifier() => $"{ToolPackageId}-{ToolPackageVersion}-{ToolCommandName}-{(NativeAOT ? "nativeaot" : SelfContained ? "selfcontained" : "managed")}";
+            public string GetIdentifier() => $"{ToolPackageId}-{ToolPackageVersion}-{ToolCommandName}-{(NativeAOT ? "nativeaot" : SelfContained ? "selfcontained" : Trimmed ? "trimmed" : "managed")}";
         }
 
 
@@ -66,6 +67,12 @@ public string CreateTestTool(ITestOutputHelper log, TestToolSettings toolSetting
                 testProject.AdditionalProperties["RuntimeIdentifiers"] = ToolsetInfo.LatestRuntimeIdentifiers;
             }
 
+            if (toolSettings.Trimmed)
+            {
+                testProject.AdditionalProperties["PublishTrimmed"] = "true";
+                testProject.AdditionalProperties["RuntimeIdentifiers"] = ToolsetInfo.LatestRuntimeIdentifiers;
+            }
+
             testProject.SourceFiles.Add("Program.cs", "Console.WriteLine(\"Hello Tool!\");");
 
             var testAssetManager = new TestAssetsManager(log);