Fixed /restore and /graph conjunction error on exit code (dotnet#9461)

Fixes dotnet#9443 Context When the /graph and /restore are used within the same command, the build exit code will always be 0. This is because of the variable used to define success of the restore action overrides the success of the graph build. Testing Made sure existing tests passed and added a unit test for this case, an some manual testing.
ladipro · Dec 8, 2023 · c70267d · c70267d
1 parent 8d10036
commit c70267d
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 9 deletions.
diff --git a/src/MSBuild.UnitTests/XMake_Tests.cs b/src/MSBuild.UnitTests/XMake_Tests.cs
@@ -697,6 +697,36 @@ public void ExecuteAppWithGetPropertyItemAndTargetResult(
             results.ShouldNotContain(ResourceUtilities.GetResourceString("BuildFailedWithPropertiesItemsOrTargetResultsRequested"));
         }
 
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void BuildFailsWithCompileErrorAndRestore(bool isGraphBuild)
+        {
+            using TestEnvironment env = TestEnvironment.Create();
+            TransientTestFile project = env.CreateFile("testProject.csproj", @"
+<Project>
+  <ItemGroup>
+    <CSFile Include=""Program.cs""/>
+  </ItemGroup>
+
+  <Target Name=""Build"">
+    <Csc Sources=""@(CSFile)"" />
+  </Target>
+</Project>
+        ");
+            TransientTestFile wrongSyntaxFile = env.CreateFile("Program.cs", @"
+            Console.WriteLine(""Hello, World!"")
+            A Line here for this to not compile right");
+
+            string graph = isGraphBuild ? "--graph" : "";
+            string result = RunnerUtilities.ExecMSBuild($" {project.Path} /restore {graph}", out bool success);
+
+            success.ShouldBeFalse();
+            result.ShouldContain("Program.cs(2,47): error CS1002: ; expected");
+            result.ShouldContain("Program.cs(3,20): error CS1003: Syntax error, ','");
+            result.ShouldContain("Program.cs(3,54): error CS1002: ; expected");
+        }
+
         /// <summary>
         /// Regression test for bug where the MSBuild.exe command-line app
         /// would sometimes set the UI culture to just "en" which is considered a "neutral" UI

diff --git a/src/MSBuild/XMake.cs b/src/MSBuild/XMake.cs
@@ -1544,6 +1544,10 @@ internal static bool BuildProject(
                                 {
                                     return false;
                                 }
+                                else
+                                {
+                                    success = result.OverallResult == BuildResultCode.Success;
+                                }
                             }
 
                             if (!restoreOnly)
@@ -1568,21 +1572,14 @@ internal static bool BuildProject(
                                                                                                                                         entryValue.Equals(propertyKvp.Value)))
                                             .Value;
                                     }
-                                    else
-                                    {
-                                        success = graphResult.OverallResult == BuildResultCode.Success;
-                                    }
+                                    success = graphResult.OverallResult == BuildResultCode.Success;
                                 }
                                 else
                                 {
                                     result = ExecuteBuild(buildManager, buildRequest);
+                                    success = result.OverallResult == BuildResultCode.Success;
                                 }
                             }
-
-                            if (result != null && result.Exception == null)
-                            {
-                                success = result.OverallResult == BuildResultCode.Success;
-                            }
                         }
                         finally
                         {