Support SkipNonexistentTargets in project reference target protocol

src/Build.UnitTests/Graph/ProjectGraph_Tests.cs

@@ -1277,6 +1277,114 @@ public void GetTargetsListProjectReferenceTargetsOrDefaultComplexPropagation()
             }
         }
 
+        [Fact]
+        public void GetTargetsListSupportsTargetsMarkedSkipNonexistentTargets()
+        {
+            ProjectGraph graph = Helpers.CreateProjectGraph(
+                _env,
+                dependencyEdges: new Dictionary<int, int[]>
+                {
+                    { 1, new[] { 2 } },
+                },
+                extraContentPerProjectNumber: new Dictionary<int, string>
+                {
+                    {
+                        1,
+                        @"
+                          <ItemGroup>
+                            <ProjectReferenceTargets Include='Build' Targets='NonskippableTarget1' />
+                            <ProjectReferenceTargets Include='Build' Targets='NonskippableTarget2' SkipNonexistentTargets='false' />
+                            <ProjectReferenceTargets Include='Build' Targets='SkippableExistingTarget;SkippableNonexistentTarget' SkipNonexistentTargets='true' />
+                            <ProjectReference Include='2.proj' />
+                          </ItemGroup>
+                          <Target Name='Build'>
+                            <MSBuild Projects='2.proj' Targets='NonskippableTarget1; NonskippableTarget2' />
+                            <MSBuild Projects='2.proj' Targets='SkippableExistingTarget;SkippableNonexistentTarget' SkipNonexistentTargets='true' />
+                          </Target>"
+                    },
+                    {
+                        2,
+                        @"<Target Name='NonskippableTarget1'>
+                          </Target>
+                          <Target Name='NonskippableTarget2'>
+                          </Target>
+                          <Target Name='SkippableExistingTarget'>
+                          </Target>"
+                    },
+                });
+            IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> targetLists = graph.GetTargetLists(entryProjectTargets: new[] { "Build" });
+            targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 1)].ShouldBe(expected: new[] { "Build" });
+            targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 2)].ShouldBe(expected: new[] { "NonskippableTarget1", "NonskippableTarget2", "SkippableExistingTarget" });
+            Dictionary<string, (BuildResult Result, MockLogger Logger)> results = ResultCacheBasedBuilds_Tests.BuildUsingCaches(
+                _env,
+                topoSortedNodes: graph.ProjectNodesTopologicallySorted,
+                generateCacheFiles: true,
+                expectedNodeBuildOutput: new Dictionary<ProjectGraphNode, string[]>(),
+                outputCaches: new Dictionary<ProjectGraphNode, string>(),
+                assertBuildResults: false,
+                targetListsPerNode: targetLists);
+            foreach (KeyValuePair<string, (BuildResult Result, MockLogger Logger)> result in results)
+            {
+                result.Value.Result.OverallResult.ShouldBe(BuildResultCode.Success);
+            }
+        }
+
+        [Fact]
+        public void SkipNonexistentTargetsDoesNotHideMissedTargetResults()
+        {
+            ProjectGraph graph = Helpers.CreateProjectGraph(
+                env: _env,
+                dependencyEdges: new Dictionary<int, int[]>
+                {
+                    { 1, new[] { 2 } },
+                },
+                extraContentPerProjectNumber: new Dictionary<int, string>
+                {
+                    {
+                        1,
+                        @"
+                          <ItemGroup>
+                            <!-- NonskippableTarget2 is not specified in the target protocol, which should cause the build to fail -->
+                            <ProjectReferenceTargets Include='Build' Targets='NonskippableTarget1;SkippableExistingTarget;SkippableNonexistentTarget' SkipNonexistentTargets='true' />
+                            <ProjectReference Include='2.proj' />
+                          </ItemGroup>
+                          <Target Name='Build'>
+                            <MSBuild Projects='2.proj' Targets='NonskippableTarget1;NonskippableTarget2;SkippableExistingTarget;SkippableNonexistentTarget' SkipNonexistentTargets='true' />
+                          </Target>"
+                    },
+                    {
+                        2,
+                        @"<Target Name='NonskippableTarget1'>
+                          </Target>
+                          <Target Name='NonskippableTarget2'>
+                          </Target>
+                          <Target Name='SkippableExistingTarget'>
+                          </Target>"
+                    },
+                });
+            IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> targetLists = graph.GetTargetLists(entryProjectTargets: new[] { "Build" });
+            targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 1)].ShouldBe(expected: new[] { "Build" });
+            targetLists[key: GetFirstNodeWithProjectNumber(graph: graph, projectNum: 2)].ShouldBe(expected: new[] { "NonskippableTarget1", "SkippableExistingTarget" });
+            Dictionary<string, (BuildResult Result, MockLogger Logger)> results = ResultCacheBasedBuilds_Tests.BuildUsingCaches(
+                env: _env,
+                topoSortedNodes: graph.ProjectNodesTopologicallySorted,
+                generateCacheFiles: true,
+                expectedNodeBuildOutput: new Dictionary<ProjectGraphNode, string[]>(),
+                outputCaches: new Dictionary<ProjectGraphNode, string>(),
+                assertBuildResults: false,
+                targetListsPerNode: targetLists);
+            results["2"].Result.OverallResult.ShouldBe(BuildResultCode.Success);
+            BuildResult project1BuildResult = results["1"].Result;
+            project1BuildResult.OverallResult.ShouldBe(BuildResultCode.Failure);
+            MockLogger project1MockLogger = results["1"].Logger;
+            project1MockLogger.ErrorCount.ShouldBe(1);
+            string project1ErrorMessage = project1MockLogger.Errors.First().Message;
+            project1ErrorMessage.ShouldContain("MSB4252");
+            project1ErrorMessage.ShouldContain("1.proj");
+            project1ErrorMessage.ShouldContain("2.proj");
+            project1ErrorMessage.ShouldContain(" with the (NonskippableTarget1;NonskippableTarget2;SkippableExistingTarget;SkippableNonexistentTarget) target(s) but the build result for the built project is not in the engine cache");
+        }
+
         [Fact]
         public void ReferencedMultitargetingEntryPointNodeTargetListContainsDefaultTarget()
         {

src/Build.UnitTests/Graph/ResultCacheBasedBuilds_Tests.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -416,6 +417,7 @@ public void MissingResultFromCacheShouldErrorDueToIsolatedBuildCacheEnforcement(
         /// <param name="generateCacheFiles"></param>
         /// <param name="assertBuildResults"></param>
         /// <param name="expectedOutputProducer"></param>
+        /// <param name="targetListsPerNode">The list of targets to build per node.</param>
         /// <param name="projectIsolationMode">The isolation mode under which to run.</param>
         /// <returns></returns>
         internal static Dictionary<string, (BuildResult Result, MockLogger Logger)> BuildUsingCaches(
@@ -427,6 +429,7 @@ public void MissingResultFromCacheShouldErrorDueToIsolatedBuildCacheEnforcement(
             bool assertBuildResults = true,
             // (current node, expected output dictionary) -> actual expected output for current node
             Func<ProjectGraphNode, ExpectedNodeBuildOutput, string[]> expectedOutputProducer = null,
+            IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> targetListsPerNode = null,
             ProjectIsolationMode projectIsolationMode = ProjectIsolationMode.False)
         {
             expectedOutputProducer ??= ((node, expectedOutputs) => expectedOutputs[node]);
@@ -460,15 +463,15 @@ public void MissingResultFromCacheShouldErrorDueToIsolatedBuildCacheEnforcement(
                     buildParameters.OutputResultsCacheFile = outputCaches[node];
                 }
 
-                var logger = new MockLogger();
+                var logger = new MockLogger(env.Output);
 
                 buildParameters.Loggers = new[] { logger };
 
                 var result = BuildProjectFileUsingBuildManager(
                     node.ProjectInstance.FullPath,
                     null,
                     buildParameters,
-                    node.ProjectInstance.DefaultTargets);
+                    targetListsPerNode?[node] != null ? targetListsPerNode?[node] : node.ProjectInstance.DefaultTargets);
 
                 results[ProjectNumber(node)] = (result, logger);
 

src/Build/BackEnd/BuildManager/BuildManager.cs

@@ -1397,7 +1397,7 @@ private void LoadSolutionIntoConfiguration(BuildRequestConfiguration config, Bui
                 ((IBuildComponentHost)this).LoggingService,
                 request.BuildEventContext,
                 false /* loaded by solution parser*/,
-                config.TargetNames,
+                config.RequestedTargets,
                 SdkResolverService,
                 request.SubmissionId);
 
@@ -2336,23 +2336,17 @@ private void HandleConfigurationRequest(int node, BuildRequestConfiguration unre
         /// </summary>
         private void HandleResult(int node, BuildResult result)
         {
-            // Update cache with the default and initial targets, as needed.
+            // Update cache with the default, initial, and project targets, as needed.
             BuildRequestConfiguration configuration = _configCache[result.ConfigurationId];
             if (result.DefaultTargets != null)
             {
-                // If the result has Default and Initial targets, we populate the configuration cache with them if it
+                // If the result has Default, Initial, and project targets, we populate the configuration cache with them if it
                 // doesn't already have entries.  This can happen if we created a configuration based on a request from
                 // an external node, but hadn't yet received a result since we may not have loaded the Project locally
-                // and thus wouldn't know what the default and initial targets were.
-                if (configuration.ProjectDefaultTargets == null)
-                {
-                    configuration.ProjectDefaultTargets = result.DefaultTargets;
-                }
-
-                if (configuration.ProjectInitialTargets == null)
-                {
-                    configuration.ProjectInitialTargets = result.InitialTargets;
-                }
+                // and thus wouldn't know what the default, initial, and project targets were.
+                configuration.ProjectDefaultTargets ??= result.DefaultTargets;
+                configuration.ProjectInitialTargets ??= result.InitialTargets;
+                configuration.ProjectTargets ??= result.ProjectTargets;
             }
 
             IEnumerable<ScheduleResponse> response = _scheduler.ReportResult(node, result);

src/Build/BackEnd/BuildManager/CacheSerialization.cs

@@ -97,7 +97,7 @@ public static string SerializeCaches(
                         // due to its dependency on a cached target whose side effects would
                         // not be taken into account. (E.g., the definition of a property.)
                         resultsCacheToSerialize.GetResultsForConfiguration(smallestConfigId)
-                            .KeepSpecificTargetResults(configCacheToSerialize[smallestConfigId].TargetNames);
+                            .KeepSpecificTargetResults(configCacheToSerialize[smallestConfigId].RequestedTargets);
                     }
 
                     translator.Translate(ref configCacheToSerialize);

src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs

@@ -809,6 +809,7 @@ private void EvaluateRequestStates()
                     // own cache.
                     completedEntry.Result.DefaultTargets = configuration.ProjectDefaultTargets;
                     completedEntry.Result.InitialTargets = configuration.ProjectInitialTargets;
+                    completedEntry.Result.ProjectTargets = configuration.ProjectTargets;
                 }
 
                 TraceEngine("ERS: Request is now {0}({1}) (nr {2}) has had its builder cleaned up.", completedEntry.Request.GlobalRequestId, completedEntry.Request.ConfigurationId, completedEntry.Request.NodeRequestId);

src/Build/BackEnd/Components/Scheduler/Scheduler.cs

@@ -1963,7 +1963,8 @@ private bool CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot(int nodeF
             var configCache = (IConfigCache)_componentHost.GetComponent(BuildComponentType.ConfigCache);
 
             // do not check root requests as nothing depends on them
-            if (isolateProjects == ProjectIsolationMode.False || request.IsRootRequest || request.SkipStaticGraphIsolationConstraints)
+            if (isolateProjects == ProjectIsolationMode.False || request.IsRootRequest || request.SkipStaticGraphIsolationConstraints
+                || SkipNonexistentTargetsIfExistentTargetsHaveResults(request))
             {
                 bool logComment = ((isolateProjects == ProjectIsolationMode.True || isolateProjects == ProjectIsolationMode.MessageUponIsolationViolation) && request.SkipStaticGraphIsolationConstraints);
                 if (logComment)
@@ -1975,14 +1976,14 @@ private bool CheckIfCacheMissOnReferencedProjectIsAllowedAndErrorIfNot(int nodeF
                             NewBuildEventContext(),
                             MessageImportance.Normal,
                             "SkippedConstraintsOnRequest",
-                            configs.parentConfig.ProjectFullPath,
-                            configs.requestConfig.ProjectFullPath);
+                            configs.ParentConfig.ProjectFullPath,
+                            configs.RequestConfig.ProjectFullPath);
                 }
 
                 return true;
             }
 
-            var (requestConfig, parentConfig) = GetConfigurations();
+            (BuildRequestConfiguration requestConfig, BuildRequestConfiguration parentConfig) = GetConfigurations();
 
             // allow self references (project calling the msbuild task on itself, potentially with different global properties)
             if (parentConfig.ProjectFullPath.Equals(requestConfig.ProjectFullPath, StringComparison.OrdinalIgnoreCase))
@@ -2021,27 +2022,61 @@ BuildEventContext NewBuildEventContext()
                     BuildEventContext.InvalidTaskId);
             }
 
-            (BuildRequestConfiguration requestConfig, BuildRequestConfiguration parentConfig) GetConfigurations()
+            (BuildRequestConfiguration RequestConfig, BuildRequestConfiguration ParentConfig) GetConfigurations()
             {
-                var buildRequestConfiguration = configCache[request.ConfigurationId];
+                BuildRequestConfiguration buildRequestConfiguration = configCache[request.ConfigurationId];
 
                 // Need the parent request. It might be blocked or executing; check both.
-                var parentRequest = _schedulingData.BlockedRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId)
-                                    ?? _schedulingData.ExecutingRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId);
+                SchedulableRequest parentRequest = _schedulingData.BlockedRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId)
+                    ?? _schedulingData.ExecutingRequests.FirstOrDefault(r => r.BuildRequest.GlobalRequestId == request.ParentGlobalRequestId);
 
                 ErrorUtilities.VerifyThrowInternalNull(parentRequest, nameof(parentRequest));
                 ErrorUtilities.VerifyThrow(
                     configCache.HasConfiguration(parentRequest.BuildRequest.ConfigurationId),
                     "All non root requests should have a parent with a loaded configuration");
 
