Revert "Populate unproxied targets to avoid dropping requested targets (#9130)" (#9358)

This reverts commit 4256aed.

Fixes AB#1906434.

Author: maridematte

src/Build.UnitTests/BackEnd/Scheduler_Tests.cs

@@ -4,15 +4,13 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
 using System.Xml;
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.Evaluation;
 using Microsoft.Build.Execution;
 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;
+
     /// <summary>
     /// Tests of the scheduler.
     /// </summary>
@@ -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<string, string> { { "foo", "bar" } }));
+            BuildRequest request1 = CreateProxyBuildRequest(1, 1, new ProxyTargets(new Dictionary<string, string> { { "foo", "bar" } }), null);
 
             BuildRequestBlocker blocker = new BuildRequestBlocker(-1, Array.Empty<string>(), new[] { request1 });
             List<ScheduleResponse> response = new List<ScheduleResponse>(_scheduler.ReportRequestBlocked(1, blocker));
@@ -812,6 +806,8 @@ private BuildRequest CreateBuildRequest(int nodeRequestId, int configId, string[
         /// </summary>
         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);
         }
 
         /// <summary>

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 = """
-                <Project>
-                  <Target Name="SomeTarget">
-                    <Message Text="SomeTarget running" />
-                  </Target>
-                  <Target Name="ProxyTarget">
-                    <Message Text="ProxyTarget running" />
-                  </Target>
-                  <Target Name="SomeOtherTarget">
-                    <Message Text="SomeOtherTarget running" />
-                  </Target>
-                </Project>
-                """;
-            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<string, string>
-                                        {
-                                            { "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,

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<string, string> realTargetsToProxyTargets = proxyTargets.RealTargetToProxyTargetMap;
-
-            ICollection<string> requestedTargets = submission.BuildRequestData.TargetNames.Count > 0
-                ? submission.BuildRequestData.TargetNames
-                : configuration.Project.DefaultTargets;
-            List<string> 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)

src/Build/BackEnd/Components/ProjectCache/ProxyTargets.cs

@@ -27,23 +27,6 @@ public class ProxyTargets : ITranslatable
         /// </summary>
         public IReadOnlyDictionary<string, string> ProxyTargetToRealTargetMap => _proxyTargetToRealTargetMap;
 
-        internal IReadOnlyDictionary<string, string> 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<string, string> realTargetsToProxyTargets = new(ProxyTargetToRealTargetMap.Count, StringComparer.OrdinalIgnoreCase);
-                foreach (KeyValuePair<string, string> 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()
         {
         }

src/Build/BackEnd/Shared/BuildRequest.cs

@@ -128,7 +128,6 @@ private BuildRequest(
         /// <param name="nodeRequestId">The id of the node issuing the request</param>
         /// <param name="configurationId">The configuration id to use.</param>
         /// <param name="proxyTargets"><see cref="ProxyTargets"/></param>
-        /// <param name="targets">The set of targets to execute</param>
         /// <param name="hostServices">Host services if any. May be null.</param>
         /// <param name="buildRequestDataFlags">Additional flags for the request.</param>
         /// <param name="requestedProjectState">Filter for desired build results.</param>
@@ -138,15 +137,14 @@ public BuildRequest(
             int nodeRequestId,
             int configurationId,
             ProxyTargets proxyTargets,
-            List<string> targets,
             HostServices hostServices,
             BuildRequestDataFlags buildRequestDataFlags = BuildRequestDataFlags.None,
             RequestedProjectState requestedProjectState = null,
             int projectContextId = BuildEventContext.InvalidProjectContextId)
             : 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;

src/Build/BackEnd/Shared/BuildRequestConfiguration.cs

@@ -745,6 +745,13 @@ public List<string> 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<string> initialTargets = _projectInitialTargets;
             List<string> nonInitialTargets = (request.Targets.Count == 0) ? _projectDefaultTargets : request.Targets;