[PATCH 0/5] buildhistory_analysis fixes

[PATCH 0/5] buildhistory_analysis fixes

[Paul Eggleton]

Some fixes for the buildhistory_analysis module to reduce the output
and make it more readable when using the buildhistory-diff tool.

The following changes since commit 5d404fdb36b0535ce758d98408b02134cdbce4ee:

  xserver-kdrive: compile xserver without dtrace support (2012-03-20 15:21:18 +0000)

are available in the git repository at:
  git://git.openembedded.org/openembedded-core-contrib paule/buildhistory-fixes5
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=paule/buildhistory-fixes5

Paul Eggleton (5):
  buildhistory_analysis: use bb.utils.explode_dep_versions
  buildhistory_analysis: hide version number increases in dependencies
  buildhistory_analysis: skip FILELIST changes for dbg packages
  buildhistory_analysis: report if all items removed from a list
  buildhistory_analysis: avoid printing PE/PV/PR more than once

 meta/lib/oe/buildhistory_analysis.py |   75 +++++++++++++++++++++++++++++----
 scripts/buildhistory-diff            |   21 ++++++++-
 2 files changed, 84 insertions(+), 12 deletions(-)

-- 
1.7.5.4

[PATCH 1/5] buildhistory_analysis: use bb.utils.explode_dep_versions

[Paul Eggleton]

Previously this had its own implementation of splitting a list of
packages with optional version e.g. "libncurses-dev (>= 5.9)"; switch to
using the already existing bitbake function which does this as it is
much better tested.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oe/buildhistory_analysis.py |   11 +++++++++--
 scripts/buildhistory-diff            |   21 ++++++++++++++++++---
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/meta/lib/oe/buildhistory_analysis.py b/meta/lib/oe/buildhistory_analysis.py
index d09911c..c0fa339 100644
--- a/meta/lib/oe/buildhistory_analysis.py
+++ b/meta/lib/oe/buildhistory_analysis.py
@@ -13,6 +13,7 @@ import os.path
 import difflib
 import git
 import re
+import bb.utils
 
 
 # How to display fields
@@ -55,8 +56,13 @@ class ChangeRecord:
             prefix = ''
 
         def pkglist_split(pkgs):
-            pkgit = re.finditer(r'[a-zA-Z0-9.+-]+( \([><=]+ [^ )]+\))?', pkgs, 0)
-            pkglist = [p.group(0) for p in pkgit]
+            depver = bb.utils.explode_dep_versions(pkgs)
+            pkglist = []
+            for k,v in depver.iteritems():
+                if v:
+                    pkglist.append("%s (%s)" % (k,v))
+                else:
+                    pkglist.append(k)
             return pkglist
 
         if self.fieldname in list_fields or self.fieldname in list_order_fields:
@@ -68,6 +74,7 @@ class ChangeRecord:
                 bitems = self.newvalue.split()
             removed = list(set(aitems) - set(bitems))
             added = list(set(bitems) - set(aitems))
+
             if removed or added:
                 out = '%s:%s%s' % (self.fieldname, ' removed "%s"' % ' '.join(removed) if removed else '', ' added "%s"' % ' '.join(added) if added else '')
             else:
diff --git a/scripts/buildhistory-diff b/scripts/buildhistory-diff
index 6b344eb..9936a4b 100755
--- a/scripts/buildhistory-diff
+++ b/scripts/buildhistory-diff
@@ -6,7 +6,7 @@
 # Author: Paul Eggleton <paul.eggleton@linux.intel.com>
 
 import sys
-import os.path
+import os
 
 # Ensure PythonGit is installed (buildhistory_analysis needs it)
 try:
@@ -24,8 +24,23 @@ def main():
         sys.exit(1)
 
     # Set path to OE lib dir so we can import the buildhistory_analysis module
-    newpath = os.path.abspath(os.path.dirname(os.path.abspath(sys.argv[0])) + '/../meta/lib')
-    sys.path = sys.path + [newpath]
+    basepath = os.path.abspath(os.path.dirname(os.path.abspath(sys.argv[0])) + '/..')
+    newpath = basepath + '/meta/lib'
+    # Set path to bitbake lib dir so the buildhistory_analysis module can load bb.utils
+    if os.path.exists(basepath + '/bitbake/lib/bb'):
+        bitbakepath = basepath + '/bitbake'
+    else:
+        # look for bitbake/bin dir in PATH
+        bitbakepath = None
+        for pth in os.environ['PATH'].split(':'):
+            if os.path.exists(os.path.join(pth, '../lib/bb')):
+                bitbakepath = os.path.abspath(os.path.join(pth, '..'))
+                break
+        if not bitbakepath:
+            print("Unable to find bitbake by searching parent directory of this script or PATH")
+            sys.exit(1)
+
+    sys.path.extend([newpath, bitbakepath + '/lib'])
     import oe.buildhistory_analysis
 
     if len(sys.argv) > 3:
