dotnet · JanKrivanek · Jun 20, 2023 · May 19, 2023 · May 22, 2023 · May 22, 2023
diff --git a/MSBuild.Dev.slnf b/MSBuild.Dev.slnf
@@ -13,7 +13,8 @@
       "src\\Tasks\\Microsoft.Build.Tasks.csproj",
       "src\\Utilities.UnitTests\\Microsoft.Build.Utilities.UnitTests.csproj",
       "src\\Utilities\\Microsoft.Build.Utilities.csproj",
-      "src\\Xunit.NetCore.Extensions\\Xunit.NetCore.Extensions.csproj"
+      "src\\Xunit.NetCore.Extensions\\Xunit.NetCore.Extensions.csproj",
+      "src\\StringTools\\StringTools.csproj"
     ]
   }
 }
diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
@@ -22,11 +22,11 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 # Change Waves & Associated Features
 
 ## Current Rotation of Change Waves
-### 17.8
-- [[RAR] Don't do I/O on SDK-provided references](https://github.com/dotnet/msbuild/pull/8688)
 
 ### 17.8
+- [[RAR] Don't do I/O on SDK-provided references](https://github.com/dotnet/msbuild/pull/8688)
 - [Delete destination file before copy](https://github.com/dotnet/msbuild/pull/8685)
+- [New serialization approach for transferring build exceptions between processes](https://github.com/dotnet/msbuild/pull/8779)
 
 ### 17.6
 - [Parse invalid property under target](https://github.com/dotnet/msbuild/pull/8190)

diff --git a/src/Build.OM.UnitTests/Microsoft.Build.Engine.OM.UnitTests.csproj b/src/Build.OM.UnitTests/Microsoft.Build.Engine.OM.UnitTests.csproj
@@ -15,8 +15,9 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <NuGetCommandLinePackageVersion>4.1.0</NuGetCommandLinePackageVersion>
-</PropertyGroup>
+    <!-- Managed manually since PackageDownload is not supported by dependabot https://github.com/dependabot/dependabot-core/issues/2920 -->
+    <NuGetCommandLinePackageVersion>4.9.6</NuGetCommandLinePackageVersion>
+  </PropertyGroup>
 
   <ItemGroup>
     <PackageReference Include="Microsoft.IO.Redist" Condition="'$(FeatureMSIORedist)' == 'true'" />
@@ -28,7 +29,7 @@
 
   <ItemGroup>
     <!-- GeneratePathProperty currently isn't enabled for PackageDownload. -->
-    <Content Condition="'$(MSBuildRuntimeType)' != 'Core' and '$(Configuration)' == 'Release'" Include="$(NuGetPackageRoot)\nuget.commandline\$(NuGetCommandLinePackageVersion)\tools\NuGet.exe" CopyToOutputDirectory="PreserveNewest" Link="nuget\NuGet.exe" />
+    <Content Include="$(NuGetPackageRoot)\nuget.commandline\$(NuGetCommandLinePackageVersion)\tools\NuGet.exe" CopyToOutputDirectory="PreserveNewest" Link="nuget\NuGet.exe" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Build.OM.UnitTests/NugetRestoreTests.cs b/src/Build.OM.UnitTests/NugetRestoreTests.cs
@@ -1,13 +1,11 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-#if !DEBUG
 using Microsoft.Build.UnitTests;
 using Microsoft.Build.UnitTests.Shared;
 using Shouldly;
 using System.IO;
 using Xunit;
-#endif
 using Xunit.Abstractions;
 using Xunit.NetCore.Extensions;
 
@@ -21,9 +19,7 @@ public NugetRestoreTests(ITestOutputHelper output)
             _output = output;
         }
 
-        // This NuGet version cannot locate other assemblies when parsing solutions at restore time. This includes localized strings required in debug mode.
-        // NuGet version 4.1.0 was somewhat arbitrarily chosen. 3.5 breaks with an unrelated error, and 4.8.2 does not fail when a new dependency is introduced. This is a safe middle point.
-#if !DEBUG
+        // Tests proper loading of msbuild assemblies by nuget.exe
         [WindowsFullFrameworkOnlyFact]
         public void TestOldNuget()
         {
@@ -54,6 +50,5 @@ public void TestOldNuget()
             RunnerUtilities.RunProcessAndGetOutput(Path.Combine(msbuildExePath, "nuget", "NuGet.exe"), "restore " + sln.Path + " -MSBuildPath \"" + msbuildExePath + "\"", out bool success, outputHelper: _output);
             success.ShouldBeTrue();
         }
-#endif
     }
 }
diff --git a/src/Build.UnitTests/BackEnd/BinaryTranslator_Tests.cs b/src/Build.UnitTests/BackEnd/BinaryTranslator_Tests.cs
@@ -6,8 +6,13 @@
 using System.Configuration.Assemblies;
 using System.Globalization;
 using System.IO;
+using System.Linq;
 using System.Reflection;
+using System.Runtime.CompilerServices;
 using Microsoft.Build.BackEnd;
+using Microsoft.Build.Exceptions;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Framework.BuildException;
 using Shouldly;
 using Xunit;
 
@@ -20,14 +25,19 @@ namespace Microsoft.Build.UnitTests.BackEnd
     /// </summary>
     public class BinaryTranslator_Tests
     {
+        static BinaryTranslator_Tests()
+        {
+            SerializationContractInitializer.Initialize();
+        }
+
         /// <summary>
         /// Tests the SerializationMode property
         /// </summary>
         [Fact]
         public void TestSerializationMode()
         {
             MemoryStream stream = new MemoryStream();
-            using ITranslator readTranslator = BinaryTranslator.GetReadTranslator(stream, null);
+            using ITranslator readTranslator = BinaryTranslator.GetReadTranslator(stream, InterningBinaryReader.PoolingBuffer);
             Assert.Equal(TranslationDirection.ReadFromStream, readTranslator.Mode);
 
             using ITranslator writeTranslator = BinaryTranslator.GetWriteTranslator(stream);
@@ -183,7 +193,7 @@ public void TestSerializeDotNet()
             ArgumentNullException deserializedValue = null;
             TranslationHelpers.GetReadTranslator().TranslateDotNet(ref deserializedValue);
 
-            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue));
+            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue, out string diffReason), diffReason);
         }
 
         /// <summary>
