zephyr: zmake: Allow warnings to break the build

Now that we are showing warnings by default (and not requiring
-l DEBUG) they are more likely to receive action. But for the commit
queue it seems useful to fail the build if there are any warnings,
just in case people are not very observant in their local builds.

Add a -w flag for this. It works by checking for any output in the
ERROR log, which is safer now that we have cleaned up the error
output to just include actual errors.

Put the check in a separate function since it will be called from
a different place in a future CL.

BUG=b:177096315
BRANCH=none
TEST=manually test by running zmake build / configure
FEATURE=test sudo -E emerge zephyr-build-tools

Signed-off-by: Simon Glass <sjg@chromium.org>
Change-Id: I4c0ec53d2836ce0ba67b3c8bcf39b53fa1af2945
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2789797
Reviewed-by: Keith Short <keithshort@chromium.org>
Reviewed-by: Yuval Peress <peress@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>

3 files changed, 58 insertions, 9 deletions

diff --git a/zephyr/zmake/zmake/__main__.py b/zephyr/zmake/zmake/__main__.py
index 654c49d085..3eb599c921 100644
--- a/zephyr/zmake/zmake/__main__.py
+++ b/zephyr/zmake/zmake/__main__.py
@@ -106,6 +106,8 @@ def main(argv=None):
     build = sub.add_parser('build')
     build.add_argument('build_dir', type=pathlib.Path,
                        help='The build directory used during configuration')
+    build.add_argument('-w', '--fail-on-warnings', action='store_true',
+                       help='Exit with code 2 if warnings are detected')
 
     test = sub.add_parser('test')
     test.add_argument('build_dir', type=pathlib.Path,
diff --git a/zephyr/zmake/zmake/multiproc.py b/zephyr/zmake/zmake/multiproc.py
index 45bf592c2c..68fc086e69 100644
--- a/zephyr/zmake/zmake/multiproc.py
+++ b/zephyr/zmake/zmake/multiproc.py
@@ -1,6 +1,7 @@
 # Copyright 2020 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
+import collections
 import logging
 import os
 import select
@@ -35,11 +36,16 @@ class LogWriter:
         _override_func: A function used to override the log level. The
             function will be called once per line prior to logging and will be
             passed the arguments of the line and the default log level.
+        _written_at_level: dict:
+            key: log_level
+            value: True if output was written at that level
     """
     def __init__(self, logger, log_level, log_level_override_func):
         self._logger = logger
         self._log_level = log_level
         self._override_func = log_level_override_func
+        # A map whether output was printed at each logging level
+        self._written_at_level = collections.defaultdict(lambda: False)
 
     def log_line(self, line):
         """Log a line of output
@@ -58,6 +64,18 @@ class LogWriter:
             # level.
             self._log_level = self._override_func(line, self._log_level)
         self._logger.log(self._log_level, line)
+        self._written_at_level[self._log_level] = True
+
+    def has_written(self, log_level):
+        """Check if output was written at a certain log level
+
+        Args:
+            log_level: log level to check
+
+        Returns:
+            True if any output was written at that log level, False if not
+        """
+        return self._written_at_level[log_level]
 
 
 def _log_fd(fd):
@@ -147,17 +165,21 @@ def log_output(logger, log_level, file_descriptor,
         log_level_override_func: A function used to override the log level. The
           function will be called once per line prior to logging and will be
           passed the arguments of the line and the default log level.
+
+    Returns:
+        LogWriter object for the resulting output
     """
     with _logging_cv:
         if not _logging_thread.is_alive():
             _logging_thread.start()
-        _logging_map[file_descriptor] = LogWriter(logger, log_level,
-                                                  log_level_override_func)
+        writer = LogWriter(logger, log_level, log_level_override_func)
+        _logging_map[file_descriptor] = writer
         # Write a dummy byte to the pipe to break the select so we can add the
         # new fd.
         os.write(_logging_interrupt_pipe[1], b'x')
         # Notify the condition so we can run the select on the current fds.
         _logging_cv.notify_all()
+    return writer
 
 
 def wait_for_log_end():
diff --git a/zephyr/zmake/zmake/zmake.py b/zephyr/zmake/zmake/zmake.py
index 0d9f9ee33c..addf422d11 100644
--- a/zephyr/zmake/zmake/zmake.py
+++ b/zephyr/zmake/zmake/zmake.py
@@ -257,12 +257,37 @@ class Zmake:
         elif build_after_configure:
             return self.build(build_dir=build_dir)
 
-    def build(self, build_dir, output_files_out=None):
+    def build(self, build_dir, output_files_out=None, fail_on_warnings=False):
         """Build a pre-configured build directory."""
-        project = zmake.project.Project(build_dir / 'project')
+        def wait_and_check_success(procs, writers):
+            """Wait for processes to complete and check for errors
+
+            Args:
+                procs: List of subprocess.Popen objects to check
+                writers: List of LogWriter objects to check
+
+            Returns:
+                True if all if OK
+                False if an error was found (so that zmake should exit)
+            """
+            for proc in procs:
+                if proc.wait():
+                    raise OSError(get_process_failure_msg(proc))
+
+            if (fail_on_warnings and
+                any(w.has_written(logging.WARNING) or
+                    w.has_written(logging.ERROR) for w in writers)):
+                self.logger.warning(
+                    "zmake: Warnings detected in build: aborting")
+                return False
+            return True
 
         procs = []
+        log_writers = []
         dirs = {}
+
+        project = zmake.project.Project(build_dir / 'project')
+
         for build_name, build_config in project.iter_builds():
             dirs[build_name] = build_dir / 'build-{}'.format(build_name)
             cmd = ['/usr/bin/ninja', '-C', dirs[build_name].as_posix()]
@@ -277,21 +302,21 @@ class Zmake:
                 stderr=subprocess.PIPE,
                 encoding='utf-8',
                 errors='replace')
-            zmake.multiproc.log_output(
+            out = zmake.multiproc.log_output(
                 logger=self.logger,
                 log_level=logging.INFO,
                 file_descriptor=proc.stdout,
                 log_level_override_func=ninja_log_level_override)
-            zmake.multiproc.log_output(
+            err = zmake.multiproc.log_output(
                 self.logger,
                 logging.ERROR,
                 proc.stderr,
                 log_level_override_func=cmake_log_level_override)
             procs.append(proc)
+            log_writers += [out, err]
 
-        for proc in procs:
-            if proc.wait():
-                raise OSError(get_process_failure_msg(proc))
+        if not wait_and_check_success(procs, log_writers):
+            return 2
 
         # Run the packer.
         packer_work_dir = build_dir / 'packer'