Change to the VS release queue for integration tests#41476
Merged
JoeRobich merged 6 commits intodotnet:masterfrom Feb 7, 2020
Merged
Change to the VS release queue for integration tests#41476JoeRobich merged 6 commits intodotnet:masterfrom
JoeRobich merged 6 commits intodotnet:masterfrom
Conversation
Solution.GetLatestProjectVersion() returns the highest project version of any of the Projects in the Solution. This is lazily computed, but the lazy computation has a fatal flaw. It's implemented with a Lazy<VersionStamp>, and if a Solution is changed and no project was changed (say, you only modified a document's text), the existing Lazy<VersionStamp> is reused. This causes us to root an older SolutionState or ProjectState until somebody were to call GetLatestProjectVersion() and cause the Lazy<VersionStamp> to be evaluated. Except...that would never happen. It turns out that nothing calls GetLatestProjectVersion() in Visual Studio anywhere that I can find. The only call is in one unit test that calls it and then ignores the result. Thus, we'd simply leak the SolutionState/ProjectState until some code ran that knew it could explicitly state the value instead of reuse the existing Lazy<VersionStamp>. Looking through history, it appears that this used to be used more regularly as a part of an older implementation of the ProjectDependencyGraph, but that was removed in abd7654. Since nobody calls this, and the actual computation (just looping through all projects to find the newest stamp) is fairly trivial, I'm just going to delete all the lazy handling and just inline the computation. In the unlikely case that this ever becomes a hot path, we can revisit the laziness at that time.
Update arcade to support SDK publishing
…rojectversion Delete the lazy caching of GetLatestProjectVersion()
…queue' into update-integration-queue
Member
|
This does a lot more than change the queues, it also changes a lot of code under |
Contributor
|
I think this is not a necessary change anymore. The original queue has been reverted to an image we expect to work. |
Member
Author
Those changes are in release/dev16.5 not sure why the versions went from V5 to V1. |
Member
Author
|
See the dev16.5 to master merge #41435 |
Member
Author
|
Merging since this is basically the dev16.5-master master |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contains same commit as #41473 but targeted at master branch