[PATCH 0/6] yocto-compat-layer.py: various enhancements

[PATCH 0/6] yocto-compat-layer.py: various enhancements

[Patrick Ohly]

While enhancing the layer structure in intel-iot-refkit I ran into
various cases where enhancements to the tool were
necessary. intel-iot-refkit now has oe-selftests that check all layers
using this tool. In addition, the signature checking utility code is
imported into a custom test that also covers a refkit specific case
(including refkit-config.inc must not change signatures).

Patrick Ohly (6):
  yocto-compat-layer.py: avoid adding layers more than once
  yocto-compat-layer.py: tolerate broken world builds during signature diff
  yocto-compat-layer.py: apply test_signatures to all layers
  yocto-compat-layer.py: add test_world
  yocto-compat-layer.py: allow README with suffix
  yocto-compat-layer.py: make signature check code reusable

 scripts/lib/compatlayer/__init__.py     | 90 +++++++++++++++++++++++++-
 scripts/lib/compatlayer/cases/common.py | 91 ++++++--------------------
 2 files changed, 111 insertions(+), 70 deletions(-)

base-commit: 598e5da5a2af2bd93ad890687dd32009e348fc85
-- 
git-series 0.9.1

[PATCH 1/6] yocto-compat-layer.py: avoid adding layers more than once

[Patrick Ohly]

add_layer_dependencies() might get called more than once, or one of
the layer dependencies might already be present. The function should
not add layers again because doing so can cause warnings like:

  WARNING: Duplicate inclusion for .../meta-openembedded/meta-oe/conf/distro/include/meta_oe_security_flags.inc in .../meta-openembedded/meta-oe/conf/layer.conf

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/lib/compatlayer/__init__.py | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index e35f8c0..eaae4e5 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 re
 import subprocess
 from enum import Enum
 
@@ -189,10 +190,22 @@ def add_layer_dependencies(bblayersconf, layer, layers, logger):
     if layer_depends is None:
         return False
     else:
+        # Don't add a layer that is already present.
+        added = set()
+        output = check_command('Getting existing layers failed.', 'bitbake-layers show-layers').decode('utf-8')
+        for layer, path, pri in re.findall(r'^(\S+) +([^\n]*?) +(\d+)$', output, re.MULTILINE):
+            added.add(path)
+
         for layer_depend in layer_depends:
-            logger.info('Adding layer dependency %s' % layer_depend['name'])
+            name = layer_depend['name']
+            path = layer_depend['path']
+            if path in added:
+                continue
+            else:
+                added.add(path)
+            logger.info('Adding layer dependency %s' % name)
             with open(bblayersconf, 'a+') as f:
-                f.write("\nBBLAYERS += \"%s\"\n" % layer_depend['path'])
+                f.write("\nBBLAYERS += \"%s\"\n" % path)
     return True
 
 def add_layer(bblayersconf, layer, layers, logger):
-- 
git-series 0.9.1

[PATCH 2/6] yocto-compat-layer.py: tolerate broken world builds during signature diff

[Patrick Ohly]

The "test_signatures" test ignored a broken world build when getting
signatures, but the code which then tried to analyze a difference
found by the test didn't, which prevented printing the difference.

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

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index eaae4e5..451e1de 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -290,7 +290,7 @@ def get_signatures(builddir, failsafe=False, machine=None):
 
     return (sigs, tune2tasks)
 
