Ensure that tool packages can still publish normally with dotnet publish.

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

@@ -51,28 +51,38 @@ NOTE: This file is imported from the following contexts, so be aware when writin
     <_UserSpecifiedToolPackageRids Condition="'$(_UserSpecifiedToolPackageRids)' == ''">$(RuntimeIdentifiers)</_UserSpecifiedToolPackageRids>
     <_HasRIDSpecificTools Condition=" '$(_UserSpecifiedToolPackageRids)' != '' ">true</_HasRIDSpecificTools>
     <_HasRIDSpecificTools Condition="'$(_HasRIDSpecificTools)' == ''">false</_HasRIDSpecificTools>
+
+    <!-- NOTE: this line is load-bearing. This impacts Restore behaviors significantly, so we can't prevent the import of these targets _in general_. -->
     <RuntimeIdentifiers Condition="'$(PackAsToolShimRuntimeIdentifiers)' != ''">$(_UserSpecifiedToolPackageRids);$(PackAsToolShimRuntimeIdentifiers)</RuntimeIdentifiers>
 
     <_IsRidSpecific>false</_IsRidSpecific>
     <_IsRidSpecific Condition="'$(RuntimeIdentifier)' != '' and '$(RuntimeIdentifier)' != 'any'">true</_IsRidSpecific>
-
-    <!-- Not determine information about this specific build of a single (or more!) tool packages -->
-    <!-- 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  -->
-    <!-- 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="!$(_IsRidSpecific) 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="!$(_IsRidSpecific) and '$(MSBuildIsRestoring)' != 'true'">false</SelfContained>
-    <PublishTrimmed Condition="!$(_IsRidSpecific) and '$(MSBuildIsRestoring)' != 'true'">false</PublishTrimmed>
-    <PublishReadyToRun Condition="!$(_IsRidSpecific) and '$(MSBuildIsRestoring)' != 'true'">false</PublishReadyToRun>
-    <PublishSingleFile Condition="!$(_IsRidSpecific) 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. -->
+      Knowing this lets us correctly decide to create the RID-specific inner tools or not when packaging the outer tool. -->
     <_InnerToolsPublishAot>false</_InnerToolsPublishAot>
     <_InnerToolsPublishAot Condition="$(_HasRIDSpecificTools) and '$(PublishAot)' == 'true'">true</_InnerToolsPublishAot>
-    <PublishAot Condition="!$(_IsRidSpecific) and '$(MSBuildIsRestoring)' != 'true'">false</PublishAot>
+
+    <!-- determining if it's safe to change publish-related properties for this evaluation. We can only override default publishing
+         behavior if
+           a) we're not restoring (since these flags contain information that influences the assets Restore acquires), and
+           b) we are actually packing
+         otherwise, we risk altering something that changes downstream behavior for 'normal' publish operations.
+    -->
+    <_IsImplicitRestore>false</_IsImplicitRestore>
+    <_IsImplicitRestore Condition="'$(MSBuildIsRestoring)' == 'true'">true</_IsImplicitRestore>
+    <_IsPacking Condition="'$(_IsPacking)' == ''">false</_IsPacking>
+
+    <!-- Now we can set load-bearing properties for the package, but only if the previously-computed conditions hold.
+         We must set these to different values for the 'outer' agnostic package, as well as any RID-agnostic child packages,
+         so that the Publish of those kinds of packages isn't implicitly coerced to be platform-specific in any way.
+         We _do_ have to set both SelfContained and PublishSelfContained here because the logic that applies PublishSelfContained to
+         SelfContained has already run. -->
+    <PublishSelfContained Condition="!$(_IsRidSpecific) and !$(_IsImplicitRestore) and $(_IsPacking)">false</PublishSelfContained>
+    <SelfContained Condition="!$(_IsRidSpecific) and !$(_IsImplicitRestore) and $(_IsPacking)">false</SelfContained>
+    <PublishTrimmed Condition="!$(_IsRidSpecific) and !$(_IsImplicitRestore) and $(_IsPacking)">false</PublishTrimmed>
+    <PublishReadyToRun Condition="!$(_IsRidSpecific) and !$(_IsImplicitRestore) and $(_IsPacking)">false</PublishReadyToRun>
+    <PublishSingleFile Condition="!$(_IsRidSpecific) and !$(_IsImplicitRestore) and $(_IsPacking)">false</PublishSingleFile>
+    <PublishAot Condition="!$(_IsRidSpecific) and !$(_IsImplicitRestore) and $(_IsPacking)">false</PublishAot>
 
     <!-- Tool implementation files are not included in the primary package when the tool has RID-specific packages.  So only pack the tool implementation
        (and only depend on publish) if there are no RID-specific packages, or if the RuntimeIdentifier is set. -->