-- 
1.7.5.4

[PATCH 2/5] buildhistory_analysis: hide version number increases in dependencies

[Paul Eggleton]

If an item in RDEPENDS or RRECOMMENDS only increases in its version
number then don't report it as a change, since we don't care about
it. This significantly reduces the noise after upgrades.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oe/buildhistory_analysis.py |   51 +++++++++++++++++++++++++++++++--
 1 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/meta/lib/oe/buildhistory_analysis.py b/meta/lib/oe/buildhistory_analysis.py
index c0fa339..a828f28 100644
--- a/meta/lib/oe/buildhistory_analysis.py
+++ b/meta/lib/oe/buildhistory_analysis.py
@@ -55,8 +55,7 @@ class ChangeRecord:
         else:
             prefix = ''
 
-        def pkglist_split(pkgs):
-            depver = bb.utils.explode_dep_versions(pkgs)
+        def pkglist_combine(depver):
             pkglist = []
             for k,v in depver.iteritems():
                 if v:
@@ -67,8 +66,9 @@ class ChangeRecord:
 
         if self.fieldname in list_fields or self.fieldname in list_order_fields:
             if self.fieldname in ['RDEPENDS', 'RRECOMMENDS']:
-                aitems = pkglist_split(self.oldvalue)
-                bitems = pkglist_split(self.newvalue)
+                (depvera, depverb) = compare_pkg_lists(self.oldvalue, self.newvalue)
+                aitems = pkglist_combine(depvera)
+                bitems = pkglist_combine(depverb)
             else:
                 aitems = self.oldvalue.split()
                 bitems = self.newvalue.split()
@@ -239,6 +239,45 @@ def compare_lists(alines, blines):
     return filechanges
 
 
+def split_version(s):
+    """Split a version string into its constituent parts (PE, PV, PR)
+    FIXME: this is a duplicate of a new function in bitbake/lib/bb/utils -
+    we should switch to that once we can bump the minimum bitbake version
+    """
+    s = s.strip(" <>=")
+    e = 0
+    if s.count(':'):
+        e = int(s.split(":")[0])
+        s = s.split(":")[1]
+    r = ""
+    if s.count('-'):
+        r = s.rsplit("-", 1)[1]
+        s = s.rsplit("-", 1)[0]
+    v = s
+    return (e, v, r)
+
+
+def compare_pkg_lists(astr, bstr):
+    depvera = bb.utils.explode_dep_versions(astr)
+    depverb = bb.utils.explode_dep_versions(bstr)
+
+    # Strip out changes where the version has increased
+    remove = []
+    for k in depvera:
+        if k in depverb:
+            dva = depvera[k]
+            dvb = depverb[k]
+            if dva != dvb:
+                if bb.utils.vercmp(split_version(dva), split_version(dvb)) < 0:
+                    remove.append(k)
+
+    for k in remove:
+        depvera.pop(k)
+        depverb.pop(k)
+
+    return (depvera, depverb)
+
+
 def compare_dict_blobs(path, ablob, bblob, report_all):
     adict = blob_to_dict(ablob)
     bdict = blob_to_dict(bblob)
@@ -259,6 +298,10 @@ def compare_dict_blobs(path, ablob, bblob, report_all):
                 if percentchg < monitor_numeric_threshold:
                     continue
             elif (not report_all) and key in list_fields:
+                if key in ['RDEPENDS', 'RRECOMMENDS']:
+                    (depvera, depverb) = compare_pkg_lists(astr, bstr)
+                    if depvera == depverb:
+                        continue
                 alist = astr.split()
                 alist.sort()
                 blist = bstr.split()
-- 
1.7.5.4

[PATCH 3/5] buildhistory_analysis: skip FILELIST changes for dbg packages

[Paul Eggleton]

Don't report when files are added or removed from dbg packages unless
it results in the package being empty.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oe/buildhistory_analysis.py |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/meta/lib/oe/buildhistory_analysis.py b/meta/lib/oe/buildhistory_analysis.py
index a828f28..2942f72 100644
--- a/meta/lib/oe/buildhistory_analysis.py
+++ b/meta/lib/oe/buildhistory_analysis.py
@@ -298,6 +298,8 @@ def compare_dict_blobs(path, ablob, bblob, report_all):
                 if percentchg < monitor_numeric_threshold:
                     continue
             elif (not report_all) and key in list_fields:
