buildman: Support single-threaded operation

At present even if only a single thread is in use, buildman still uses
threading.

For some debugging it is helpful to do everything in the main process.
Allow -T0 to support this.

Signed-off-by: Simon Glass <sjg@chromium.org>

6 files changed, 68 insertions, 30 deletions

diff --git a/tools/buildman/README b/tools/buildman/README
index b7442a95e5..600794790a 100644
--- a/tools/buildman/README
+++ b/tools/buildman/README
@@ -1128,6 +1128,11 @@ If there are both warnings and errors, errors win, so buildman returns 100.
 The -y option is provided (for use with -s) to ignore the bountiful device-tree
 warnings. Similarly, -Y tells buildman to ignore the migration warnings.
 
+Sometimes you might get an error in a thread that is not handled by buildman,
+perhaps due to a failure of a tool that it calls. You might see the output, but
+then buildman hangs. Failing to handle any eventuality is a bug in buildman and
+should be reported. But you can use -T0 to disable threading and hopefully
+figure out the root cause of the build failure.
 
 Build summary
 =============
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 6f6d759329..be8a8fa13a 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -197,6 +197,8 @@ class Builder:
             last _timestamp_count builds. Each is a datetime object.
         _timestamp_count: Number of timestamps to keep in our list.
         _working_dir: Base working directory containing all threads
+        _single_builder: BuilderThread object for the singer builder, if
+            threading is not being used
     """
     class Outcome:
         """Records a build outcome for a single make invocation
@@ -309,19 +311,24 @@ class Builder:
         self._re_migration_warning = re.compile(r'^={21} WARNING ={22}\n.*\n=+\n',
                                                 re.MULTILINE | re.DOTALL)
 
-        self.queue = queue.Queue()
-        self.out_queue = queue.Queue()
-        for i in range(self.num_threads):
-            t = builderthread.BuilderThread(self, i, mrproper,
-                    per_board_out_dir)
+        if self.num_threads:
+            self._single_builder = None
+            self.queue = queue.Queue()
+            self.out_queue = queue.Queue()
+            for i in range(self.num_threads):
+                t = builderthread.BuilderThread(self, i, mrproper,
+                        per_board_out_dir)
+                t.setDaemon(True)
+                t.start()
+                self.threads.append(t)
+
+            t = builderthread.ResultThread(self)
             t.setDaemon(True)
             t.start()
             self.threads.append(t)
-
-        t = builderthread.ResultThread(self)
-        t.setDaemon(True)
-        t.start()
-        self.threads.append(t)
+        else:
+            self._single_builder = builderthread.BuilderThread(
+                self, -1, mrproper, per_board_out_dir)
 
         ignore_lines = ['(make.*Waiting for unfinished)', '(Segmentation fault)']
         self.re_make_err = re.compile('|'.join(ignore_lines))
@@ -1531,11 +1538,12 @@ class Builder:
         """Get the directory path to the working dir for a thread.
 
         Args:
-            thread_num: Number of thread to check.
+            thread_num: Number of thread to check (-1 for main process, which
+                is treated as 0)
         """
         if self.work_in_output:
             return self._working_dir
-        return os.path.join(self._working_dir, '%02d' % thread_num)
+        return os.path.join(self._working_dir, '%02d' % max(thread_num, 0))
 
     def _PrepareThread(self, thread_num, setup_git):
         """Prepare the working directory for a thread.
@@ -1594,7 +1602,9 @@ class Builder:
         if git-worktree is available, or clones the repo if it isn't.
 
         Args:
-            max_threads: Maximum number of threads we expect to need.
+            max_threads: Maximum number of threads we expect to need. If 0 then
+                1 is set up, since the main process still needs somewhere to
+                work
             setup_git: True to set up a git worktree or a git clone
         """
         builderthread.Mkdir(self._working_dir)
@@ -1608,7 +1618,9 @@ class Builder:
                 gitutil.PruneWorktrees(src_dir)
             else:
                 setup_git = 'clone'
-        for thread in range(max_threads):
+
+        # Always do at least one thread
+        for thread in range(max(max_threads, 1)):
             self._PrepareThread(thread, setup_git)
 
     def _GetOutputSpaceRemovals(self):
@@ -1686,16 +1698,20 @@ class Builder:
             job.keep_outputs = keep_outputs
             job.work_in_output = self.work_in_output
             job.step = self._step
-            self.queue.put(job)
+            if self.num_threads:
+                self.queue.put(job)
+            else:
+                results = self._single_builder.RunJob(job)
 