-def get_depgraph(targets=['world']):
+def get_depgraph(targets=['world'], failsafe=False):
     '''
     Returns the dependency graph for the given target(s).
     The dependency graph is taken directly from DepTreeEvent.
@@ -309,6 +309,11 @@ def get_depgraph(targets=['world']):
                 elif isinstance(event, bb.command.CommandCompleted):
                     break
                 elif isinstance(event, bb.event.NoProvider):
+                    if failsafe:
+                        # The event is informational, we will get information about the
+                        # remaining dependencies eventually and thus can ignore this
+                        # here like we do in get_signatures(), if desired.
+                        continue
                     if event._reasons:
                         raise RuntimeError('Nothing provides %s: %s' % (event._item, event._reasons))
                     else:
diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 8eeada9..2dfcbb1 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -50,7 +50,7 @@ class CommonCompatLayer(OECompatLayerTestCase):
             def graph2sig(task):
                 pn, taskname = task.rsplit('.', 1)
                 return pn + ':' + taskname
-            depgraph = get_depgraph()
+            depgraph = get_depgraph(failsafe=True)
             depends = depgraph['tdepends']
 
             # If a task A has a changed signature, but none of its
-- 
git-series 0.9.1

[PATCH 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Patrick Ohly]

Software layers were previously allowed to change signatures, but
that's not desired for those layers either. The rule that a layer
which is "Yocto Compatible 2.0" must not change signatures unless
explicitly requested holds for all kinds of layers.

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

diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 2dfcbb1..fe4936e 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -27,10 +27,6 @@ class CommonCompatLayer(OECompatLayerTestCase):
                       'bitbake -e')
 
     def test_signatures(self):
-        if self.tc.layer['type'] == LayerType.SOFTWARE:
-            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)
-- 
git-series 0.9.1

Re: [PATCH 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Aníbal Limón]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

On 05/29/2017 10:32 AM, Patrick Ohly wrote:
> Software layers were previously allowed to change signatures, but
> that's not desired for those layers either. The rule that a layer
> which is "Yocto Compatible 2.0" must not change signatures unless
> explicitly requested holds for all kinds of layers.

If i understand correctly now a software layer can't change a signature
but how do we handle this?, currently if a software layer is added and
has bbappends or newer version of a recipe the signature will change.

May be we need to postpone this validation removal until we have a
manner to avoid a software layer automatically change the signatures.

Cheers,
Anibal


> 
> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
>  scripts/lib/compatlayer/cases/common.py | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
> index 2dfcbb1..fe4936e 100644
> --- a/scripts/lib/compatlayer/cases/common.py
> +++ b/scripts/lib/compatlayer/cases/common.py
> @@ -27,10 +27,6 @@ class CommonCompatLayer(OECompatLayerTestCase):
>                        'bitbake -e')
>  
>      def test_signatures(self):
> -        if self.tc.layer['type'] == LayerType.SOFTWARE:
> -            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)
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Patrick Ohly]

On Mon, 2017-05-29 at 11:13 -0500, Aníbal Limón wrote:
> On 05/29/2017 10:32 AM, Patrick Ohly wrote:
> > Software layers were previously allowed to change signatures, but
> > that's not desired for those layers either. The rule that a layer
> > which is "Yocto Compatible 2.0" must not change signatures unless
> > explicitly requested holds for all kinds of layers.
> 
> If i understand correctly now a software layer can't change a signature
> but how do we handle this?, currently if a software layer is added and
> has bbappends or newer version of a recipe the signature will change.

We've touched on this topic in the "[Openembedded-architecture] Yocto
Compatible 2.0 + signature changes" mail thread. I had asked about
software layers and Richard said "I do think we need to do this [strict
signature check also for software layers]"

For .bbappends, the solution from that mail thread is to turn
DISTRO_FEATURES into overrides and make changes in the .bbappend depend
on that, or include the .bbappend code only for certain features. That
reminds me, I still need to turn my prototype code for that into a
specific patch for bitbake.conf...

Changing software versions is indeed a bit more problematic. One could
argue that layers shouldn't fight over who provides a certain recipe in
the first place. If they do, perhaps the "additional layers" (= the ones
with lower priority) need to provide explicit .inc files with
PREFERRED_VERSION assignments without which the overriding recipes
aren't used?

Also remember that this is only a problem for layers who want to be
"Yocto Compatible 2.0". Other, private layers can simply ignore the
rules and do what needs to be done.

Alternatively, we can make the stricter checking of software layers
optional in the tool. Not sure what that'll mean for the "Yocto
Compatible 2.0" - all layers are created compatible, some more than
others? ;-}

-- 
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.

Re: [PATCH 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Aníbal Limón]

[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]



On 05/29/2017 02:18 PM, Patrick Ohly wrote:
> On Mon, 2017-05-29 at 11:13 -0500, Aníbal Limón wrote:
>> On 05/29/2017 10:32 AM, Patrick Ohly wrote:
>>> Software layers were previously allowed to change signatures, but
>>> that's not desired for those layers either. The rule that a layer
>>> which is "Yocto Compatible 2.0" must not change signatures unless
>>> explicitly requested holds for all kinds of layers.
>>
>> If i understand correctly now a software layer can't change a signature
>> but how do we handle this?, currently if a software layer is added and
>> has bbappends or newer version of a recipe the signature will change.
> 
> We've touched on this topic in the "[Openembedded-architecture] Yocto
> Compatible 2.0 + signature changes" mail thread. I had asked about
> software layers and Richard said "I do think we need to do this [strict
> signature check also for software layers]"

Sorry, i did not see that thread, i need to be more aware on the arch ML. :)

> 
> For .bbappends, the solution from that mail thread is to turn
> DISTRO_FEATURES into overrides and make changes in the .bbappend depend
> on that, or include the .bbappend code only for certain features. That
> reminds me, I still need to turn my prototype code for that into a
> specific patch for bitbake.conf...

Yes, sounds good to me.

> 
> Changing software versions is indeed a bit more problematic. One could
> argue that layers shouldn't fight over who provides a certain recipe in
> the first place. If they do, perhaps the "additional layers" (= the ones
> with lower priority) need to provide explicit .inc files with
> PREFERRED_VERSION assignments without which the overriding recipes
> aren't used?

Yes, so in this case will need to set automatically the preferred
versions to oe-core recipes, and then let the distro layer to change the
preferred version in this way when test a distro layer with oe-core the
signatures not will change only when add the combo of distro layer +
software layer.

> 
> Also remember that this is only a problem for layers who want to be
> "Yocto Compatible 2.0". Other, private layers can simply ignore the
> rules and do what needs to be done.

Agree,

> 
> Alternatively, we can make the stricter checking of software layers
> optional in the tool. Not sure what that'll mean for the "Yocto
> Compatible 2.0" - all layers are created compatible, some more than
> others? ;-}

If we have the decision to include this checking in software layers, i
will go to make it as a default mode with the option to disable it.

Cheers,
Anibal
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Christopher Larson]

[-- Attachment #1: Type: text/plain, Size: 1662 bytes --]

On Mon, May 29, 2017 at 12:26 PM, Aníbal Limón <anibal.limon@linux.intel.com
> wrote:

> >
> > Changing software versions is indeed a bit more problematic. One could
> > argue that layers shouldn't fight over who provides a certain recipe in
> > the first place. If they do, perhaps the "additional layers" (= the ones
> > with lower priority) need to provide explicit .inc files with
> > PREFERRED_VERSION assignments without which the overriding recipes
> > aren't used?
>
> Yes, so in this case will need to set automatically the preferred
> versions to oe-core recipes, and then let the distro layer to change the
> preferred version in this way when test a distro layer with oe-core the
> signatures not will change only when add the combo of distro layer +
> software layer.


Currently, if you add a high priority layer with an older version recipe,
it will change the default selected recipe, lacking a PREFERRED_VERSION.
You can’t use DEFAULT_PREFERENCE to make the new old version of a recipe
not be used, since bitbake treats layer priority as more important than
default preference, so adding a .inc won’t do much good, since you can’t
make the recipe not be preferred by default without a preferred version set
to the current oe-core version. You could add such a line to the
layer.conf, but then you’re hardcoding the oe-core recipe version into a
separate layer, which is pretty ugly. I don’t htink we should be enforcing
this signature change without resolving this.
-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 2163 bytes --]

Re: [PATCH 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Patrick Ohly]

On Mon, 2017-05-29 at 13:14 -0700, Christopher Larson wrote:
> 
> On Mon, May 29, 2017 at 12:26 PM, Aníbal Limón
> <anibal.limon@linux.intel.com> wrote:
>         >
>         > Changing software versions is indeed a bit more problematic.
>         One could
>         > argue that layers shouldn't fight over who provides a
>         certain recipe in
>         > the first place. If they do, perhaps the "additional
>         layers" (= the ones
>         > with lower priority) need to provide explicit .inc files
>         with
>         > PREFERRED_VERSION assignments without which the overriding
>         recipes
>         > aren't used?
>         
>         Yes, so in this case will need to set automatically the
>         preferred
>         versions to oe-core recipes, and then let the distro layer to
>         change the
>         preferred version in this way when test a distro layer with
>         oe-core the
>         signatures not will change only when add the combo of distro
>         layer +
>         software layer.
> 
> Currently, if you add a high priority layer with an older version
> recipe, it will change the default selected recipe, lacking a
> PREFERRED_VERSION. You can’t use DEFAULT_PREFERENCE to make the new
> old version of a recipe not be used, since bitbake treats layer
> priority as more important than default preference, so adding a .inc
> won’t do much good, since you can’t make the recipe not be preferred
> by default without a preferred version set to the current oe-core
> version. You could add such a line to the layer.conf, but then you’re
> hardcoding the oe-core recipe version into a separate layer, which is
> pretty ugly. I don’t htink we should be enforcing this signature
> change without resolving this.

The key question is whether overriding existing recipes implicitly
merely by adding a layer is considered acceptable. Any opinions about
that?

We agree that changing via .bbappend unconditionally is bad, even for a
software layer, right?

That means that the signature check must be applied, but with an
exception for wholesale recipe replacements. I can think of one
solution, and that is to artificially lower the priority of the
collections in the new layer so that the recipes in it cannot override
the ones in the base configuration anymore. But this is kind of a hack
and might lead to broken world builds (for example, the layer overrides
recipe A with a version which adds an RPROVIDES foo, and another recipe
B in the layer RDEPENDS on foo).

The alternative would be to do a deep dive into the signature diff and
filter out those changes which are caused by a replaced recipe. This
could be fairly tricky to implement, in particular as these changes then
also affect all recipes depending on the replace recipe. The code behind
bitbake-diffsigs has known limitations regarding recursively identifying
changes [1], so this probably would have to be solved first.

[1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=6428#c4

-- 
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.

[PATCH 4/6] yocto-compat-layer.py: add test_world

[Patrick Ohly]

"test_signatures" ignores wold build breakage for the sake of
reporting differences also when a world build is broken. Therefore we
need a dedicated test that a world build at least theoretically can
proceed without obvious parse time problems (dependencies, parse
errors, dangling .bbappends, etc.).

This is similar to the BSP test_machine_world. The difference is
that test_world doesn't change the MACHINE.

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

diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index fe4936e..b9cd656 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -26,6 +26,15 @@ class CommonCompatLayer(OECompatLayerTestCase):
         check_command('Layer %s failed to show environment.' % self.tc.layer['name'],
                       'bitbake -e')
 
+    def test_world(self):
+        '''
+        "bitbake world" is expected to work. test_signatures does not cover that
+        because it is more lenient and ignores recipes in a world build that
+        are not actually buildable, so here we fail when "bitbake -S none world"
+        fails.
+        '''
+        get_signatures(self.td['builddir'], failsafe=False)
+
     def test_signatures(self):
         # task -> (old signature, new signature)
         sig_diff = {}
-- 
git-series 0.9.1

[PATCH 5/6] yocto-compat-layer.py: allow README with suffix

[Patrick Ohly]

It may be useful to append a suffix denoting the file format. For
example, README.rst is rendered differently when viewed on Github, and
also helps editors to switch to a mode more suitable for the format.

The tests uses a file pattern to find the README file(s) and treats
the one with the shortest name as the main one which must not be
empty.

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

diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index b9cd656..284d1cc 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -1,6 +1,7 @@
 # Copyright (C) 2017 Intel Corporation
 # Released under the MIT license (see COPYING.MIT)
 
+import glob
 import os
 import unittest
 from compatlayer import get_signatures, LayerType, check_command, get_depgraph
@@ -8,15 +9,20 @@ from compatlayer.case import OECompatLayerTestCase
 
 class CommonCompatLayer(OECompatLayerTestCase):
     def test_readme(self):
-        readme_file = os.path.join(self.tc.layer['path'], 'README')
-        self.assertTrue(os.path.isfile(readme_file),
-                msg="Layer doesn't contains README file.")
+        # The top-level README file may have a suffix (like README.rst or README.txt).
+        readme_files = glob.glob(os.path.join(self.tc.layer['path'], 'README*'))
+        self.assertTrue(len(readme_files) > 0,
+                        msg="Layer doesn't contains README file.")
 
+        # There might be more than one file matching the file pattern above
+        # (for example, README.rst and README-COPYING.rst). The one with the shortest
+        # name is considered the "main" one.
+        readme_file = sorted(readme_files)[0]
         data = ''
         with open(readme_file, 'r') as f:
             data = f.read()
         self.assertTrue(data,
-                msg="Layer contains README file but is empty.")
+                msg="Layer contains a README file but it is empty.")
 
     def test_parse(self):
         check_command('Layer %s failed to parse.' % self.tc.layer['name'],
-- 
git-series 0.9.1

[PATCH 6/6] yocto-compat-layer.py: make signature check code reusable

[Patrick Ohly]

This moves the main content of test_signature into a helper
function. It can be reused by arbitrary tests that need to do
a before/after signature comparison. Long-term this might even
be useful in oeqa itself.

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

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index 451e1de..7197e85 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -324,3 +324,69 @@ def get_depgraph(targets=['world'], failsafe=False):
     if depgraph is None:
         raise RuntimeError('Could not retrieve the depgraph.')
     return depgraph
+
+def compare_signatures(old_sigs, curr_sigs):
+    '''
+    Compares the result of two get_signatures() calls. Returns None if no
+    problems found, otherwise a string that can be used as additional
+    explanation in self.fail().
+    '''
+    # task -> (old signature, new signature)
+    sig_diff = {}
+    for task in old_sigs:
+        if task in curr_sigs and \
+           old_sigs[task] != curr_sigs[task]:
+            sig_diff[task] = (old_sigs[task], curr_sigs[task])
+
+    if not sig_diff:
+        return None
+
+    # 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(failsafe=True)
+    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('%d signatures changed, initial differences (first hash before, second after):' %
+               len(sig_diff))
+    for diff in sorted(sig_diff_filtered):
+        recipe, taskname = diff[0].rsplit(':', 1)
+        cmd = 'bitbake-diffsigs --task %s %s --signature %s %s' % \
+              (recipe, taskname, diff[1], diff[2])
+        msg.append('   %s: %s -> %s' % diff)
+        msg.append('      %s' % cmd)
+        try:
+            output = check_command('Determining signature difference failed.',
+                                   cmd).decode('utf-8')
+        except RuntimeError as error:
+            output = str(error)
+        if output:
+            msg.extend(['      ' + line for line in output.splitlines()])
+            msg.append('')
+    return '\n'.join(msg)
diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 284d1cc..b8d7c3a 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -4,7 +4,7 @@
 import glob
 import os
 import unittest
-from compatlayer import get_signatures, LayerType, check_command, get_depgraph
+from compatlayer import get_signatures, LayerType, check_command, get_depgraph, compare_signatures
 from compatlayer.case import OECompatLayerTestCase
 
 class CommonCompatLayer(OECompatLayerTestCase):
@@ -42,61 +42,7 @@ class CommonCompatLayer(OECompatLayerTestCase):
         get_signatures(self.td['builddir'], failsafe=False)
 
     def test_signatures(self):
-        # task -> (old signature, new signature)
-        sig_diff = {}
         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]:
-                sig_diff[task] = (self.td['sigs'][task], curr_sigs[task])
-
-        if sig_diff:
-            # 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(failsafe=True)
-            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):
-                recipe, taskname = diff[0].rsplit(':', 1)
-                cmd = 'bitbake-diffsigs --task %s %s --signature %s %s' % \
-                      (recipe, taskname, diff[1], diff[2])
-                msg.append('   %s: %s -> %s' % diff)
-                msg.append('      %s' % cmd)
-                try:
-                    output = check_command('Determining signature difference failed.',
-                                           cmd).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))
+        msg = compare_signatures(self.td['sigs'], curr_sigs)
+        if msg is not None:
+            self.fail('Adding layer %s changed signatures.\n%s' % (self.tc.layer['name'], msg))
-- 
git-series 0.9.1

Re: [PATCH 0/6] yocto-compat-layer.py: various enhancements

[Aníbal Limón]

[-- Attachment #1: Type: text/plain, Size: 1332 bytes --]

Most of the patches looks good, i only have comments on:

- yocto-compat-layer.py: apply test_signatures to all layers

For the rest:

Acked-by: Aníbal Limón <anibal.limon@linux.intel.com>

Cheers,
Anibal

On 05/29/2017 10:32 AM, Patrick Ohly wrote:
> While enhancing the layer structure in intel-iot-refkit I ran into
> various cases where enhancements to the tool were
> necessary. intel-iot-refkit now has oe-selftests that check all layers
> using this tool. In addition, the signature checking utility code is
> imported into a custom test that also covers a refkit specific case
> (including refkit-config.inc must not change signatures).
> 
> Patrick Ohly (6):
>   yocto-compat-layer.py: avoid adding layers more than once
>   yocto-compat-layer.py: tolerate broken world builds during signature diff
>   yocto-compat-layer.py: apply test_signatures to all layers
>   yocto-compat-layer.py: add test_world
>   yocto-compat-layer.py: allow README with suffix
>   yocto-compat-layer.py: make signature check code reusable
> 
>  scripts/lib/compatlayer/__init__.py     | 90 +++++++++++++++++++++++++-
>  scripts/lib/compatlayer/cases/common.py | 91 ++++++--------------------
>  2 files changed, 111 insertions(+), 70 deletions(-)
> 
> base-commit: 598e5da5a2af2bd93ad890687dd32009e348fc85
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH v2 0/6] yocto-compat-layer.py: various enhancements

[Patrick Ohly]

While enhancing the layer structure in intel-iot-refkit I ran into
various cases where enhancements to the tool were
necessary. intel-iot-refkit now has oe-selftests that check all layers
using this tool. In addition, the signature checking utility code is
imported into a custom test that also covers a refkit specific case
(including refkit-config.inc must not change signatures).

Changes in V2:
- turn signature checking in software layers on/off via command line flags

Patrick Ohly (6):
  yocto-compat-layer.py: avoid adding layers more than once
  yocto-compat-layer.py: tolerate broken world builds during signature diff
  yocto-compat-layer.py: apply test_signatures to all layers
  yocto-compat-layer.py: add test_world
  yocto-compat-layer.py: allow README with suffix
  yocto-compat-layer.py: make signature check code reusable

 scripts/lib/compatlayer/__init__.py     | 90 ++++++++++++++++++++++++-
 scripts/lib/compatlayer/cases/common.py | 92 +++++++-------------------
 scripts/lib/compatlayer/context.py      |  3 +-
 scripts/yocto-compat-layer.py           | 12 ++-
 4 files changed, 125 insertions(+), 72 deletions(-)

base-commit: 20b3574749420a1fef2cb2e0579584453dd4c5c5
-- 
git-series 0.9.1

[PATCH v2 1/6] yocto-compat-layer.py: avoid adding layers more than once

[Patrick Ohly]

add_layer_dependencies() might get called more than once, or one of
the layer dependencies might already be present. The function should
not add layers again because doing so can cause warnings like:

  WARNING: Duplicate inclusion for .../meta-openembedded/meta-oe/conf/distro/include/meta_oe_security_flags.inc in .../meta-openembedded/meta-oe/conf/layer.conf

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/lib/compatlayer/__init__.py | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index e35f8c0..eaae4e5 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 re
 import subprocess
 from enum import Enum
 
@@ -189,10 +190,22 @@ def add_layer_dependencies(bblayersconf, layer, layers, logger):
     if layer_depends is None:
         return False
     else:
+        # Don't add a layer that is already present.
+        added = set()
+        output = check_command('Getting existing layers failed.', 'bitbake-layers show-layers').decode('utf-8')
+        for layer, path, pri in re.findall(r'^(\S+) +([^\n]*?) +(\d+)$', output, re.MULTILINE):
+            added.add(path)
+
         for layer_depend in layer_depends:
-            logger.info('Adding layer dependency %s' % layer_depend['name'])
+            name = layer_depend['name']
+            path = layer_depend['path']
+            if path in added:
+                continue
+            else:
+                added.add(path)
+            logger.info('Adding layer dependency %s' % name)
             with open(bblayersconf, 'a+') as f:
-                f.write("\nBBLAYERS += \"%s\"\n" % layer_depend['path'])
+                f.write("\nBBLAYERS += \"%s\"\n" % path)
     return True
 
 def add_layer(bblayersconf, layer, layers, logger):
-- 
git-series 0.9.1

Re: [PATCH v2 1/6] yocto-compat-layer.py: avoid adding layers more than once

[Christopher Larson]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Tue, Jun 27, 2017 at 8:33 AM, Patrick Ohly <patrick.ohly@intel.com>
wrote:

> add_layer_dependencies() might get called more than once, or one of
> the layer dependencies might already be present. The function should
> not add layers again because doing so can cause warnings like:
>
>   WARNING: Duplicate inclusion for .../meta-openembedded/meta-oe/conf
> /distro/include/meta_oe_security_flags.inc in
> .../meta-openembedded/meta-oe/conf/layer.conf
>
> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
>

I’m curious about why this isn’t either just calling out to `bitbake-layers
add-layer` or using one of the existing functions we have for modifying
bblayers, rather than doing its own +=.
-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 1490 bytes --]

Re: [PATCH v2 1/6] yocto-compat-layer.py: avoid adding layers more than once

[Patrick Ohly]

On Tue, 2017-06-27 at 15:46 -0700, Christopher Larson wrote:
> 
> On Tue, Jun 27, 2017 at 8:33 AM, Patrick Ohly <patrick.ohly@intel.com>
> wrote:
>         add_layer_dependencies() might get called more than once, or
>         one of
>         the layer dependencies might already be present. The function
>         should
>         not add layers again because doing so can cause warnings like:
>         
>           WARNING: Duplicate inclusion
>         for .../meta-openembedded/meta-oe/conf/distro/include/meta_oe_security_flags.inc in .../meta-openembedded/meta-oe/conf/layer.conf
>         
>         Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
>         ---
> 
> I’m curious about why this isn’t either just calling out to
> `bitbake-layers add-layer` or using one of the existing functions we
> have for modifying bblayers, rather than doing its own +=.

I don't know exactly, Mark implemented it like that. My guess is that
more control was needed over which layers get added. For example,
"bitbake-layers add-layer" only adds one layer, but does not recursively
add layers it depends on. This could be improved of course.

-- 
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.

Re: [PATCH v2 1/6] yocto-compat-layer.py: avoid adding layers more than once

[Mark Hatle]

On 6/28/17 9:50 AM, Patrick Ohly wrote:
> On Tue, 2017-06-27 at 15:46 -0700, Christopher Larson wrote:
>>
>> On Tue, Jun 27, 2017 at 8:33 AM, Patrick Ohly <patrick.ohly@intel.com>
>> wrote:
>>         add_layer_dependencies() might get called more than once, or
>>         one of
>>         the layer dependencies might already be present. The function
>>         should
>>         not add layers again because doing so can cause warnings like:
>>         
>>           WARNING: Duplicate inclusion
>>         for .../meta-openembedded/meta-oe/conf/distro/include/meta_oe_security_flags.inc in .../meta-openembedded/meta-oe/conf/layer.conf
>>         
>>         Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
>>         ---
>>
>> I’m curious about why this isn’t either just calling out to
>> `bitbake-layers add-layer` or using one of the existing functions we
>> have for modifying bblayers, rather than doing its own +=.
> 
> I don't know exactly, Mark implemented it like that. My guess is that
> more control was needed over which layers get added. For example,
> "bitbake-layers add-layer" only adds one layer, but does not recursively
> add layers it depends on. This could be improved of course.
> 

Correct.  Dependencies had to be evaluated for the layers to be added properly.
The existing add-layer does not process dependencies, it simply adds the layer.

The code was also based on the prior version which did it directly and not via
bitbake add-layer.  I did not change that behavior, but simply added the ability
to evaluate the dependency set and include the necessary combinations for the
layer to work properly.

I have no objections to using bitbake-layers add-layer.  (Note, I'm working on
general enhancements that may allow this to process dependencies, but I'm not
sure when I will have that work done.)

--Mark

[PATCH v2 2/6] yocto-compat-layer.py: tolerate broken world builds during signature diff

[Patrick Ohly]

The "test_signatures" test ignored a broken world build when getting
signatures, but the code which then tried to analyze a difference
found by the test didn't, which prevented printing the difference.

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

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index eaae4e5..451e1de 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -290,7 +290,7 @@ def get_signatures(builddir, failsafe=False, machine=None):
 
     return (sigs, tune2tasks)
 
-def get_depgraph(targets=['world']):
+def get_depgraph(targets=['world'], failsafe=False):
     '''
     Returns the dependency graph for the given target(s).
     The dependency graph is taken directly from DepTreeEvent.