+                if key == "FILELIST" and path.endswith("-dbg") and bstr.strip() != '':
+                    continue
                 if key in ['RDEPENDS', 'RRECOMMENDS']:
                     (depvera, depverb) = compare_pkg_lists(astr, bstr)
                     if depvera == depverb:
-- 
1.7.5.4

[PATCH 4/5] buildhistory_analysis: report if all items removed from a list

[Paul Eggleton]

If all items have been removed from a list then state that explicitly.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oe/buildhistory_analysis.py |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/meta/lib/oe/buildhistory_analysis.py b/meta/lib/oe/buildhistory_analysis.py
index 2942f72..4f55d9a 100644
--- a/meta/lib/oe/buildhistory_analysis.py
+++ b/meta/lib/oe/buildhistory_analysis.py
@@ -76,7 +76,10 @@ class ChangeRecord:
             added = list(set(bitems) - set(aitems))
 
             if removed or added:
-                out = '%s:%s%s' % (self.fieldname, ' removed "%s"' % ' '.join(removed) if removed else '', ' added "%s"' % ' '.join(added) if added else '')
+                if removed and not bitems:
+                    out = '%s: removed all items "%s"' % (self.fieldname, ' '.join(removed))
+                else:
+                    out = '%s:%s%s' % (self.fieldname, ' removed "%s"' % ' '.join(removed) if removed else '', ' added "%s"' % ' '.join(added) if added else '')
             else:
                 out = '%s changed order' % self.fieldname
         elif self.fieldname in numeric_fields:
-- 
1.7.5.4

[PATCH 5/5] buildhistory_analysis: avoid printing PE/PV/PR more than once

[Paul Eggleton]

Don't print PE/PV/PR changes as related field changes of related field
changes (i.e. only print them once at the top level).

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oe/buildhistory_analysis.py |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/meta/lib/oe/buildhistory_analysis.py b/meta/lib/oe/buildhistory_analysis.py
index 4f55d9a..313416c 100644
--- a/meta/lib/oe/buildhistory_analysis.py
+++ b/meta/lib/oe/buildhistory_analysis.py
@@ -49,8 +49,8 @@ class ChangeRecord:
     def __str__(self):
         return self._str_internal(True)
 
-    def _str_internal(self, pathprefix):
-        if pathprefix:
+    def _str_internal(self, outer):
+        if outer:
             prefix = '%s: ' % self.path
         else:
             prefix = ''
@@ -91,7 +91,7 @@ class ChangeRecord:
                 percentchg = 100
             out = '%s changed from %s to %s (%s%d%%)' % (self.fieldname, self.oldvalue or "''", self.newvalue or "''", '+' if percentchg > 0 else '', percentchg)
         elif self.fieldname in img_monitor_files:
-            if pathprefix:
+            if outer:
                 prefix = 'Changes to %s ' % self.path
             out = '(%s):\n  ' % self.fieldname
             if self.filechanges:
@@ -107,6 +107,8 @@ class ChangeRecord:
 
         if self.related:
             for chg in self.related:
+                if not outer and chg.fieldname in ['PE', 'PV', 'PR']:
+                    continue
                 for line in chg._str_internal(False).splitlines():
                     out += '\n  * %s' % line
 
-- 
1.7.5.4

Re: [PATCH 0/5] buildhistory_analysis fixes

[Richard Purdie]

On Tue, 2012-03-20 at 16:06 +0000, Paul Eggleton wrote:
> Some fixes for the buildhistory_analysis module to reduce the output
> and make it more readable when using the buildhistory-diff tool.
> 
> The following changes since commit 5d404fdb36b0535ce758d98408b02134cdbce4ee:
> 
>   xserver-kdrive: compile xserver without dtrace support (2012-03-20 15:21:18 +0000)
> 
> are available in the git repository at:
>   git://git.openembedded.org/openembedded-core-contrib paule/buildhistory-fixes5
>   http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=paule/buildhistory-fixes5
> 
> Paul Eggleton (5):
>   buildhistory_analysis: use bb.utils.explode_dep_versions
>   buildhistory_analysis: hide version number increases in dependencies
>   buildhistory_analysis: skip FILELIST changes for dbg packages
>   buildhistory_analysis: report if all items removed from a list
>   buildhistory_analysis: avoid printing PE/PV/PR more than once

Merged to master, thanks.

Richard