-        term = threading.Thread(target=self.queue.join)
-        term.setDaemon(True)
-        term.start()
-        while term.is_alive():
-            term.join(100)
+        if self.num_threads:
+            term = threading.Thread(target=self.queue.join)
+            term.setDaemon(True)
+            term.start()
+            while term.is_alive():
+                term.join(100)
 
-        # Wait until we have processed all output
-        self.out_queue.join()
+            # Wait until we have processed all output
+            self.out_queue.join()
         Print()
 
         msg = 'Completed: %d total built' % self.count
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index d664868582..6c6dbd7872 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -89,7 +89,8 @@ class BuilderThread(threading.Thread):
     Members:
         builder: The builder which contains information we might need
         thread_num: Our thread number (0-n-1), used to decide on a
-                temporary directory
+                temporary directory. If this is -1 then there are no threads
+                and we are the (only) main process
     """
     def __init__(self, builder, thread_num, mrproper, per_board_out_dir):
         """Set up a new builder thread"""
@@ -445,6 +446,9 @@ class BuilderThread(threading.Thread):
 
         Args:
             job: Job to build
+
+        Returns:
+            List of Result objects
         """
         brd = job.board
         work_dir = self.builder.GetThreadDir(self.thread_num)
@@ -508,7 +512,10 @@ class BuilderThread(threading.Thread):
 
                 # We have the build results, so output the result
                 self._WriteResult(result, job.keep_outputs, job.work_in_output)
-                self.builder.out_queue.put(result)
+                if self.thread_num != -1:
+                    self.builder.out_queue.put(result)
+                else:
+                    self.builder.ProcessResult(result)
         else:
             # Just build the currently checked-out build
             result, request_config = self.RunCommit(None, brd, work_dir, True,
@@ -517,7 +524,10 @@ class BuilderThread(threading.Thread):
                         work_in_output=job.work_in_output)
             result.commit_upto = 0
             self._WriteResult(result, job.keep_outputs, job.work_in_output)
-            self.builder.out_queue.put(result)
+            if self.thread_num != -1:
+                self.builder.out_queue.put(result)
+            else:
+                self.builder.ProcessResult(result)
 
     def run(self):
         """Our thread's run function
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 680c072d66..274b5ac3f4 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -97,7 +97,8 @@ def ParseArgs():
     parser.add_option('-t', '--test', action='store_true', dest='test',
                       default=False, help='run tests')
     parser.add_option('-T', '--threads', type='int',
-          default=None, help='Number of builder threads to use')
+          default=None,
+          help='Number of builder threads to use (0=single-thread)')
     parser.add_option('-u', '--show_unknown', action='store_true',
           default=False, help='Show boards with unknown build result')
     parser.add_option('-U', '--show-environment', action='store_true',
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index fe874b8165..a767570146 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -294,7 +294,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None,
 
     # By default we have one thread per CPU. But if there are not enough jobs
     # we can have fewer threads and use a high '-j' value for make.
-    if not options.threads:
+    if options.threads is None:
         options.threads = min(multiprocessing.cpu_count(), len(selected))
     if not options.jobs:
         options.jobs = max(1, (multiprocessing.cpu_count() +
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index 1a259d54ab..b9c65c0d32 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -187,7 +187,7 @@ class TestBuild(unittest.TestCase):
             expect += col.Color(expected_colour, ' %s' % board)
         self.assertEqual(text, expect)
 
-    def _SetupTest(self, echo_lines=False, **kwdisplay_args):
+    def _SetupTest(self, echo_lines=False, threads=1, **kwdisplay_args):
         """Set up the test by running a build and summary
 
         Args:
@@ -199,8 +199,8 @@ class TestBuild(unittest.TestCase):
         Returns:
             Iterator containing the output lines, each a PrintLine() object
         """
-        build = builder.Builder(self.toolchains, self.base_dir, None, 1, 2,
-                                checkout=False, show_unknown=False)
+        build = builder.Builder(self.toolchains, self.base_dir, None, threads,
+                                2, checkout=False, show_unknown=False)
         build.do_make = self.Make
         board_selected = self.boards.GetSelectedDict()
 
@@ -438,6 +438,12 @@ class TestBuild(unittest.TestCase):
                                 filter_migration_warnings=True)
         self._CheckOutput(lines, filter_migration_warnings=True)
 
+    def testSingleThread(self):
+        """Test operation without threading"""
+        lines = self._SetupTest(show_errors=True, threads=0)
+        self._CheckOutput(lines, list_error_boards=False,
+                          filter_dtb_warnings=False)
+
     def _testGit(self):
         """Test basic builder operation by building a branch"""
         options = Options()