@@ -82,11 +92,13 @@ NOTE: This file is imported from the following contexts, so be aware when writin
     <_ToolPackageShouldIncludeImplementation Condition="'$(_ToolPackageShouldIncludeImplementation)' == ''">false</_ToolPackageShouldIncludeImplementation>
 
     <!-- inner builds and non-RID-specific outer builds need publish content-->
-    <_PackToolPublishDependency Condition=" '$(_ToolPackageShouldIncludeImplementation)' != '' and '$(GeneratePackageOnBuild)' != 'true' and $(IsPublishable) == 'true' ">Publish</_PackToolPublishDependency>
+    <_PackToolPublishDependency Condition=" $(_ToolPackageShouldIncludeImplementation) and '$(GeneratePackageOnBuild)' != 'true' and $(IsPublishable) == 'true' ">Publish</_PackToolPublishDependency>
     <!-- BUT builds that GeneratePackageOnBuild can't directly rely on Publish since Publish would trigger Build, causing an infinite loop. -->
     <!-- To get around this, we try a bit of a workaround: since we can't set NoBuild=true and call Publish that way, we instead trigger all of the
          dependencies of the no-build Publish Target, _PublishNoBuildAlternative -->
-    <_PackToolPublishDependency Condition=" '$(_ToolPackageShouldIncludeImplementation)' != '' and '$(GeneratePackageOnBuild)' == 'true' and $(IsPublishable) == 'true' ">$(_PublishNoBuildAlternativeDependsOn)</_PackToolPublishDependency>
+    <_PackToolPublishDependency Condition=" $(_ToolPackageShouldIncludeImplementation) and '$(GeneratePackageOnBuild)' == 'true' and $(IsPublishable) == 'true' ">$(_PublishNoBuildAlternativeDependsOn)</_PackToolPublishDependency>
+    <!-- No implementation required? Then don't call either target. -->
+    <_PackToolPublishDependency Condition="!$(_ToolPackageShouldIncludeImplementation)"></_PackToolPublishDependency>
 
     <!-- GenerateNuspec is called in two places: crossTargeting NuGet targets, and single-targeting NuGet targets.
          Tools need to ensure that

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

@@ -96,7 +96,7 @@ Copyright (c) .NET Foundation. All rights reserved.
   ... for these properties is limited to publishing only scenarios.
 
   .NET Tools that build RID-specific packages will also need a primary package without a RuntimeIdentifier, so we disable RID inference for them
-  in order to build the primary package
+  in order to build the primary package, but only when we are actually packing.
 
   Finally, library projects and non-executable projects have awkward interactions here so they are excluded.
   -->
@@ -105,7 +105,7 @@ Copyright (c) .NET Foundation. All rights reserved.
                  '$(RuntimeIdentifier)' == '' and
                  '$(_IsExecutable)' == 'true' and
                  '$(IsRidAgnostic)' != 'true' and
