All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] [RFC] yocto-compat-layer: various enhancements + bitbake-diffsigs support
@ 2017-04-07 16:38 Patrick Ohly
  2017-04-07 16:38 ` [PATCH 1/5] yocto-compat-layer: fix also other command invocations Patrick Ohly
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Patrick Ohly @ 2017-04-07 16:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

I started applying yocto-compat-layer to some real BSP layers and ran
into some usability issues with the tool.

I also didn't want to do the root cause analysis manually, so I
automated the dependency analysis and the running of
bitbake-diffsigs.

This patch series is based on Mark's "yocto-compat-layer.py updates"
series. The last commit depends on Paul's "bitbake-diffsigs: add an
option to find and compare specific signatures" patch from
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=paule/sigstuff&id=5bb69edfb4bbaa7373061daeb4f233a7e2f43a43

Regarding the BSP example that I ended up using: it actually was one
of the better BSP layers and only had one problem in a "bitbake world"
build instead of several as in other BSP layers. Nevertheless I
obscured the name to protect the (not so) guilty in the commit
messages ;-}

The two changes that show up in test_signatures look harmless at first
glance, but probably would need to be done differently to avoid a
false positive when doing the signature check.

V2: - use self.fail() instead of self.assertTrue(False, ...)
    - test_machine_signatures check (work in progress)

Patrick Ohly (5):
  yocto-compat-layer: fix also other command invocations
  yocto-compat-layer: limit report of signature changes
  yocto-compat-layer: include bitbake-diffsigs output
  yocto-compat-layer: also determine tune flags for each task
  yocto-compat-layer: test signature differences when setting MACHINE

 scripts/lib/compatlayer/__init__.py     | 71 ++++++++++++++++++---
 scripts/lib/compatlayer/cases/bsp.py    | 54 +++++++++++++++-
 scripts/lib/compatlayer/cases/common.py | 89 ++++++++++++++++----------
 scripts/yocto-compat-layer.py           | 22 +++++-
 4 files changed, 189 insertions(+), 47 deletions(-)

base-commit: 719f6ff68135dc2ad143c8572c8321974f6a3357
-- 
git-series 0.9.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] yocto-compat-layer: fix also other command invocations
  2017-04-07 16:38 [PATCH 0/5] [RFC] yocto-compat-layer: various enhancements + bitbake-diffsigs support Patrick Ohly
@ 2017-04-07 16:38 ` Patrick Ohly
  2017-04-07 16:38 ` [PATCH 2/5] yocto-compat-layer: limit report of signature changes Patrick Ohly
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Patrick Ohly @ 2017-04-07 16:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

In commit 5b9ac62ab535d, one place was fixed where a command was
invoked such that failures caused double stack traces and stderr was
lost. The same problem also occurs elsewhere, triggered for example by
a layer with parsing problems.

Now a new utility method is used instead of repeating the code.

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

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index 86f86eb..9eb862d 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -4,6 +4,7 @@
 # Released under the MIT license (see COPYING.MIT)
 
 import os
+import subprocess
 from enum import Enum
 
 class LayerType(Enum):
@@ -199,8 +200,20 @@ def add_layer(bblayersconf, layer, layers, logger):
 
     return True
 
+def check_command(error_msg, cmd):
+    '''
+    Run a command under a shell, capture stdout and stderr in a single stream,
+    throw an error when command returns non-zero exit code. Returns the output.
+    '''
+
+    p = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+    output, _ = p.communicate()
+    if p.returncode:
+        msg = "%s\nCommand: %s\nOutput:\n%s" % (error_msg, cmd, output.decode('utf-8'))
+        raise RuntimeError(msg)
+    return output
+
 def get_signatures(builddir, failsafe=False):
-    import subprocess
     import re
 
     # some recipes needs to be excluded like meta-world-pkgdata
@@ -214,12 +227,8 @@ def get_signatures(builddir, failsafe=False):
     if failsafe:
         cmd += '-k '
     cmd += '-S none world'
-    p = subprocess.Popen(cmd, shell=True,
-                         stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-    output, _ = p.communicate()
-    if p.returncode:
-        msg = "Generating signatures failed. This might be due to some parse error and/or general layer incompatibilities.\nCommand: %s\nOutput:\n%s" % (cmd, output.decode('utf-8'))
-        raise RuntimeError(msg)
+    check_command('Generating signatures failed. This might be due to some parse error and/or general layer incompatibilities.',
+                  cmd)
     sigs_file = os.path.join(builddir, 'locked-sigs.inc')
 
     sig_regex = re.compile("^(?P<task>.*:.*):(?P<hash>.*) .$")
diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 4d328ec..9cc682e 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -2,9 +2,8 @@
 # Released under the MIT license (see COPYING.MIT)
 
 import os
-import subprocess
 import unittest
-from compatlayer import get_signatures, LayerType
+from compatlayer import get_signatures, LayerType, check_command
 from compatlayer.case import OECompatLayerTestCase
 
 class CommonCompatLayer(OECompatLayerTestCase):
@@ -20,26 +19,12 @@ class CommonCompatLayer(OECompatLayerTestCase):
                 msg="Layer contains README file but is empty.")
 
     def test_parse(self):
-        try:
-            output = subprocess.check_output('bitbake -p', shell=True,
-                    stderr=subprocess.PIPE)
-        except subprocess.CalledProcessError as e:
-            import traceback
-            exc = traceback.format_exc()
-            msg = 'Layer %s failed to parse.\n%s\n%s\n' % (self.tc.layer['name'],
-                    exc, e.output.decode('utf-8'))
-            raise RuntimeError(msg)
+        check_command('Layer %s failed to parse.' % self.tc.layer['name'],
+                      'bitbake -p')
 
     def test_show_environment(self):
-        try:
-            output = subprocess.check_output('bitbake -e', shell=True,
-                    stderr=subprocess.PIPE)
-        except subprocess.CalledProcessError as e:
-            import traceback
-            exc = traceback.format_exc()
-            msg = 'Layer %s failed to show environment.\n%s\n%s\n' % \
-                    (self.tc.layer['name'], exc, e.output.decode('utf-8'))
-            raise RuntimeError(msg)
+        check_command('Layer %s failed to show environment.' % self.tc.layer['name'],
+                      'bitbake -e')
 
     def test_signatures(self):
         if self.tc.layer['type'] == LayerType.SOFTWARE:
-- 
git-series 0.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] yocto-compat-layer: limit report of signature changes
  2017-04-07 16:38 [PATCH 0/5] [RFC] yocto-compat-layer: various enhancements + bitbake-diffsigs support Patrick Ohly
  2017-04-07 16:38 ` [PATCH 1/5] yocto-compat-layer: fix also other command invocations Patrick Ohly