@@ -198,7 +208,125 @@ public void TestSerializeDotNetNull()
             ArgumentNullException deserializedValue = null;
             TranslationHelpers.GetReadTranslator().TranslateDotNet(ref deserializedValue);
 
-            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue));
+            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue, out string diffReason), diffReason);
+        }
+
+        [Fact]
+        public void TestSerializeException()
+        {
+            Exception value = new ArgumentNullException("The argument was null");
+            TranslationHelpers.GetWriteTranslator().TranslateException(ref value);
+
+            Exception deserializedValue = null;
+            TranslationHelpers.GetReadTranslator().TranslateException(ref deserializedValue);
+
+            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue, out string diffReason), diffReason);
+        }
+
+        [Fact]
+        public void TestSerializeException_NestedWithStack()
+        {
+            Exception value = null;
+            try
+            {
+                // Intentionally throw a nested exception with a stack trace.
+                value = value.InnerException;
+            }
+            catch (Exception e)
+            {
+                value = new ArgumentNullException("The argument was null", e);
+            }
+
+            TranslationHelpers.GetWriteTranslator().TranslateException(ref value);
+
+            Exception deserializedValue = null;
+            TranslationHelpers.GetReadTranslator().TranslateException(ref deserializedValue);
+
+            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue, out string diffReason), diffReason);
+        }
+
+        [Fact]
+        public void TestSerializeBuildException_NestedWithStack()
+        {
+            Exception value = null;
+            try
+            {
+                throw new InvalidProjectFileException("sample message");
+            }
+            catch (Exception e)
+            {
+                try
+                {
+                    throw new ArgumentNullException("The argument was null", e);
+                }
+                catch (Exception exception)
+                {
+                    value = new InternalErrorException("Another message", exception);
+                }
+            }
+
+            Assert.NotNull(value);
+            TranslationHelpers.GetWriteTranslator().TranslateException(ref value);
+
+            Exception deserializedValue = null;
+            TranslationHelpers.GetReadTranslator().TranslateException(ref deserializedValue);
+
+            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue, out string diffReason), diffReason);
+        }
+
+        public static IEnumerable<object[]> GetBuildExceptionsAsTestData()
+            => AppDomain
+                .CurrentDomain
+                .GetAssemblies()
+                // TaskHost is copying code files - so has a copy of types with identical names.
+                .Where(a => !a.FullName!.StartsWith("MSBuildTaskHost", StringComparison.CurrentCultureIgnoreCase))
+                .SelectMany(s => s.GetTypes())
+                .Where(BuildExceptionSerializationHelper.IsSupportedExceptionType)
+                .Select(t => new object[] { t });
+
+        [Theory]
+        [MemberData(nameof(GetBuildExceptionsAsTestData))]
+        public void TestSerializationOfBuildExceptions(Type exceptionType)
+        {
+            Exception e = (Exception)Activator.CreateInstance(exceptionType, BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.CreateInstance | BindingFlags.Instance, null, new object[]{"msg", new GenericBuildTransferredException() }, System.Globalization.CultureInfo.CurrentCulture);
+            Exception remote;
+            try
+            {
+                throw e;
+            }
+            catch (Exception exception)
+            {
+                remote = exception;
+            }
+
+            Assert.NotNull(remote);
+            TranslationHelpers.GetWriteTranslator().TranslateException(ref remote);
+
+            Exception deserializedValue = null;
+            TranslationHelpers.GetReadTranslator().TranslateException(ref deserializedValue);
+
+            Assert.True(TranslationHelpers.CompareExceptions(remote, deserializedValue, out string diffReason, true), $"Exception type {exceptionType.FullName} not properly de/serialized: {diffReason}");
+        }
+
+        [Fact]
+        public void TestInvalidProjectFileException_NestedWithStack()
+        {
+            Exception value = null;
+            try
+            {
+                throw new InvalidProjectFileException("sample message", new InternalErrorException("Another message"));
+            }
+            catch (Exception e)
+            {
+                value = e;
+            }
+
+            TranslationHelpers.GetWriteTranslator().TranslateException(ref value);
+
+            Exception deserializedValue = null;
+            TranslationHelpers.GetReadTranslator().TranslateException(ref deserializedValue);
+
+            Assert.True(TranslationHelpers.CompareExceptions(value, deserializedValue, out string diffReason, true), diffReason);
         }
 
         /// <summary>

