Fix missed build data on BuildRequestDataFlags provided #9565
Conversation
There was a problem hiding this comment.
This seems like it should work, but it's also a big more aggressive than I think is necessary. Specifically, BuildRequestDataFlags have a number of options, some of which seem like they'd make it invalid to use the same result for another project but some of which seem like they wouldn't affect the result. Can we be more precise in this comparison than just a straight comparison?
src/Build/BackEnd/Components/BuildRequestEngine/BuildRequestEngine.cs
Outdated
Show resolved
Hide resolved
YuliiaKovalova
force-pushed
the
dev/ykovalova/fix_missed_ProjectStateAfterBuild
branch
from
677494e to
475e215
Compare
There was a problem hiding this comment.
I've left a few comments inline. I think we need to split BuildRequestDataFlags into those that affect the result and those that don't to make the right check in CheckBuildDataFlagsResults.
Also, please take a look at ProvideSubsetOfStateAfterBuild. This is similar to ProvideProjectStateAfterBuild but the returned state goes through a user-specified filter. If the filtered ProjectInstance is stored in the cache, we will likely need to use the filter (as defined in RequestedProjectState) as an additional cache key.
There was a problem hiding this comment.
I've added a few minor comments. Please add an entry to the ChangeWave document.
Fixes #9458 ### Context #9565 had to be reverted because the assumption that results with `BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild` don't have to be cached broke design-time builds in Visual Studio. ### Changes Made Re-did the change, now with full handling of `BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild`. ### Testing New and existing unit tests, experimental insertion, manual testing with the problematic design-time build scenario. ### Notes Compared to #9565, we now: - Copy the flags from the original `BuildResult` in the `BuildResult` copy constructor. - Have `ProjectInstance` remember the project state filter it was created from. - Implement `IsSubsetOf` operator on `RequestedProjectState`. - Use the `IsSubsetOf` to determine if a request with `ProvideSubsetOfStateAfterBuild` can be satisfied by the cache. - Don't consider `SkipNonexistentTargets` to be a flag affecting build results.
4 participants
Fixes #9458
Context
The two build requests appear to be treated as identical by the results cache even when different BuildRequestDataFlags were provided .
Changes Made
Before returning cache results, check the cached BuildRequestDataFlags for the previous BuildResult.
Testing
Manual + UT