Make sure to use BuildNodeManager in any build run, and corrected error handling...
authorAtsushi Eno <atsushieno@veritas-vos-liberabit.com>
Thu, 28 Nov 2013 16:00:52 +0000 (01:00 +0900)
committerAtsushi Eno <atsushieno@veritas-vos-liberabit.com>
Tue, 3 Dec 2013 07:53:22 +0000 (16:53 +0900)
Also, now EndBuild() won't finish until all callbacks are done.

mcs/class/Microsoft.Build/Microsoft.Build.Execution/BuildManager.cs
mcs/class/Microsoft.Build/Microsoft.Build.Execution/BuildSubmission.cs
mcs/class/Microsoft.Build/Microsoft.Build.Internal/BuildEngine4.cs
mcs/class/Microsoft.Build/Microsoft.Build.Internal/BuildNodeManager.cs
mcs/class/Microsoft.Build/Test/Microsoft.Build.Evaluation/ProjectTest.cs
mcs/class/Microsoft.Build/Test/Microsoft.Build.Execution/BuildManagerTest.cs

index e369eb6740e0f9b3c487d9ad7ff89da0377b0c91..0c179afa48efedf470ffea5b6c884576b8a7d6c8 100644 (file)
@@ -45,7 +45,6 @@ namespace Microsoft.Build.Execution
                
                public BuildManager ()
                {
-                       build_node_manager = new BuildNodeManager (this);
                }
 
                public BuildManager (string hostName)
@@ -83,7 +82,9 @@ namespace Microsoft.Build.Execution
                public BuildResult Build (BuildParameters parameters, BuildRequestData requestData)
                {
                        BeginBuild (parameters);
-                       return BuildRequest (requestData);
+                       var ret = BuildRequest (requestData);
+                       EndBuild ();
+                       return ret;
                }
 
                public BuildResult BuildRequest (BuildRequestData requestData)
@@ -152,13 +153,17 @@ namespace Microsoft.Build.Execution
                        if (OngoingBuildParameters != null)
                                throw new InvalidOperationException ("Cannot reset caches while builds are in progress.");
                        
-                       build_node_manager.ResetCaches ();
+                       BuildNodeManager.ResetCaches ();
                }
                
                BuildNodeManager build_node_manager;
                
                internal BuildNodeManager BuildNodeManager {
-                       get { return build_node_manager; }
+                       get {
+                               if (build_node_manager == null)
+                                               build_node_manager = new BuildNodeManager (this);
+                               return build_node_manager;
+                       }
                }
        }
 }
index ea8e79e255908e431970654b66071bea5f44a0db..91a9823ce810a2f539fddff0e37d0f120efebdc3 100644 (file)
@@ -75,6 +75,13 @@ namespace Microsoft.Build.Execution
                }
 
                public BuildResult Execute ()
