From 777ec86a1a980ac85cf07db6101b4f61c34b1a89 Mon Sep 17 00:00:00 2001 From: Atsushi Eno Date: Fri, 22 Nov 2013 16:25:07 +0900 Subject: [PATCH] Fix ContinueOnError evaluation (was almost always true). Add more logging. --- .../Microsoft.Build.Internal/BuildEngine4.cs | 43 +++++++++++-------- .../ProjectTargetInstanceTest.cs | 5 ++- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/mcs/class/Microsoft.Build/Microsoft.Build.Internal/BuildEngine4.cs b/mcs/class/Microsoft.Build/Microsoft.Build.Internal/BuildEngine4.cs index e321f8ceec9..071122a84e2 100644 --- a/mcs/class/Microsoft.Build/Microsoft.Build.Internal/BuildEngine4.cs +++ b/mcs/class/Microsoft.Build/Microsoft.Build.Internal/BuildEngine4.cs @@ -132,7 +132,6 @@ namespace Microsoft.Build.Internal // process DependsOnTargets first. foreach (var dep in project.ExpandString (target.DependsOnTargets).Split (';').Where (s => !string.IsNullOrEmpty (s))) { var result = BuildTarget (dep, args); - if (result != null) args.Result.AddResultsForTarget (dep, result); if (result.ResultCode == TargetResultCode.Failure) { targetResult.Failure (null); @@ -192,31 +191,37 @@ namespace Microsoft.Build.Internal throw new InvalidOperationException (string.Format ("Task {0} has property {1} but it is read-only.", ti.Name, p.Key)); prop.SetValue (task, ConvertTo (p.Value, prop.PropertyType), null); } + event_source.FireTaskStarted (this, new TaskStartedEventArgs ("Task Started", null, project.FullPath, ti.FullPath, ti.Name)); if (!task.Execute ()) { + event_source.FireTaskFinished (this, new TaskFinishedEventArgs ("Task Finished", null, project.FullPath, ti.FullPath, ti.Name, false)); targetResult.Failure (null); - if (!ContinueOnError) - break; - } - foreach (var to in ti.Outputs) { - var toItem = to as ProjectTaskOutputItemInstance; - var toProp = to as ProjectTaskOutputPropertyInstance; - string taskParameter = toItem != null ? toItem.TaskParameter : toProp.TaskParameter; - var pi = task.GetType ().GetProperty (taskParameter); - if (pi == null) - throw new InvalidOperationException (string.Format ("Task {0} does not have property {1} specified as TaskParameter", ti.Name, toItem.TaskParameter)); - if (!pi.CanRead) - throw new InvalidOperationException (string.Format ("Task {0} has property {1} specified as TaskParameter, but it is write-only", ti.Name, toItem.TaskParameter)); - if (toItem != null) - args.Project.AddItem (toItem.ItemType, ConvertFrom (pi.GetValue (task, null))); - else - args.Project.SetProperty (toProp.PropertyName, ConvertFrom (pi.GetValue (task, null))); + if (!ContinueOnError) { + event_source.FireTargetFinished (this, new TargetFinishedEventArgs ("Target Failed", null, targetName, project.FullPath, target.FullPath, false)); + return targetResult; + } + } else { + event_source.FireTaskFinished (this, new TaskFinishedEventArgs ("Task Finished", null, project.FullPath, ti.FullPath, ti.Name, true)); + foreach (var to in ti.Outputs) { + var toItem = to as ProjectTaskOutputItemInstance; + var toProp = to as ProjectTaskOutputPropertyInstance; + string taskParameter = toItem != null ? toItem.TaskParameter : toProp.TaskParameter; + var pi = task.GetType ().GetProperty (taskParameter); + if (pi == null) + throw new InvalidOperationException (string.Format ("Task {0} does not have property {1} specified as TaskParameter", ti.Name, toItem.TaskParameter)); + if (!pi.CanRead) + throw new InvalidOperationException (string.Format ("Task {0} has property {1} specified as TaskParameter, but it is write-only", ti.Name, toItem.TaskParameter)); + if (toItem != null) + args.Project.AddItem (toItem.ItemType, ConvertFrom (pi.GetValue (task, null))); + else + args.Project.SetProperty (toProp.PropertyName, ConvertFrom (pi.GetValue (task, null))); + } } } Func creator = s => new TargetOutputTaskItem () { ItemSpec = s }; var items = args.Project.GetAllItems (target.Outputs, string.Empty, creator, creator, s => true, (t, s) => { }); targetResult.Success (items); - event_source.FireTargetFinished (this, new TargetFinishedEventArgs ("Target Started", null, targetName, project.FullPath, target.FullPath, true)); + event_source.FireTargetFinished (this, new TargetFinishedEventArgs ("Target Finished", null, targetName, project.FullPath, target.FullPath, true)); } return targetResult; } @@ -440,7 +445,7 @@ namespace Microsoft.Build.Internal case "ErrorAndStop": return false; } - return project.EvaluateCondition (current_task.ContinueOnError); + return !string.IsNullOrEmpty (current_task.ContinueOnError) && project.EvaluateCondition (current_task.ContinueOnError); } } diff --git a/mcs/class/Microsoft.Build/Test/Microsoft.Build.Execution/ProjectTargetInstanceTest.cs b/mcs/class/Microsoft.Build/Test/Microsoft.Build.Execution/ProjectTargetInstanceTest.cs index daccc6c55d0..490feda5bd0 100644 --- a/mcs/class/Microsoft.Build/Test/Microsoft.Build.Execution/ProjectTargetInstanceTest.cs +++ b/mcs/class/Microsoft.Build/Test/Microsoft.Build.Execution/ProjectTargetInstanceTest.cs @@ -133,15 +133,16 @@ namespace MonoTests.Microsoft.Build.Execution public void DependsOnTargets () { string project_xml = @" + - "; var xml = XmlReader.Create (new StringReader (project_xml)); var root = ProjectRootElement.Create (xml); var proj = new ProjectInstance (root); - Assert.IsFalse (proj.Build ("Bar", new ILogger [] {new ConsoleLogger (LoggerVerbosity.Diagnostic, Console.Error.WriteLine, null, null)}), "#1"); + Assert.AreEqual (2, proj.Targets.Count, "#1"); + Assert.IsFalse (proj.Build ("Bar", new ILogger [0]), "#2"); } } } -- 2.25.1