All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick Ohly <patrick.ohly@intel.com>
To: openembedded-core@lists.openembedded.org
Cc: paul.eggleton@linux.intel.com
Subject: [PATCH 2/3] yocto-compat-layer: limit report of signature changes
Date: Wed,  5 Apr 2017 15:36:05 +0200	[thread overview]
Message-ID: <ddd6caf52e347f925286bfe9e37b894996037679.1491399200.git-series.patrick.ohly@intel.com> (raw)
In-Reply-To: <cover.3970fab18c2598adf378d62e8fcba4ca004fbd13.1491399200.git-series.patrick.ohly@intel.com>

Typically a single change cascades through the entire task dependency
chain. Developers had to figure that out themselves, based on hard to
read and interpret output (not sorted, no indention, no explanations):

   $ yocto-compat-layer.py -n meta-xxxx
   ...
   AssertionError: True is not false : Layer meta-xxxx changed signatures.
   webkitgtk:do_install changed fe2edc9082bc0da98f9cb1391c52f565 -> b3a44684c5cd9aacd3f7c6ed88eefab5
   gstreamer1.0-plugins-good:do_configure changed 3b2f8211be3fe08422bf6087f3af16d1 -> 7d80e42fa1f4f01ff4dfe2ea4477d382
   pulseaudio:do_package_qa changed 5d0a58ada66ff17f5576555302ac319a -> 0e13bcb96143d1ae54c451bc3de0aa30
   epiphany:do_prepare_recipe_sysroot changed 29e1b277dbcb005bd54950594c50d91b -> d3c45527b37677a0668ce483c6db3052
   ...
   gst-player:do_packagedata changed 9ce6efdd357dd74919bc4957458b1e95 -> d0c083ce629f37adfc9c4ba9eff81f83
   gstreamer1.0-plugins-base:do_install changed 1161cd867d15bea63e5dd5d9abf0519c -> 5bf2b652a2d77fee3eedb35af2f201a0
   gstreamer1.0-rtsp-server:do_packagedata changed 6781dc3070f80b843ed1970d74dd323e -> 454620c2e3b9fea87e525d14b6ed0344
   alsa-plugins:do_packagedata changed 1808c3f737cb805b169d004e948ea19c -> 480124b7fa5eab1f73bf96440d725231

Now the tool automates the problem analysis: it retrieves the depgraph
using the tinfoil API and only reports those tasks with modified
signatures whose dependencies have not changed, i.e. those tasks which
definitely introduce a change.

From the previous example, that just leaves two tasks that need to be
checked:

   AssertionError: False is not true : Layer meta-xxxx changed 120 signatures, initial differences (first hash without, second with layer):
      gstreamer1.0-plugins-base:do_fetch: 76973f19f2e30d282152bdd7e4efe5bb -> e6e7c6fa9f2bd59d7d8d107f7c6ca1ac
      pulseaudio:do_install: 668eb1e30af129df9806b0aa0d7c10cd -> 1196bdb88eef56eeee4613bb06b9387e

This pruning might be a bit too aggressive in the sense that tasks
which inherit a change and then add more changes themselves won't be
reported initially. They will be found when fixing the reported tasks
and re-running the check.

For a developer it seems better to have something listed which
definitely is a problem and needs fixing instead of everything,
including the tasks which don't need fixes.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/lib/compatlayer/__init__.py     | 32 +++++++++++++++-
 scripts/lib/compatlayer/cases/common.py | 54 +++++++++++++++++++-------
 2 files changed, 73 insertions(+), 13 deletions(-)

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index 9eb862d..b46527a 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -7,6 +7,8 @@ import os
 import subprocess
 from enum import Enum
 
+import bb.tinfoil
+
 class LayerType(Enum):
     BSP = 0
     DISTRO = 1
@@ -252,3 +254,33 @@ def get_signatures(builddir, failsafe=False):
         raise RuntimeError('Can\'t load signatures from %s' % sigs_file)
 
     return sigs
+
+def get_depgraph(targets=['world']):
+    '''
+    Returns the dependency graph for the given target(s).
+    The dependency graph is taken directly from DepTreeEvent.
+    '''
+    depgraph = None
+    with bb.tinfoil.Tinfoil() as tinfoil:
+        tinfoil.prepare(config_only=False)
+        tinfoil.set_event_mask(['bb.event.NoProvider', 'bb.event.DepTreeGenerated', 'bb.command.CommandCompleted'])
+        if not tinfoil.run_command('generateDepTreeEvent', targets, 'do_build'):
+            raise RuntimeError('starting generateDepTreeEvent failed')
+        while True:
+            event = tinfoil.wait_event(timeout=1000)
+            if event:
+                if isinstance(event, bb.command.CommandFailed):
+                    raise RuntimeError('Generating dependency information failed: %s' % event.error)
+                elif isinstance(event, bb.command.CommandCompleted):
+                    break
+                elif isinstance(event, bb.event.NoProvider):
+                    if event._reasons:
+                        raise RuntimeError('Nothing provides %s: %s' % (event._item, event._reasons))
+                    else:
+                        raise RuntimeError('Nothing provides %s.' % (event._item))
+                elif isinstance(event, bb.event.DepTreeGenerated):
+                    depgraph = event._depgraph
+
+    if depgraph is None:
+        raise RuntimeError('Could not retrieve the depgraph.')
+    return depgraph
diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 9cc682e..b91da9b 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -3,7 +3,7 @@
 
 import os
 import unittest
