Allow MSBuild to use BinaryFormatter #33227
Conversation
With EnableUnsafeBinaryFormatterSerialization=false, MSBuild can crash instead of passing along an error, dotnet/msbuild#8786. Temporarily opt back into the old behavior while fixing this in MSBuild.
dotnet-issue-labeler
bot
added
Area-Infrastructure
untriaged
|
Crashes caused by the BinaryFormatter changes have been problematic for ASP.NET, so let's get an SDK out that fixes them since dotnet/msbuild#8779 has been a bit more difficult than we expected. |
| <ReplaceFileContents | ||
| InputFiles="@(ToolRuntimeConfigPath)" | ||
| DestinationFiles="@(ToolRuntimeConfigPath)" | ||
| ReplacementPatterns="$(DisabledBinaryFormatterPattern)" |
There was a problem hiding this comment.
I'm failing to find where this is being populated by the original value that we are replacing - is the key allways expected to be present?
There was a problem hiding this comment.
Yeah the process in this repo for things is pretty convoluted! The gist is that there's a redist.csproj that is basically an empty project targeted at the latest runtime, and its output config files get used as the prototype for the tool executables like MSBuild.
I don't know for sure that the setting is guaranteed to be there and explicitly set to false forever, but it is right now, which seems good enough to me since we hopefully fix the underlying reason with your MSBuild PR soon and then revert this tweak.
| @@ -438,6 +438,9 @@ | |||
| <PropertyGroup> | |||
| <ReplacementPattern>"version": ".*"</ReplacementPattern> | |||
| <ReplacementString>"version": "$(MicrosoftNETCoreAppRuntimePackageVersion)"</ReplacementString> | |||
|
|
|||
| <DisabledBinaryFormatterPattern>"System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": false</DisabledBinaryFormatterPattern> | |||
| <EnableBinaryFormatterString>"System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": true</EnableBinaryFormatterString> | |||
There was a problem hiding this comment.
Do true and false need to be quoted?
There was a problem hiding this comment.
They are not in the current JSON.
There was a problem hiding this comment.
correct - true/false are literals in JSON and don't need quote-wrapping
|
I'm not quite sure if there's something special about the runtimeconfig.json logic here, but in theory all you need to do is put |
|
@GrabYourPitchforks this is the moral equivalent of that. Things that ship as tools in the SDK are more convoluted because they're retargeted to the runtime that the SDK will ship with, and one side effect of that was inheriting the latest/greatest .NET 8 rules. |
|
I should also point out that the SDK's runtimeconfig.json including an explicit false value by default is an error. See also: #32969 Hopefully that will also make life a little less painful for you once that is addressed. |
This reverts commit 5f90e87.
With EnableUnsafeBinaryFormatterSerialization=false, MSBuild can crash
instead of passing along an error, dotnet/msbuild#8786. Temporarily
opt back into the old behavior while fixing this in MSBuild.