+               {
+                       ExecuteAsync (null, null);
+                       WaitHandle.WaitOne ();
+                       return BuildResult;
+               }
+               
+               internal BuildResult InternalExecute ()
                {
                        BuildResult = new BuildResult () { SubmissionId = SubmissionId };
                        try {
@@ -83,24 +90,24 @@ namespace Microsoft.Build.Execution
                                var outputs = new Dictionary<string,string> ();
                                engine.BuildProject (() => is_canceled, BuildResult, request.ProjectInstance, request.TargetNames, BuildManager.OngoingBuildParameters.GlobalProperties, outputs, toolsVersion);
                        } catch (Exception ex) {
-// FIXME: remove this. It's here only for diagnositc purpose
-Console.Error.WriteLine (ex);
                                BuildResult.Exception = ex;
                                BuildResult.OverallResult = BuildResultCode.Failure;
                        }
                        is_completed = true;
-                       wait_handle.Set ();
                        if (callback != null)
                                callback (this);
+                       wait_handle.Set ();
                        return BuildResult;
                }
 
                public void ExecuteAsync (BuildSubmissionCompleteCallback callback, object context)
                {
-                       if (is_started)
-                               throw new InvalidOperationException ("Build has already started");
+                       if (is_completed)
+                               throw new InvalidOperationException ("Build has already completed");
                        if (is_canceled)
                                throw new InvalidOperationException ("Build has already canceled");
+                       if (is_started)
+                               throw new InvalidOperationException ("Build has already started");
                        is_started = true;
                        this.AsyncContext = context;
                        this.callback = callback;
index 4edd2ba936e9c83c43d3f5e564ce5d102ca22897..040d31731abdcf50728e4790ee82121e29736250 100644 (file)
@@ -84,7 +84,7 @@ namespace Microsoft.Build.Internal
                        var toolset = parameters.GetToolset (toolsVersion);
                        if (toolset == null)
                                throw new InvalidOperationException (string.Format ("Toolset version '{0}' was not resolved to valid toolset", toolsVersion));
-                       event_source.FireMessageRaised (this, new BuildMessageEventArgs (string.Format ("Using Toolset version {0}.", toolsVersion), null, null, MessageImportance.Low));
+                       LogMessageEvent (new BuildMessageEventArgs (string.Format ("Using Toolset version {0}.", toolsVersion), null, null, MessageImportance.Low));
                        var buildTaskFactory = new BuildTaskFactory (BuildTaskDatabase.GetDefaultTaskDatabase (toolset), submission.BuildRequest.ProjectInstance.TaskDatabase);
                        BuildProject (new InternalBuildArguments () { CheckCancel = checkCancel, Result = result, Project = project, TargetNames = targetNames, GlobalProperties = globalProperties, TargetOutputs = targetOutputs, ToolsVersion = toolsVersion, BuildTaskFactory = buildTaskFactory });
                }
@@ -118,7 +118,7 @@ namespace Microsoft.Build.Internal
                        try {
                                
                                var initialPropertiesFormatted = "Initial Properties:\n" + string.Join (Environment.NewLine, project.Properties.OrderBy (p => p.Name).Select (p => string.Format ("{0} = {1}", p.Name, p.EvaluatedValue)).ToArray ());
-                               event_source.FireMessageRaised (this, new BuildMessageEventArgs (initialPropertiesFormatted, null, null, MessageImportance.Low));
+                               LogMessageEvent (new BuildMessageEventArgs (initialPropertiesFormatted, null, null, MessageImportance.Low));
                                
                                // null targets -> success. empty targets -> success(!)
                                if (request.TargetNames == null)
@@ -128,12 +128,12 @@ namespace Microsoft.Build.Internal
                                                BuildTargetByName (targetName, args);
                        
                                        // FIXME: check .NET behavior, whether cancellation always results in failure.
-                                       args.Result.OverallResult = args.CheckCancel () ? BuildResultCode.Failure : args.Result.ResultsByTarget.Select (p => p.Value).Any (r => r.ResultCode == TargetResultCode.Failure) ? BuildResultCode.Failure : BuildResultCode.Success;
+                                       args.Result.OverallResult = args.CheckCancel () ? BuildResultCode.Failure : args.Result.ResultsByTarget.Any (p => p.Value.ResultCode == TargetResultCode.Failure) ? BuildResultCode.Failure : BuildResultCode.Success;
                                }
                        } catch (Exception ex) {
-                               event_source.FireErrorRaised (this, new BuildErrorEventArgs (null, null, project.FullPath, 0, 0, 0, 0, "Unhandled exception occured during a build", null, null));
-                               event_source.FireMessageRaised (this, new BuildMessageEventArgs ("Exception details: " + ex, null, null, MessageImportance.Low));
-                               args.Result.OverallResult = BuildResultCode.Failure;
+                               LogErrorEvent (new BuildErrorEventArgs (null, null, project.FullPath, 0, 0, 0, 0, "Unhandled exception occured during a build", null, null));
+                               LogMessageEvent (new BuildMessageEventArgs ("Exception details: " + ex, null, null, MessageImportance.Low));
+                               throw; // BuildSubmission re-catches this.
                        } finally {
                                event_source.FireBuildFinished (this, new BuildFinishedEventArgs ("Build Finished.", null, args.Result.OverallResult == BuildResultCode.Success, DateTime.Now));
                        }
