diff --git a/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs b/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs index 0d43d7f9d4a..4aea24b2a95 100644 --- a/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs +++ b/src/Build.UnitTests/BackEnd/Scheduler_Tests.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Xml; using Microsoft.Build.BackEnd; using Microsoft.Build.Evaluation; @@ -12,7 +11,6 @@ using Microsoft.Build.Experimental.ProjectCache; using Microsoft.Build.Framework; using Microsoft.Build.Shared; -using Microsoft.Build.Unittest; using Shouldly; using Xunit; using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem; @@ -21,6 +19,8 @@ namespace Microsoft.Build.UnitTests.BackEnd { + using Microsoft.Build.Unittest; + /// /// Tests of the scheduler. /// @@ -544,13 +544,7 @@ public void TestProxyAffinityIsInProc() CreateConfiguration(1, "foo.csproj"); - BuildRequest request1 = CreateBuildRequest( - nodeRequestId: 1, - configId: 1, - targets: new[] { "foo" }, - NodeAffinity.Any, - parentRequest: null, - new ProxyTargets(new Dictionary { { "foo", "bar" } })); + BuildRequest request1 = CreateProxyBuildRequest(1, 1, new ProxyTargets(new Dictionary { { "foo", "bar" } }), null); BuildRequestBlocker blocker = new BuildRequestBlocker(-1, Array.Empty(), new[] { request1 }); List response = new List(_scheduler.ReportRequestBlocked(1, blocker)); @@ -812,6 +806,8 @@ private BuildRequest CreateBuildRequest(int nodeRequestId, int configId, string[ /// private BuildRequest CreateBuildRequest(int nodeRequestId, int configId, string[] targets, NodeAffinity nodeAffinity, BuildRequest parentRequest, ProxyTargets proxyTargets = null) { + (targets == null ^ proxyTargets == null).ShouldBeTrue(); + HostServices hostServices = null; if (nodeAffinity != NodeAffinity.Any) @@ -820,26 +816,36 @@ private BuildRequest CreateBuildRequest(int nodeRequestId, int configId, string[ hostServices.SetNodeAffinity(String.Empty, nodeAffinity); } - if (proxyTargets != null) + if (targets != null) { - parentRequest.ShouldBeNull(); return new BuildRequest( submissionId: 1, nodeRequestId, configId, - proxyTargets, - targets.ToList(), - hostServices); + targets, + hostServices, + BuildEventContext.Invalid, + parentRequest); } + parentRequest.ShouldBeNull(); return new BuildRequest( submissionId: 1, nodeRequestId, configId, - targets, - hostServices, - BuildEventContext.Invalid, - parentRequest); + proxyTargets, + hostServices); + } + + private BuildRequest CreateProxyBuildRequest(int nodeRequestId, int configId, ProxyTargets proxyTargets, BuildRequest parentRequest) + { + return CreateBuildRequest( + nodeRequestId, + configId, + null, + NodeAffinity.Any, + parentRequest, + proxyTargets); } /// diff --git a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs index 3d7a9e96ab6..6debe82528e 100644 --- a/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs +++ b/src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs @@ -507,6 +507,7 @@ public void ProjectCacheByBuildParametersAndBottomUpBuildWorks(GraphCacheRespons } } + AssertCacheBuild(graph, testData, mockCache, logger, nodesToBuildResults, targets: null); } @@ -765,59 +766,6 @@ public void RunningProxyBuildsOnOutOfProcNodesShouldIssueWarning(bool disableInp logger.AssertMessageCount("MSB4274", 1); } - // A common scenario is to get a request for N targets, but only some of them can be handled by the cache. - // In this case, missing targets should be passed through. - [Fact] - public async Task PartialProxyTargets() - { - const string ProjectContent = """ - - - - - - - - - - - - """; - TransientTestFile project = _env.CreateFile($"project.proj", ProjectContent); - - BuildParameters buildParameters = new() - { - ProjectCacheDescriptor = ProjectCacheDescriptor.FromInstance( - new ConfigurableMockCache - { - GetCacheResultImplementation = (_, _, _) => - { - return Task.FromResult( - CacheResult.IndicateCacheHit( - new ProxyTargets( - new Dictionary - { - { "ProxyTarget", "SomeTarget" }, - }))); - } - }), - }; - - MockLogger logger; - using (Helpers.BuildManagerSession buildSession = new(_env, buildParameters)) - { - logger = buildSession.Logger; - BuildResult buildResult = await buildSession.BuildProjectFileAsync(project.Path, new[] { "SomeTarget", "SomeOtherTarget" }); - - buildResult.Exception.ShouldBeNull(); - buildResult.ShouldHaveSucceeded(); - } - - logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("SomeTarget running"); - logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("ProxyTarget running"); - logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("SomeOtherTarget running"); - } - private void AssertCacheBuild( ProjectGraph graph, GraphCacheResponse testData, diff --git a/src/Build/BackEnd/BuildManager/BuildManager.cs b/src/Build/BackEnd/BuildManager/BuildManager.cs index 66bed57e5ea..bc1e535abab 100644 --- a/src/Build/BackEnd/BuildManager/BuildManager.cs +++ b/src/Build/BackEnd/BuildManager/BuildManager.cs @@ -1783,30 +1783,15 @@ private static void AddBuildRequestToSubmission(BuildSubmission submission, int private static void AddProxyBuildRequestToSubmission( BuildSubmission submission, - BuildRequestConfiguration configuration, + int configurationId, ProxyTargets proxyTargets, int projectContextId) { - IReadOnlyDictionary realTargetsToProxyTargets = proxyTargets.RealTargetToProxyTargetMap; - - ICollection requestedTargets = submission.BuildRequestData.TargetNames.Count > 0 - ? submission.BuildRequestData.TargetNames - : configuration.Project.DefaultTargets; - List targets = new(requestedTargets.Count); - foreach (string requestedTarget in requestedTargets) - { - string effectiveTarget = realTargetsToProxyTargets.TryGetValue(requestedTarget, out string proxyTarget) - ? proxyTarget - : requestedTarget; - targets.Add(effectiveTarget); - } - submission.BuildRequest = new BuildRequest( submission.SubmissionId, BackEnd.BuildRequest.InvalidNodeRequestId, - configuration.ConfigurationId, + configurationId, proxyTargets, - targets, submission.BuildRequestData.HostServices, submission.BuildRequestData.Flags, submission.BuildRequestData.RequestedProjectState, @@ -2309,7 +2294,7 @@ void HandleCacheResult() { // Setup submission.BuildRequest with proxy targets. The proxy request is built on the inproc node (to avoid // ProjectInstance serialization). The proxy target results are used as results for the real targets. - AddProxyBuildRequestToSubmission(submission, configuration, cacheResult.ProxyTargets, projectContextId); + AddProxyBuildRequestToSubmission(submission, configuration.ConfigurationId, cacheResult.ProxyTargets, projectContextId); IssueBuildRequestForBuildSubmission(submission, configuration, allowMainThreadBuild: false); } else if (cacheResult.ResultType == CacheResultType.CacheHit && cacheResult.BuildResult != null) diff --git a/src/Build/BackEnd/Components/ProjectCache/ProxyTargets.cs b/src/Build/BackEnd/Components/ProjectCache/ProxyTargets.cs index 03f7892e8b3..970dfdd3332 100644 --- a/src/Build/BackEnd/Components/ProjectCache/ProxyTargets.cs +++ b/src/Build/BackEnd/Components/ProjectCache/ProxyTargets.cs @@ -27,23 +27,6 @@ public class ProxyTargets : ITranslatable /// public IReadOnlyDictionary ProxyTargetToRealTargetMap => _proxyTargetToRealTargetMap; - internal IReadOnlyDictionary RealTargetToProxyTargetMap - { - get - { - // The ProxyTargetToRealTargetMap is "backwards" from how most users would want to use it and doesn't provide as much flexibility as it could if reversed. - // Unfortunately this is part of a public API so cannot easily change at this point. - Dictionary realTargetsToProxyTargets = new(ProxyTargetToRealTargetMap.Count, StringComparer.OrdinalIgnoreCase); - foreach (KeyValuePair kvp in ProxyTargetToRealTargetMap) - { - // In the case of multiple proxy targets pointing to the same real target, the last one wins. Another awkwardness of ProxyTargetToRealTargetMap being "backwards". - realTargetsToProxyTargets[kvp.Value] = kvp.Key; - } - - return realTargetsToProxyTargets; - } - } - private ProxyTargets() { } diff --git a/src/Build/BackEnd/Shared/BuildRequest.cs b/src/Build/BackEnd/Shared/BuildRequest.cs index 94ca6b40fcd..428eea19656 100644 --- a/src/Build/BackEnd/Shared/BuildRequest.cs +++ b/src/Build/BackEnd/Shared/BuildRequest.cs @@ -128,7 +128,6 @@ private BuildRequest( /// The id of the node issuing the request /// The configuration id to use. /// - /// The set of targets to execute /// Host services if any. May be null. /// Additional flags for the request. /// Filter for desired build results. @@ -138,7 +137,6 @@ public BuildRequest( int nodeRequestId, int configurationId, ProxyTargets proxyTargets, - List targets, HostServices hostServices, BuildRequestDataFlags buildRequestDataFlags = BuildRequestDataFlags.None, RequestedProjectState requestedProjectState = null, @@ -146,7 +144,7 @@ public BuildRequest( : this(submissionId, nodeRequestId, configurationId, hostServices, buildRequestDataFlags, requestedProjectState, projectContextId) { _proxyTargets = proxyTargets; - _targets = targets; + _targets = proxyTargets.ProxyTargetToRealTargetMap.Keys.ToList(); // Only root requests can have proxy targets. _parentGlobalRequestId = InvalidGlobalRequestId; diff --git a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs index 4dfa7e4332d..838210573f6 100644 --- a/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs +++ b/src/Build/BackEnd/Shared/BuildRequestConfiguration.cs @@ -745,6 +745,13 @@ public List GetTargetsUsedToBuildRequest(BuildRequest request) ErrorUtilities.VerifyThrow(_projectInitialTargets != null, "Initial targets have not been set."); ErrorUtilities.VerifyThrow(_projectDefaultTargets != null, "Default targets have not been set."); + if (request.ProxyTargets != null) + { + ErrorUtilities.VerifyThrow( + CollectionHelpers.SetEquivalent(request.Targets, request.ProxyTargets.ProxyTargetToRealTargetMap.Keys), + "Targets must be same as proxy targets"); + } + List initialTargets = _projectInitialTargets; List nonInitialTargets = (request.Targets.Count == 0) ? _projectDefaultTargets : request.Targets;