Fix missed build data on BuildRequestDataFlags provided

src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs

@@ -13,8 +13,6 @@
 using Shouldly;
 using Xunit;
 
-
-
 #nullable disable
 
 namespace Microsoft.Build.UnitTests.BackEnd
@@ -190,6 +188,127 @@ public void TestRetrieveSubsetTargetsFromResult()
             Assert.Equal(BuildResultCode.Success, response.Results.OverallResult);
         }
 
+        [Fact]
+        public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideProjectStateAfterBuild()
+        {
+            string targetName = "testTarget1";
+            int submissionId = 1;
+            int nodeRequestId = 0;
+            int configurationId = 1;
+
+            BuildRequest requestWithNoBuildDataFlags = new BuildRequest(
+               submissionId,
+               nodeRequestId,
+               configurationId,
+               new string[1] { targetName } /* escapedTargets */,
+               null /* hostServices */,
+               BuildEventContext.Invalid /* parentBuildEventContext */,
+               null /* parentRequest */,
+               BuildRequestDataFlags.None);
+
+            BuildRequest requestWithProjectStateFlag = new BuildRequest(
+               submissionId,
+               nodeRequestId,
+               configurationId,
+               new string[1] { targetName } /* escapedTargets */,
+               null /* hostServices */,
+               BuildEventContext.Invalid /* parentBuildEventContext */,
+               null /* parentRequest */,
+               BuildRequestDataFlags.ProvideProjectStateAfterBuild);
+
+            BuildRequest requestWithNoBuildDataFlags2 = new BuildRequest(
+               submissionId,
+               nodeRequestId,
+               configurationId,
+               new string[1] { targetName } /* escapedTargets */,
+               null /* hostServices */,
+               BuildEventContext.Invalid /* parentBuildEventContext */,
+               null /* parentRequest */,
+               BuildRequestDataFlags.None);
+
+            BuildResult resultForRequestWithNoBuildDataFlags = new(requestWithNoBuildDataFlags);
+            resultForRequestWithNoBuildDataFlags.AddResultsForTarget(targetName, BuildResultUtilities.GetEmptySucceedingTargetResult());
+            ResultsCache cache = new();
+            cache.AddResult(resultForRequestWithNoBuildDataFlags);
+
+            ResultsCacheResponse cacheResponseForRequestWithNoBuildDataFlags = cache.SatisfyRequest(
+               requestWithNoBuildDataFlags,
+               new List<string>(),
+               new List<string>(new string[] { targetName }),
+               skippedResultsDoNotCauseCacheMiss: false);
+
+            ResultsCacheResponse cachedResponseForProjectState = cache.SatisfyRequest(
+               requestWithProjectStateFlag,
+               new List<string>(),
+               new List<string>(new string[] { targetName }),
+               skippedResultsDoNotCauseCacheMiss: false);
+
+            ResultsCacheResponse cacheResponseForNoBuildDataFlags2 = cache.SatisfyRequest(
+               requestWithNoBuildDataFlags2,
+               new List<string>(),
+               new List<string>(new string[] { targetName }),
+               skippedResultsDoNotCauseCacheMiss: false);
+
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cacheResponseForRequestWithNoBuildDataFlags.Type);
+
+            // Because ProvideProjectStateAfterBuildFlag was provided as a part of BuildRequest
+            Assert.Equal(ResultsCacheResponseType.NotSatisfied, cachedResponseForProjectState.Type);
+
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cacheResponseForNoBuildDataFlags2.Type);
+        }
+
+        [Fact]
+        public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBuild()
+        {
+            string targetName = "testTarget1";
+            int submissionId = 1;
+            int nodeRequestId = 0;
+            int configurationId = 1;
+
+            BuildRequest requestWithSubsetFlag1 = new BuildRequest(
+                submissionId,
+                nodeRequestId,
+                configurationId,
+                new string[1] { targetName } /* escapedTargets */,
+                null /* hostServices */,
+                BuildEventContext.Invalid /* parentBuildEventContext */,
+                null /* parentRequest */,
+                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild);
+
+            BuildRequest requestWithSubsetFlag2 = new BuildRequest(
+                submissionId,
+                nodeRequestId,
+                configurationId,
+                new string[1] { targetName } /* escapedTargets */,
+                null /* hostServices */,
+                BuildEventContext.Invalid /* parentBuildEventContext */,
+                null /* parentRequest */,
+                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild);
+
+            BuildResult resultForRequestWithSubsetFlag1 = new(requestWithSubsetFlag1);
+            resultForRequestWithSubsetFlag1.AddResultsForTarget(targetName, BuildResultUtilities.GetEmptySucceedingTargetResult());
+            ResultsCache cache = new();
+            cache.AddResult(resultForRequestWithSubsetFlag1);
+
+            ResultsCacheResponse cachedResponseWithSubsetFlag1 = cache.SatisfyRequest(
+            requestWithSubsetFlag1,
+            new List<string>(),
+            new List<string>(new string[] { targetName }),
+            skippedResultsDoNotCauseCacheMiss: false);
+
+            ResultsCacheResponse cachedResponseWithSubsetFlag2 = cache.SatisfyRequest(
+                requestWithSubsetFlag2,
+                new List<string>(),
+                new List<string>(new string[] { targetName }),
+                skippedResultsDoNotCauseCacheMiss: false);
+
+            // It was agreed not to return cache results if ProvideSubsetOfStateAfterBuild is passed,
+            // because RequestedProjectState may have different user filters defined.
+            // It is more reliable to ignore the cached value. 
+            Assert.Equal(ResultsCacheResponseType.NotSatisfied, cachedResponseWithSubsetFlag1.Type);
+            Assert.Equal(ResultsCacheResponseType.NotSatisfied, cachedResponseWithSubsetFlag2.Type);    
+        }
+
         [Fact]
         public void TestClearResultsCache()
         {

src/Build/BackEnd/Components/Caching/ResultsCache.cs

@@ -6,6 +6,7 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using Microsoft.Build.Execution;
+using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
 
 #nullable disable
@@ -17,6 +18,11 @@ namespace Microsoft.Build.BackEnd
     /// </summary>
     internal class ResultsCache : IResultsCache
     {
+        /// <summary>
+        /// The presence of any of these flags affects build result for the specified request.
+        /// </summary>
+        private readonly BuildRequestDataFlags _flagsAffectingBuildResults = BuildRequestDataFlags.ProvideProjectStateAfterBuild;
+
         /// <summary>
         /// The table of all build results.  This table is indexed by configuration id and
         /// contains BuildResult objects which have all of the target information.
@@ -139,13 +145,14 @@ public BuildResult GetResultsForConfiguration(int configurationId)
 
         /// <summary>
         /// Attempts to satisfy the request from the cache.  The request can be satisfied only if:
-        /// 1. All specified targets in the request have successful results in the cache or if the sequence of target results
+        /// 1. The passed BuildRequestDataFlags can not affect the result data.
+        /// 2. All specified targets in the request have successful results in the cache or if the sequence of target results
         ///    includes 0 or more successful targets followed by at least one failed target.
-        /// 2. All initial targets in the configuration for the request have non-skipped results in the cache.
-        /// 3. If there are no specified targets, then all default targets in the request must have non-skipped results
+        /// 3. All initial targets in the configuration for the request have non-skipped results in the cache.
+        /// 4. If there are no specified targets, then all default targets in the request must have non-skipped results
         ///    in the cache.
         /// </summary>
-        /// <param name="request">The request whose results we should return</param>
+        /// <param name="request">The request whose results we should return.</param>
         /// <param name="configInitialTargets">The initial targets for the request's configuration.</param>
         /// <param name="configDefaultTargets">The default targets for the request's configuration.</param>
         /// <param name="skippedResultsDoNotCauseCacheMiss">If false, a cached skipped target will cause this method to return "NotSatisfied".
@@ -156,60 +163,61 @@ public BuildResult GetResultsForConfiguration(int configurationId)
         public ResultsCacheResponse SatisfyRequest(BuildRequest request, List<string> configInitialTargets, List<string> configDefaultTargets, bool skippedResultsDoNotCauseCacheMiss)
         {
             ErrorUtilities.VerifyThrow(request.IsConfigurationResolved, "UnresolvedConfigurationInRequest");
-            ResultsCacheResponse response = new ResultsCacheResponse(ResultsCacheResponseType.NotSatisfied);
+            ResultsCacheResponse response = new(ResultsCacheResponseType.NotSatisfied);
 
             lock (_resultsByConfiguration)
             {
                 if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult allResults))
                 {
-                    // Check for targets explicitly specified.
-                    bool explicitTargetsSatisfied = CheckResults(allResults, request.Targets, response.ExplicitTargetsToBuild, skippedResultsDoNotCauseCacheMiss);
+                    bool buildDataFlagsSatisfied = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)
+                        ? CheckBuildDataFlagsResults(request.BuildRequestDataFlags, allResults.BuildRequestDataFlags) : true;
 
-                    if (explicitTargetsSatisfied)
+                    if (buildDataFlagsSatisfied)
                     {
-                        // All of the explicit targets, if any, have been satisfied
-                        response.Type = ResultsCacheResponseType.Satisfied;
+                        // Check for targets explicitly specified.
+                        bool explicitTargetsSatisfied = CheckResults(allResults, request.Targets, response.ExplicitTargetsToBuild, skippedResultsDoNotCauseCacheMiss);
 
-                        // Check for the initial targets.  If we don't know what the initial targets are, we assume they are not satisfied.
-                        if (configInitialTargets == null || !CheckResults(allResults, configInitialTargets, null, skippedResultsDoNotCauseCacheMiss))
+                        if (explicitTargetsSatisfied)
                         {
-                            response.Type = ResultsCacheResponseType.NotSatisfied;
-                        }
+                            // All of the explicit targets, if any, have been satisfied
+                            response.Type = ResultsCacheResponseType.Satisfied;
 
-                        // We could still be missing implicit targets, so check those...
-                        if (request.Targets.Count == 0)
-                        {
-                            // Check for the default target, if necessary.  If we don't know what the default targets are, we
-                            // assume they are not satisfied.
-                            if (configDefaultTargets == null || !CheckResults(allResults, configDefaultTargets, null, skippedResultsDoNotCauseCacheMiss))
+                            // Check for the initial targets.  If we don't know what the initial targets are, we assume they are not satisfied.
+                            if (configInitialTargets == null || !CheckResults(allResults, configInitialTargets, null, skippedResultsDoNotCauseCacheMiss))
                             {
                                 response.Type = ResultsCacheResponseType.NotSatisfied;
                             }
-                        }
-
-                        // Now report those results requested, if they are satisfied.
-                        if (response.Type == ResultsCacheResponseType.Satisfied)
-                        {
-                            List<string> targetsToAddResultsFor = new List<string>(configInitialTargets);
 
-                            // Now report either the explicit targets or the default targets
-                            if (request.Targets.Count > 0)
+                            // We could still be missing implicit targets, so check those...
+                            if (request.Targets.Count == 0)
                             {
-                                targetsToAddResultsFor.AddRange(request.Targets);
+                                // Check for the default target, if necessary.  If we don't know what the default targets are, we
+                                // assume they are not satisfied.
+                                if (configDefaultTargets == null || !CheckResults(allResults, configDefaultTargets, null, skippedResultsDoNotCauseCacheMiss))
+                                {
+                                    response.Type = ResultsCacheResponseType.NotSatisfied;
+                                }
                             }
-                            else
+
+                            // Now report those results requested, if they are satisfied.
+                            if (response.Type == ResultsCacheResponseType.Satisfied)
                             {
-                                targetsToAddResultsFor.AddRange(configDefaultTargets);
+                                List<string> targetsToAddResultsFor = new List<string>(configInitialTargets);
+
+                                // Now report either the explicit targets or the default targets
+                                if (request.Targets.Count > 0)
+                                {
+                                    targetsToAddResultsFor.AddRange(request.Targets);
+                                }
+                                else
+                                {
+                                    targetsToAddResultsFor.AddRange(configDefaultTargets);
+                                }
+
+                                response.Results = new BuildResult(request, allResults, targetsToAddResultsFor.ToArray(), null);
                             }
-
-                            response.Results = new BuildResult(request, allResults, targetsToAddResultsFor.ToArray(), null);
                         }
                     }
-                    else
-                    {
-                        // Some targets were not satisfied.
-                        response.Type = ResultsCacheResponseType.NotSatisfied;
-                    }
                 }
             }
 