-from compatlayer import get_signatures, LayerType, check_command
+from compatlayer import get_signatures, LayerType, check_command, get_depgraph
 from compatlayer.case import OECompatLayerTestCase
 
 class CommonCompatLayer(OECompatLayerTestCase):
@@ -31,21 +31,49 @@ class CommonCompatLayer(OECompatLayerTestCase):
             raise unittest.SkipTest("Layer %s isn't BSP or DISTRO one." \
                      % self.tc.layer['name'])
 
+        # task -> (old signature, new signature)
         sig_diff = {}
-
         curr_sigs = get_signatures(self.td['builddir'], failsafe=True)
         for task in self.td['sigs']:
-            if task not in curr_sigs:
-                continue
-
-            if self.td['sigs'][task] != curr_sigs[task]:
-                sig_diff[task] = '%s -> %s' % \
-                        (self.td['sigs'][task], curr_sigs[task])
+            if task in curr_sigs and \
+               self.td['sigs'][task] != curr_sigs[task]:
+                sig_diff[task] = (self.td['sigs'][task], curr_sigs[task])
 
-        detail = ''
         if sig_diff:
-            for task in sig_diff:
-                detail += "%s changed %s\n" % (task, sig_diff[task])
-        self.assertFalse(bool(sig_diff), "Layer %s changed signatures.\n%s" % \
-                (self.tc.layer['name'], detail))
+            # Beware, depgraph uses task=<pn>.<taskname> whereas get_signatures()
+            # uses <pn>:<taskname>. Need to convert sometimes. The output follows
+            # the convention from get_signatures() because that seems closer to
+            # normal bitbake output.
+            def sig2graph(task):
+                pn, taskname = task.rsplit(':', 1)
+                return pn + '.' + taskname
+            def graph2sig(task):
+                pn, taskname = task.rsplit('.', 1)
+                return pn + ':' + taskname
+            depgraph = get_depgraph()
+            depends = depgraph['tdepends']
+
+            # If a task A has a changed signature, but none of its
+            # dependencies, then we need to report it because it is
+            # the one which introduces a change. Any task depending on
+            # A (directly or indirectly) will also have a changed
+            # signature, but we don't need to report it. It might have
+            # its own changes, which will become apparent once the
+            # issues that we do report are fixed and the test gets run
+            # again.
+            sig_diff_filtered = []
+            for task, (old_sig, new_sig) in sig_diff.items():
+                deps_tainted = False
+                for dep in depends.get(sig2graph(task), ()):
+                    if graph2sig(dep) in sig_diff:
+                        deps_tainted = True
+                        break
+                if not deps_tainted:
+                    sig_diff_filtered.append((task, old_sig, new_sig))
 
+            msg = []
+            msg.append('Layer %s changed %d signatures, initial differences (first hash without, second with layer):' %
+                       (self.tc.layer['name'], len(sig_diff)))
+            for diff in sorted(sig_diff_filtered):
+                msg.append('   %s: %s -> %s' % diff)
+            self.assertTrue(False, '\n'.join(msg))
-- 
git-series 0.9.1


  parent reply	other threads:[~2017-04-05 13:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-05 13:36 [PATCH 0/3] yocto-compat-layer: various enhancements + bitbake-diffsigs support Patrick Ohly
2017-04-05 13:36 ` [PATCH 1/3] yocto-compat-layer: fix also other command invocations Patrick Ohly
2017-04-05 13:36 ` Patrick Ohly [this message]
2017-04-06 20:38   ` [PATCH 2/3] yocto-compat-layer: limit report of signature changes Paul Eggleton
2017-04-07  6:41     ` Patrick Ohly
2017-04-05 13:36 ` [PATCH 3/3] yocto-compat-layer: include bitbake-diffsigs output Patrick Ohly
2017-04-05 15:26   ` Leonardo Sandoval
2017-04-05 18:10     ` Patrick Ohly
2017-04-06 16:25 ` [PATCH 0/3] yocto-compat-layer: various enhancements + bitbake-diffsigs support Aníbal Limón

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ddd6caf52e347f925286bfe9e37b894996037679.1491399200.git-series.patrick.ohly@intel.com \
    --to=patrick.ohly@intel.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=paul.eggleton@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.