Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix oversharing of build results in ResultsCache #9987

Merged
merged 14 commits into from
Apr 19, 2024

Conversation

ladipro
Copy link
Member

@ladipro ladipro commented Apr 10, 2024

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.

@ladipro ladipro marked this pull request as ready for review April 11, 2024 12:42
@ladipro
Copy link
Member Author

ladipro commented Apr 18, 2024

I have removed SkipNonexistentTargets from the flags that have to match. This is a cache of target results so if the target is there, it has to exist. Re-tested everything and will merge tomorrow if there are no objections.

@ladipro ladipro merged commit 1e80cef into dotnet:main Apr 19, 2024
9 checks passed
@ladipro ladipro deleted the 9458-build-cache branch April 19, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: ResultsCache ignores some of the BuildRequest data, may return incorrect results
3 participants