From 1185ec749e9c3049d9cde64e833a8eecc627e9b0 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Thu, 28 Mar 2024 14:57:28 +0100
Subject: [PATCH 01/14] Revert "revert changes for "ResultsCache ignores some
 of the BuildRequest data,.." (#9718)"

This reverts commit 553649bd6d8765beaead2f6ccee45c041afc3bb3.
---
 documentation/wiki/ChangeWaves.md             |   1 +
 .../BackEnd/ResultsCache_Tests.cs             | 121 ++++++++++++++++++
 .../Components/Caching/ResultsCache.cs        |  92 ++++++++-----
 src/Build/BackEnd/Shared/BuildRequest.cs      |   5 +
 src/Build/BackEnd/Shared/BuildResult.cs       |  13 ++
 5 files changed, 201 insertions(+), 31 deletions(-)

diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
index 000d4675069..32eadbdb38f 100644
--- a/documentation/wiki/ChangeWaves.md
+++ b/documentation/wiki/ChangeWaves.md
@@ -35,6 +35,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 - [Add Link metadata to Resources in AssignLinkMetadata target](https://github.com/dotnet/msbuild/pull/9464)
 - [Change Version switch output to finish with a newline](https://github.com/dotnet/msbuild/pull/9485)
 - [Load NuGet.Frameworks into secondary AppDomain (MSBuild.exe only)](https://github.com/dotnet/msbuild/pull/9446)
+- [ResultsCache ignores some of the BuildRequest data, may return incorrect results](https://github.com/dotnet/msbuild/pull/9565)
 - [Update Traits when environment has been changed](https://github.com/dotnet/msbuild/pull/9655)
 - [Exec task does not trim leading whitespaces for ConsoleOutput](https://github.com/dotnet/msbuild/pull/9722)
 - [Introduce [MSBuild]::StableStringHash overloads](https://github.com/dotnet/msbuild/issues/9519)
diff --git a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
index d832aa878b3..39f61d819a9 100644
--- a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
@@ -188,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()
         {
diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
index 0823f86cffe..ec5c888265a 100644
--- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs
+++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
@@ -18,6 +18,15 @@ 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 const BuildRequestDataFlags FlagsAffectingBuildResults =
+            BuildRequestDataFlags.ProvideProjectStateAfterBuild
+            | BuildRequestDataFlags.SkipNonexistentTargets
+            | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports
+            | BuildRequestDataFlags.FailOnUnresolvedSdk;
+
         /// <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.
@@ -140,10 +149,11 @@ 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>
@@ -163,47 +173,53 @@ public ResultsCacheResponse SatisfyRequest(BuildRequest request, List<string> co
             {
                 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);
                         }
                     }
                 }
@@ -328,6 +344,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 static 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();
diff --git a/src/Build/BackEnd/Shared/BuildRequest.cs b/src/Build/BackEnd/Shared/BuildRequest.cs
index 428eea19656..8951500b8d6 100644
--- a/src/Build/BackEnd/Shared/BuildRequest.cs
+++ b/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>
diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs
index 68aa197381f..cee4212033e 100644
--- a/src/Build/BackEnd/Shared/BuildResult.cs
+++ b/src/Build/BackEnd/Shared/BuildResult.cs
@@ -116,6 +116,11 @@ public class BuildResult : INodePacket, IBuildResults
         /// </summary>
         private ProjectInstance _projectStateAfterBuild;
 
+        /// <summary>
+        /// The flags provide additional control over the build results and may affect the cached value.
+        /// </summary>
+        private BuildRequestDataFlags _buildRequestDataFlags;
+
         private string _schedulerInducedError;
 
         private HashSet<string> _projectTargets;
@@ -204,6 +209,7 @@ internal BuildResult(BuildRequest request, BuildResult existingResults, string[]
             _nodeRequestId = request.NodeRequestId;
             _circularDependency = false;
             _baseOverallResult = true;
+            _buildRequestDataFlags = request.BuildRequestDataFlags;
 
             if (existingResults == null)
             {
@@ -380,6 +386,12 @@ public ProjectInstance ProjectStateAfterBuild
             set => _projectStateAfterBuild = value;
         }
 
+        /// <summary>
+        /// Gets the flags that were used in the build request to which these results are associated.
+        /// See <see cref="Execution.BuildRequestDataFlags"/> for examples of the available flags.
+        /// </summary>
+        public BuildRequestDataFlags BuildRequestDataFlags => _buildRequestDataFlags;
+
         /// <summary>
         /// Returns the node packet type.
         /// </summary>
@@ -581,6 +593,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>

From 31ae8135759bffa57617a38f2c9ae252e9771e30 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Thu, 28 Mar 2024 15:01:41 +0100
Subject: [PATCH 02/14] Fix up the revert

---
 documentation/wiki/ChangeWaves.md                    | 2 +-
 src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs    | 4 ++--
 src/Build/BackEnd/Components/Caching/ResultsCache.cs | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
index 32eadbdb38f..d961b8d13c9 100644
--- a/documentation/wiki/ChangeWaves.md
+++ b/documentation/wiki/ChangeWaves.md
@@ -26,6 +26,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 ### 17.12
 - [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908)
 - [Convert.ToString during a property evaluation uses the InvariantCulture for all types](https://github.com/dotnet/msbuild/pull/9874)
+- [ResultsCache ignores some of the BuildRequest data, may return incorrect results](https://github.com/dotnet/msbuild/pull/9987)
 
 ### 17.10
 - [AppDomain configuration is serialized without using BinFmt](https://github.com/dotnet/msbuild/pull/9320) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json`
@@ -35,7 +36,6 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 - [Add Link metadata to Resources in AssignLinkMetadata target](https://github.com/dotnet/msbuild/pull/9464)
 - [Change Version switch output to finish with a newline](https://github.com/dotnet/msbuild/pull/9485)
 - [Load NuGet.Frameworks into secondary AppDomain (MSBuild.exe only)](https://github.com/dotnet/msbuild/pull/9446)
-- [ResultsCache ignores some of the BuildRequest data, may return incorrect results](https://github.com/dotnet/msbuild/pull/9565)
 - [Update Traits when environment has been changed](https://github.com/dotnet/msbuild/pull/9655)
 - [Exec task does not trim leading whitespaces for ConsoleOutput](https://github.com/dotnet/msbuild/pull/9722)
 - [Introduce [MSBuild]::StableStringHash overloads](https://github.com/dotnet/msbuild/issues/9519)
diff --git a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
index 39f61d819a9..da24e32046c 100644
--- a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
@@ -304,9 +304,9 @@ public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBu
 
             // 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. 
+            // It is more reliable to ignore the cached value.
             Assert.Equal(ResultsCacheResponseType.NotSatisfied, cachedResponseWithSubsetFlag1.Type);
-            Assert.Equal(ResultsCacheResponseType.NotSatisfied, cachedResponseWithSubsetFlag2.Type);    
+            Assert.Equal(ResultsCacheResponseType.NotSatisfied, cachedResponseWithSubsetFlag2.Type);
         }
 
         [Fact]
diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
index ec5c888265a..9b9ffa8e99c 100644
--- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs
+++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
@@ -173,7 +173,7 @@ public ResultsCacheResponse SatisfyRequest(BuildRequest request, List<string> co
             {
                 if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult allResults))
                 {
-                    bool buildDataFlagsSatisfied = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)
+                    bool buildDataFlagsSatisfied = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12)
                         ? CheckBuildDataFlagsResults(request.BuildRequestDataFlags, allResults.BuildRequestDataFlags) : true;
 
                     if (buildDataFlagsSatisfied)
@@ -354,7 +354,7 @@ private static bool CheckBuildDataFlagsResults(BuildRequestDataFlags buildReques
 
             // 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. 
+            // It is more reliable to ignore the cached value.
             !buildRequestDataFlags.HasFlag(BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild)
             && (buildRequestDataFlags & FlagsAffectingBuildResults) == (buildResultDataFlags & FlagsAffectingBuildResults);
 

From 6488acc9754c57ea0e553afd3498ca18c5e72e14 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Wed, 3 Apr 2024 10:08:37 +0200
Subject: [PATCH 03/14] Implement request/result compatibility test

---
 .../Components/Caching/ResultsCache.cs        | 57 ++++++++++++++-----
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
index 9b9ffa8e99c..b36b13a4648 100644
--- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs
+++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
@@ -19,11 +19,11 @@ namespace Microsoft.Build.BackEnd
     internal class ResultsCache : IResultsCache
     {
         /// <summary>
-        /// The presence of any of these flags affects build result for the specified request.
+        /// The presence of any of these flags affects build result for the specified request. Not included are ProvideProjectStateAfterBuild
+        /// and ProvideSubsetOfStateAfterBuild which require additional checks.
         /// </summary>
         private const BuildRequestDataFlags FlagsAffectingBuildResults =
-            BuildRequestDataFlags.ProvideProjectStateAfterBuild
-            | BuildRequestDataFlags.SkipNonexistentTargets
+            BuildRequestDataFlags.SkipNonexistentTargets
             | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports
             | BuildRequestDataFlags.FailOnUnresolvedSdk;
 
@@ -174,7 +174,7 @@ public ResultsCacheResponse SatisfyRequest(BuildRequest request, List<string> co
                 if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult allResults))
                 {
                     bool buildDataFlagsSatisfied = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_12)
-                        ? CheckBuildDataFlagsResults(request.BuildRequestDataFlags, allResults.BuildRequestDataFlags) : true;
+                        ? AreBuildResultFlagsCompatible(request, allResults) : true;
 
                     if (buildDataFlagsSatisfied)
                     {
@@ -345,18 +345,45 @@ private static bool CheckResults(BuildResult result, List<string> targets, HashS
         }
 
         /// <summary>
-        /// Checks results for the specified build flags.
+        /// Returns true if the giveChecks 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 static 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);
+        /// <param name="buildRequest">The current build request.</param>
+        /// <param name="buildResult">The candidate build result.</param>
+        /// <returns>False if there is any difference in the flags that can cause missed build data, true otherwise.</returns>
+        private static bool AreBuildResultFlagsCompatible(BuildRequest buildRequest, BuildResult buildResult)
+        {
+            BuildRequestDataFlags buildRequestDataFlags = buildRequest.BuildRequestDataFlags;
+            BuildRequestDataFlags buildResultDataFlags = buildResult.BuildRequestDataFlags;
+
+            if ((buildRequestDataFlags & FlagsAffectingBuildResults) != (buildResultDataFlags & FlagsAffectingBuildResults))
+            {
+                // Mismatch in flags that can affect build results -> not compatible.
+                return false;
+            }
+
+            if (buildRequestDataFlags.HasFlag(BuildRequestDataFlags.ProvideProjectStateAfterBuild))
+            {
+                // If full state is requested, we must have full state in the result.
+                return buildResultDataFlags.HasFlag(BuildRequestDataFlags.ProvideProjectStateAfterBuild);
+            }
+
+            if (buildRequestDataFlags.HasFlag(BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild))
+            {
+                // If partial state is requested, we must have full or partial-and-compatible state in the result.
+                if (buildResultDataFlags.HasFlag(BuildRequestDataFlags.ProvideProjectStateAfterBuild))
+                {
+                    return true;
+                }
+                if (!buildResultDataFlags.HasFlag(BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild))
+                {
+                    return false;
+                }
+
+                // Verify that the requested subset is compatible with the result.
+            }
+
+            return true;
+        }
 
         public IEnumerator<BuildResult> GetEnumerator()
         {

From 91a9c7b316f8640eaabddaddd157ffd613e8d533 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Wed, 10 Apr 2024 13:35:12 +0200
Subject: [PATCH 04/14] Disable SA1010

---
 .editorconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.editorconfig b/.editorconfig
index 7b0f1419bb8..58a8ef2c257 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -409,3 +409,6 @@ dotnet_diagnostic.IDE0290.severity = suggestion
 dotnet_diagnostic.IDE0300.severity = suggestion
 dotnet_diagnostic.IDE0301.severity = suggestion
 dotnet_diagnostic.IDE0305.severity = suggestion
+
+# Temporarily disable SA1010 "Opening square brackets should not be preceded by a space" until https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3687 if fixed
+dotnet_diagnostic.SA1010.severity = none

From 0d816cb3295c62b97864e5add3f6df07d86e0f34 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Wed, 10 Apr 2024 13:37:19 +0200
Subject: [PATCH 05/14] Add ProjectInstance.RequestedProjectStateFilter

---
 src/Build/Instance/ProjectInstance.cs | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/Build/Instance/ProjectInstance.cs b/src/Build/Instance/ProjectInstance.cs
index fe63676c1d2..5251df89247 100644
--- a/src/Build/Instance/ProjectInstance.cs
+++ b/src/Build/Instance/ProjectInstance.cs
@@ -185,6 +185,12 @@ public class ProjectInstance : IPropertyProvider<ProjectPropertyInstance>, IItem
         private bool _translateEntireState;
         private int _evaluationId = BuildEventContext.InvalidEvaluationId;
 
+        /// <summary>
+        /// The property and item filter used when creating this instance, or null if this is not a filtered copy
+        /// of another ProjectInstance. <seealso cref="ProjectInstance(ProjectInstance, bool, RequestedProjectState)"/>
+        /// </summary>
+        private RequestedProjectState _requestedProjectStateFilter;
+
         /// <summary>
         /// Creates a ProjectInstance directly.
         /// No intermediate Project object is created.
@@ -679,6 +685,7 @@ private ProjectInstance(ProjectInstance that, bool isImmutable, RequestedProject
             _isImmutable = isImmutable;
             _evaluationId = that.EvaluationId;
             _translateEntireState = that._translateEntireState;
+            _requestedProjectStateFilter = filter?.DeepClone();
 
             if (filter == null)
             {
@@ -1115,6 +1122,12 @@ public bool IsImmutable
             get { return _isImmutable; }
         }
 
+        /// <summary>
+        /// The property and item filter used when creating this instance, or null if this is not a filtered copy
+        /// of another ProjectInstance. <seealso cref="ProjectInstance(ProjectInstance, bool, RequestedProjectState)"/>
+        /// </summary>
+        internal RequestedProjectState RequestedProjectStateFilter => _requestedProjectStateFilter;
+
         /// <summary>
         /// Task classes and locations known to this project.
         /// This is the project-specific task registry, which is consulted before
@@ -2228,6 +2241,7 @@ internal void TranslateMinimalState(ITranslator translator)
         {
             translator.TranslateDictionary(ref _globalProperties, ProjectPropertyInstance.FactoryForDeserialization);
             translator.TranslateDictionary(ref _properties, ProjectPropertyInstance.FactoryForDeserialization);
+            translator.Translate(ref _requestedProjectStateFilter);
             translator.Translate(ref _isImmutable);
             TranslateItems(translator);
         }

From 0352f2fd1f7b6d121a62fccf5da9b5cfc0e9b361 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Wed, 10 Apr 2024 13:38:38 +0200
Subject: [PATCH 06/14] Add DeepClone and IsSubsetOf to RequestedProjectState

---
 .../BuildManager/RequestedProjectState.cs     | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/src/Build/BackEnd/BuildManager/RequestedProjectState.cs b/src/Build/BackEnd/BuildManager/RequestedProjectState.cs
index d195131a51c..fa324777269 100644
--- a/src/Build/BackEnd/BuildManager/RequestedProjectState.cs
+++ b/src/Build/BackEnd/BuildManager/RequestedProjectState.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.Build.BackEnd;
 
 #nullable disable
@@ -35,6 +36,94 @@ public IDictionary<string, List<string>> ItemFilters
             set => _itemFilters = value;
         }
 
+        /// <summary>
+        /// Creates a deep copy of this instance.
+        /// </summary>
+        internal RequestedProjectState DeepClone()
+        {
+            RequestedProjectState result = new RequestedProjectState();
+            if (PropertyFilters is not null)
+            {
+                result.PropertyFilters = new List<string>(PropertyFilters);
+            }
+            if (ItemFilters is not null)
+            {
+                result.ItemFilters = ItemFilters.ToDictionary(
+                    kvp => kvp.Key,
+                    kvp => kvp.Value == null ? null : new List<string>(kvp.Value));
+            }
+            return result;
+        }
+
+        /// <summary>
+        /// Returns true if this instance contains all property and item filters present in another instance.
+        /// </summary>
+        /// <param name="another">The instance to compare against.</param>
+        /// <returns>True if this instance is equivalent or a strict subset of <paramref name="another"/>.</returns>
+        internal bool IsSubsetOf(RequestedProjectState another)
+        {
+            if (PropertyFilters is null)
+            {
+                if (another.PropertyFilters is not null)
+                {
+                    // The instance to compare against has filtered props and we need everything -> not a subset.
+                    return false;
+                }
+            }
+            else if (another.PropertyFilters is not null)
+            {
+                HashSet<string> anotherPropertyFilters = new HashSet<string>(another.PropertyFilters);
+                foreach (string propertyFilter in PropertyFilters)
+                {
+                    if (!anotherPropertyFilters.Contains(propertyFilter))
+                    {
+                        return false;
+                    }
+                }
+            }
+
+            if (ItemFilters is null)
+            {
+                if (another.ItemFilters is not null)
+                {
+                    // The instance to compare against has filtered items and we need everything -> not a subset.
+                    return false;
+                }
+            }
+            else if (another.ItemFilters is not null)
+            {
+                foreach (KeyValuePair<string, List<string>> kvp in ItemFilters)
+                {
+                    if (!another.ItemFilters.TryGetValue(kvp.Key, out List<string> metadata))
+                    {
+                        // The instance to compare against doesn't have this item -> not a subset.
+                        return false;
+                    }
+                    if (kvp.Value is null)
+                    {
+                        if (metadata is not null)
+                        {
+                            // The instance to compare against has filtered metadata for this item and we need everything - not a subset.
+                            return false;
+                        }
+                    }
+                    else if (metadata is not null)
+                    {
+                        HashSet<string> anotherMetadata = new HashSet<string>(metadata);
+                        foreach (string metadatum in kvp.Value)
+                        {
+                            if (!anotherMetadata.Contains(metadatum))
+                            {
+                                return false;
+                            }
+                        }
+                    }
+                }
+            }
+
+            return true;
+        }
+
         void ITranslatable.Translate(ITranslator translator)
         {
             translator.Translate(ref _propertyFilters);

From 9335dbe0fd061ade1949765a1dd877f7557f5f24 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Wed, 10 Apr 2024 13:40:47 +0200
Subject: [PATCH 07/14] Call IsSubsetOf from ResultsCache

---
 src/Build/BackEnd/Components/Caching/ResultsCache.cs | 5 ++++-
 src/Build/BackEnd/Shared/BuildResult.cs              | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
index b36b13a4648..9428728f3b7 100644
--- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs
+++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
@@ -345,7 +345,7 @@ private static bool CheckResults(BuildResult result, List<string> targets, HashS
         }
 
         /// <summary>
-        /// Returns true if the giveChecks results for the specified build flags.
+        /// Returns true if the flags of the given build request are compatible with the given build result.
         /// </summary>
         /// <param name="buildRequest">The current build request.</param>
         /// <param name="buildResult">The candidate build result.</param>
@@ -380,6 +380,9 @@ private static bool AreBuildResultFlagsCompatible(BuildRequest buildRequest, Bui
                 }
 
                 // Verify that the requested subset is compatible with the result.
+                return buildRequest.RequestedProjectState is not null &&
+                    buildResult.ProjectStateAfterBuild?.RequestedProjectStateFilter is not null &&
+                    buildRequest.RequestedProjectState.IsSubsetOf(buildResult.ProjectStateAfterBuild.RequestedProjectStateFilter);
             }
 
             return true;
diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs
index cee4212033e..aa23c02113c 100644
--- a/src/Build/BackEnd/Shared/BuildResult.cs
+++ b/src/Build/BackEnd/Shared/BuildResult.cs
@@ -220,6 +220,10 @@ internal BuildResult(BuildRequest request, BuildResult existingResults, string[]
             {
                 _requestException = exception ?? existingResults._requestException;
                 _resultsByTarget = targetNames == null ? existingResults._resultsByTarget : CreateTargetResultDictionaryWithContents(existingResults, targetNames);
+                if (request.RequestedProjectState != null)
+                {
+                    _projectStateAfterBuild = existingResults._projectStateAfterBuild?.FilteredCopy(request.RequestedProjectState);
+                }
             }
         }
 

From d24ccfc07f95d69614a2f81d4225a22fe53c59d5 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Wed, 10 Apr 2024 13:42:18 +0200
Subject: [PATCH 08/14] Update existing and add new tests

---
 .../BackEnd/RequestedProjectState_Tests.cs    | 208 ++++++++++++++++++
 .../BackEnd/ResultsCache_Tests.cs             |  50 ++++-
 2 files changed, 247 insertions(+), 11 deletions(-)
 create mode 100644 src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs

diff --git a/src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs b/src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs
new file mode 100644
index 00000000000..cc0b1f39faf
--- /dev/null
+++ b/src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs
@@ -0,0 +1,208 @@
+// 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 FluentAssertions;
+using Microsoft.Build.Execution;
+using Shouldly;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Microsoft.Build.UnitTests.BackEnd
+{
+    public class RequestedProjectState_Tests
+    {
+        [Fact]
+        public void DeepCloneEmpty()
+        {
+            RequestedProjectState state = new();
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeNull();
+            clone.ItemFilters.Should().BeNull();
+        }
+
+        [Fact]
+        public void DeepCloneProperties()
+        {
+            List<string> properties = ["prop1", "prop2"];
+            RequestedProjectState state = new()
+            {
+                PropertyFilters = properties,
+            };
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeEquivalentTo(properties);
+            clone.ItemFilters.Should().BeNull();
+
+            // Mutating the original instance is not reflected in the clone.
+            properties.Add("prop3");
+            clone.PropertyFilters.Count.Should().NotBe(properties.Count);
+        }
+
+        [Fact]
+        public void DeepCloneItemsNoMetadata()
+        {
+            Dictionary<string, List<string>> items = new()
+            {
+                { "item1", null! },
+                { "item2", null! },
+            };
+            RequestedProjectState state = new()
+            {
+                ItemFilters = items,
+            };
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeNull();
+            clone.ItemFilters.Should().BeEquivalentTo(items);
+
+            // Mutating the original instance is not reflected in the clone.
+            items.Add("item3", null!);
+            clone.ItemFilters.Count.Should().NotBe(items.Count);
+        }
+
+        [Fact]
+        public void DeepCloneItemsWithMetadata()
+        {
+            Dictionary<string, List<string>> items = new()
+            {
+                { "item1", ["metadatum1", "metadatum2"] },
+                { "item2", ["metadatum3"] },
+            };
+            RequestedProjectState state = new()
+            {
+                ItemFilters = items,
+            };
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeNull();
+            clone.ItemFilters.Should().BeEquivalentTo(items);
+
+            // Mutating the original instance is not reflected in the clone.
+            items.Add("item3", ["metadatum4"]);
+            clone.ItemFilters.Count.Should().NotBe(items.Count);
+        }
+
+        [Fact]
+        public void IsSubsetOfEmpty()
+        {
+            RequestedProjectState state1 = new();
+            RequestedProjectState state2 = new();
+
+            // Empty instances are subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeTrue();
+
+            state1.PropertyFilters = ["prop1"];
+            state1.ItemFilters = new Dictionary<string, List<string>>()
+            {
+                { "item1", null! },
+            };
+
+            // Non-empty instance is a subset of empty instance but not the other way round.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+
+        [Fact]
+        public void IsSubsetOfProperties()
+        {
+            RequestedProjectState state1 = new()
+            {
+                PropertyFilters = ["prop1"],
+            };
+            RequestedProjectState state2 = new()
+            {
+                PropertyFilters = ["prop1", "prop2"],
+            };
+
+            // "prop1" is a subset of "prop1", "prop2".
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.PropertyFilters.Add("prop3");
+
+            // Disjoint sets are not subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeFalse();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.PropertyFilters.Clear();
+
+            // Empty props is a subset of anything.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+
+        [Fact]
+        public void IsSubsetOfItemsNoMetadata()
+        {
+            RequestedProjectState state1 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", null! },
+                },
+            };
+            RequestedProjectState state2 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", null! },
+                    { "item2", null! },
+                },
+            };
+
+            // "item1" is a subset of "item1", "item2".
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.ItemFilters.Add("item3", null!);
+
+            // Disjoint sets are not subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeFalse();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.ItemFilters.Clear();
+
+            // Empty items is a subset of anything.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+
+        [Fact]
+        public void IsSubsetOfItemsWithMetadata()
+        {
+            RequestedProjectState state1 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", ["metadatum1"] },
+                },
+            };
+            RequestedProjectState state2 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", null! },
+                },
+            };
+
+            // "item1" with "metadatum1" is a subset of "item1" with no metadata filter.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state2.ItemFilters["item1"] = ["metadatum2"];
+
+            // Disjoint metadata filters are not subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeFalse();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.ItemFilters["item1"] = [];
+
+            // Empty metadata filter is a subset of any other metadata filter.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+    }
+}
diff --git a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
index da24e32046c..8448538e7c7 100644
--- a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
@@ -3,8 +3,12 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
+using System.Xml;
 using Microsoft.Build.BackEnd;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Evaluation;
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Internal;
@@ -265,6 +269,10 @@ public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBu
             int nodeRequestId = 0;
             int configurationId = 1;
 
+            RequestedProjectState requestedProjectState1 = new()
+            {
+                PropertyFilters = ["property1", "property2"],
+            };
             BuildRequest requestWithSubsetFlag1 = new BuildRequest(
                 submissionId,
                 nodeRequestId,
@@ -273,8 +281,13 @@ public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBu
                 null /* hostServices */,
                 BuildEventContext.Invalid /* parentBuildEventContext */,
                 null /* parentRequest */,
-                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild);
+                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild,
+                requestedProjectState1);
 
+            RequestedProjectState requestedProjectState2 = new()
+            {
+                PropertyFilters = ["property1"],
+            };
             BuildRequest requestWithSubsetFlag2 = new BuildRequest(
                 submissionId,
                 nodeRequestId,
@@ -283,18 +296,31 @@ public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBu
                 null /* hostServices */,
                 BuildEventContext.Invalid /* parentBuildEventContext */,
                 null /* parentRequest */,
-                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild);
+                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild,
+                requestedProjectState2);
 
             BuildResult resultForRequestWithSubsetFlag1 = new(requestWithSubsetFlag1);
             resultForRequestWithSubsetFlag1.AddResultsForTarget(targetName, BuildResultUtilities.GetEmptySucceedingTargetResult());
+
+            using TextReader textReader = new StringReader(@"
+              <Project>
+                <PropertyGroup>
+                  <property1>Value1</property1>
+                  <property2>Value2</property2>
+                </PropertyGroup>
+              </Project>
+            ");
+            using XmlReader xmlReader = XmlReader.Create(textReader);
+            resultForRequestWithSubsetFlag1.ProjectStateAfterBuild = new ProjectInstance(ProjectRootElement.Create(xmlReader)).FilteredCopy(requestedProjectState1);
+
             ResultsCache cache = new();
             cache.AddResult(resultForRequestWithSubsetFlag1);
 
             ResultsCacheResponse cachedResponseWithSubsetFlag1 = cache.SatisfyRequest(
-            requestWithSubsetFlag1,
-            new List<string>(),
-            new List<string>(new string[] { targetName }),
-            skippedResultsDoNotCauseCacheMiss: false);
+                requestWithSubsetFlag1,
+                new List<string>(),
+                new List<string>(new string[] { targetName }),
+                skippedResultsDoNotCauseCacheMiss: false);
 
             ResultsCacheResponse cachedResponseWithSubsetFlag2 = cache.SatisfyRequest(
                 requestWithSubsetFlag2,
@@ -302,11 +328,13 @@ public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBu
                 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);
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cachedResponseWithSubsetFlag1.Type);
+            Assert.Equal("Value1", cachedResponseWithSubsetFlag1.Results.ProjectStateAfterBuild.GetPropertyValue("property1"));
+            Assert.Equal("Value2", cachedResponseWithSubsetFlag1.Results.ProjectStateAfterBuild.GetPropertyValue("property2"));
+
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cachedResponseWithSubsetFlag2.Type);
+            Assert.Equal("Value1", cachedResponseWithSubsetFlag2.Results.ProjectStateAfterBuild.GetPropertyValue("property1"));
+            Assert.Equal("", cachedResponseWithSubsetFlag2.Results.ProjectStateAfterBuild.GetPropertyValue("property2"));
         }
 
         [Fact]

From 3122ed87a11239bc070c91ec5fa9e2c7e700f95d Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Thu, 11 Apr 2024 14:04:24 +0200
Subject: [PATCH 09/14] Copy all state in BuildResult copy ctor

---
 src/Build/BackEnd/Shared/BuildResult.cs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs
index aa23c02113c..208fa2e7b9a 100644
--- a/src/Build/BackEnd/Shared/BuildResult.cs
+++ b/src/Build/BackEnd/Shared/BuildResult.cs
@@ -177,6 +177,8 @@ internal BuildResult(BuildResult existingResults, string[] targetNames)
             _requestException = existingResults._requestException;
             _resultsByTarget = CreateTargetResultDictionaryWithContents(existingResults, targetNames);
             _baseOverallResult = existingResults.OverallResult == BuildResultCode.Success;
+            _buildRequestDataFlags = existingResults._buildRequestDataFlags;
+            _projectStateAfterBuild = existingResults._projectStateAfterBuild;
 
             _circularDependency = existingResults._circularDependency;
         }

From 10d9c3addbd500077859f694802f10f7019dbdca Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Thu, 11 Apr 2024 14:05:21 +0200
Subject: [PATCH 10/14] Make DeepCloneItemsWithMetadata more meaningful

---
 src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs b/src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs
index cc0b1f39faf..034b3d1594a 100644
--- a/src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs
@@ -80,8 +80,8 @@ public void DeepCloneItemsWithMetadata()
             clone.ItemFilters.Should().BeEquivalentTo(items);
 
             // Mutating the original instance is not reflected in the clone.
-            items.Add("item3", ["metadatum4"]);
-            clone.ItemFilters.Count.Should().NotBe(items.Count);
+            items["item2"].Add("metadatum4");
+            clone.ItemFilters["item2"].Count.Should().NotBe(items["item2"].Count);
         }
 
         [Fact]

From a29be1a4d44414baca8749996577e98fc3474c7f Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Thu, 11 Apr 2024 14:05:42 +0200
Subject: [PATCH 11/14] Update change wave entry

---
 documentation/wiki/ChangeWaves.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
index d961b8d13c9..7744d96a090 100644
--- a/documentation/wiki/ChangeWaves.md
+++ b/documentation/wiki/ChangeWaves.md
@@ -26,7 +26,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 ### 17.12
 - [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908)
 - [Convert.ToString during a property evaluation uses the InvariantCulture for all types](https://github.com/dotnet/msbuild/pull/9874)
-- [ResultsCache ignores some of the BuildRequest data, may return incorrect results](https://github.com/dotnet/msbuild/pull/9987)
+- [Fix oversharing of build results in ResultsCache](https://github.com/dotnet/msbuild/pull/9987)
 
 ### 17.10
 - [AppDomain configuration is serialized without using BinFmt](https://github.com/dotnet/msbuild/pull/9320) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json`

From 73b3c3effad2115505038dbd66986d5693c18db0 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Thu, 11 Apr 2024 14:24:02 +0200
Subject: [PATCH 12/14] Tweak comments

---
 .editorconfig                                        | 2 +-
 src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs    | 2 ++
 src/Build/BackEnd/Components/Caching/ResultsCache.cs | 6 +++---
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/.editorconfig b/.editorconfig
index 58a8ef2c257..6c218296356 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -410,5 +410,5 @@ dotnet_diagnostic.IDE0300.severity = suggestion
 dotnet_diagnostic.IDE0301.severity = suggestion
 dotnet_diagnostic.IDE0305.severity = suggestion
 
-# Temporarily disable SA1010 "Opening square brackets should not be preceded by a space" until https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3687 if fixed
+# Temporarily disable SA1010 "Opening square brackets should not be preceded by a space" until https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3687 is fixed
 dotnet_diagnostic.SA1010.severity = none
diff --git a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
index 8448538e7c7..ac421399121 100644
--- a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
+++ b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs
@@ -328,10 +328,12 @@ public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBu
                 new List<string>(new string[] { targetName }),
                 skippedResultsDoNotCauseCacheMiss: false);
 
+            // We used the same filter that was used for the ProjectInstance in the cache -> cache hit.
             Assert.Equal(ResultsCacheResponseType.Satisfied, cachedResponseWithSubsetFlag1.Type);
             Assert.Equal("Value1", cachedResponseWithSubsetFlag1.Results.ProjectStateAfterBuild.GetPropertyValue("property1"));
             Assert.Equal("Value2", cachedResponseWithSubsetFlag1.Results.ProjectStateAfterBuild.GetPropertyValue("property2"));
 
+            // We used a filter that's a subset of the one used for the ProjectInstance in the cache -> cache hit.
             Assert.Equal(ResultsCacheResponseType.Satisfied, cachedResponseWithSubsetFlag2.Type);
             Assert.Equal("Value1", cachedResponseWithSubsetFlag2.Results.ProjectStateAfterBuild.GetPropertyValue("property1"));
             Assert.Equal("", cachedResponseWithSubsetFlag2.Results.ProjectStateAfterBuild.GetPropertyValue("property2"));
diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
index 9428728f3b7..78ad8aea99b 100644
--- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs
+++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
@@ -149,7 +149,7 @@ public BuildResult GetResultsForConfiguration(int configurationId)
 
         /// <summary>
         /// Attempts to satisfy the request from the cache.  The request can be satisfied only if:
-        /// 1. The passed BuildRequestDataFlags can not affect the result data.
+        /// 1. The passed BuildRequestDataFlags and RequestedProjectStateFilter are compatible with 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.
         /// 3. All initial targets in the configuration for the request have non-skipped results in the cache.
@@ -345,11 +345,11 @@ private static bool CheckResults(BuildResult result, List<string> targets, HashS
         }
 
         /// <summary>
-        /// Returns true if the flags of the given build request are compatible with the given build result.
+        /// Returns true if the flags and project state filter of the given build request are compatible with the given build result.
         /// </summary>
         /// <param name="buildRequest">The current build request.</param>
         /// <param name="buildResult">The candidate build result.</param>
-        /// <returns>False if there is any difference in the flags that can cause missed build data, true otherwise.</returns>
+        /// <returns>True if the flags and project state filter of the build request is compatible with the build result.</returns>
         private static bool AreBuildResultFlagsCompatible(BuildRequest buildRequest, BuildResult buildResult)
         {
             BuildRequestDataFlags buildRequestDataFlags = buildRequest.BuildRequestDataFlags;

From ee2afbff8039291f7f0ae4cec290974d883cd0f5 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Tue, 16 Apr 2024 10:12:16 +0200
Subject: [PATCH 13/14] Use bit operations instead of HasFlag

---
 .../BackEnd/Components/Caching/ResultsCache.cs   | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
index 78ad8aea99b..b6917cae8b1 100644
--- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs
+++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
@@ -361,20 +361,20 @@ private static bool AreBuildResultFlagsCompatible(BuildRequest buildRequest, Bui
                 return false;
             }
 
-            if (buildRequestDataFlags.HasFlag(BuildRequestDataFlags.ProvideProjectStateAfterBuild))
+            if (HasProvideProjectStateAfterBuild(buildRequestDataFlags))
             {
                 // If full state is requested, we must have full state in the result.
-                return buildResultDataFlags.HasFlag(BuildRequestDataFlags.ProvideProjectStateAfterBuild);
+                return HasProvideProjectStateAfterBuild(buildResultDataFlags);
             }
 
-            if (buildRequestDataFlags.HasFlag(BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild))
+            if (HasProvideSubsetOfStateAfterBuild(buildRequestDataFlags))
             {
                 // If partial state is requested, we must have full or partial-and-compatible state in the result.
-                if (buildResultDataFlags.HasFlag(BuildRequestDataFlags.ProvideProjectStateAfterBuild))
+                if (HasProvideProjectStateAfterBuild(buildResultDataFlags))
                 {
                     return true;
                 }
-                if (!buildResultDataFlags.HasFlag(BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild))
+                if (!HasProvideSubsetOfStateAfterBuild(buildResultDataFlags))
                 {
                     return false;
                 }
@@ -386,6 +386,12 @@ private static bool AreBuildResultFlagsCompatible(BuildRequest buildRequest, Bui
             }
 
             return true;
+
+            static bool HasProvideProjectStateAfterBuild(BuildRequestDataFlags flags)
+                => (flags & BuildRequestDataFlags.ProvideProjectStateAfterBuild) == BuildRequestDataFlags.ProvideProjectStateAfterBuild;
+
+            static bool HasProvideSubsetOfStateAfterBuild(BuildRequestDataFlags flags)
+                => (flags & BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild) == BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild;
         }
 
         public IEnumerator<BuildResult> GetEnumerator()

From b8441095835abdb9ea6e7cad58766a3ea904d6e0 Mon Sep 17 00:00:00 2001
From: Ladi Prosek <laprosek@microsoft.com>
Date: Tue, 16 Apr 2024 17:48:47 +0200
Subject: [PATCH 14/14] Remove SkipNonexistentTargets from
 FlagsAffectingBuildResults

---
 src/Build/BackEnd/Components/Caching/ResultsCache.cs | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/Build/BackEnd/Components/Caching/ResultsCache.cs b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
index b6917cae8b1..34480ff2142 100644
--- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs
+++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs
@@ -23,8 +23,7 @@ internal class ResultsCache : IResultsCache
         /// and ProvideSubsetOfStateAfterBuild which require additional checks.
         /// </summary>
         private const BuildRequestDataFlags FlagsAffectingBuildResults =
-            BuildRequestDataFlags.SkipNonexistentTargets
-            | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports
+            BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports
             | BuildRequestDataFlags.FailOnUnresolvedSdk;
 
         /// <summary>