From b82492bbccb0ecdee17108ed43c4220f00f3f4f3 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 30 Jan 2021 22:17:46 -0700 Subject: 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 --- tools/buildman/README | 5 ++++ tools/buildman/builder.py | 60 ++++++++++++++++++++++++++--------------- tools/buildman/builderthread.py | 16 ++++++++--- tools/buildman/cmdline.py | 3 ++- tools/buildman/control.py | 2 +- tools/buildman/test.py | 12 ++++++--- 6 files changed, 68 insertions(+), 30 deletions(-) (limited to 'tools/buildman') diff --git a/tools/buildman/README b/tools/buildman/README index b7442a95e56..600794790a0 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 6f6d759329a..be8a8fa13a6 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 d6648685823..6c6dbd78725 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 680c072d662..274b5ac3f45 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 fe874b8165b..a7675701466 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 1a259d54ab0..b9c65c0d326 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() -- cgit v1.2.3