[PATCH 0/7] Improvements for the rootfs log check

[PATCH 0/7] Improvements for the rootfs log check

[Peter Kjellerstedt]

This is a set of improvements to the log check code in
meta/lib/oe/rootfs.py. It started out with me wanting to reduce the
spam I got when there were many warnings in the log, and ended up with
quite a lot of refactoring. I also activated the fix to bug 7789 along
the way...

I have only tested this with rpm builds, but I expect it to work for
deb and ipk builds as well...

//Peter

The following changes since commit 7707cf6562e6a477ea953743a2d9aba682d90949:

  sdk-manual: Fixed three broken links to sections within manual. (2016-05-17 21:35:00 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib pkj/improve_rootfs_log_check
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=pkj/improve_rootfs_log_check

Peter Kjellerstedt (7):
  Revert "rootfs.py: add more info to the warning message"
  rootfs.py: Remove _log_check_error() from the RpmRootfs class
  rootfs.py: Simplify the regular expression used in _log_check_warn()
  rootfs.py: Use one way to exclude lines in _log_check_error()
  rootfs.py: Exclude lines in _log_check_warn() as well
  rootfs.py: Reduce spam from _log_check_warn()
  rootfs.py: Unify _log_check_warn() and _log_check_error()

 meta/lib/oe/rootfs.py | 93 +++++++++++++++++----------------------------------
 1 file changed, 31 insertions(+), 62 deletions(-)

-- 
2.1.0

[PATCH 1/7] Revert "rootfs.py: add more info to the warning message"

[Peter Kjellerstedt]

The reverted commit added a warning with the log line that triggered
the log check for error messages before the warning that states that
an error has been found in the log. However, the error line is output
by the call to bb.fatal() that follows immediately after the original
warning, which makes it redundant. Additionaly, having two warnings
contradicts the intent of commit 8dfdd329 where the log warnings were
tidied up.

This reverts commit f9cf31525fc885e1a0f65bd55654631257f87078.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/rootfs.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 95fd3ab..f6fb06c 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -75,7 +75,6 @@ class Rootfs(object):
                 m = r.search(line)
                 if m:
                     found_error = 1
-                    bb.warn('[log_check] In line: [%s]' % line)
                     bb.warn('[log_check] %s: found an error message in the logfile (keyword \'%s\'):\n[log_check] %s'
 				    % (self.d.getVar('PN', True), m.group(), line))
 
-- 
2.1.0

[PATCH 2/7] rootfs.py: Remove _log_check_error() from the RpmRootfs class

[Peter Kjellerstedt]

The fact that this function was overridden in the RpmRootfs class
seems to have led to a number of misstakes when changes have been made
to the base function in the Rootfs class. E.g., this change will
properly solve ticket 7789, which was supposedly solved in 38871dc0,
but that change had no effect in practice as the log_check_regex that
was modified for RpmRootfs class was not used by the RpmRootfs version
of _log_check_error()...

The only thing _log_check_error() in RpmRootfs did that the base
function in Rootfs did not do was to skip lines in the log that begin
with a + sign. This has now been moved to the base function instead.

[YOCTO #7789]

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/rootfs.py | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index f6fb06c..0a2753e 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -62,6 +62,9 @@ class Rootfs(object):
             for line in log:
                 if 'log_check' in line:
                     continue
+                # sh -x may emit code which isn't actually executed
+                if line.startswith('+'):
+                    continue
 
                 if hasattr(self, 'log_check_expected_errors_regexes'):
                     m = None
@@ -473,32 +476,6 @@ class RpmRootfs(Rootfs):
         # already saved in /etc/rpm-postinsts
         pass
 
-    def _log_check_error(self):
-        r = re.compile('(unpacking of archive failed|Cannot find package|exit 1|ERR|Fail)')
-        log_path = self.d.expand("${T}/log.do_rootfs")
-        with open(log_path, 'r') as log:
-            found_error = 0
-            message = "\n"
-            for line in log.read().split('\n'):
-                if 'log_check' in line:
-                    continue
-                # sh -x may emit code which isn't actually executed
-                if line.startswith('+'):
-		    continue
-
-                m = r.search(line)
-                if m:
-                    found_error = 1
-                    bb.warn('log_check: There were error messages in the logfile')
-                    bb.warn('log_check: Matched keyword: [%s]\n\n' % m.group())
-
-                if found_error >= 1 and found_error <= 5:
-                    message += line + '\n'
-                    found_error += 1
-
-                if found_error == 6:
-                    bb.fatal(message)
-
     def _log_check(self):
         self._log_check_warn()
         self._log_check_error()
-- 
2.1.0

[PATCH 3/7] rootfs.py: Simplify the regular expression used in _log_check_warn()

[Peter Kjellerstedt]

In commit 0387d095 lines with "NOTE:" in them were excluded from the
log check for warnings. However, those lines were only there in the
first place since the regular expression that is used to find warning
messages explicitly included those lines...

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/rootfs.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 0a2753e..a92aa22 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -41,11 +41,11 @@ class Rootfs(object):
         pass
 
     def _log_check_warn(self):
-        r = re.compile('^(warn|Warn|NOTE: warn|NOTE: Warn|WARNING:)')
+        r = re.compile('^(warn|Warn|WARNING:)')
         log_path = self.d.expand("${T}/log.do_rootfs")
         with open(log_path, 'r') as log:
             for line in log:
-                if 'log_check' in line or 'NOTE:' in line:
+                if 'log_check' in line:
                     continue
 
                 m = r.search(line)
-- 
2.1.0

[PATCH 4/7] rootfs.py: Use one way to exclude lines in _log_check_error()

[Peter Kjellerstedt]

Before there were three different ways to exclude a line from being
searched for error messages in _log_check_error(). Now there is only
one: an array of regular expressions. This should make it easy to add
more excludes if nedded.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/rootfs.py | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index a92aa22..63ca22f 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -54,26 +54,25 @@ class Rootfs(object):
 				    % (self.d.getVar('PN', True), m.group(), line))
 
     def _log_check_error(self):
+        # Ignore any lines containing log_check to avoid recursion, and ignore
+        # lines beginning with a + since sh -x may emit code which isn't
+        # actually executed, but may contain error messages
+        excludes = [ 'log_check', r'^\+' ]
+        if hasattr(self, 'log_check_expected_errors_regexes'):
+            excludes.extend(self.log_check_expected_errors_regexes)
+        excludes = [re.compile(x) for x in excludes]
         r = re.compile(self.log_check_regex)
         log_path = self.d.expand("${T}/log.do_rootfs")
         with open(log_path, 'r') as log:
             found_error = 0
             message = "\n"
             for line in log:
-                if 'log_check' in line:
-                    continue
-                # sh -x may emit code which isn't actually executed
-                if line.startswith('+'):
-                    continue
-
-                if hasattr(self, 'log_check_expected_errors_regexes'):
-                    m = None
-                    for ee in self.log_check_expected_errors_regexes:
-                        m = re.search(ee, line)
-                        if m:
-                            break
+                for ee in excludes:
+                    m = ee.search(line)
                     if m:
-                        continue
+                        break
+                if m:
+                    continue
 
                 m = r.search(line)
                 if m:
-- 
2.1.0

[PATCH 5/7] rootfs.py: Exclude lines in _log_check_warn() as well

[Peter Kjellerstedt]

This will make _log_check_warn() exclude the same lines as
_log_check_error() does.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/rootfs.py | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 63ca22f..741399a 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -41,11 +41,22 @@ class Rootfs(object):
         pass
 
     def _log_check_warn(self):
+        # Ignore any lines containing log_check to avoid recursion, and ignore
+        # lines beginning with a + since sh -x may emit code which isn't
+        # actually executed, but may contain error messages
+        excludes = [ 'log_check', r'^\+' ]
+        if hasattr(self, 'log_check_expected_regexes'):
+            excludes.extend(self.log_check_expected_regexes)
+        excludes = [re.compile(x) for x in excludes]
         r = re.compile('^(warn|Warn|WARNING:)')
         log_path = self.d.expand("${T}/log.do_rootfs")
         with open(log_path, 'r') as log:
             for line in log:
-                if 'log_check' in line:
+                for ee in excludes:
+                    m = ee.search(line)
+                    if m:
+                        break
+                if m:
                     continue
 
                 m = r.search(line)
@@ -58,8 +69,8 @@ class Rootfs(object):
         # lines beginning with a + since sh -x may emit code which isn't
         # actually executed, but may contain error messages
         excludes = [ 'log_check', r'^\+' ]
-        if hasattr(self, 'log_check_expected_errors_regexes'):
-            excludes.extend(self.log_check_expected_errors_regexes)
+        if hasattr(self, 'log_check_expected_regexes'):
+            excludes.extend(self.log_check_expected_regexes)
         excludes = [re.compile(x) for x in excludes]
         r = re.compile(self.log_check_regex)
         log_path = self.d.expand("${T}/log.do_rootfs")
@@ -597,7 +608,7 @@ class DpkgRootfs(DpkgOpkgRootfs):
     def __init__(self, d, manifest_dir):
         super(DpkgRootfs, self).__init__(d)
         self.log_check_regex = '^E:'
-        self.log_check_expected_errors_regexes = \
+        self.log_check_expected_regexes = \
         [
             "^E: Unmet dependencies."
         ]
-- 
2.1.0

[PATCH 6/7] rootfs.py: Reduce spam from _log_check_warn()

[Peter Kjellerstedt]

For each warning found in the log, _log_check_warn() would output a
line stating that it had found a warning, then the actual warning and
finally an empty line. This is quite excessive when there are many
warnings in the log.

With this change the output is instead a line stating how many
warnings were found, followed by the warnings. This makes the output
much more compact and actually much more readable.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/rootfs.py | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 741399a..479e4cc 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -50,6 +50,7 @@ class Rootfs(object):
         excludes = [re.compile(x) for x in excludes]
         r = re.compile('^(warn|Warn|WARNING:)')
         log_path = self.d.expand("${T}/log.do_rootfs")
+        messages = []
         with open(log_path, 'r') as log:
             for line in log:
                 for ee in excludes:
@@ -61,8 +62,14 @@ class Rootfs(object):
 
                 m = r.search(line)
                 if m:
-                    bb.warn('[log_check] %s: found a warning message in the logfile (keyword \'%s\'):\n[log_check] %s'
-				    % (self.d.getVar('PN', True), m.group(), line))
+                    messages.append('[log_check] %s' % line)
+        if messages:
+            if len(messages) == 1:
+                msg = 'a warning message'
+            else:
+                msg = '%d warning messages' % len(messages)
+            bb.warn('[log_check] %s: found %s in the logfile:\n%s'
+                    % (self.d.getVar('PN', True), msg, ''.join(messages)))
 
     def _log_check_error(self):
         # Ignore any lines containing log_check to avoid recursion, and ignore
-- 
2.1.0

[PATCH 7/7] rootfs.py: Unify _log_check_warn() and _log_check_error()

[Peter Kjellerstedt]

Use a common _log_check_common() function (based on the old
_log_check_warn() function) to implement the logic for both
_log_check_warn() and _log_check_error().

The main benefit of this is that now all error messages will be
reported again, not just the first one found. Additionally the output
will now look the same for both error and warning messages.

This removes the context for the error messages. However, since there
was no indication in the output that some of the lines were context,
they were more confusing than helping.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/lib/oe/rootfs.py | 52 ++++++++++++++-------------------------------------
 1 file changed, 14 insertions(+), 38 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 479e4cc..f5c465f 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -40,7 +40,7 @@ class Rootfs(object):
     def _log_check(self):
         pass
 
-    def _log_check_warn(self):
+    def _log_check_common(self, type, match):
         # Ignore any lines containing log_check to avoid recursion, and ignore
         # lines beginning with a + since sh -x may emit code which isn't
         # actually executed, but may contain error messages
@@ -48,7 +48,7 @@ class Rootfs(object):
         if hasattr(self, 'log_check_expected_regexes'):
             excludes.extend(self.log_check_expected_regexes)
         excludes = [re.compile(x) for x in excludes]
-        r = re.compile('^(warn|Warn|WARNING:)')
+        r = re.compile(match)
         log_path = self.d.expand("${T}/log.do_rootfs")
         messages = []
         with open(log_path, 'r') as log:
@@ -65,45 +65,21 @@ class Rootfs(object):
                     messages.append('[log_check] %s' % line)
         if messages:
             if len(messages) == 1:
-                msg = 'a warning message'
+                msg = '1 %s message' % type
             else:
-                msg = '%d warning messages' % len(messages)
-            bb.warn('[log_check] %s: found %s in the logfile:\n%s'
-                    % (self.d.getVar('PN', True), msg, ''.join(messages)))
-
-    def _log_check_error(self):
-        # Ignore any lines containing log_check to avoid recursion, and ignore
-        # lines beginning with a + since sh -x may emit code which isn't
-        # actually executed, but may contain error messages
-        excludes = [ 'log_check', r'^\+' ]
-        if hasattr(self, 'log_check_expected_regexes'):
-            excludes.extend(self.log_check_expected_regexes)
-        excludes = [re.compile(x) for x in excludes]
-        r = re.compile(self.log_check_regex)
-        log_path = self.d.expand("${T}/log.do_rootfs")
-        with open(log_path, 'r') as log:
-            found_error = 0
-            message = "\n"
-            for line in log:
-                for ee in excludes:
-                    m = ee.search(line)
-                    if m:
-                        break
-                if m:
-                    continue
-
-                m = r.search(line)
-                if m:
-                    found_error = 1
-                    bb.warn('[log_check] %s: found an error message in the logfile (keyword \'%s\'):\n[log_check] %s'
-				    % (self.d.getVar('PN', True), m.group(), line))
+                msg = '%d %s messages' % (len(messages), type)
+            msg = '[log_check] %s: found %s in the logfile:\n%s' % \
+                (self.d.getVar('PN', True), msg, ''.join(messages))
+            if type == 'error':
+                bb.fatal(msg)
+            else:
+                bb.warn(msg)
 
-                if found_error >= 1 and found_error <= 5:
-                    message += line + '\n'
-                    found_error += 1
+    def _log_check_warn(self):
+        self._log_check_common('warning', '^(warn|Warn|WARNING:)')
 
-                if found_error == 6:
-                    bb.fatal(message)
+    def _log_check_error(self):
+        self._log_check_common('error', self.log_check_regex)
 
     def _insert_feed_uris(self):
         if bb.utils.contains("IMAGE_FEATURES", "package-management",
-- 
2.1.0