@ 2017-04-07 16:38 ` Patrick Ohly
  2017-04-07 16:38 ` [PATCH 3/5] yocto-compat-layer: include bitbake-diffsigs output Patrick Ohly
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Patrick Ohly @ 2017-04-07 16:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] yocto-compat-layer: include bitbake-diffsigs output
  2017-04-07 16:38 [PATCH 0/5] [RFC] yocto-compat-layer: various enhancements + bitbake-diffsigs support Patrick Ohly
  2017-04-07 16:38 ` [PATCH 1/5] yocto-compat-layer: fix also other command invocations Patrick Ohly
  2017-04-07 16:38 ` [PATCH 2/5] yocto-compat-layer: limit report of signature changes Patrick Ohly
@ 2017-04-07 16:38 ` Patrick Ohly
  2017-04-07 16:38 ` [PATCH 4/5] yocto-compat-layer: also determine tune flags for each task Patrick Ohly
  2017-04-07 16:38 ` [PATCH 5/5] yocto-compat-layer: test signature differences when setting MACHINE Patrick Ohly
  4 siblings, 0 replies; 7+ messages in thread
From: Patrick Ohly @ 2017-04-07 16:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

After filtering out potential false positives, it becomes feasible to
include the output of bitbake-diffsigs for those tasks which
definitely have a change.

Depends on bitbake-diffsigs with the "--signature" parameter.

Enhanced output now is:

   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
         Task dependencies changed from:
         ['PV', 'SRCREV', 'SRC_URI', 'SRC_URI[md5sum]', 'SRC_URI[sha256sum]', 'base_do_fetch']
         to:
         ['GST_IMX_PATCHES_TO_APPEND', 'PV', 'SRCREV', 'SRC_URI', 'SRC_URI[md5sum]', 'SRC_URI[sha256sum]', 'base_do_fetch']
         basehash changed from d679d30bd1ea41c56e57419b57587f3c to 090a79b45f5fa26d10f9d34e2ed7a1e6
            List of dependencies for variable SRC_URI changed from '{'PV', 'SRC_URI[md5sum]', 'SRC_URI[sha256sum]'}' to '{'GST_IMX_PATCHES_TO_APPEND', 'PV', 'SRC_URI[md5sum]', 'SRC_URI[sha256sum]'}'
         changed items: {'GST_IMX_PATCHES_TO_APPEND'}
         Dependency on variable GST_IMX_PATCHES_TO_APPEND was added
         Variable SRC_URI value changed:
         "     http://gstreamer.freedesktop.org/src/gst-plugins-base/gst-plugins-base-${PV}.tar.xz     file://get-caps-from-src-pad-when-query-caps.patch     file://0003-ssaparse-enhance-SSA-text-lines-parsing.patch     file://0004-subparse-set-need_segment-after-sink-pad-received-GS.patch     file://encodebin-Need-more-buffers-in-output-queue-for-bett.patch     file://make-gio_unix_2_0-dependency-configurable.patch     file://0001-introspection.m4-prefix-pkgconfig-paths-with-PKG_CON.patch     file://0001-Makefile.am-don-t-hardcode-libtool-name-when-running.patch     file://0002-Makefile.am-prefix-calls-to-pkg-config-with-PKG_CONF.patch     file://0003-riff-add-missing-include-directories-when-calling-in.patch     file://0004-rtsp-drop-incorrect-reference-to-gstreamer-sdp-in-Ma.patch [--] {+${GST_IMX_PATCHES_TO_APPEND}+}"

      pulseaudio:do_install: 6bb6fe23e11a6d5fef9c3a25e73e4f9c -> 3f54ea75673a792e307197cfa6ef2694
         basehash changed from ac4efcfa783bd04a5a98a2c38719aedd to 37679d99623a37c8df955da3a01415a5
         Variable do_install value changed:
         @@ -1,3 +1,7 @@
              autotools_do_install
           	install -d ${D}${sysconfdir}/default/volatiles
          	install -m 0644 ${WORKDIR}/volatiles.04_pulse  ${D}${sysconfdir}/default/volatiles/volatiles.04_pulse
         +    if [ -e "${WORKDIR}/daemon.conf" ] && [ -e "${WORKDIR}/default.pa" ]; then
         +        install -m 0644 ${WORKDIR}/daemon.conf ${D}${sysconfdir}/pulse/daemon.conf
         +        install -m 0644 ${WORKDIR}/default.pa ${D}${sysconfdir}/pulse/default.pa
         +    fi

[YOCTO #11161]

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/lib/compatlayer/cases/common.py | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index b91da9b..6eb29c1 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -76,4 +76,14 @@ class CommonCompatLayer(OECompatLayerTestCase):
                        (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))
+                try:
+                    recipe, taskname = diff[0].rsplit(':', 1)
+                    output = check_command('Determining signature difference failed.',
+                                           'bitbake-diffsigs --task %s %s --signature %s %s' %
+                                           (recipe, taskname, diff[1], diff[2])).decode('utf-8')
+                except RuntimeError as error:
+                    output = str(error)
+                if output:
+                    msg.extend(['      ' + line for line in output.splitlines()])
+                    msg.append('')
+            self.fail('\n'.join(msg))
-- 
git-series 0.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] yocto-compat-layer: also determine tune flags for each task
  2017-04-07 16:38 [PATCH 0/5] [RFC] yocto-compat-layer: various enhancements + bitbake-diffsigs support Patrick Ohly
                   ` (2 preceding siblings ...)
  2017-04-07 16:38 ` [PATCH 3/5] yocto-compat-layer: include bitbake-diffsigs output Patrick Ohly
@ 2017-04-07 16:38 ` Patrick Ohly
  2017-04-07 16:38 ` [PATCH 5/5] yocto-compat-layer: test signature differences when setting MACHINE Patrick Ohly
  4 siblings, 0 replies; 7+ messages in thread
From: Patrick Ohly @ 2017-04-07 16:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

locked-sigs.inc groups tasks according to their tune flags (allarch,
i586, etc.). Also retrieve that information while getting signatures,
it will be needed to determine when setting a machine changes tasks
that aren't machine-specific.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/lib/compatlayer/__init__.py     |  9 ++++++++-
 scripts/lib/compatlayer/cases/common.py |  2 +-
 scripts/yocto-compat-layer.py           |  2 +-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index b46527a..6130b85 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -224,6 +224,7 @@ def get_signatures(builddir, failsafe=False):
     exclude_recipes = ('meta-world-pkgdata',)
 
     sigs = {}
+    tune2tasks = {}
 
     cmd = 'bitbake '
     if failsafe:
@@ -234,9 +235,14 @@ def get_signatures(builddir, failsafe=False):
     sigs_file = os.path.join(builddir, 'locked-sigs.inc')
 
     sig_regex = re.compile("^(?P<task>.*:.*):(?P<hash>.*) .$")
+    tune_regex = re.compile("(^|\s)SIGGEN_LOCKEDSIGS_t-(?P<tune>\S*)\s*=\s*")
+    current_tune = None
     with open(sigs_file, 'r') as f:
         for line in f.readlines():
             line = line.strip()
+            t = tune_regex.search(line)
+            if t:
+                current_tune = t.group('tune')
             s = sig_regex.match(line)
             if s:
                 exclude = False
@@ -249,11 +255,12 @@ def get_signatures(builddir, failsafe=False):
                     continue
 
                 sigs[s.group('task')] = s.group('hash')
+                tune2tasks.setdefault(current_tune, []).append(s.group('task'))
 
     if not sigs:
         raise RuntimeError('Can\'t load signatures from %s' % sigs_file)
 
-    return sigs
+    return (sigs, tune2tasks)
 
 def get_depgraph(targets=['world']):
     '''
diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 6eb29c1..aa46984 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -33,7 +33,7 @@ class CommonCompatLayer(OECompatLayerTestCase):
 
         # task -> (old signature, new signature)
         sig_diff = {}
-        curr_sigs = get_signatures(self.td['builddir'], failsafe=True)
+        curr_sigs, _ = get_signatures(self.td['builddir'], failsafe=True)
         for task in self.td['sigs']:
             if task in curr_sigs and \
                self.td['sigs'][task] != curr_sigs[task]:
diff --git a/scripts/yocto-compat-layer.py b/scripts/yocto-compat-layer.py
index 22c0c2d..2ebddb6 100755
--- a/scripts/yocto-compat-layer.py
+++ b/scripts/yocto-compat-layer.py
@@ -139,7 +139,7 @@ def main():
         td['bbvars'] = get_bb_vars()
         logger.info('Getting initial signatures ...')
         td['builddir'] = builddir
-        td['sigs'] = get_signatures(td['builddir'])
+        td['sigs'], td['tunetasks'] = get_signatures(td['builddir'])
 
         if not add_layer(bblayersconf, layer, dep_layers, logger):
             logger.info('Skipping %s ???.' % layer['name'])
-- 
git-series 0.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] yocto-compat-layer: test signature differences when setting MACHINE
  2017-04-07 16:38 [PATCH 0/5] [RFC] yocto-compat-layer: various enhancements + bitbake-diffsigs support Patrick Ohly
                   ` (3 preceding siblings ...)
  2017-04-07 16:38 ` [PATCH 4/5] yocto-compat-layer: also determine tune flags for each task Patrick Ohly
@ 2017-04-07 16:38 ` Patrick Ohly
  2017-04-08 10:14   ` Patrick Ohly
  4 siblings, 1 reply; 7+ messages in thread
From: Patrick Ohly @ 2017-04-07 16:38 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

Selecting a machine may only affect the signature of tasks that are specific
to that machine. In other words, when MACHINE=A and MACHINE=B share a recipe
foo and the output of foo, then both machine configurations must build foo
in exactly the same way. Otherwise it is not possible to use both machines
in the same distribution.

This criteria can only be tested by testing different machines in combination,
i.e. one main layer, potentially several additional BSP layers and an explicit
choice of machines:
yocto-compat-layer --additional-layers .../meta-intel --machines intel-corei7-64 imx6slevk -- .../meta-freescale

This is work in progress.

Right now it finds do_build differences in all-arch ca-certificates
because of RDEPENDS_${PN} += "openssl". This causes do_build to run
more often then necessary, but probably needs to be ignored by the
test because fixing it in bitbake would be too hard and is harmless in
practice.

There is also a difference for gdb-cross-x86_64:

$ yocto-compat-layer.py -n --machines intel-corei7-64 intel-core2-32 qemux86 qemux86-64 -- /fast/work/meta-intel
INFO: Detected layers:
INFO: meta-intel: LayerType.BSP, /fast/work/meta-intel
INFO:
INFO: Setting up for meta-intel(LayerType.BSP), /fast/work/meta-intel
INFO: Getting initial bitbake variables ...
INFO: Getting initial signatures ...
INFO: Adding layer meta-intel
INFO: Adding layer meta-intel
INFO: Starting to analyze: meta-intel
INFO: ----------------------------------------------------------------------
...
INFO: ======================================================================
INFO: FAIL: test_machine_signatures (bsp.BSPCompatLayer)
INFO: ----------------------------------------------------------------------
INFO: Traceback (most recent call last):
  File "/fast/work/openembedded-core/scripts/lib/compatlayer/cases/bsp.py", line 78, in test_machine_signatures
    self.fail('\n'.join(msg))
AssertionError: The machines have conflicting signatures for some shared tasks:
   allarch ca-certificates:do_build: 0e3027d1bdab8f84c6db04eedf3daa1d (intel-core2-32) != 95083aeb6a3a8179ffe49fa2459d1ef9 (intel-corei7-64) != a3234da44f5203dd7fcd96bb79b6b164 (qemux86) != f6d4810f5faad32491c4f1a6f28fc900 (qemux86-64)
   allarch cantarell-fonts:do_build: 8b3ed1c153b2bfb49ce2e03814a0075e (intel-core2-32) != d37c32facfc8aae57044cdab3aa7e1b6 (intel-corei7-64) != 1e348f57ef18743d1329ab0b2dc51bd5 (qemux86) != de6cb47d25cae6204067d949bcc47d23 (qemux86-64)
...
   allarch xuser-account:do_build: 81cd497e08b84846f0b70d0f6877b1ff (intel-core2-32) != f038a12c4ac671c286783171bf7a8e8c (intel-corei7-64) != 7fb6d3e9a830959095aebc12df6d3d1e (qemux86) != c3bd05c580cacc6ba06fc909bf6e7656 (qemux86-64)
   x86-64-x86-64 gdb-cross-x86_64:do_build: 68a82197720db0f0dbad8955635bc1c0 (intel-corei7-64) != 5631f484d848541bdbf6a4c1f8117f79 (qemux86-64)
   ...
   x86-64-x86-64 gdb-cross-x86_64:do_configure: 16a264dadaca2897b52871d0c884d564 (intel-corei7-64) != 9345d347d27f735ef441aad853370acb (qemux86-64)
   ...
   x86-64-x86-64 go-cross-x86_64:do_compile: a297977ab6362dc875643a50f597cd44 (intel-corei7-64) != 2a7f9da09ac83e2ff2ef2be21b29c19a (qemux86-64)
   ...

$ bitbake-diffsigs -d -t gdb-cross-x86_64 do_configure -s 16a264dadaca2897b52871d0c884d564 9345d347d27f735ef441aad853370acb
DEBUG: Signature file (previous): /fast/build/nodistro/x86/tmp-glibc/stamps/x86_64-linux/gdb-cross-x86_64/7.12.1-r0.do_configure.sigdata.16a264dadaca2897b52871d0c884d564
DEBUG: Signature file (latest): /fast/build/nodistro/x86/tmp-glibc/stamps/x86_64-linux/gdb-cross-x86_64/7.12.1-r0.do_configure.sigdata.9345d347d27f735ef441aad853370acb
Task dependencies changed from:
...
List of dependencies for variable TUNE_PKGARCH changed from '{'DEFAULTTUNE', 'TUNE_PKGARCH_tune-corei7-64'}' to '{'TUNE_PKGARCH_tune-core2-64', 'DEFAULTTUNE'}'
changed items: {'TUNE_PKGARCH_tune-core2-64', 'TUNE_PKGARCH_tune-corei7-64'}
Dependency on variable TUNE_PKGARCH_tune-core2-64 was added
Dependency on Variable TUNE_PKGARCH_tune-corei7-64 was removed
Variable DEFAULTTUNE value changed from 'corei7-64' to 'core2-64'

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/lib/compatlayer/__init__.py  |  7 ++--
 scripts/lib/compatlayer/cases/bsp.py | 54 ++++++++++++++++++++++++++++-
 scripts/yocto-compat-layer.py        | 20 +++++++++--
 3 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index 6130b85..0d6f4e9 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -215,7 +215,7 @@ def check_command(error_msg, cmd):
         raise RuntimeError(msg)
     return output
 
-def get_signatures(builddir, failsafe=False):
+def get_signatures(builddir, failsafe=False, machine=None):
     import re
 
     # some recipes needs to be excluded like meta-world-pkgdata
@@ -226,7 +226,10 @@ def get_signatures(builddir, failsafe=False):
     sigs = {}
     tune2tasks = {}
 
-    cmd = 'bitbake '
+    cmd = ''
+    if machine:
+        cmd += 'MACHINE=%s ' % machine
+    cmd += 'bitbake '
     if failsafe:
         cmd += '-k '
     cmd += '-S none world'
diff --git a/scripts/lib/compatlayer/cases/bsp.py b/scripts/lib/compatlayer/cases/bsp.py
index 5d9bf93..636367f 100644
--- a/scripts/lib/compatlayer/cases/bsp.py
+++ b/scripts/lib/compatlayer/cases/bsp.py
@@ -3,7 +3,7 @@
 
 import unittest
 
-from compatlayer import LayerType
+from compatlayer import LayerType, get_signatures
 from compatlayer.case import OECompatLayerTestCase
 
 class BSPCompatLayer(OECompatLayerTestCase):
@@ -24,3 +24,55 @@ class BSPCompatLayer(OECompatLayerTestCase):
         self.assertEqual(self.td['bbvars']['MACHINE'], machine,
                 msg="Layer %s modified machine %s -> %s" % \
                     (self.tc.layer['name'], self.td['bbvars']['MACHINE'], machine))
+
+    def test_machine_signatures(self):
+        '''
+        Selecting a machine may only affect the signature of tasks that are specific
+        to that machine. In other words, when MACHINE=A and MACHINE=B share a recipe
+        foo and the output of foo, then both machine configurations must build foo
+        in exactly the same way. Otherwise it is not possible to use both machines
+        in the same distribution.
+
+        This criteria can only be tested by testing different machines in combination,
+        i.e. one main layer, potentially several additional BSP layers and an explicit
+        choice of machines:
+        yocto-compat-layer --additional-layers .../meta-intel --machines intel-corei7-64 imx6slevk -- .../meta-freescale
+        '''
+
+        if not self.td['machines']:
+            self.skipTest('No machines set with --machines.')
+
+        # Collect signatures for all machines that we are testing
+        # and merge that into a hash:
+        # tune -> task -> signature -> list of machines with that combination
+        #
+        # It is an error if any tune/task pair has more than one signature,
+        # because that implies that the machines that caused those different
+        # signatures do not agree on how to execute the task.
+        tunes = {}
+        # Preserve ordering of machines as chosen by the user.
+        for machine in self.td['machines']:
+            curr_sigs, tune2tasks = get_signatures(self.td['builddir'], failsafe=True, machine=machine)
+            # Invert the tune -> [tasks] mapping.
+            tasks2tune = {}
+            for tune, tasks in tune2tasks.items():
+                for task in tasks:
+                    tasks2tune[task] = tune
+            for task, sighash in curr_sigs.items():
+                tunes.setdefault(tasks2tune[task], {}).setdefault(task, {}).setdefault(sighash, []).append(machine)
+
+        msg = []
+        for tune in sorted(tunes.keys()):
+            tasks = tunes[tune]
+            for task in sorted(tasks.keys()):
+                signatures = tasks[task]
+                if len(signatures.keys()) > 1:
+                    # Error! Sort signatures by machines, because the hex values don't mean anything.
+                    # => all-arch adwaita-icon-theme:do_build: 1234... (beaglebone, qemux86) != abcdf... (qemux86-64)
+                    line = '   %s %s: ' % (tune, task)
+                    line += ' != '.join(['%s (%s)' % (signature, ', '.join([machine for machine in signatures[signature]])) for
+                                         signature in sorted(signatures.keys(), key=lambda signature: signatures[signature])])
+                    msg.append(line)
+        if msg:
+            msg.insert(0, 'The machines have conflicting signatures for some shared tasks:')
+            self.fail('\n'.join(msg))
diff --git a/scripts/yocto-compat-layer.py b/scripts/yocto-compat-layer.py
index 2ebddb6..d4fb435 100755
--- a/scripts/yocto-compat-layer.py
+++ b/scripts/yocto-compat-layer.py
@@ -49,6 +49,10 @@ def main():
             help='File to output log (optional)', action='store')
     parser.add_argument('--dependency', nargs="+",
             help='Layers to process for dependencies', action='store')
+    parser.add_argument('--machines', nargs="+",
+            help='List of MACHINEs to be used during testing', action='store')
+    parser.add_argument('--additional-layers', nargs="+",
+            help='List of additional layers to add during testing', action='store')
     parser.add_argument('-n', '--no-auto', help='Disable auto layer discovery',
             action='store_true')
     parser.add_argument('-d', '--debug', help='Enable debug output',
@@ -82,6 +86,7 @@ def main():
     if not layers:
         logger.error("Fail to detect layers")
         return 1
+    additional_layers = detect_layers(args.additional_layers, args.no_auto)
     if args.dependency:
         dep_layers = detect_layers(args.dependency, args.no_auto)
         dep_layers = dep_layers + layers
@@ -128,7 +133,15 @@ def main():
 
         shutil.copyfile(bblayersconf + '.backup', bblayersconf)
 
-        if not add_layer_dependencies(bblayersconf, layer, dep_layers, logger):
+        missing_dependencies = not add_layer_dependencies(bblayersconf, layer, dep_layers, logger)
+        if not missing_dependencies:
+            for additional_layer in additional_layers:
+                if not add_layer_dependencies(bblayersconf, additional_layer, dep_layers, logger):
+                    missing_dependencies = True
+                    break
+        if not add_layer_dependencies(bblayersconf, layer, dep_layers, logger) or \
+           any(map(lambda additional_layer: not add_layer_dependencies(bblayersconf, additional_layer, dep_layers, logger),
+                   additional_layers)):
             logger.info('Skipping %s due to missing dependencies.' % layer['name'])
             results[layer['name']] = None
             results_status[layer['name']] = 'SKIPPED (Missing dependencies)'
@@ -140,8 +153,11 @@ def main():
         logger.info('Getting initial signatures ...')
         td['builddir'] = builddir
         td['sigs'], td['tunetasks'] = get_signatures(td['builddir'])
+        td['machines'] = args.machines
 
-        if not add_layer(bblayersconf, layer, dep_layers, logger):
+        if not add_layer(bblayersconf, layer, dep_layers, logger) or \
+           any(map(lambda additional_layer: not add_layer(bblayersconf, additional_layer, dep_layers, logger),
+                   additional_layers)):
             logger.info('Skipping %s ???.' % layer['name'])
             results[layer['name']] = None
             results_status[layer['name']] = 'SKIPPED (Unknown)'
-- 
git-series 0.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 5/5] yocto-compat-layer: test signature differences when setting MACHINE
  2017-04-07 16:38 ` [PATCH 5/5] yocto-compat-layer: test signature differences when setting MACHINE Patrick Ohly