@@ -309,6 +309,11 @@ def get_depgraph(targets=['world']):
                 elif isinstance(event, bb.command.CommandCompleted):
                     break
                 elif isinstance(event, bb.event.NoProvider):
+                    if failsafe:
+                        # The event is informational, we will get information about the
+                        # remaining dependencies eventually and thus can ignore this
+                        # here like we do in get_signatures(), if desired.
+                        continue
                     if event._reasons:
                         raise RuntimeError('Nothing provides %s: %s' % (event._item, event._reasons))
                     else:
diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 8eeada9..2dfcbb1 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -50,7 +50,7 @@ class CommonCompatLayer(OECompatLayerTestCase):
             def graph2sig(task):
                 pn, taskname = task.rsplit('.', 1)
                 return pn + ':' + taskname
-            depgraph = get_depgraph()
+            depgraph = get_depgraph(failsafe=True)
             depends = depgraph['tdepends']
 
             # If a task A has a changed signature, but none of its
-- 
git-series 0.9.1

[PATCH v2 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Patrick Ohly]

Software layers were previously allowed to change signatures, but
that's not desired for those layers either. The rule that a layer
which is "Yocto Compatible 2.0" must not change signatures unless
explicitly requested holds for all kinds of layers.

However, as this is something that software layers might not be able
to do right away, testing for signature changes in software layers can
be disabled. It's on by default, as that was Richard's
recommendation. Whether that should change needs further discussion as
part of finalizing "Yocto Compatible 2.0".