-                 '$(PackAsTool)' != true and
+                 ('$(PackAsTool)' != 'true' or '$(_IsPacking)' != 'true') and
                  (
                    '$(SelfContained)' == 'true' or
                    ('$(_IsPublishing)' == 'true' and
@@ -216,10 +216,8 @@ Copyright (c) .NET Foundation. All rights reserved.
                                   $([MSBuild]::VersionLessThan('$(_TargetFrameworkVersionWithoutV)', '5.0'))">$(DefaultAppHostRuntimeIdentifier.Replace("arm64", "x64"))</DefaultAppHostRuntimeIdentifier>
   </PropertyGroup>
 
-  <Target Name="_CheckForUnsupportedAppHostUsage"
-          BeforeTargets="_CheckForInvalidConfigurationAndPlatform"
-          Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' and '$(HasRuntimeOutput)' == 'true'">
-
+  <Target Name="_CheckRIDAsserts"
+          Condition="'$(PackAsTool)' != 'true' or '$(_IsPacking)' != 'true'">
     <!-- The following RID errors are asserts, and we don't expect them to ever occur. The error message is added as a safeguard.-->
     <NETSdkError Condition="'$(SelfContained)' == 'true' and '$(RuntimeIdentifier)' == '' and '$(AllowSelfContainedWithoutRuntimeIdentifier)' != 'true'"
                  ResourceName="ImplicitRuntimeIdentifierResolutionForPublishPropertyFailed"
@@ -237,6 +235,14 @@ Copyright (c) .NET Foundation. All rights reserved.
                 ResourceName="ImplicitRuntimeIdentifierResolutionForPublishPropertyFailed"
                 FormatArguments="PublishAot"/>
 
+  </Target>
+
+  <Target Name="_CheckForUnsupportedAppHostUsage"
+          BeforeTargets="_CheckForInvalidConfigurationAndPlatform"
+          DependsOnTargets="_CheckRIDAsserts"
+          Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' and '$(HasRuntimeOutput)' == 'true'">
+
+
     <!-- End of implicit RID resolver checks.-->
 
     <NETSdkError Condition="'$(PublishSelfContained)' != 'true' and '$(PublishSelfContained)' != 'false' and '$(PublishSelfContained)' != ''"

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.props

@@ -121,6 +121,7 @@ Copyright (c) .NET Foundation. All rights reserved.
     <PreferNativeArm64 Condition="'$(PreferNativeArm64)'==''">false</PreferNativeArm64>
     <SignAssembly Condition="'$(SignAssembly)'==''">false</SignAssembly>
     <DelaySign Condition="'$(DelaySign)'==''">false</DelaySign>
+    <!-- TODO: should we set _IsPacking=true on the NuGet Pack targets when they hook up the 'generate package on build' stuff -->
     <GeneratePackageOnBuild Condition="'$(GeneratePackageOnBuild)'==''">false</GeneratePackageOnBuild>
     <PackageRequireLicenseAcceptance Condition="'$(PackageRequireLicenseAcceptance)'==''">false</PackageRequireLicenseAcceptance>
     <DebugSymbols Condition="'$(DebugSymbols)'==''">false</DebugSymbols>

test/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildAnAppWithoutTransitiveProjectRefs.cs

@@ -29,14 +29,17 @@ public void It_builds_the_project_successfully_with_static_graph_and_isolation()
         public void It_cleans_the_project_successfully_with_static_graph_and_isolation()
         {
             var (testAsset, outputDirectories) = BuildAppWithTransitiveDependenciesAndTransitiveCompileReference(new[] { "/graph", "/bl:build-{}.binlog" });
+            var binlogDestPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is { } ciOutputRoot && Environment.GetEnvironmentVariable("HELIX_WORKITEM_ID") is { } helixGuid ?
+                Path.Combine(ciOutputRoot, "binlog", helixGuid, $"{nameof(It_cleans_the_project_successfully_with_static_graph_and_isolation)}.binlog") :
+                "./msbuild.binlog";
 
             var cleanCommand = new DotnetCommand(
                 Log,
                 "msbuild",
                 Path.Combine(testAsset.TestRoot, "1", "1.csproj"),
                 "/t:clean",
                 "/graph",
-                "/bl:clean-{}.binlog");
+                $"/bl:{binlogDestPath}");
 
             cleanCommand
                 .Execute()
@@ -175,12 +178,16 @@ public void It_builds_the_project_successfully_when_RAR_does_not_find_all_refere
         private (CommandResult BuildResult, IReadOnlyDictionary<string, DirectoryInfo> OutputDirectories) Build(
             TestAsset testAsset,
             IEnumerable<string> targetFrameworks,
-            string[] msbuildArguments
+            string[] msbuildArguments,
+            [CallerMemberName] string callingMethod = ""
             )
         {
             var buildCommand = new BuildCommand(testAsset, "1");
             buildCommand.WithWorkingDirectory(testAsset.TestRoot);
-            var buildResult = buildCommand.ExecuteWithoutRestore(msbuildArguments);
+            var binlogDestPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is { } ciOutputRoot && Environment.GetEnvironmentVariable("HELIX_WORKITEM_ID") is { } helixGuid ?
+                Path.Combine(ciOutputRoot, "binlog", helixGuid, $"{callingMethod}.binlog") :
+                "./msbuild.binlog";
+            var buildResult = buildCommand.ExecuteWithoutRestore([..msbuildArguments, $"/bl:{binlogDestPath}"]);
 
             var outputDirectories = targetFrameworks.ToImmutableDictionary(tf => tf, tf => buildCommand.GetOutputDirectory(tf));
 

test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishASingleFileApp.cs

@@ -736,7 +736,7 @@ public void EnableSingleFile_warns_when_expected_for_not_correctly_multitargeted
             testProject.AdditionalProperties["CheckEolTargetFramework"] = "false"; // Silence warning about targeting EOL TFMs
             var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: targetFrameworks)
                 .WithProjectChanges(AddTargetFrameworkAliases);
-            
+
             var buildCommand = new BuildCommand(testAsset);
             var resultAssertion = buildCommand.Execute("/p:CheckEolTargetFramework=false")
                 .Should().Pass();
@@ -853,9 +853,14 @@ public void It_can_disable_cetcompat(bool? cetCompat)
                 testProject.AdditionalProperties.Add("CetCompat", cetCompat.ToString());
             }
 
+            var binlogDestPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is { } ciOutputRoot && Environment.GetEnvironmentVariable("HELIX_WORKITEM_ID") is { } helixGuid ?
+                Path.Combine(ciOutputRoot, "binlog", helixGuid, $"{nameof(It_can_disable_cetcompat)}_{cetCompat?.ToString() ?? "null"}.binlog") :
+                "./msbuild.binlog";
+
+
             var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: cetCompat.HasValue ? cetCompat.Value.ToString() : "default");
             var publishCommand = new PublishCommand(testAsset);