@@ -151,7 +151,7 @@ namespace Microsoft.Build.Internal
                        if (!request.ProjectInstance.Targets.TryGetValue (targetName, out target))
                                throw new InvalidOperationException (string.Format ("target '{0}' was not found in project '{1}'", targetName, project.FullPath));
                        else if (!args.Project.EvaluateCondition (target.Condition)) {
-                               event_source.FireMessageRaised (this, new BuildMessageEventArgs (string.Format ("Target '{0}' was skipped because condition '{1}' wasn't met.", target.Name, target.Condition), null, null, MessageImportance.Low));
+                               LogMessageEvent (new BuildMessageEventArgs (string.Format ("Target '{0}' was skipped because condition '{1}' wasn't met.", target.Name, target.Condition), null, null, MessageImportance.Low));
                                targetResult.Skip ();
                        } else {
                                // process DependsOnTargets first.
@@ -173,16 +173,16 @@ namespace Microsoft.Build.Internal
                                                        var inputs = args.Project.GetAllItems (target.Inputs, string.Empty, creator, creator, s => true, (t, s) => {
                                                        });
                                                        if (!inputs.Any ()) {
-                                                               event_source.FireMessageRaised (this, new BuildMessageEventArgs (string.Format ("Target '{0}' was skipped because there is no input.", target.Name), null, null, MessageImportance.Low));
+                                                               LogMessageEvent (new BuildMessageEventArgs (string.Format ("Target '{0}' was skipped because there is no input.", target.Name), null, null, MessageImportance.Low));
                                                                skip = true;
                                                        } else {
                                                                var outputs = args.Project.GetAllItems (target.Outputs, string.Empty, creator, creator, s => true, (t, s) => {
                                                                });
                                                                var needsUpdates = GetOlderOutputsThanInputs (inputs, outputs).FirstOrDefault ();
                                                                if (needsUpdates != null)
-                                                                       event_source.FireMessageRaised (this, new BuildMessageEventArgs (string.Format ("Target '{0}' needs to be built because new output {1} is needed.", target.Name, needsUpdates.ItemSpec), null, null, MessageImportance.Low));
+                                                                       LogMessageEvent (new BuildMessageEventArgs (string.Format ("Target '{0}' needs to be built because new output {1} is needed.", target.Name, needsUpdates.ItemSpec), null, null, MessageImportance.Low));
                                                                else {
-                                                                       event_source.FireMessageRaised (this, new BuildMessageEventArgs (string.Format ("Target '{0}' was skipped because all the outputs are newer than all the inputs.", target.Name), null, null, MessageImportance.Low));
+                                                                       LogMessageEvent (new BuildMessageEventArgs (string.Format ("Target '{0}' was skipped because all the outputs are newer than all the inputs.", target.Name), null, null, MessageImportance.Low));
                                                                        skip = true;
                                                                }
                                                        }
@@ -258,7 +258,7 @@ namespace Microsoft.Build.Internal
                                foreach (var ti in target.Children.OfType<ProjectTaskInstance> ()) {
                                        current_task = ti;
                                        if (!args.Project.EvaluateCondition (ti.Condition)) {
-                                               event_source.FireMessageRaised (this, new BuildMessageEventArgs (string.Format ("Task '{0}' was skipped because condition '{1}' wasn't met.", ti.Name, ti.Condition), null, null, MessageImportance.Low));
+                                               LogMessageEvent (new BuildMessageEventArgs (string.Format ("Task '{0}' was skipped because condition '{1}' wasn't met.", ti.Name, ti.Condition), null, null, MessageImportance.Low));
                                                continue;
                                        }
                                        if (!RunBuildTask (target, ti, targetResult, args))
@@ -291,7 +291,7 @@ namespace Microsoft.Build.Internal
                        factoryIdentityParameters ["MSBuildArchitecture"] = taskInstance.MSBuildArchitecture;
                        #endif
                        var task = args.BuildTaskFactory.CreateTask (taskInstance.Name, factoryIdentityParameters, this);
-                       event_source.FireMessageRaised (this, new BuildMessageEventArgs (string.Format ("Using task {0} from {1}", taskInstance.Name, task.GetType ().AssemblyQualifiedName), null, null, MessageImportance.Low));
+                       LogMessageEvent (new BuildMessageEventArgs (string.Format ("Using task {0} from {1}", taskInstance.Name, task.GetType ().AssemblyQualifiedName), null, null, MessageImportance.Low));
                        task.HostObject = host;
                        task.BuildEngine = this;
                        
index 2ef40243b33633dcd23e7d51d7a3e4cc6adc56a3..ebfad5ba752ff0ec11cd51eb452d64bb8a186b62 100644 (file)
@@ -43,6 +43,12 @@ namespace Microsoft.Build.Internal
                        BuildManager = buildManager;
                        new Thread (RunLoop).Start ();
                }
+
+               ~BuildNodeManager ()
+               {
+                       run_loop = false;
+                       queue_wait_handle.Set ();
+               }
                
                public BuildManager BuildManager { get; private set; }
                
@@ -63,9 +69,8 @@ namespace Microsoft.Build.Internal
                {
                        while (run_loop) {
                                try {
-                                       if (queued_builds.Count == 0) {
+                                       if (queued_builds.Count == 0)
                                                queue_wait_handle.WaitOne ();
-                                       }
                                        if (!run_loop)
                                                break;
                                        BuildSubmission build;
@@ -73,6 +78,7 @@ namespace Microsoft.Build.Internal
                                                continue;
                                        StartOneBuild (build);
                                } catch (Exception ex) {
+                                       // FIXME: I guess INodeLogger should be used instead.
                                        Console.Error.WriteLine ("Uncaught build node exception occured");
                                        Console.Error.WriteLine (ex);
                                }
@@ -100,9 +106,16 @@ namespace Microsoft.Build.Internal
                void StartOneBuild (BuildSubmission build)
                {
                        var node = TakeNode (build);
-                       // FIXME: Task here causes NotImplementedException in somewhere in Interlocked. It does not make sense.
-                       //ongoing_builds [build] = task_factory.StartNew (node.ExecuteBuild);
-                       new Thread (() => { node.ExecuteBuild (); }).Start ();
+                       // FIXME: Task (non-generic) here causes NotImplementedException in somewhere in Interlocked. It does not make sense.
+                       ongoing_builds [build] = task_factory.StartNew (node.ExecuteBuild);
+                       //new Thread (() => { node.ExecuteBuild (); }).Start ();
+               }
+               
+               void EndOneBuild (BuildNode node)
+               {
+                       var task = ongoing_builds [node.Build];
+                       ongoing_builds [node.Build] = null;
+                       node.Release ();
                }
                
                // FIXME: take max nodes into account here, and get throttling working.
@@ -167,12 +180,29 @@ namespace Microsoft.Build.Internal
                                Build = build;
                        }
                        
+                       public void Release ()
+                       {
+                               Build = null;
+                               IsAvailable = true;
+                       }
+                       
                        public BuildResult ExecuteBuild ()
                        {
-                               // FIXME: depending on NodeAffinity, build it through another MSBuild process.
-                               if (Affinity == NodeAffinity.OutOfProc)
-                                       throw new NotImplementedException ();
-                               return Build.Execute ();
+                               BuildResult result;
+                               try {
+                                       // FIXME: depending on NodeAffinity, build it through another MSBuild process.
+                                       if (Affinity == NodeAffinity.OutOfProc)
+                                               throw new NotImplementedException ();
+                                       result = Build.InternalExecute ();
+                               } catch (Exception ex) {
+                                       // FIXME: I guess INodeLogger should be used instead.
+                                       Console.Error.WriteLine ("Uncaught build node exception occured");
+                                       Console.Error.WriteLine (ex);
+                                       result = null;
+                               } finally {
+                                       Manager.EndOneBuild (this);
+                               }
+                               return result;
                        }
                }
        }
