Bug 1360764 - Decouple warnings database from log parser; r?froydnj draft
authorGregory Szorc <gps@mozilla.com>
Fri, 28 Apr 2017 16:21:44 -0700
changeset 570501 99276eec4f66a014c540ce1e2a8f0453f4ff45bc
parent 570263 b3b32894d9326d147238d9f071e1814ab3fdc850
child 570502 fec091da190b7c434ea78d54e71edb35b753aad2
push id56506
push userbmo:gps@mozilla.com
push dateSat, 29 Apr 2017 00:24:04 +0000
reviewersfroydnj
bugs1360764
milestone55.0a1
Bug 1360764 - Decouple warnings database from log parser; r?froydnj Currently, the WarningsCollector (which parses warnings from line inputs) accepts a WarningsDatabase (an object representing a collection of warnings) and populates that instance as a new warning is parsed. In an upcoming commit I want to introduce a 2nd WarningsDatabase. Rather that add it to WarningsCollector, I figure it will be easier to decouple WarningsDatabase from WarningsCollector. This commit refactors WarningsCollector to call a callback when a warning is parsed. This allows consumers to do anything they want with a warning, including potentially write it to multiple databases. MozReview-Commit-ID: 7Z9x4FMwyof
python/mozbuild/mozbuild/compilation/warnings.py
python/mozbuild/mozbuild/controller/building.py
python/mozbuild/mozbuild/test/compilation/test_warnings.py
--- a/python/mozbuild/mozbuild/compilation/warnings.py
+++ b/python/mozbuild/mozbuild/compilation/warnings.py
@@ -278,32 +278,34 @@ class WarningsDatabase(object):
         with open(filename, 'wb') as fh:
             self.serialize(fh)
 
 
 class WarningsCollector(object):
     """Collects warnings from text data.
 
     Instances of this class receive data (usually the output of compiler
-    invocations) and parse it into warnings and add these warnings to a
-    database.
+    invocations) and parse it into warnings.
 
     The collector works by incrementally receiving data, usually line-by-line
     output from the compiler. Therefore, it can maintain state to parse
     multi-line warning messages.
     """
-    def __init__(self, database=None, objdir=None, resolve_files=True):
-        self.database = database
+    def __init__(self, cb, objdir=None):
+        """Initialize a new collector.
+
+        ``cb`` is a callable that is called with a ``CompilerWarning``
+        instance whenever a new warning is parsed.
+
+         ``objdir`` is the object directory. Used for normalizing paths.
+         """
+        self.cb = cb
         self.objdir = objdir
-        self.resolve_files = resolve_files
         self.included_from = []
 
-        if database is None:
-            self.database = WarningsDatabase()
-
     def process_line(self, line):
         """Take a line of text and process it for a warning."""
 
         filtered = RE_STRIP_COLORS.sub('', line)
 
         # Clang warnings in files included from the one(s) being compiled will
         # start with "In file included from /path/to/file:line:". Here, we
         # record those.
@@ -344,23 +346,19 @@ class WarningsCollector(object):
 
         filename = os.path.normpath(filename)
 
         # Sometimes we get relative includes. These typically point to files in
         # the object directory. We try to resolve the relative path.
         if not os.path.isabs(filename):
             filename = self._normalize_relative_path(filename)
 
-        if not os.path.exists(filename) and self.resolve_files:
-            raise Exception('Could not find file containing warning: %s' %
-                    filename)
-
         warning['filename'] = filename
 
-        self.database.insert(warning, compute_hash=self.resolve_files)
+        self.cb(warning)
 
         return warning
 
     def _normalize_relative_path(self, filename):
         # Special case files in dist/include.
         idx = filename.find('/dist/include')
         if idx != -1:
             return self.objdir + filename[idx:]
--- a/python/mozbuild/mozbuild/controller/building.py
+++ b/python/mozbuild/mozbuild/controller/building.py
@@ -167,18 +167,27 @@ class BuildMonitor(MozbuildObject):
 
         self.warnings_database = WarningsDatabase()
         if os.path.exists(warnings_path):
             try:
                 self.warnings_database.load_from_file(warnings_path)
             except ValueError:
                 os.remove(warnings_path)
 
-        self._warnings_collector = WarningsCollector(
-            database=self.warnings_database, objdir=self.topobjdir)
+        def on_warning(warning):
+            filename = warning['filename']
+
+            if not os.path.exists(filename):
+                raise Exception('Could not find file containing warning: %s' %
+                                filename)
+
+            self.warnings_database.insert(warning)
+
+        self._warnings_collector = WarningsCollector(on_warning,
+                                                     objdir=self.topobjdir)
 
         self.build_objects = []
 
     def start(self):
         """Record the start of the build."""
         self.start_time = time.time()
         self._finder_start_cpu = self._get_finder_cpu_usage()
 
--- a/python/mozbuild/mozbuild/test/compilation/test_warnings.py
+++ b/python/mozbuild/mozbuild/test/compilation/test_warnings.py
@@ -120,30 +120,30 @@ class TestCompilerWarning(unittest.TestC
         self.assertLessEqual(w1, w2)
         self.assertLessEqual(w2, w1)
         self.assertGreaterEqual(w2, w1)
         self.assertGreaterEqual(w1, w2)
 
 class TestWarningsParsing(unittest.TestCase):
     def test_clang_parsing(self):
         for source, filename, line, column, message, flag in CLANG_TESTS:
-            collector = WarningsCollector(resolve_files=False)
+            collector = WarningsCollector(lambda w: None)
             warning = collector.process_line(source)
 
             self.assertIsNotNone(warning)
 
             self.assertEqual(warning['filename'], filename)
             self.assertEqual(warning['line'], line)
             self.assertEqual(warning['column'], column)
             self.assertEqual(warning['message'], message)
             self.assertEqual(warning['flag'], flag)
 
     def test_msvc_parsing(self):
         for source, filename, line, flag, message in MSVC_TESTS:
-            collector = WarningsCollector(resolve_files=False)
+            collector = WarningsCollector(lambda w: None)
             warning = collector.process_line(source)
 
             self.assertIsNotNone(warning)
 
             self.assertEqual(warning['filename'], os.path.normpath(filename))
             self.assertEqual(warning['line'], line)
             self.assertEqual(warning['flag'], flag)
             self.assertEqual(warning['message'], message)