@@ -332,6 +340,20 @@ private static bool CheckResults(BuildResult result, List<string> targets, HashS
             return returnValue;
         }
 
+        /// <summary>
+        /// Checks results for the specified build flags.
+        /// </summary>
+        /// <param name="buildRequestDataFlags">The current request build flags.</param>
+        /// <param name="buildResultDataFlags">The existing build result data flags.</param>
+        /// <returns>False if there is any difference in the data flags that can cause missed build data, true otherwise.</returns>
+        private bool CheckBuildDataFlagsResults(BuildRequestDataFlags buildRequestDataFlags, BuildRequestDataFlags buildResultDataFlags) =>
+
+            // Even if both buildRequestDataFlags and buildResultDataFlags have ProvideSubsetOfStateAfterBuild flag,
+            // the underlying RequestedProjectState may have different user filters defined.
+            // It is more reliable to ignore the cached value. 
+            !buildRequestDataFlags.HasFlag(BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild)
+            & (buildRequestDataFlags & _flagsAffectingBuildResults) == (buildResultDataFlags & _flagsAffectingBuildResults);
+
         public IEnumerator<BuildResult> GetEnumerator()
         {
             return _resultsByConfiguration.Values.GetEnumerator();

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

@@ -47,8 +47,11 @@ public enum CacheResultType
     public class CacheResult
     {
         public CacheResultType ResultType { get; }
+
         public BuildResult? BuildResult { get; }
+
         public ProxyTargets? ProxyTargets { get; }
+
         internal Exception? Exception { get; }
 
         private CacheResult(

src/Build/BackEnd/Shared/BuildRequest.cs

@@ -119,6 +119,11 @@ private BuildRequest(
             _nodeRequestId = nodeRequestId;
             _buildRequestDataFlags = buildRequestDataFlags;
             _requestedProjectState = requestedProjectState;
+
+            if (_requestedProjectState != null)
+            {
+                _buildRequestDataFlags |= BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild;
+            }
         }
 
         /// <summary>

src/Build/BackEnd/Shared/BuildResult.cs

@@ -116,6 +116,8 @@ public class BuildResult : INodePacket, IBuildResults
         /// </summary>
         private ProjectInstance _projectStateAfterBuild;
 
+        private BuildRequestDataFlags _buildRequestDataFlags;
+
         private string _schedulerInducedError;
 
         private HashSet<string> _projectTargets;
@@ -204,6 +206,7 @@ internal BuildResult(BuildRequest request, BuildResult existingResults, string[]
             _nodeRequestId = request.NodeRequestId;
             _circularDependency = false;
             _baseOverallResult = true;
+            _buildRequestDataFlags = request.BuildRequestDataFlags;
 
             if (existingResults == null)
             {
@@ -380,6 +383,16 @@ public ProjectInstance ProjectStateAfterBuild
             set => _projectStateAfterBuild = value;
         }
 
+        /// <summary>
+        /// Gets or sets the flags that provide additional control over the build request.
+        /// See <see cref="Execution.BuildRequestDataFlags"/> for examples of the available flags.
+        /// </summary>
+        public BuildRequestDataFlags BuildRequestDataFlags
+        {
+            get => _buildRequestDataFlags;
+            set => _buildRequestDataFlags = value;
+        }
+
         /// <summary>
         /// Returns the node packet type.
         /// </summary>
@@ -581,6 +594,7 @@ void ITranslatable.Translate(ITranslator translator)
             translator.Translate(ref _savedCurrentDirectory);
             translator.Translate(ref _schedulerInducedError);
             translator.TranslateDictionary(ref _savedEnvironmentVariables, StringComparer.OrdinalIgnoreCase);
+            translator.TranslateEnum(ref _buildRequestDataFlags, (int)_buildRequestDataFlags);
         }
 
         /// <summary>