Fix issue where targets are executed after an error. (dotnet#2133)

Fixes an issue which occurs when two targets are scheduled to be built 
(e.g. ProduceError1 and ProduceError2), the condition of the first target
prevents it from executing, and a dependent target
(BeforeTargets='ProduceError1') fails. This change prevents
ProduceError2 from executing by marking the parent of the failure
(ProduceError1) with the Stop WorkUnitActionCode.

Fixes dotnet#2116

src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs

@@ -1208,6 +1208,42 @@ public void TestCancelWithNoEntriesAfterBuild()
             }
         }
 
+        [Fact]
+        public void SkippedTargetWithFailedDependenciesStopsBuild()
+        {
+            string projectContents = @"
+  <Target Name=""Build"" DependsOnTargets=""ProduceError1;ProduceError2"" />
+  <Target Name=""ProduceError1"" Condition=""false"" />
+  <Target Name=""ProduceError2"">
+    <ErrorTask2 />
+  </Target>
+  <Target Name=""_Error1"" BeforeTargets=""ProduceError1"">
+    <ErrorTask1 />
+  </Target>
+";
+
+            var project = CreateTestProject(projectContents, string.Empty, "Build");
+            TargetBuilder builder = (TargetBuilder)_host.GetComponent(BuildComponentType.TargetBuilder);
+            MockTaskBuilder taskBuilder = (MockTaskBuilder)_host.GetComponent(BuildComponentType.TaskBuilder);
+            // Fail the first task
+            taskBuilder.FailTaskNumber = 1;
+
+            IConfigCache cache = (IConfigCache)_host.GetComponent(BuildComponentType.ConfigCache);
+            BuildRequestEntry entry = new BuildRequestEntry(CreateNewBuildRequest(1, new[] { "Build" }), cache[1]);
+
+            var buildResult = builder.BuildTargets(GetProjectLoggingContext(entry), entry, this, entry.Request.Targets.ToArray(), CreateStandardLookup(project), CancellationToken.None).Result;
+
+            IResultsCache resultsCache = (IResultsCache)_host.GetComponent(BuildComponentType.ResultsCache);
+            Assert.True(resultsCache.GetResultForRequest(entry.Request).HasResultsForTarget("Build"));
+            Assert.True(resultsCache.GetResultForRequest(entry.Request).HasResultsForTarget("ProduceError1"));
+            Assert.False(resultsCache.GetResultForRequest(entry.Request).HasResultsForTarget("ProduceError2"));
+            Assert.True(resultsCache.GetResultForRequest(entry.Request).HasResultsForTarget("_Error1"));
+
+            Assert.Equal(TargetResultCode.Failure, resultsCache.GetResultForRequest(entry.Request)["Build"].ResultCode);
+            Assert.Equal(TargetResultCode.Skipped, resultsCache.GetResultForRequest(entry.Request)["ProduceError1"].ResultCode);
+            Assert.Equal(TargetResultCode.Failure, resultsCache.GetResultForRequest(entry.Request)["_Error1"].ResultCode);
+        }
+
         #region IRequestBuilderCallback Members
 
         /// <summary>

src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs

@@ -609,11 +609,17 @@ private void PopDependencyTargetsOnTargetFailure(TargetEntry topEntry, TargetRes
                     }
                 }
 
-                // Mark our parent for error execution
+                // Mark our parent for error execution when it is not Completed (e.g. Executing)
                 if (topEntry.ParentEntry != null && topEntry.ParentEntry.State != TargetEntryState.Completed)
                 {
                     topEntry.ParentEntry.MarkForError();
                 }
+                // In cases where we need to indicate a failure but the ParentEntry was Skipped (due to condition) it must be
+                // marked stop to prevent other targets from executing.
+                else if (topEntry.ParentEntry?.Result?.ResultCode == TargetResultCode.Skipped)
+                {
+                    topEntry.ParentEntry.MarkForStop();
+                }
             }
         }
 

src/Build/BackEnd/Components/RequestBuilder/TargetEntry.cs

@@ -752,6 +752,20 @@ internal void MarkForError()
             _state = TargetEntryState.ErrorExecution;
         }
 
+        /// <summary>
+        /// This method is used by the Target Builder to indicate that a child of this target has failed and that work should not
+        /// continue in Completed / Skipped mode. We do not want to mark the state to run in ErrorExecution mode so that the
+        /// OnError targets do not run (the target was skipped due to condition so OnError targets should not run).
+        /// </summary>
+        internal void MarkForStop()
+        {
+            ErrorUtilities.VerifyThrow(_state == TargetEntryState.Completed, "State must be Completed. State is {0}.", _state);
+            ErrorUtilities.VerifyThrow(_targetResult.ResultCode == TargetResultCode.Skipped, "ResultCode must be Skipped. ResultCode is {0}.", _state);
+            ErrorUtilities.VerifyThrow(_targetResult.WorkUnitResult.ActionCode == WorkUnitActionCode.Continue, "ActionCode must be Continue. ActionCode is {0}.", _state);
+
+            _targetResult.WorkUnitResult.ActionCode = WorkUnitActionCode.Stop;
+        }
+
         /// <summary>
         /// Leaves all the call target scopes in the order they were entered.
         /// </summary>

src/Build/BackEnd/Shared/WorkUnitResult.cs

@@ -115,6 +115,7 @@ internal WorkUnitResultCode ResultCode
         internal WorkUnitActionCode ActionCode
         {
             get { return _actionCode; }
+            set { _actionCode = value; }
         }
 
         /// <summary>