index 69e088393b323c21bfa74fecb97203f509024833..4ea45518163d0302702b1c67f6a36d633d5e0b24 100644 (file)
@@ -149,7 +149,7 @@ namespace MonoTests.Microsoft.Build.Evaluation
             var root = ProjectRootElement.Create (xml);
             var proj = new Project (root);
                        root.FullPath = "ProjectTest.BuildCSharpTargetGetFrameworkPaths.proj";
-                       Assert.IsTrue (proj.Build ("GetFrameworkPaths", new ILogger [] {new ConsoleLogger ()}));
+                       Assert.IsTrue (proj.Build ("GetFrameworkPaths", new ILogger [] {/*new ConsoleLogger ()*/}));
                }
                
                [Test]
@@ -165,7 +165,7 @@ namespace MonoTests.Microsoft.Build.Evaluation
             var root = ProjectRootElement.Create (xml);
                        root.FullPath = "ProjectTest.BuildCSharpTargetBuild.proj";
                        var proj = new Project (root, null, "4.0");
-                       Assert.IsFalse (proj.Build ("Build", new ILogger [] {new ConsoleLogger (LoggerVerbosity.Diagnostic)})); // missing mandatory properties
+                       Assert.IsFalse (proj.Build ("Build", new ILogger [] {/*new ConsoleLogger (LoggerVerbosity.Diagnostic)*/})); // missing mandatory properties
                }
                
                [Test]
