-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Keep our System.Text.Json version used in HostModel in sync with MSBuild #101199
Conversation
…ild. This follows the same pattern as Roslyn, and ensures that the binding redirects in MSBuild kick in on .NET Framework. This also happens to help unblock dotnet/installer#19491 by working around the downgrade problem.
Tagging subscribers to this area: @vitek-karas, @agocke |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
<SystemReflectionMetadataLoadContextToolsetVersion Condition="'$(DotNetBuildSourceOnly)' == 'true'">$(SystemReflectionMetadataLoadContextVersion)</SystemReflectionMetadataLoadContextToolsetVersion> | ||
<SystemTextJsonToolsetVersion Condition="'$(DotNetBuildSourceOnly)' == 'true'">$(SystemTextJsonVersion)</SystemTextJsonToolsetVersion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we override SystemCollectionsImmutableToolsetVersion
and SystemReflectionMetadataToolsetVersion
too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have source-build-reference-packages for them for the versions we use, so it's not necessary. We could for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that SBRP can only be used when the assembly in question doesn't get loaded which isn't true for build tasks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSBuild loads these ones in this case (that's why we're trying to version-match)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer the consistency
This follows the same pattern as Roslyn, and ensures that the binding redirects in MSBuild kick in on .NET Framework.
This also happens to help unblock dotnet/installer#19491 by working around the downgrade problem.