diff --git a/src/Build.UnitTests/BackEnd/BuildResult_Tests.cs b/src/Build.UnitTests/BackEnd/BuildResult_Tests.cs
@@ -326,16 +326,16 @@ public void TestTranslation()
 
             Assert.Equal(result.ConfigurationId, deserializedResult.ConfigurationId);
             Assert.True(TranslationHelpers.CompareCollections(result.DefaultTargets, deserializedResult.DefaultTargets, StringComparer.Ordinal));
-            Assert.True(TranslationHelpers.CompareExceptions(result.Exception, deserializedResult.Exception));
+            Assert.True(TranslationHelpers.CompareExceptions(result.Exception, deserializedResult.Exception, out string diffReason), diffReason);
             Assert.Equal(result.Exception.Message, deserializedResult.Exception.Message);
             Assert.Equal(result.GlobalRequestId, deserializedResult.GlobalRequestId);
             Assert.True(TranslationHelpers.CompareCollections(result.InitialTargets, deserializedResult.InitialTargets, StringComparer.Ordinal));
             Assert.Equal(result.NodeRequestId, deserializedResult.NodeRequestId);
             Assert.Equal(result["alpha"].ResultCode, deserializedResult["alpha"].ResultCode);
-            Assert.True(TranslationHelpers.CompareExceptions(result["alpha"].Exception, deserializedResult["alpha"].Exception));
+            Assert.True(TranslationHelpers.CompareExceptions(result["alpha"].Exception, deserializedResult["alpha"].Exception, out diffReason), diffReason);
             Assert.True(TranslationHelpers.CompareCollections(result["alpha"].Items, deserializedResult["alpha"].Items, TaskItemComparer.Instance));
             Assert.Equal(result["omega"].ResultCode, deserializedResult["omega"].ResultCode);
-            Assert.True(TranslationHelpers.CompareExceptions(result["omega"].Exception, deserializedResult["omega"].Exception));
+            Assert.True(TranslationHelpers.CompareExceptions(result["omega"].Exception, deserializedResult["omega"].Exception, out diffReason), diffReason);
             Assert.True(TranslationHelpers.CompareCollections(result["omega"].Items, deserializedResult["omega"].Items, TaskItemComparer.Instance));
         }
 

diff --git a/src/Build.UnitTests/BackEnd/TargetResult_Tests.cs b/src/Build.UnitTests/BackEnd/TargetResult_Tests.cs
@@ -102,7 +102,7 @@ public void TestTranslationNoException()
 
             Assert.Equal(result.ResultCode, deserializedResult.ResultCode);
             Assert.True(TranslationHelpers.CompareCollections(result.Items, deserializedResult.Items, TaskItemComparer.Instance));
-            Assert.True(TranslationHelpers.CompareExceptions(result.Exception, deserializedResult.Exception));
+            Assert.True(TranslationHelpers.CompareExceptions(result.Exception, deserializedResult.Exception, out string diffReason), diffReason);
             Assert.Equal(result.OriginalBuildEventContext, deserializedResult.OriginalBuildEventContext);
         }
 
@@ -122,7 +122,7 @@ public void TestTranslationWithException()
 
             Assert.Equal(result.ResultCode, deserializedResult.ResultCode);
             Assert.True(TranslationHelpers.CompareCollections(result.Items, deserializedResult.Items, TaskItemComparer.Instance));