index 6812f52a2fe5697af674a04cee84e66da73dd2c2..74c3b4379c95a678d58c64987ac87bd90dfcf8a8 100644 (file)
@@ -138,7 +138,7 @@ namespace MonoTests.Microsoft.Build.Execution
                        var root = ProjectRootElement.Create (xml);
                        var proj = new ProjectInstance (root);
                        var bm = new BuildManager ();
-                       bm.BeginBuild (new BuildParameters () { Loggers = new ILogger [] { new ConsoleLogger () } });
+                       bm.BeginBuild (new BuildParameters ());
                        DateTime waitDone = DateTime.MinValue;
                        DateTime beforeExec = DateTime.Now;
                        var l = new List<BuildSubmission> ();
@@ -153,6 +153,33 @@ namespace MonoTests.Microsoft.Build.Execution
                        Assert.IsTrue (endBuildDone - beforeExec >= TimeSpan.FromSeconds (1), "#2");
                        Assert.IsTrue (endBuildDone > waitDone, "#3");
                }
+               
+               [Test]
+               public void BuildCommonResolveAssemblyReferences ()
+               {
+            string project_xml = @"<Project xmlns='http://schemas.microsoft.com/developer/msbuild/2003'>
+  <Import Project='$(MSBuildToolsPath)\Microsoft.Common.targets' />
+  <ItemGroup>
+    <Reference Include='System.Core' />
+    <Reference Include='System.Xml' />
+  </ItemGroup>
+</Project>";
+            var xml = XmlReader.Create (new StringReader (project_xml));
+            var root = ProjectRootElement.Create (xml);
+                       root.FullPath = "BuildManagerTest.BuildCommonResolveAssemblyReferences.proj";
+            var proj = new ProjectInstance (root);
+                       var manager = new BuildManager ();
+                       var parameters = new BuildParameters () { Loggers = new ILogger [] {new ConsoleLogger (LoggerVerbosity.Diagnostic)} };
+                       var request = new BuildRequestData (proj, new string [] {"ResolveAssemblyReferences"});
+                       var result = manager.Build (parameters, request);
+                       var items = result.ResultsByTarget ["ResolveAssemblyReferences"].Items;
+                       Assert.AreEqual (2, items.Count (), "#0");
+                       Assert.IsTrue (items.Any (i => Path.GetFileName (i.ItemSpec) == "System.Core.dll"), "#1");
+                       Assert.IsTrue (items.Any (i => Path.GetFileName (i.ItemSpec) == "System.Xml.dll"), "#2");
+                       Assert.IsTrue (File.Exists (items.First (i => Path.GetFileName (i.ItemSpec) == "System.Core.dll").ItemSpec), "#3");
+                       Assert.IsTrue (File.Exists (items.First (i => Path.GetFileName (i.ItemSpec) == "System.Xml.dll").ItemSpec), "#4");
+                       Assert.AreEqual (BuildResultCode.Success, result.OverallResult, "#5");
+               }
        }
 }