Cache SDK resolver data process-wide#9335
Merged
Merged
Conversation
jeffkl
approved these changes
Oct 17, 2023
JanKrivanek
approved these changes
Oct 19, 2023
JanKrivanek
left a comment
Member
There was a problem hiding this comment.
Overall looks good.
I love the extra diligence in ensuring the encapsulation!!
9aedf0f to
d845e21
Compare
MattKotsenas
added a commit
to MattKotsenas/reproducible-builds
that referenced
this pull request
Jun 4, 2026
Upstream investigation across dotnet/sdk, dotnet/msbuild, microsoft/MSBuildLocator identified the actual mechanism for issue dotnet#79 and one real correctness fix: 1. The reason ValidateGlobalJsonSdkVersion's [DllImport("hostfxr")] fails inside VS 2022+'s IDE-hosted MSBuild (but not in dotnet build or command-line MSBuild.exe) is that VS's in-process MSBuild engine is compiled for .NET (8/9/10) and its SdkResolverService.ResolveSdk() contains a #if NET fast path (enabled by ChangeWave 17.10, see PR dotnet/msbuild#9335) that resolves in-box SDKs like Microsoft.NET.Sdk directly from MSBuildSDKsPath via DefaultSdkResolver. The plugin chain never runs, Microsoft.DotNet.MSBuildSdkResolver is never loaded, and the Interop.PreloadWindowsLibrary("hostfxr") call that would otherwise bring hostfxr into the process never fires. Standalone netfx MSBuild.exe lacks the fast path (it's net472), so its plugin chain runs and the preload happens; that's why CLI succeeds and VS IDE fails. Update HostFxrResolver's class remarks and the netfx-branch comment to describe this mechanism accurately rather than the previous "safety net" hand-wave. 2. Switch from LoadLibraryW to LoadLibraryExW with LOAD_WITH_ALTERED_SEARCH_PATH (0x8). hostfxr.dll has its own transitive dependencies (e.g. hostpolicy.dll in the same versioned directory). LOAD_WITH_ALTERED_SEARCH_PATH makes the loader resolve those deps from hostfxr's directory rather than the process default DLL search path - matching what Microsoft.DotNet.NativeWrapper.Interop does for the same reason. 3. Document the StubTaskHarness x64 coverage gaps in the csproj comment (VS 2019 x86 MSBuild, MSBuildTaskHost.exe, arm64-native MSBuild on arm64 Windows). All are acceptable misses for a regression-prevention test focused on the issue dotnet#79 scenario.
MattKotsenas
added a commit
to MattKotsenas/reproducible-builds
that referenced
this pull request
Jun 4, 2026
Upstream investigation across dotnet/sdk, dotnet/msbuild, microsoft/MSBuildLocator identified the actual mechanism for issue dotnet#79 and one real correctness fix: 1. The reason ValidateGlobalJsonSdkVersion's [DllImport("hostfxr")] fails inside VS 2022+'s IDE-hosted MSBuild (but not in dotnet build or command-line MSBuild.exe) is that VS's in-process MSBuild engine is compiled for .NET (8/9/10) and its SdkResolverService.ResolveSdk() contains a #if NET fast path (enabled by ChangeWave 17.10, see PR dotnet/msbuild#9335) that resolves in-box SDKs like Microsoft.NET.Sdk directly from MSBuildSDKsPath via DefaultSdkResolver. The plugin chain never runs, Microsoft.DotNet.MSBuildSdkResolver is never loaded, and the Interop.PreloadWindowsLibrary("hostfxr") call that would otherwise bring hostfxr into the process never fires. Standalone netfx MSBuild.exe lacks the fast path (it's net472), so its plugin chain runs and the preload happens; that's why CLI succeeds and VS IDE fails. Update HostFxrResolver's class remarks and the netfx-branch comment to describe this mechanism accurately rather than the previous "safety net" hand-wave. 2. Switch from LoadLibraryW to LoadLibraryExW with LOAD_WITH_ALTERED_SEARCH_PATH (0x8). hostfxr.dll has its own transitive dependencies (e.g. hostpolicy.dll in the same versioned directory). LOAD_WITH_ALTERED_SEARCH_PATH makes the loader resolve those deps from hostfxr's directory rather than the process default DLL search path - matching what Microsoft.DotNet.NativeWrapper.Interop does for the same reason. 3. Document the StubTaskHarness x64 coverage gaps in the csproj comment (VS 2019 x86 MSBuild, MSBuildTaskHost.exe, arm64-native MSBuild on arm64 Windows). All are acceptable misses for a regression-prevention test focused on the issue dotnet#79 scenario.
MattKotsenas
added a commit
to MattKotsenas/reproducible-builds
that referenced
this pull request
Jun 12, 2026
Upstream investigation across dotnet/sdk, dotnet/msbuild, microsoft/MSBuildLocator identified the actual mechanism for issue dotnet#79 and one real correctness fix: 1. The reason ValidateGlobalJsonSdkVersion's [DllImport("hostfxr")] fails inside VS 2022+'s IDE-hosted MSBuild (but not in dotnet build or command-line MSBuild.exe) is that VS's in-process MSBuild engine is compiled for .NET (8/9/10) and its SdkResolverService.ResolveSdk() contains a #if NET fast path (enabled by ChangeWave 17.10, see PR dotnet/msbuild#9335) that resolves in-box SDKs like Microsoft.NET.Sdk directly from MSBuildSDKsPath via DefaultSdkResolver. The plugin chain never runs, Microsoft.DotNet.MSBuildSdkResolver is never loaded, and the Interop.PreloadWindowsLibrary("hostfxr") call that would otherwise bring hostfxr into the process never fires. Standalone netfx MSBuild.exe lacks the fast path (it's net472), so its plugin chain runs and the preload happens; that's why CLI succeeds and VS IDE fails. Update HostFxrResolver's class remarks and the netfx-branch comment to describe this mechanism accurately rather than the previous "safety net" hand-wave. 2. Switch from LoadLibraryW to LoadLibraryExW with LOAD_WITH_ALTERED_SEARCH_PATH (0x8). hostfxr.dll has its own transitive dependencies (e.g. hostpolicy.dll in the same versioned directory). LOAD_WITH_ALTERED_SEARCH_PATH makes the loader resolve those deps from hostfxr's directory rather than the process default DLL search path - matching what Microsoft.DotNet.NativeWrapper.Interop does for the same reason. 3. Document the StubTaskHarness x64 coverage gaps in the csproj comment (VS 2019 x86 MSBuild, MSBuildTaskHost.exe, arm64-native MSBuild on arm64 Windows). All are acceptable misses for a regression-prevention test focused on the issue dotnet#79 scenario.
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.
Fixes #9302
Context
Unless a shared evaluation context is used, each project evaluation creates a fresh new
SdkResolverLoader, walks the disk to discover resolver manifests, callsAssembly.LoadFromon resolver assemblies and so on. The cost of this is non-trivial (close to 3% of total evaluation time for solution like OrchardCore) while arguably there is no way how any of this can change while an MSBuild process is running.Changes Made
IList<>toIReadOnlyList<>to codify the fact that they are immutable.CachingSdkResolverLoaderas a subclass ofSdkResolverLoaderand plugged it intoSdkResolverService.Testing
Existing unit test and an experimental VS insertion.
Notes
Consider reviewing commit by commit.