-            publishCommand.Execute(PublishSingleFile)
+            publishCommand.Execute(PublishSingleFile, "/bl:" + binlogDestPath)
                 .Should()
                 .Pass();
 

test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAToolProject.cs

@@ -10,6 +10,12 @@ namespace Microsoft.NET.Publish.Tests
 {
     public class GivenThatWeWantToPublishAToolProject : SdkTest
     {
+
+        public static string HostfxrName =
+                RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "hostfxr.dll" :
+                RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "libhostfxr.so" :
+                "libhostfxr.dylib";
+
         public GivenThatWeWantToPublishAToolProject(ITestOutputHelper log) : base(log)
         {
         }
@@ -37,5 +43,24 @@ public void It_can_publish_and_has_apphost()
             publishCommand.GetOutputDirectory(targetFramework: ToolsetInfo.CurrentTargetFramework)
                 .Should().HaveFile("consoledemo" + Constants.ExeSuffix);
         }
+
+        [Fact]
+        // this test verifies that we don't regress the 'normal' publish experience accidentally in the
+        // PackTool.targets
+        public void It_can_publish_selfcontained_and_has_apphost()
+        {
+            var testAsset = SetupTestAsset().SetProjProperty("PublishSelfContained", "true");
+            var publishCommand = new PublishCommand(testAsset);
+
+            var binlogDestPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is { } ciOutputRoot ?
+                Path.Combine(ciOutputRoot, "binlog", $"{nameof(It_can_publish_selfcontained_and_has_apphost)}.binlog") :
+                "./msbuild.binlog";
+
+            publishCommand.WithWorkingDirectory(testAsset.Path).Execute($"-bl:{binlogDestPath}");
+
+            publishCommand.GetOutputDirectory(targetFramework: ToolsetInfo.CurrentTargetFramework, runtimeIdentifier: System.Runtime.InteropServices.RuntimeInformation.RuntimeIdentifier)
+                .Should().HaveFile("consoledemo" + Constants.ExeSuffix)
+                .And.HaveFile(HostfxrName);
+        }
     }
 }