@ 2017-04-08 10:14   ` Patrick Ohly
  0 siblings, 0 replies; 7+ messages in thread
From: Patrick Ohly @ 2017-04-08 10:14 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

On Fri, 2017-04-07 at 18:38 +0200, Patrick Ohly wrote:
> +    parser.add_argument('--additional-layers', nargs="+",
> +            help='List of additional layers to add during testing',
> action='store')
>      parser.add_argument('-n', '--no-auto', help='Disable auto layer
> discovery',
>              action='store_true')
>      parser.add_argument('-d', '--debug', help='Enable debug output',
> @@ -82,6 +86,7 @@ def main():
>      if not layers:
>          logger.error("Fail to detect layers")
>          return 1
> +    additional_layers = detect_layers(args.additional_layers,
> args.no_auto)
>      if args.dependency:
>          dep_layers = detect_layers(args.dependency, args.no_auto)
>          dep_layers = dep_layers + layers
> @@ -128,7 +133,15 @@ def main():
>  
>          shutil.copyfile(bblayersconf + '.backup', bblayersconf)
>  
> -        if not add_layer_dependencies(bblayersconf, layer,
> dep_layers, logger):
> +        missing_dependencies = not
> add_layer_dependencies(bblayersconf, layer, dep_layers, logger)
> +        if not missing_dependencies:
> +            for additional_layer in additional_layers:
> +                if not add_layer_dependencies(bblayersconf,
> additional_layer, dep_layers, logger):
> +                    missing_dependencies = True
> +                    break
> +        if not add_layer_dependencies(bblayersconf, layer,
> dep_layers, logger) or \
> +           any(map(lambda additional_layer: not
> add_layer_dependencies(bblayersconf, additional_layer, dep_layers,
> logger),
> +                   additional_layers)):
>              logger.info('Skipping %s due to missing dependencies.' %
> layer['name'])
>              results[layer['name']] = None
>              results_status[layer['name']] = 'SKIPPED (Missing
> dependencies)'
> @@ -140,8 +153,11 @@ def main():
>          logger.info('Getting initial signatures ...')
>          td['builddir'] = builddir
>          td['sigs'], td['tunetasks'] = get_signatures(td['builddir'])
> +        td['machines'] = args.machines
>  
> -        if not add_layer(bblayersconf, layer, dep_layers, logger):
> +        if not add_layer(bblayersconf, layer, dep_layers, logger) or
> \
> +           any(map(lambda additional_layer: not
> add_layer(bblayersconf, additional_layer, dep_layers, logger),
> +                   additional_layers)):

Note to myself: this --additional-layers argument should be in its own
commit, and the additional layers need to be added before obtaining the
initial signatures. Otherwise the test_signatures does not test the
changes introduced by the layer under testing, but also all these
additional layers.

That's not the intention - the additional layers basically represents
what's already in a distro before adding the new layer.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.





^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-08 10:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 16:38 [PATCH 0/5] [RFC] yocto-compat-layer: various enhancements + bitbake-diffsigs support Patrick Ohly
2017-04-07 16:38 ` [PATCH 1/5] yocto-compat-layer: fix also other command invocations Patrick Ohly
2017-04-07 16:38 ` [PATCH 2/5] yocto-compat-layer: limit report of signature changes Patrick Ohly
2017-04-07 16:38 ` [PATCH 3/5] yocto-compat-layer: include bitbake-diffsigs output Patrick Ohly
2017-04-07 16:38 ` [PATCH 4/5] yocto-compat-layer: also determine tune flags for each task Patrick Ohly
2017-04-07 16:38 ` [PATCH 5/5] yocto-compat-layer: test signature differences when setting MACHINE Patrick Ohly
2017-04-08 10:14   ` Patrick Ohly

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.