-                var parentConfiguration = configCache[parentRequest.BuildRequest.ConfigurationId];
+                BuildRequestConfiguration parentConfiguration = configCache[parentRequest.BuildRequest.ConfigurationId];
                 return (buildRequestConfiguration, parentConfiguration);
             }
 
             string ConcatenateGlobalProperties(BuildRequestConfiguration configuration)
             {
                 return string.Join("; ", configuration.GlobalProperties.Select<ProjectPropertyInstance, string>(p => $"{p.Name}={p.EvaluatedValue}"));
             }
+
+            bool SkipNonexistentTargetsIfExistentTargetsHaveResults(BuildRequest buildRequest)
+            {
+                // Return early if the top-level target(s) of this build request weren't requested to be skipped if nonexistent.
+                if ((buildRequest.BuildRequestDataFlags & BuildRequestDataFlags.SkipNonexistentTargets) != BuildRequestDataFlags.SkipNonexistentTargets)
+                {
+                    return false;
+                }
+
+                BuildResult requestResults = _resultsCache.GetResultsForConfiguration(buildRequest.ConfigurationId);
+
+                // On a self-referenced build, cache misses are allowed.
+                if (requestResults == null)
+                {
+                    return false;
+                }
+
+                // A cache miss on at least one existing target without results is disallowed,
+                // as it violates isolation constraints.
+                foreach (string target in request.Targets)
+                {
+                    if (_configCache[buildRequest.ConfigurationId]
+                        .ProjectTargets
+                        .Contains(target) &&
+                        !requestResults.HasResultsForTarget(target))
+                    {
+                        return false;
+                    }
+                }
+
+                // A cache miss on nonexistent targets on the reference is allowed, given the request
+                // to skip nonexistent targets.
+                return true;
+            }
         }
 
         /// <summary>