test/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishIncrementally.cs

@@ -62,11 +62,14 @@ public void It_cleans_between_renames()
             };
             var testAsset = _testAssetsManager.CreateTestProject(testProject, testProject.Name);
 
+            var binlogDestPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is { } ciOutputRoot && Environment.GetEnvironmentVariable("HELIX_WORKITEM_ID") is { } helixGuid ?
+                Path.Combine(ciOutputRoot, "binlog", helixGuid, $"{nameof(It_cleans_between_renames)}.binlog") :
+                "./msbuild.binlog";
 
             // Publish as a single file
             var publishCommand = new PublishCommand(testAsset);
             publishCommand
-                .Execute(@"/p:PublishSingleFile=true")
+                .Execute(@"/p:PublishSingleFile=true", $"-bl:{binlogDestPath}")
                 .Should()
                 .Pass();
 
@@ -104,10 +107,14 @@ public void It_cleans_between_single_file_publishes()
             };
             var testAsset = _testAssetsManager.CreateTestProject(testProject, testProject.Name);
 
-            // Publish as a single file
-            var publishCommand = new PublishCommand(testAsset);
+            var binlogDestPath = Environment.GetEnvironmentVariable("HELIX_WORKITEM_UPLOAD_ROOT") is { } ciOutputRoot && Environment.GetEnvironmentVariable("HELIX_WORKITEM_ID") is { } helixGuid ?
+                Path.Combine(ciOutputRoot, "binlog", helixGuid, $"{nameof(It_cleans_between_single_file_publishes)}.binlog") :
+                "./msbuild.binlog";
+
+
+            var publishCommand = new PublishCommand(testAsset).WithWorkingDirectory(testAsset.Path) as PublishCommand;
             publishCommand
-                .Execute(@"/p:PublishSingleFile=true")
+                .Execute(@"/p:PublishSingleFile=true", $"-bl:{binlogDestPath}")
                 .Should()
                 .Pass();
 

test/Microsoft.NET.Publish.Tests/GivenThatWeWantToRunILLink.cs

@@ -1662,8 +1662,14 @@ public void ILLink_can_treat_warnings_as_errors_independently(string targetFrame
                 .And.NotHaveStdOutContaining("error IL2075");
         }
 
+        /// <summary>
+        /// The reason we test this on net7 and below is because in net8 _IsPublishing was added which changes
+        /// the RID-defaulting behavior such that 8+ apps are not 'portable apps' when published for configurations that
+        /// require a RID (self-contained, or trimmed).
+        /// </summary>
+        /// <param name="targetFramework"></param>
         [RequiresMSBuildVersionTheory("17.0.0.32901")]
-        [MemberData(nameof(SupportedTfms), MemberType = typeof(PublishTestUtils))]
+        [MemberData(nameof(TFMsThatDoNotInferPublishSelfContained), MemberType = typeof(PublishTestUtils))]
         public void ILLink_error_on_portable_app(string targetFramework)
         {
             var projectName = "HelloWorld";