-            Assert.True(TranslationHelpers.CompareExceptions(result.Exception, deserializedResult.Exception));
+            Assert.True(TranslationHelpers.CompareExceptions(result.Exception, deserializedResult.Exception, out string diffReason), diffReason);
         }
 
         /// <summary>

diff --git a/src/Build.UnitTests/BackEnd/TranslationHelpers.cs b/src/Build.UnitTests/BackEnd/TranslationHelpers.cs
@@ -9,6 +9,7 @@
 using System.Text;
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.Framework;
+using Xunit;
 
 #nullable disable
 
@@ -40,7 +41,7 @@ internal static ITranslator GetWriteTranslator()
         internal static ITranslator GetReadTranslator()
         {
             s_serializationStream.Seek(0, SeekOrigin.Begin);
-            return BinaryTranslator.GetReadTranslator(s_serializationStream, null);
+            return BinaryTranslator.GetReadTranslator(s_serializationStream, InterningBinaryReader.PoolingBuffer);
         }
 
         /// <summary>
@@ -85,29 +86,84 @@ internal static bool CompareCollections<T>(ICollection<T> left, ICollection<T> r
         /// <summary>
         /// Compares two exceptions.
         /// </summary>
-        internal static bool CompareExceptions(Exception left, Exception right)
+        internal static bool CompareExceptions(Exception left, Exception right, out string diffReason, bool detailed = false)
         {
+            diffReason = null;
             if (ReferenceEquals(left, right))
             {
                 return true;
             }
 
             if ((left == null) ^ (right == null))
             {
+                diffReason = "One exception is null and the other is not.";
                 return false;
             }
 
             if (left.Message != right.Message)
             {
+                diffReason = $"Exception messages are different ({left.Message} vs {right.Message}).";
                 return false;
             }
 
             if (left.StackTrace != right.StackTrace)
             {
+                diffReason = $"Exception stack traces are different ({left.StackTrace} vs {right.StackTrace}).";
                 return false;
             }
 
-            return CompareExceptions(left.InnerException, right.InnerException);
+            if (!CompareExceptions(left.InnerException, right.InnerException, out diffReason, detailed))
+            {
+                diffReason = "Inner exceptions are different: " + diffReason;
+                return false;
+            }
+
+            if (detailed)
+            {
+                if (left.GetType() != right.GetType())
+                {
+                    diffReason = $"Exception types are different ({left.GetType().FullName} vs {right.GetType().FullName}).";
+                    return false;
+                }
+
+                foreach (var prop in left.GetType().GetProperties())
+                {
+                    if (!IsSimpleType(prop.PropertyType))
+                    {
+                        continue;
+                    }
+
+                    object leftProp = prop.GetValue(left, null);
+                    object rightProp = prop.GetValue(right, null);
+
+                    if (leftProp == null && rightProp != null)
+                    {
+                        diffReason = $"Property {prop.Name} is null on left but not on right.";
+                        return false;
+                    }
+
+                    if (leftProp != null && !prop.GetValue(left, null).Equals(prop.GetValue(right, null)))
+                    {
+                        diffReason = $"Property {prop.Name} is different ({prop.GetValue(left, null)} vs {prop.GetValue(rightProp, null)}).";
+                        return false;
+                    }
+                }
+            }
+
+            return true;
+        }
+
+        internal static bool IsSimpleType(Type type)
+        {
+            // Nullables
+            if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(Nullable<>))
+            {
+                return IsSimpleType(type.GetGenericArguments()[0]);
+            }
+            return type.IsPrimitive
+                   || type.IsEnum
+                   || type == typeof(string)
+                   || type == typeof(decimal);
         }
 
         internal static string GetPropertiesString(IEnumerable properties)

diff --git a/src/Build.UnitTests/InternalEngineHelpers.cs b/src/Build.UnitTests/InternalEngineHelpers.cs
@@ -175,7 +175,7 @@ internal static void AssertBuildResultsEqual(BuildResult actualBuildResult, Buil
 
             internal static void AssertTargetResultsEqual(TargetResult a, TargetResult b)
             {
-                TranslationHelpers.CompareExceptions(a.Exception, b.Exception).ShouldBeTrue();
+                TranslationHelpers.CompareExceptions(a.Exception, b.Exception, out string diffReason).ShouldBeTrue(diffReason);
                 TranslationHelpers.CompareCollections(a.Items, b.Items, TaskItemComparer.Instance).ShouldBeTrue();
 
                 a.ResultCode.ShouldBe(b.ResultCode);