As it might still change, the tool now has both a with/without
parameter so that users of the tool can choose the desired behavior
without being affected by future changes to the default.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 scripts/lib/compatlayer/cases/common.py |  5 +++--
 scripts/lib/compatlayer/context.py      |  3 ++-
 scripts/yocto-compat-layer.py           | 12 +++++++++---
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 2dfcbb1..a1cdbab 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -27,8 +27,9 @@ class CommonCompatLayer(OECompatLayerTestCase):
                       'bitbake -e')
 
     def test_signatures(self):
-        if self.tc.layer['type'] == LayerType.SOFTWARE:
-            raise unittest.SkipTest("Layer %s isn't BSP or DISTRO one." \
+        if self.tc.layer['type'] == LayerType.SOFTWARE and \
+           not self.tc.test_software_layer_signatures:
+            raise unittest.SkipTest("Not testing for signature changes in a software layer %s." \
                      % self.tc.layer['name'])
 
         # task -> (old signature, new signature)
diff --git a/scripts/lib/compatlayer/context.py b/scripts/lib/compatlayer/context.py
index 4932238..7811d4a 100644
--- a/scripts/lib/compatlayer/context.py
+++ b/scripts/lib/compatlayer/context.py
@@ -9,6 +9,7 @@ import re
 from oeqa.core.context import OETestContext
 
 class CompatLayerTestContext(OETestContext):
-    def __init__(self, td=None, logger=None, layer=None):
+    def __init__(self, td=None, logger=None, layer=None, test_software_layer_signatures=True):
         super(CompatLayerTestContext, self).__init__(td, logger)
         self.layer = layer
+        self.test_software_layer_signatures = test_software_layer_signatures
diff --git a/scripts/yocto-compat-layer.py b/scripts/yocto-compat-layer.py
index 30c55a9..a16974f 100755
--- a/scripts/yocto-compat-layer.py
+++ b/scripts/yocto-compat-layer.py
@@ -30,12 +30,12 @@ CASES_PATHS = [os.path.join(os.path.abspath(os.path.dirname(__file__)),
                 'lib', 'compatlayer', 'cases')]
 logger = scriptutils.logger_create(PROGNAME, stream=sys.stdout)
 
-def test_layer_compatibility(td, layer):
+def test_layer_compatibility(td, layer, test_software_layer_signatures):
     from compatlayer.context import CompatLayerTestContext
     logger.info("Starting to analyze: %s" % layer['name'])
     logger.info("----------------------------------------------------------------------")
 
-    tc = CompatLayerTestContext(td=td, logger=logger, layer=layer)
+    tc = CompatLayerTestContext(td=td, logger=logger, layer=layer, test_software_layer_signatures=test_software_layer_signatures)
     tc.loadTests(CASES_PATHS)
     return tc.runTests()
 
@@ -53,6 +53,12 @@ def main():
             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')
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument('--with-software-layer-signature-check', action='store_true', dest='test_software_layer_signatures',
+                       default=True,
+                       help='check that software layers do not change signatures (on by default)')
+    group.add_argument('--without-software-layer-signature-check', action='store_false', dest='test_software_layer_signatures',
+                       help='disable signature checking for software layers')
     parser.add_argument('-n', '--no-auto', help='Disable auto layer discovery',
             action='store_true')
     parser.add_argument('-d', '--debug', help='Enable debug output',
@@ -173,7 +179,7 @@ def main():
             layers_tested = layers_tested + 1
             continue
 
-        result = test_layer_compatibility(td, layer)
+        result = test_layer_compatibility(td, layer, args.test_software_layer_signatures)
         results[layer['name']] = result
         results_status[layer['name']] = 'PASS' if results[layer['name']].wasSuccessful() else 'FAIL'
         layers_tested = layers_tested + 1
-- 
git-series 0.9.1

Re: [PATCH v2 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Mark Hatle]

On 6/27/17 5:33 PM, Patrick Ohly wrote:
> Software layers were previously allowed to change signatures, but
> that's not desired for those layers either. The rule that a layer
> which is "Yocto Compatible 2.0" must not change signatures unless
> explicitly requested holds for all kinds of layers.
> 
> However, as this is something that software layers might not be able
> to do right away, testing for signature changes in software layers can
> be disabled. It's on by default, as that was Richard's
> recommendation. Whether that should change needs further discussion as
> part of finalizing "Yocto Compatible 2.0".
> 
> As it might still change, the tool now has both a with/without
> parameter so that users of the tool can choose the desired behavior
> without being affected by future changes to the default.

How would you regulate the behavior of a software layer that is doing bbappends
or similar to a system provided component.

My understanding is that the inclusion of the software layer SHOULD be able to
modify the existing recipe behavior, as there currently is no other way to
'control' activation.

This is different then BSP and distro layers, which have a specific activation
(and thus override) behavior.

--Mark

> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
>  scripts/lib/compatlayer/cases/common.py |  5 +++--
>  scripts/lib/compatlayer/context.py      |  3 ++-
>  scripts/yocto-compat-layer.py           | 12 +++++++++---
>  3 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
> index 2dfcbb1..a1cdbab 100644
> --- a/scripts/lib/compatlayer/cases/common.py
> +++ b/scripts/lib/compatlayer/cases/common.py
> @@ -27,8 +27,9 @@ class CommonCompatLayer(OECompatLayerTestCase):
>                        'bitbake -e')
>  
>      def test_signatures(self):
> -        if self.tc.layer['type'] == LayerType.SOFTWARE:
> -            raise unittest.SkipTest("Layer %s isn't BSP or DISTRO one." \
> +        if self.tc.layer['type'] == LayerType.SOFTWARE and \
> +           not self.tc.test_software_layer_signatures:
> +            raise unittest.SkipTest("Not testing for signature changes in a software layer %s." \
>                       % self.tc.layer['name'])
>  
>          # task -> (old signature, new signature)
> diff --git a/scripts/lib/compatlayer/context.py b/scripts/lib/compatlayer/context.py
> index 4932238..7811d4a 100644
> --- a/scripts/lib/compatlayer/context.py
> +++ b/scripts/lib/compatlayer/context.py
> @@ -9,6 +9,7 @@ import re
>  from oeqa.core.context import OETestContext
>  
>  class CompatLayerTestContext(OETestContext):
> -    def __init__(self, td=None, logger=None, layer=None):
> +    def __init__(self, td=None, logger=None, layer=None, test_software_layer_signatures=True):
>          super(CompatLayerTestContext, self).__init__(td, logger)
>          self.layer = layer
> +        self.test_software_layer_signatures = test_software_layer_signatures
> diff --git a/scripts/yocto-compat-layer.py b/scripts/yocto-compat-layer.py
> index 30c55a9..a16974f 100755
> --- a/scripts/yocto-compat-layer.py
> +++ b/scripts/yocto-compat-layer.py
> @@ -30,12 +30,12 @@ CASES_PATHS = [os.path.join(os.path.abspath(os.path.dirname(__file__)),
>                  'lib', 'compatlayer', 'cases')]
>  logger = scriptutils.logger_create(PROGNAME, stream=sys.stdout)
>  
> -def test_layer_compatibility(td, layer):
> +def test_layer_compatibility(td, layer, test_software_layer_signatures):
>      from compatlayer.context import CompatLayerTestContext
>      logger.info("Starting to analyze: %s" % layer['name'])
>      logger.info("----------------------------------------------------------------------")
>  
> -    tc = CompatLayerTestContext(td=td, logger=logger, layer=layer)
> +    tc = CompatLayerTestContext(td=td, logger=logger, layer=layer, test_software_layer_signatures=test_software_layer_signatures)
>      tc.loadTests(CASES_PATHS)
>      return tc.runTests()
>  
> @@ -53,6 +53,12 @@ def main():
>              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')
> +    group = parser.add_mutually_exclusive_group()
> +    group.add_argument('--with-software-layer-signature-check', action='store_true', dest='test_software_layer_signatures',
> +                       default=True,
> +                       help='check that software layers do not change signatures (on by default)')
> +    group.add_argument('--without-software-layer-signature-check', action='store_false', dest='test_software_layer_signatures',
> +                       help='disable signature checking for software layers')
>      parser.add_argument('-n', '--no-auto', help='Disable auto layer discovery',
>              action='store_true')
>      parser.add_argument('-d', '--debug', help='Enable debug output',
> @@ -173,7 +179,7 @@ def main():
>              layers_tested = layers_tested + 1
>              continue
>  
> -        result = test_layer_compatibility(td, layer)
> +        result = test_layer_compatibility(td, layer, args.test_software_layer_signatures)
>          results[layer['name']] = result
>          results_status[layer['name']] = 'PASS' if results[layer['name']].wasSuccessful() else 'FAIL'
>          layers_tested = layers_tested + 1
> 

Re: [PATCH v2 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Patrick Ohly]

On Wed, 2017-06-28 at 11:08 +0200, Mark Hatle wrote:
> On 6/27/17 5:33 PM, Patrick Ohly wrote:
> > Software layers were previously allowed to change signatures, but
> > that's not desired for those layers either. The rule that a layer
> > which is "Yocto Compatible 2.0" must not change signatures unless
> > explicitly requested holds for all kinds of layers.
> > 
> > However, as this is something that software layers might not be able
> > to do right away, testing for signature changes in software layers can
> > be disabled. It's on by default, as that was Richard's
> > recommendation. Whether that should change needs further discussion as
> > part of finalizing "Yocto Compatible 2.0".
> > 
> > As it might still change, the tool now has both a with/without
> > parameter so that users of the tool can choose the desired behavior
> > without being affected by future changes to the default.
> 
> How would you regulate the behavior of a software layer that is doing bbappends
> or similar to a system provided component.

By adding a PACKAGECONFIG that is off by default?

But I haven't tried this and whether it influences task signatures. Do
you have a specific example?

Regarding these patches, is it okay to merge them as they are now?
Without them, we cannot test software layers for signature changes, so
won't know how much of a problem it would be.

The tool and "Yocto Compatible 2.0" are work in progress, so there's
still time to refine it after merging. My motivation for getting them
merged already now is a) to make the change available to others and b)
to use the strict version of the check in refkit (where we currently
satisfy the criteria).

-- 
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.

Re: [PATCH v2 3/6] yocto-compat-layer.py: apply test_signatures to all layers

[Mark Hatle]

On 6/28/17 11:33 AM, Patrick Ohly wrote:
> On Wed, 2017-06-28 at 11:08 +0200, Mark Hatle wrote:
>> On 6/27/17 5:33 PM, Patrick Ohly wrote:
>>> Software layers were previously allowed to change signatures, but
>>> that's not desired for those layers either. The rule that a layer
>>> which is "Yocto Compatible 2.0" must not change signatures unless
>>> explicitly requested holds for all kinds of layers.
>>>
>>> However, as this is something that software layers might not be able
>>> to do right away, testing for signature changes in software layers can
>>> be disabled. It's on by default, as that was Richard's
>>> recommendation. Whether that should change needs further discussion as
>>> part of finalizing "Yocto Compatible 2.0".
>>>
>>> As it might still change, the tool now has both a with/without
>>> parameter so that users of the tool can choose the desired behavior
>>> without being affected by future changes to the default.
>>
>> How would you regulate the behavior of a software layer that is doing bbappends
>> or similar to a system provided component.
> 
> By adding a PACKAGECONFIG that is off by default?
> 
> But I haven't tried this and whether it influences task signatures. Do
> you have a specific example?
> 
> Regarding these patches, is it okay to merge them as they are now?
> Without them, we cannot test software layers for signature changes, so
> won't know how much of a problem it would be.
> 
> The tool and "Yocto Compatible 2.0" are work in progress, so there's
> still time to refine it after merging. My motivation for getting them
> merged already now is a) to make the change available to others and b)
> to use the strict version of the check in refkit (where we currently
> satisfy the criteria).
> 

Yes, this was a comment about the commit message, not the technical content of
the patch.

I'm not sure that PACKAGECONFIG can be used to control this type of stuff.  Perhaps?

In many cases, the bbappends seem to add patches or change the way the
compile/install work.  Often these are integration specific for something -- so
there is no 'easy' way to use PACKAGECONFIG to control this.

The only way I can think of is using a PACKAGECONFIG to define a an override.
But as far as I know just adding the evaluation to the overrides and such will
affect the task hashes in some way.

--Mark

[PATCH v2 4/6] yocto-compat-layer.py: add test_world

[Patrick Ohly]

"test_signatures" ignores wold build breakage for the sake of
reporting differences also when a world build is broken. Therefore we
need a dedicated test that a world build at least theoretically can
proceed without obvious parse time problems (dependencies, parse
errors, dangling .bbappends, etc.).

This is similar to the BSP test_machine_world. The difference is
that test_world doesn't change the MACHINE.

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

diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index a1cdbab..ede002d 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -26,6 +26,15 @@ class CommonCompatLayer(OECompatLayerTestCase):
         check_command('Layer %s failed to show environment.' % self.tc.layer['name'],
                       'bitbake -e')
 
+    def test_world(self):
+        '''
+        "bitbake world" is expected to work. test_signatures does not cover that
+        because it is more lenient and ignores recipes in a world build that
+        are not actually buildable, so here we fail when "bitbake -S none world"
+        fails.
+        '''
+        get_signatures(self.td['builddir'], failsafe=False)
+
     def test_signatures(self):
         if self.tc.layer['type'] == LayerType.SOFTWARE and \
            not self.tc.test_software_layer_signatures:
-- 
git-series 0.9.1

[PATCH v2 5/6] yocto-compat-layer.py: allow README with suffix

[Patrick Ohly]

It may be useful to append a suffix denoting the file format. For
example, README.rst is rendered differently when viewed on Github, and
also helps editors to switch to a mode more suitable for the format.

The tests uses a file pattern to find the README file(s) and treats
the one with the shortest name as the main one which must not be
empty.

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

diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index ede002d..4c8a543 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -1,6 +1,7 @@
 # Copyright (C) 2017 Intel Corporation
 # Released under the MIT license (see COPYING.MIT)
 
+import glob
 import os
 import unittest
 from compatlayer import get_signatures, LayerType, check_command, get_depgraph
@@ -8,15 +9,20 @@ from compatlayer.case import OECompatLayerTestCase
 
 class CommonCompatLayer(OECompatLayerTestCase):
     def test_readme(self):
-        readme_file = os.path.join(self.tc.layer['path'], 'README')
-        self.assertTrue(os.path.isfile(readme_file),
-                msg="Layer doesn't contains README file.")
+        # The top-level README file may have a suffix (like README.rst or README.txt).
+        readme_files = glob.glob(os.path.join(self.tc.layer['path'], 'README*'))
+        self.assertTrue(len(readme_files) > 0,
+                        msg="Layer doesn't contains README file.")
 
+        # There might be more than one file matching the file pattern above
+        # (for example, README.rst and README-COPYING.rst). The one with the shortest
+        # name is considered the "main" one.
+        readme_file = sorted(readme_files)[0]
         data = ''
         with open(readme_file, 'r') as f:
             data = f.read()
         self.assertTrue(data,
-                msg="Layer contains README file but is empty.")
+                msg="Layer contains a README file but it is empty.")
 
     def test_parse(self):
         check_command('Layer %s failed to parse.' % self.tc.layer['name'],
-- 
git-series 0.9.1

[PATCH v2 6/6] yocto-compat-layer.py: make signature check code reusable

[Patrick Ohly]

This moves the main content of test_signature into a helper
function. It can be reused by arbitrary tests that need to do
a before/after signature comparison. Long-term this might even
be useful in oeqa itself.

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

diff --git a/scripts/lib/compatlayer/__init__.py b/scripts/lib/compatlayer/__init__.py
index 451e1de..7197e85 100644
--- a/scripts/lib/compatlayer/__init__.py
+++ b/scripts/lib/compatlayer/__init__.py
@@ -324,3 +324,69 @@ def get_depgraph(targets=['world'], failsafe=False):
     if depgraph is None:
         raise RuntimeError('Could not retrieve the depgraph.')
     return depgraph
+
+def compare_signatures(old_sigs, curr_sigs):
+    '''
+    Compares the result of two get_signatures() calls. Returns None if no
+    problems found, otherwise a string that can be used as additional
+    explanation in self.fail().
+    '''
+    # task -> (old signature, new signature)
+    sig_diff = {}
+    for task in old_sigs:
+        if task in curr_sigs and \
+           old_sigs[task] != curr_sigs[task]:
+            sig_diff[task] = (old_sigs[task], curr_sigs[task])
+
+    if not sig_diff:
+        return None
+
+    # 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(failsafe=True)
+    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('%d signatures changed, initial differences (first hash before, second after):' %
+               len(sig_diff))
+    for diff in sorted(sig_diff_filtered):
+        recipe, taskname = diff[0].rsplit(':', 1)
+        cmd = 'bitbake-diffsigs --task %s %s --signature %s %s' % \
+              (recipe, taskname, diff[1], diff[2])
+        msg.append('   %s: %s -> %s' % diff)
+        msg.append('      %s' % cmd)
+        try:
+            output = check_command('Determining signature difference failed.',
+                                   cmd).decode('utf-8')
+        except RuntimeError as error:
+            output = str(error)
+        if output:
+            msg.extend(['      ' + line for line in output.splitlines()])
+            msg.append('')
+    return '\n'.join(msg)
diff --git a/scripts/lib/compatlayer/cases/common.py b/scripts/lib/compatlayer/cases/common.py
index 4c8a543..55e8ba4 100644
--- a/scripts/lib/compatlayer/cases/common.py
+++ b/scripts/lib/compatlayer/cases/common.py
@@ -4,7 +4,7 @@
 import glob
 import os
 import unittest
-from compatlayer import get_signatures, LayerType, check_command, get_depgraph
+from compatlayer import get_signatures, LayerType, check_command, get_depgraph, compare_signatures
 from compatlayer.case import OECompatLayerTestCase
 
 class CommonCompatLayer(OECompatLayerTestCase):
@@ -47,61 +47,7 @@ class CommonCompatLayer(OECompatLayerTestCase):
             raise unittest.SkipTest("Not testing for signature changes in a software layer %s." \
                      % 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 in curr_sigs and \
-               self.td['sigs'][task] != curr_sigs[task]:
-                sig_diff[task] = (self.td['sigs'][task], curr_sigs[task])
-
-        if sig_diff:
-            # 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(failsafe=True)
-            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):
-                recipe, taskname = diff[0].rsplit(':', 1)
-                cmd = 'bitbake-diffsigs --task %s %s --signature %s %s' % \
-                      (recipe, taskname, diff[1], diff[2])
-                msg.append('   %s: %s -> %s' % diff)
-                msg.append('      %s' % cmd)
-                try:
-                    output = check_command('Determining signature difference failed.',
-                                           cmd).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))
+        msg = compare_signatures(self.td['sigs'], curr_sigs)
+        if msg is not None:
+            self.fail('Adding layer %s changed signatures.\n%s' % (self.tc.layer['name'], msg))
-- 
git-series 0.9.1