Only restore based on assets file changes if the actual content changed #80341
Conversation
| using var assetsFileStream = File.OpenRead(filePath); | ||
| var sourceText = SourceText.From(assetsFileStream); |
There was a problem hiding this comment.
Is this the right way to do read the assets file in this context? Is there some other pattern that should be used instead?
There was a problem hiding this comment.
It should be wrapped in an IOUtilities.PerformIO() to deal with the case of this throwing some sort of I/O exception.
There was a problem hiding this comment.
I am going to make an assumption that we don't need to handle a scenario where we are notified the assets file changed, and, the file was actually deleted from disk.
I guess I am also making an assumption that if we got an IOException when reading the file for some other reason, that there isn't a need to start a design time build.
When I use this helper, is there any need to log or anything in the case an IO exception is thrown?
|
@dibarbet @jasonmalinowski for review |
...ageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs
Show resolved
Hide resolved
| using var assetsFileStream = File.OpenRead(filePath); | ||
| var sourceText = SourceText.From(assetsFileStream); |
There was a problem hiding this comment.
It should be wrapped in an IOUtilities.PerformIO() to deal with the case of this throwing some sort of I/O exception.
src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LoadedProject.cs
Outdated
Show resolved
Hide resolved
src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LoadedProject.cs
Outdated
Show resolved
Hide resolved
| NeedsReload?.Invoke(this, EventArgs.Empty); | ||
| } | ||
|
|
||
| private void AssetsFileChangeContext_FileChanged(object? sender, string filePath) |
There was a problem hiding this comment.
There's a theoretical race here where the file path being passed might be to an assets file that isn't actually being watched anymore. Not sure that matters in any useful way; just deal with the IO exceptions in case it's gone.
* upstream/main: (206 commits) Remove bogus xlf tag (#80357) Fix missing type argument checks Add tests Use dotnet run file for generating compiler code (#80248) Only restore based on assets file changes if the actual content changed (#80341) make expressionbody analyzer use semanticspananalysis (#80339) [EnC] Use ignoreAssemblyKey: false to resolve symbol keys (#80342) Properly populate ExportedType metadata table in presence of extension block. (#80311) Propagate `params` to lambdas and local functions (#79880) Change 17.15 to VS 2026 preview. (#80325) Improve virtualproject support for older .NET SDKs (#80324) Update dependencies from https://github.com/dotnet/dotnet build 283666 (#80344) Update dependencies from https://github.com/dotnet/arcade build 20250917.6 (#80343) Simplifying Fix tests Fix tests Fix introduce variable placement in top level statements move to immutable types in signature help move to immutable types in signature help Fix check ...
* upstream/main: (31 commits) Remove bogus xlf tag (dotnet#80357) Fix missing type argument checks Add tests Use dotnet run file for generating compiler code (dotnet#80248) Only restore based on assets file changes if the actual content changed (dotnet#80341) make expressionbody analyzer use semanticspananalysis (dotnet#80339) [EnC] Use ignoreAssemblyKey: false to resolve symbol keys (dotnet#80342) Properly populate ExportedType metadata table in presence of extension block. (dotnet#80311) Propagate `params` to lambdas and local functions (dotnet#79880) Change 17.15 to VS 2026 preview. (dotnet#80325) Improve virtualproject support for older .NET SDKs (dotnet#80324) Update dependencies from https://github.com/dotnet/dotnet build 283666 (dotnet#80344) Update dependencies from https://github.com/dotnet/arcade build 20250917.6 (dotnet#80343) Simplifying Fix tests Fix tests Fix introduce variable placement in top level statements move to immutable types in signature help move to immutable types in signature help Fix check ...
3 participants
Closes #80242
What was happening was, a restore would modify the assets file and cause our watcher to get notified, even though the actual content didn't change. The design time build would notice that assets were still missing and kick off another restore, which would fail again, but generate the file watcher event, in a cycle.
The change is to hash the assets file content when we get a change notification for it, and compare it with the hash from the previous notification, to make sure a meaningful content change has occurred, before kicking off a design time build based on it.
With this change we no longer get a restore loop for a package directive like
#:package Newtonsoft.Json:13.0.4.