[bitbake-devel,09/11] lib/bb/siggen: add collapsed mode to compare_sigfiles()

Submitted by Paul Eggleton on April 6, 2017, 9:52 p.m. | Patch ID: 138903

Details

Message ID 9a743ad396af642fc20961e38fb7f6c5752f2446.1491514854.git.paul.eggleton@linux.intel.com
State New
Headers show

Commit Message

Paul Eggleton April 6, 2017, 9:52 p.m.
If we just want to drill down to the actual differences then we don't
need to see certain things in the output, e.g. basehash changing or the
signature of dependent tasks. This will be used for comparing signatures
within buildhistory-diff in OE-Core; the default mode as used by
bitbake-diffsigs and bitbake -S printdiff remains unchanged for the
moment.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 lib/bb/siggen.py | 51 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 23 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/bb/siggen.py b/lib/bb/siggen.py
index c6b14c2..3c5d862 100644
--- a/lib/bb/siggen.py
+++ b/lib/bb/siggen.py
@@ -375,7 +375,7 @@  def clean_basepaths_list(a):
         b.append(clean_basepath(x))
     return b
 
-def compare_sigfiles(a, b, recursecb = None):
+def compare_sigfiles(a, b, recursecb=None, collapsed=False):
     output = []
 
     with open(a, 'rb') as f:
@@ -443,7 +443,7 @@  def compare_sigfiles(a, b, recursecb = None):
     if a_data['taskdeps'] != b_data['taskdeps']:
         output.append("Task dependencies changed from:\n%s\nto:\n%s" % (sorted(a_data['taskdeps']), sorted(b_data['taskdeps'])))
 
-    if a_data['basehash'] != b_data['basehash']:
+    if a_data['basehash'] != b_data['basehash'] and not collapsed:
         output.append("basehash changed from %s to %s" % (a_data['basehash'], b_data['basehash']))
 
     changed, added, removed = dict_diff(a_data['gendeps'], b_data['gendeps'], a_data['basewhitelist'] & b_data['basewhitelist'])
@@ -495,24 +495,25 @@  def compare_sigfiles(a, b, recursecb = None):
     if not 'runtaskdeps' in b_data:
          b_data['runtaskdeps'] = {}
 
-    if len(a_data['runtaskdeps']) != len(b_data['runtaskdeps']):
-        changed = ["Number of task dependencies changed"]
-    else:
-        changed = []
-        for idx, task in enumerate(a_data['runtaskdeps']):
-            a = a_data['runtaskdeps'][idx]
-            b = b_data['runtaskdeps'][idx]
-            if a_data['runtaskhashes'][a] != b_data['runtaskhashes'][b]:
-                changed.append("%s with hash %s\n changed to\n%s with hash %s" % (a, a_data['runtaskhashes'][a], b, b_data['runtaskhashes'][b]))
-
-    if changed:
-        clean_a = clean_basepaths_list(a_data['runtaskdeps'])
-        clean_b = clean_basepaths_list(b_data['runtaskdeps'])
-        if clean_a != clean_b:
-            output.append("runtaskdeps changed from %s to %s" % (clean_a, clean_b))
+    if not collapsed:
+        if len(a_data['runtaskdeps']) != len(b_data['runtaskdeps']):
+            changed = ["Number of task dependencies changed"]
         else:
-            output.append("runtaskdeps changed:")
-        output.append("\n".join(changed))
+            changed = []
+            for idx, task in enumerate(a_data['runtaskdeps']):
+                a = a_data['runtaskdeps'][idx]
+                b = b_data['runtaskdeps'][idx]
+                if a_data['runtaskhashes'][a] != b_data['runtaskhashes'][b]:
+                    changed.append("%s with hash %s\n changed to\n%s with hash %s" % (a, a_data['runtaskhashes'][a], b, b_data['runtaskhashes'][b]))
+
+        if changed:
+            clean_a = clean_basepaths_list(a_data['runtaskdeps'])
+            clean_b = clean_basepaths_list(b_data['runtaskdeps'])
+            if clean_a != clean_b:
+                output.append("runtaskdeps changed from %s to %s" % (clean_a, clean_b))
+            else:
+                output.append("runtaskdeps changed:")
+            output.append("\n".join(changed))
 
     if 'runtaskhashes' in a_data and 'runtaskhashes' in b_data:
         a = a_data['runtaskhashes']
@@ -540,13 +541,17 @@  def compare_sigfiles(a, b, recursecb = None):
                     output.append("Dependency on task %s was removed with hash %s" % (clean_basepath(dep), a[dep]))
         if changed:
             for dep in changed:
-                output.append("Hash for dependent task %s changed from %s to %s" % (clean_basepath(dep), a[dep], b[dep]))
+                if not collapsed:
+                    output.append("Hash for dependent task %s changed from %s to %s" % (clean_basepath(dep), a[dep], b[dep]))
                 if callable(recursecb):
-                    # If a dependent hash changed, might as well print the line above and then defer to the changes in 
-                    # that hash since in all likelyhood, they're the same changes this task also saw.
                     recout = recursecb(dep, a[dep], b[dep])
                     if recout:
-                        output = [output[-1]] + recout
+                        if collapsed:
+                            output.extend(recout)
+                        else:
+                            # If a dependent hash changed, might as well print the line above and then defer to the changes in 
+                            # that hash since in all likelyhood, they're the same changes this task also saw.
+                            output = [output[-1]] + recout
 
     a_taint = a_data.get('taint', None)
     b_taint = b_data.get('taint', None)

Comments

Patrick Ohly April 7, 2017, 6:37 a.m.
On Fri, 2017-04-07 at 09:52 +1200, Paul Eggleton wrote:
> If we just want to drill down to the actual differences then we don't
> need to see certain things in the output, e.g. basehash changing or the
> signature of dependent tasks. This will be used for comparing signatures
> within buildhistory-diff in OE-Core; the default mode as used by
> bitbake-diffsigs and bitbake -S printdiff remains unchanged for the
> moment.

This sounds interesting. The bitbake-diffsigs output I got for the
yocto-compat-layer had such useless "basehash changed" comments. Should
we add a mode parameter to bitbake-diffsigs that allows choosing this
collapsed mode?
Paul Eggleton April 7, 2017, 8:26 a.m.
On Friday, 7 April 2017 6:37:49 PM NZST Patrick Ohly wrote:
> On Fri, 2017-04-07 at 09:52 +1200, Paul Eggleton wrote:
> > If we just want to drill down to the actual differences then we don't
> > need to see certain things in the output, e.g. basehash changing or the
> > signature of dependent tasks. This will be used for comparing signatures
> > within buildhistory-diff in OE-Core; the default mode as used by
> > bitbake-diffsigs and bitbake -S printdiff remains unchanged for the
> > moment.
> 
> This sounds interesting. The bitbake-diffsigs output I got for the
> yocto-compat-layer had such useless "basehash changed" comments. Should
> we add a mode parameter to bitbake-diffsigs that allows choosing this
> collapsed mode?

We could, but to be effective the caller needs to handle the output in a 
particular way (see what I've done in the buildhistory-diff patchset I sent 
today).

FYI my plan for 2.4 is to evaluate whether this collapsed mode or something 
like it is better as a default display mode.

Cheers,
Paul