Skip to content

Conversation

@ladipro
Copy link
Member

@ladipro ladipro commented Mar 12, 2024

This reverts commit 6257b8e (#9439)

Fixes AB#1974814 / AB#1993507

Context

The change added a binding entry for Newtonsoft.Json to msbuild.exe.config. It turned out that Microsoft.DotNet.MSBuildSdkResolver may not be installed (it is not an always-present component like MSBuild itself). The dangling entry makes Newtonsoft.Json fail to load into the MSBuild process. The assembly may be needed as a dependency of a task or another SDK resolver, for example.

Changes Made

The change is reverted.

Testing

Verified that the failing scenarios work without the problematic entry in MSBuild.exe.config.

@ladipro ladipro requested a review from a team as a code owner March 12, 2024 21:54
Copy link
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Btw. what's the proper way of speeding the MSBuildSdkResolver?

@ladipro
Copy link
Member Author

ladipro commented Mar 13, 2024

Btw. what's the proper way of speeding the MSBuildSdkResolver?

I am still going to pursue this direction. I think the best thing to do is switch MSBuildSdkResolver to use System.Text.Json instead of Newtonsoft.Json. Then re-do this change, without the problematic assembly binding entry.

@rainersigwald rainersigwald merged commit b34f758 into dotnet:vs17.9 Mar 13, 2024
YuliiaKovalova pushed a commit that referenced this pull request Mar 14, 2024
…ntext" (#9857) (#9861)

* Revert "Load Microsoft.DotNet.MSBuildSdkResolver into default load context (MSBuild.exe only) (#9439)"

This reverts commit 6257b8e.

* Bump version

Co-authored-by: Ladi Prosek <[email protected]>
ladipro added a commit that referenced this pull request Jun 24, 2024
ladipro added a commit that referenced this pull request Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants