diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md index fbbc8868cc1..16823e9cd00 100644 --- a/documentation/wiki/ChangeWaves.md +++ b/documentation/wiki/ChangeWaves.md @@ -32,7 +32,6 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t - [Change Version switch output to finish with a newline](https://github.com/dotnet/msbuild/pull/9485) - [Load Microsoft.DotNet.MSBuildSdkResolver into default load context (MSBuild.exe only)](https://github.com/dotnet/msbuild/pull/9439) - [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) diff --git a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs index 39f61d819a9..d832aa878b3 100644 --- a/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs +++ b/src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs @@ -188,127 +188,6 @@ 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(), - new List(new string[] { targetName }), - skippedResultsDoNotCauseCacheMiss: false); - - ResultsCacheResponse cachedResponseForProjectState = cache.SatisfyRequest( - requestWithProjectStateFlag, - new List(), - new List(new string[] { targetName }), - skippedResultsDoNotCauseCacheMiss: false); - - ResultsCacheResponse cacheResponseForNoBuildDataFlags2 = cache.SatisfyRequest( - requestWithNoBuildDataFlags2, - new List(), - new List(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(), - new List(new string[] { targetName }), - skippedResultsDoNotCauseCacheMiss: false); - - ResultsCacheResponse cachedResponseWithSubsetFlag2 = cache.SatisfyRequest( - requestWithSubsetFlag2, - new List(), - new List(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 ec5c888265a..0823f86cffe 100644 --- a/src/Build/BackEnd/Components/Caching/ResultsCache.cs +++ b/src/Build/BackEnd/Components/Caching/ResultsCache.cs @@ -18,15 +18,6 @@ namespace Microsoft.Build.BackEnd /// internal class ResultsCache : IResultsCache { - /// - /// The presence of any of these flags affects build result for the specified request. - /// - private const BuildRequestDataFlags FlagsAffectingBuildResults = - BuildRequestDataFlags.ProvideProjectStateAfterBuild - | BuildRequestDataFlags.SkipNonexistentTargets - | BuildRequestDataFlags.IgnoreMissingEmptyAndInvalidImports - | BuildRequestDataFlags.FailOnUnresolvedSdk; - /// /// The table of all build results. This table is indexed by configuration id and /// contains BuildResult objects which have all of the target information. @@ -149,11 +140,10 @@ public BuildResult GetResultsForConfiguration(int configurationId) /// /// 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. - /// 2. All specified targets in the request have successful results in the cache or if the sequence of target results + /// 1. 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. - /// 4. If there are no specified targets, then all default targets in the request must have non-skipped results + /// 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 /// in the cache. /// /// The request whose results we should return. @@ -173,53 +163,47 @@ public ResultsCacheResponse SatisfyRequest(BuildRequest request, List co { if (_resultsByConfiguration.TryGetValue(request.ConfigurationId, out BuildResult allResults)) { - bool buildDataFlagsSatisfied = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10) - ? CheckBuildDataFlagsResults(request.BuildRequestDataFlags, allResults.BuildRequestDataFlags) : true; + // Check for targets explicitly specified. + bool explicitTargetsSatisfied = CheckResults(allResults, request.Targets, response.ExplicitTargetsToBuild, skippedResultsDoNotCauseCacheMiss); - if (buildDataFlagsSatisfied) + if (explicitTargetsSatisfied) { - // Check for targets explicitly specified. - bool explicitTargetsSatisfied = CheckResults(allResults, request.Targets, response.ExplicitTargetsToBuild, skippedResultsDoNotCauseCacheMiss); + // All of the explicit targets, if any, have been satisfied + response.Type = ResultsCacheResponseType.Satisfied; - if (explicitTargetsSatisfied) + // 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)) { - // All of the explicit targets, if any, have been satisfied - response.Type = ResultsCacheResponseType.Satisfied; + response.Type = ResultsCacheResponseType.NotSatisfied; + } - // 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)) + // 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)) { response.Type = ResultsCacheResponseType.NotSatisfied; } + } - // We could still be missing implicit targets, so check those... - if (request.Targets.Count == 0) + // Now report those results requested, if they are satisfied. + if (response.Type == ResultsCacheResponseType.Satisfied) + { + List targetsToAddResultsFor = new List(configInitialTargets); + + // Now report either the explicit targets or the default targets + 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)) - { - response.Type = ResultsCacheResponseType.NotSatisfied; - } + targetsToAddResultsFor.AddRange(request.Targets); } - - // Now report those results requested, if they are satisfied. - if (response.Type == ResultsCacheResponseType.Satisfied) + else { - List targetsToAddResultsFor = new List(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); + targetsToAddResultsFor.AddRange(configDefaultTargets); } + + response.Results = new BuildResult(request, allResults, targetsToAddResultsFor.ToArray(), null); } } } @@ -344,20 +328,6 @@ private static bool CheckResults(BuildResult result, List targets, HashS return returnValue; } - /// - /// Checks results for the specified build flags. - /// - /// The current request build flags. - /// The existing build result data flags. - /// False if there is any difference in the data flags that can cause missed build data, true otherwise. - 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 GetEnumerator() { return _resultsByConfiguration.Values.GetEnumerator(); diff --git a/src/Build/BackEnd/Shared/BuildRequest.cs b/src/Build/BackEnd/Shared/BuildRequest.cs index 8951500b8d6..428eea19656 100644 --- a/src/Build/BackEnd/Shared/BuildRequest.cs +++ b/src/Build/BackEnd/Shared/BuildRequest.cs @@ -119,11 +119,6 @@ private BuildRequest( _nodeRequestId = nodeRequestId; _buildRequestDataFlags = buildRequestDataFlags; _requestedProjectState = requestedProjectState; - - if (_requestedProjectState != null) - { - _buildRequestDataFlags |= BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild; - } } /// diff --git a/src/Build/BackEnd/Shared/BuildResult.cs b/src/Build/BackEnd/Shared/BuildResult.cs index cee4212033e..68aa197381f 100644 --- a/src/Build/BackEnd/Shared/BuildResult.cs +++ b/src/Build/BackEnd/Shared/BuildResult.cs @@ -116,11 +116,6 @@ public class BuildResult : INodePacket, IBuildResults /// private ProjectInstance _projectStateAfterBuild; - /// - /// The flags provide additional control over the build results and may affect the cached value. - /// - private BuildRequestDataFlags _buildRequestDataFlags; - private string _schedulerInducedError; private HashSet _projectTargets; @@ -209,7 +204,6 @@ internal BuildResult(BuildRequest request, BuildResult existingResults, string[] _nodeRequestId = request.NodeRequestId; _circularDependency = false; _baseOverallResult = true; - _buildRequestDataFlags = request.BuildRequestDataFlags; if (existingResults == null) { @@ -386,12 +380,6 @@ public ProjectInstance ProjectStateAfterBuild set => _projectStateAfterBuild = value; } - /// - /// Gets the flags that were used in the build request to which these results are associated. - /// See for examples of the available flags. - /// - public BuildRequestDataFlags BuildRequestDataFlags => _buildRequestDataFlags; - /// /// Returns the node packet type. /// @@ -593,7 +581,6 @@ 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); } ///