Setting changewave 17.8 causes crashes on .NET 8

[rainersigwald]

Since dotnet/sdk#34402, triggering the old behavior by opting into the escape hatch introduced in #8779 causes crashes instead of cleanly reported MSBuild errors.

> set ms
MSBUILDDISABLEFEATURESFROMVERSION=17.8
MSBUILDNOINPROCNODE=1

> dotnet msbuild .\foo.proj
MSBuild version 17.8.0-preview-23418-03+0125fc9fb for .NET

S:\msbuild\foo.proj(5,17): error MSB4113: Specified condition "''" evaluates to "" instead of a boolean.
MSBUILD : error MSB4166: Child node "2" exited prematurely. Shutting down. Diagnostic information may be found in files
 in "C:\Users\raines\AppData\Local\Temp\MSBuildTempraines\" and will be named MSBuild_*.failure.txt. This location can
be changed by setting the MSBUILDDEBUGPATH environment variable to a different directory.
MSBUILD : error MSB4166: C:\Users\raines\AppData\Local\Temp\MSBuildTempraines\MSBuild_pid-1588_93c93e5ec6e5461799877462
8589f648.failure.txt:
MSBUILD : error MSB4166: UNHANDLED EXCEPTIONS FROM PROCESS 1588:
MSBUILD : error MSB4166: =====================
MSBUILD : error MSB4166: 9/22/2023 11:36:44 AM
MSBUILD : error MSB4166: System.NotSupportedException: BinaryFormatter serialization and deserialization are disabled w
ithin this application. See https://aka.ms/binaryformatter for more information.
MSBUILD : error MSB4166:    at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializ
ationStream, Object graph)
MSBUILD : error MSB4166:    at Microsoft.Build.Execution.BuildResult.Microsoft.Build.BackEnd.ITranslatable.Translate(IT
ranslator translator)
MSBUILD : error MSB4166:    at Microsoft.Build.BackEnd.NodeEndpointOutOfProcBase.RunReadLoop(Stream localReadPipe, Stre
am localWritePipe, ConcurrentQueue`1 localPacketQueue, AutoResetEvent localPacketAvailable, AutoResetEvent localTermina
tePacketPump)
MSBUILD : error MSB4166: ===================
MSBUILD : error MSB4166:
MSBUILD : error MSB4166:

foo.proj is from #6215 (comment).

[JanKrivanek]

FYI @rokonec - this is the scenario we discussed the other day (customer opting-out from the reworked serialization, but not having the BinFmt opted-in in msbuild runtimeconfig.json).

Discussed briefly with @rainersigwald and we should probably completely disallow opting-out of the new serialization via 17.8 changevawe. Namely:

• Allowing BinFmt serialization of legacy Custom events:

  msbuild/src/Build/BackEnd/Node/OutOfProcNode.cs

  Line 590 in 3847162

  && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8)

  (so

  msbuild/src/Build/BackEnd/Components/Logging/LoggingService.cs

  Line 918 in 3847162

  && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8)

  should go away as well)
• Allowing BinFmt serialization of Exceptions:

  msbuild/src/Framework/BinaryTranslator.cs

  Line 585 in 3847162

  if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8))

  and

  msbuild/src/Framework/BinaryTranslator.cs

  Line 1294 in 3847162

  if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8))

Unless there is a reliable and fast check of BinFmt being opted in (via EnableUnsafeBinaryFormatterSerialization), then we cannot determine whather fallbacking to legacy behavior won't cause unhandled exception - and we should not allow users to switch on unhandled exceptions. Hence the decision not to control this behavior by changewave

[JanKrivanek]

Seems like:

AppContext.TryGetSwitch("System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization", out bool enabled)

is the way to determine whether legacy serialization will crash or not.
So we can keep the changewave, but error out proactively if the BinFmt is not enabled.

Btw. the legacy Custom events should probably be mentioned in the ChangeWave doc

[AR-May]

@JanKrivanek did your PR fix that?