diff --git a/azure-pipelines/vmr-sb-validation.yml b/azure-pipelines/vmr-sb-validation.yml index 305dea8e8b5..97995be9161 100644 --- a/azure-pipelines/vmr-sb-validation.yml +++ b/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 diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index d65d97beb89..27854c3cc9a 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/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) diff --git a/src/Build.UnitTests/BackEnd/GetGlobalPropertiesTask.cs b/src/Build.UnitTests/BackEnd/GetGlobalPropertiesTask.cs new file mode 100644 index 00000000000..910a6f59ee5 --- /dev/null +++ b/src/Build.UnitTests/BackEnd/GetGlobalPropertiesTask.cs @@ -0,0 +1,40 @@ +// 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 +{ + /// + /// 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. + /// + public class GetGlobalPropertiesTask : Task + { + [Output] + public int GlobalPropertyCount { get; set; } + + public override bool Execute() + { + if (BuildEngine is IBuildEngine6 engine6) + { + IReadOnlyDictionary globalProperties = engine6.GetGlobalProperties(); + GlobalPropertyCount = globalProperties.Count; + + foreach (KeyValuePair 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; + } + } +} diff --git a/src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs b/src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs index ab3cd481f87..f54cde2470e 100644 --- a/src/Build.UnitTests/BackEnd/TaskHostCallback_Tests.cs +++ b/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) ="); } + /// + /// 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. + /// + [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 = $@" + + + + <{nameof(GetGlobalPropertiesTask)}> + + + +"; + + 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 + { + ["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"); + } + + /// + /// 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. + /// + [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 = $@" + + + + <{nameof(GetGlobalPropertiesTask)}> + + + +"; + + 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 + { + ["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"); + } + /// /// Regression test for https://github.com/dotnet/msbuild/issues/13333 /// When callbacks are not supported (cross-version OOP TaskHost), RequestCores must diff --git a/src/Build/Instance/TaskFactories/TaskHostTask.cs b/src/Build/Instance/TaskFactories/TaskHostTask.cs index 269362b1c7a..04bc5679d8a 100644 --- a/src/Build/Instance/TaskFactories/TaskHostTask.cs +++ b/src/Build/Instance/TaskFactories/TaskHostTask.cs @@ -344,7 +344,7 @@ public bool Execute() _projectFile, _buildComponentHost.BuildParameters.LogTaskInputs, _setParameters, - new Dictionary(_buildComponentHost.BuildParameters.GlobalProperties), + GetGlobalPropertiesForTaskHost(), _taskLoggingContext.GetWarningsAsErrors(), _taskLoggingContext.GetWarningsNotAsErrors(), _taskLoggingContext.GetWarningsAsMessages()); @@ -412,6 +412,30 @@ public bool Execute() return _taskExecutionSucceeded; } + /// + /// Gets the global properties to send to the out-of-proc TaskHost. + /// Under ChangeWave 18.6, uses request-level properties from BuildEngine via + /// because those include per-request + /// properties like MSBuildRestoreSessionId that are added by ExecuteRestore() but are + /// not present in the build-level BuildParameters.GlobalProperties. + /// + private Dictionary GetGlobalPropertiesForTaskHost() + { + if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_6) && BuildEngine is IBuildEngine6 buildEngine6) + { + IReadOnlyDictionary requestProperties = buildEngine6.GetGlobalProperties(); + var result = new Dictionary(requestProperties.Count); + foreach (KeyValuePair kvp in requestProperties) + { + result[kvp.Key] = kvp.Value; + } + + return result; + } + + return new Dictionary(_buildComponentHost.BuildParameters.GlobalProperties); + } + /// /// Registers the specified handler for a particular packet type. ///