-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Add ChecksumAlgorithm to project snapshot #62840
Merged
Merged
Conversation
This file contains 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
This was referenced Jul 22, 2022
tmat
force-pushed
the
Checksum
branch
2 times, most recently
from
July 25, 2022 22:13
f173424
to
0b6ad0c
Compare
tmat
force-pushed
the
Checksum
branch
5 times, most recently
from
July 27, 2022 02:04
51b539b
to
32adda2
Compare
@jasonmalinowski @dotnet/roslyn-compiler PTAL |
davidwengier
approved these changes
Jul 27, 2022
src/Compilers/Core/CodeAnalysisTest/MetadataReferences/MetadataReferenceTests.cs
Outdated
Show resolved
Hide resolved
Found some issues and redoing the representation of checksum alg in Document snapshot. |
jaredpar
reviewed
Aug 3, 2022
/azp run |
Azure Pipelines successfully started running 4 pipeline(s). |
ghost
added this to the Next milestone
Oct 10, 2022
333fred
added a commit
to 333fred/roslyn
that referenced
this pull request
Oct 10, 2022
* upstream/main: (252 commits) Use the source-built version of ref packs and don't use app host when building in source-build (dotnet#64055) Enable rich LSIF hover information. (dotnet#64580) Add ChecksumAlgorithm to project snapshot (dotnet#62840) Utility for uploading artifact on test failure (dotnet#64578) Enable diagnostics Revert "Remove unused TS brace completion code" Publish additional packages to vssdk feed (dotnet#64571) spelling Move check Simplify SymbolKey implementation lint Update publish data to test PR validation fix (dotnet#64559) Simplify Update src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/SymbolKey/SymbolKey.cs Lint Proper equality checks Update src/Workspaces/Remote/Core/RemoteCallback.cs Revert "Not wait for solution crawler because it can be very busy" Add and use TargetFramework.Net70 (dotnet#64490) Not wait for solution crawler because it can be very busy ...
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.
The goal is to allow EnC analyzer to check whether content of a
Document
matches the checksum stored in the PDB when applying EnC/Hot Reload changes. Currently we read the content from disk since we can't get the correct checksum fromDocument
and such approach runs into issues when the file is updated on disk before we can read its content. This results in EnC/Hot Reload incorrectly blocking or ignoring changes.Adds
ChecksumAlgorithm
to Project snapshot (onProjectAttributes
). The value is derived from the corresponding msbuild property set in project file.Checksum algorithm of a
Document
is stored onDocumentState
and initialized from theTextLoader
when the loader is set to the state. This is needed because the state does not directly hold on the loader, but instead it holds on a value source for the SourceText or SyntaxTree. It's not part ofDocumentAttributes
as we would need to keep the attributes in sync with the value of the loader.Fixes #51993
Fixes AB#1567043
Implements #64220
Bans use of APIs in Roslyn whose implementations create
SourceText
but do not have a checksum algorithm parameter and always use the default (SHA1). In Roslyn product code we should be explicit about what checksum algorithm is used.Allow usage of APIs listed in
eng\config\BannedSymbols.txt
in tests without warnings. These APIs would ideally not be used at all but usage in tests doesn't matter much. In fact, we still need to test these APIs even though we don't want them used in product code. Avoiding usage of APIs banned in this PR in all tests is not worth the significant effort it would take to clean up all tests.This PR keeps new TextLoader API internal. These will become public in the follow up change: #64192
PdbMatchingSourceTextProvider (96e0f77)
EnC needs to ensure for each modified document that it has a baseline snapshot of that document whose content exactly matches the source used for compilation of the baseline binary. We use checksum stored in the PDB for the comparison.
When debugging session starts, we stash away the current Solution snapshot so that we can compare updated documents against it. However, this snapshot doesn't necessarily have all file content loaded at this time. By the time we need to compare the content the file content on disk might have changed. To mitigate this issue, we pre-loaded all open documents at the start of debugging session and then watched the workspace for new documents being opened. When the document was opened, we notified the EnC service to check if it already has the PDB matching content snapshot for this document. If not, it would try to read the current file content and see if it's matching the PDB.
This approach has an inherent race condition. This PR changes implements a different approach. We record document snapshots of interest (candidates for PDB matching content) in reaction to workspace events. Instead of notifying the EnC service at that point we let it to call back to the workspace listener when it encounters a document whose baseline it doesn't have yet.
VS validation 1: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/426348
VS validation 2: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=6790099&view=results