Fix TaskHostTask to pass request-level global properties to TaskHost

azure-pipelines/vmr-sb-validation.yml

@@ -13,7 +13,7 @@ parameters:
 - name: extraPropertiesStage2
   displayName: 'Extra MSBuild properties for stage 2 (e.g., /p:DotNetBuildMT=true)'
   type: string
-  default: '/p:DotNetBuildMT=true /p:MSBuildMTExcludedReposOverride=runtime%3Broslyn%3Baspnetcore%3Bsdk%3Befcore%3Bwinforms%3Bwpf%3Brazor%3Bsource-build-reference-packages'
+  default: '/p:DotNetBuildMT=true /p:MSBuildMTExcludedReposOverride=roslyn%3Baspnetcore%3Bsdk%3Befcore%3Bwinforms%3Bwpf%3Brazor%3Bsource-build-reference-packages'
 
 variables:
 - template: /eng/common/templates/variables/pool-providers.yml@self

documentation/wiki/ChangeWaves.md

@@ -31,6 +31,7 @@ Change wave checks around features will be removed in the release that accompani
 
 ### 18.6
 - [AbsolutePath.GetCanonicalForm optimization - avoid expensive Path.GetFullPath calls when paths don't need canonicalization](https://github.com/dotnet/msbuild/pull/13369)
+- [TaskHostTask forwards request-level global properties (e.g. MSBuildRestoreSessionId) to out-of-proc TaskHost in -mt mode](https://github.com/dotnet/msbuild/pull/13443)
 - [Fix ShouldTreatWarningAsError in OOP TaskHost checking wrong collection (WarningsAsMessages instead of WarningsAsErrors)](https://github.com/dotnet/msbuild/issues/11952)
 - [Fix ToolTask hang when tool spawns grandchild processes that inherit stdout/stderr pipe handles](https://github.com/dotnet/msbuild/issues/2981)
 

src/Build.UnitTests/BackEnd/GetGlobalPropertiesTask.cs

@@ -0,0 +1,43 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+
+namespace Microsoft.Build.UnitTests.BackEnd
+{
+    /// <summary>
+    /// A simple task that queries global properties from the build engine via IBuildEngine6.
+    /// Used by TaskHostCallback_Tests to verify that request-level global properties
+    /// (not just build-level properties) are forwarded through TaskHostTask to the out-of-proc TaskHost.
+    /// </summary>
+    public class GetGlobalPropertiesTask : Task
+    {
+        [Output]
+        public int GlobalPropertyCount { get; set; }
+
+        [Output]
+        public string GlobalPropertyLog { get; set; } = string.Empty;
+
+        public override bool Execute()
+        {
+            if (BuildEngine is IBuildEngine6 engine6)
+            {
+                IReadOnlyDictionary<string, string> globalProperties = engine6.GetGlobalProperties();
+                GlobalPropertyCount = globalProperties.Count;
+
+                foreach (KeyValuePair<string, string> kvp in globalProperties)
+                {
+                    Log.LogMessage(MessageImportance.High, $"GlobalProperty: {kvp.Key}={kvp.Value}");
+                }
+
+                Log.LogMessage(MessageImportance.High, $"GlobalPropertyCount = {GlobalPropertyCount}");
+                return true;
+            }
+
+            Log.LogError("BuildEngine does not implement IBuildEngine6");
+            return false;
+        }
+    }
+}

src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs

@@ -5,6 +5,7 @@
 using System.IO;
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.Execution;
+using Microsoft.Build.Framework;
 using Microsoft.Build.UnitTests.Shared;
 using Shouldly;
 using Xunit;
@@ -247,6 +248,116 @@ public void RequestCores_WorksWhenAutoEjectedInMultiThreadedMode()
             logger.FullLog.ShouldContain("RequestCores(1) =");
         }
 
+        /// <summary>
+        /// Regression test for https://github.com/dotnet/msbuild/issues/13153
+        /// Verifies that request-level global properties (passed via BuildRequestData, not just
+        /// BuildParameters.GlobalProperties) are forwarded through TaskHostTask to the out-of-proc
+        /// TaskHost when a task is auto-ejected in multithreaded mode.
+        ///
+        /// Before the fix, TaskHostTask.Execute() used BuildParameters.GlobalProperties (build-level),
+        /// which did not include per-request properties like MSBuildRestoreSessionId. This caused
+        /// NuGet static graph restore to fail for conditional ProjectReference items.
+        /// </summary>
+        [Fact]
+        public void GlobalProperties_ForwardedToAutoEjectedTaskInMultiThreadedMode()
+        {
+            using TestEnvironment env = TestEnvironment.Create(_output);
+            string testDir = env.CreateFolder().Path;
+
+            // GetGlobalPropertiesTask lacks MSBuildMultiThreadableTask attribute,
+            // so it's auto-ejected to TaskHost in MT mode
+            string projectContents = $@"
+<Project>
+    <UsingTask TaskName=""{nameof(GetGlobalPropertiesTask)}"" AssemblyFile=""{typeof(GetGlobalPropertiesTask).Assembly.Location}"" />
+    <Target Name=""Test"">
+        <{nameof(GetGlobalPropertiesTask)}>
+            <Output PropertyName=""PropCount"" TaskParameter=""GlobalPropertyCount"" />
+        </{nameof(GetGlobalPropertiesTask)}>
+    </Target>
+</Project>";
+
+            string projectFile = Path.Combine(testDir, "Test.proj");
+            File.WriteAllText(projectFile, projectContents);
+
+            // Pass request-level global properties via BuildRequestData (simulates what
+            // ExecuteRestore() does when adding MSBuildRestoreSessionId)
+            var requestGlobalProperties = new Dictionary<string, string?>
+            {
+                ["TestRequestProperty"] = "RequestValue",
+                ["AnotherRequestProp"] = "AnotherValue",
+            };
+
+            var logger = new MockLogger(_output);
+            BuildResult buildResult = BuildManager.DefaultBuildManager.Build(
+                new BuildParameters
+                {
+                    MultiThreaded = true,
+                    MaxNodeCount = 4,
+                    Loggers = [logger],
+                    EnableNodeReuse = false,
+                },
+                new BuildRequestData(projectFile, requestGlobalProperties, null, ["Test"], null));
+
+            buildResult.OverallResult.ShouldBe(BuildResultCode.Success);
+
+            // Verify task was ejected to TaskHost
+            logger.FullLog.ShouldContain("external task host");
+
+            // Verify request-level global properties were forwarded to the TaskHost
+            logger.FullLog.ShouldContain("GlobalProperty: TestRequestProperty=RequestValue");
+            logger.FullLog.ShouldContain("GlobalProperty: AnotherRequestProp=AnotherValue");
+        }
+
+        /// <summary>
+        /// Verifies that when ChangeWave 18.6 is disabled, the old behavior is preserved:
+        /// TaskHostTask sends build-level properties (BuildParameters.GlobalProperties) instead
+        /// of request-level properties. This is the opt-out for the fix in #13153.
+        /// </summary>
+        [Fact]
+        public void GlobalProperties_UseBuildLevelWhenChangeWaveDisabled()
+        {
+            using TestEnvironment env = TestEnvironment.Create(_output);
+            env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave18_6.ToString());
+            string testDir = env.CreateFolder().Path;
+
+            string projectContents = $@"
+<Project>
+    <UsingTask TaskName=""{nameof(GetGlobalPropertiesTask)}"" AssemblyFile=""{typeof(GetGlobalPropertiesTask).Assembly.Location}"" />
+    <Target Name=""Test"">
+        <{nameof(GetGlobalPropertiesTask)}>
+            <Output PropertyName=""PropCount"" TaskParameter=""GlobalPropertyCount"" />
+        </{nameof(GetGlobalPropertiesTask)}>
+    </Target>
+</Project>";
+
+            string projectFile = Path.Combine(testDir, "Test.proj");
+            File.WriteAllText(projectFile, projectContents);
+
+            // These request-level properties should NOT be forwarded when the wave is disabled
+            var requestGlobalProperties = new Dictionary<string, string?>
+            {
+                ["TestRequestProperty"] = "RequestValue",
+            };
+
+            var logger = new MockLogger(_output);
+            BuildResult buildResult = BuildManager.DefaultBuildManager.Build(
+                new BuildParameters
+                {
+                    MultiThreaded = true,
+                    MaxNodeCount = 4,
+                    Loggers = [logger],
+                    EnableNodeReuse = false,
+                },
+                new BuildRequestData(projectFile, requestGlobalProperties, null, ["Test"], null));
+
+            buildResult.OverallResult.ShouldBe(BuildResultCode.Success);
+
+            // With wave disabled, build-level properties are used (empty in this test),
+            // so request-level properties should NOT appear
+            logger.FullLog.ShouldNotContain("GlobalProperty: TestRequestProperty=RequestValue");
+            logger.FullLog.ShouldContain("GlobalPropertyCount = 0");
+        }
+
         /// <summary>
         /// Regression test for https://github.com/dotnet/msbuild/issues/13333
         /// When callbacks are not supported (cross-version OOP TaskHost), RequestCores must

src/Build/Instance/TaskFactories/TaskHostTask.cs

@@ -344,7 +344,7 @@ public bool Execute()
                         _projectFile,
                         _buildComponentHost.BuildParameters.LogTaskInputs,
                         _setParameters,
-                        new Dictionary<string, string>(_buildComponentHost.BuildParameters.GlobalProperties),
+                        GetGlobalPropertiesForTaskHost(),
                         _taskLoggingContext.GetWarningsAsErrors(),
                         _taskLoggingContext.GetWarningsNotAsErrors(),
                         _taskLoggingContext.GetWarningsAsMessages());
@@ -412,6 +412,30 @@ public bool Execute()
             return _taskExecutionSucceeded;
         }
 
+        /// <summary>
+        /// Gets the global properties to send to the out-of-proc TaskHost.
+        /// Under ChangeWave 18.6, uses request-level properties from BuildEngine via
+        /// <see cref="IBuildEngine6.GetGlobalProperties"/> because those include per-request
+        /// properties like MSBuildRestoreSessionId that are added by ExecuteRestore() but are
+        /// not present in the build-level BuildParameters.GlobalProperties.
+        /// </summary>
+        private Dictionary<string, string> GetGlobalPropertiesForTaskHost()
+        {
+            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_6) && BuildEngine is IBuildEngine6 buildEngine6)
+            {
+                IReadOnlyDictionary<string, string> requestProperties = buildEngine6.GetGlobalProperties();
+                var result = new Dictionary<string, string>(requestProperties.Count);
+                foreach (KeyValuePair<string, string> kvp in requestProperties)
+                {
+                    result[kvp.Key] = kvp.Value;
+                }
+
+                return result;
+            }
+
+            return new Dictionary<string, string>(_buildComponentHost.BuildParameters.GlobalProperties);
+        }
+
         /// <summary>
         /// Registers the specified handler for a particular packet type.
         /// </summary>