[PATCH 0/1] insane.bbclass: make package_qa_walk() can print all the messages

[PATCH 0/1] insane.bbclass: make package_qa_walk() can print all the messages

[Robert Yang]

The following changes since commit 541315d6c56df6448f64c262f99d43d5c1e9400b:

  update_font_cache: only scan system font directories (2016-01-11 23:23:18 +0000)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib rbt/qa
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=rbt/qa

Robert Yang (1):
  insane.bbclass: make package_qa_walk() can print all the messages

 meta/classes/insane.bbclass |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[PATCH 1/1] insane.bbclass: make package_qa_walk() can print all the messages

[Robert Yang]

* If more than one files have the same QA issue, package_qa_walk() can
  only print the last one, others are overrided, for example:
  messages["host-user-contaminated"] = "foo1"
  messages["host-user-contaminated"] = "foo2"
  Only foo2 will be printed, this patch fixes the issue.

* Remove unused parameter "path" from package_qa_walk()
  The path is a useless parameter in package_qa_walk() since it has been
  redined inside.

[YOCTO #8436]

Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 meta/classes/insane.bbclass |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index bf6a598..bb58f79 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -753,8 +753,15 @@ def package_qa_check_staged(path,d):
 
     return sane
 
+# Merge src dict's content into dest
+def package_qa_merge_dict(dest, src):
+    if len(dest) == len(src) and dest != src:
+        for w in src:
+            if dest[w] != src[w]:
+                dest[w] = src[w] + '\n' + dest[w]
+
 # Walk over all files in a directory and call func
-def package_qa_walk(path, warnfuncs, errorfuncs, skip, package, d):
+def package_qa_walk(warnfuncs, errorfuncs, skip, package, d):
     import oe.qa
 
     #if this will throw an exception, then fix the dict above
@@ -770,9 +777,17 @@ def package_qa_walk(path, warnfuncs, errorfuncs, skip, package, d):
             except:
                 elf = None
             for func in warnfuncs:
+                warnings_orig = warnings.copy()
                 func(path, package, d, elf, warnings)
+                # warnings[foo] = "foo1" might be overrided by
+                # warnings [foo] = "foo2", check and merge.
+                package_qa_merge_dict(warnings, warnings_orig)
             for func in errorfuncs:
+                errors_orig = errors.copy()
                 func(path, package, d, elf, errors)
+                # errors[foo] = "foo1" might be overrided by
+                # errors [foo] = "foo2", check and merge.
+                package_qa_merge_dict(errors, errors_orig)
 
     for w in warnings:
         package_qa_handle_error(w, warnings[w], d)
@@ -1098,8 +1113,7 @@ python do_package_qa () {
             package_qa_handle_error("pkgname",
                     "%s doesn't match the [a-z0-9.+-]+ regex" % package, d)
 
-        path = "%s/%s" % (pkgdest, package)
-        if not package_qa_walk(path, warnchecks, errorchecks, skip, package, d):
+        if not package_qa_walk(warnchecks, errorchecks, skip, package, d):
             walk_sane  = False
         if not package_qa_check_rdepends(package, pkgdest, skip, taskdeps, packages, d):
             rdepends_sane = False
-- 
1.7.9.5

Re: [PATCH 1/1] insane.bbclass: make package_qa_walk() can print all the messages

[Richard Purdie]

On Thu, 2016-01-14 at 18:15 -0800, Robert Yang wrote:
> * If more than one files have the same QA issue, package_qa_walk()
> can
>   only print the last one, others are overrided, for example:
>   messages["host-user-contaminated"] = "foo1"
>   messages["host-user-contaminated"] = "foo2"
>   Only foo2 will be printed, this patch fixes the issue.
> 
> * Remove unused parameter "path" from package_qa_walk()
>   The path is a useless parameter in package_qa_walk() since it has
> been
>   redined inside.
> 
> [YOCTO #8436]
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  meta/classes/insane.bbclass |   20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)

Whilst your proposal is self contained, it doesn't really do anything
to help the readability of this code which is pretty horrific already,
if anything it just makes the code even more convoluted. I'd propose
something more radical, like:

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index acf8639..03028f8 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -189,6 +189,12 @@ def package_qa_handle_error(error_class, error_msg, d):
         bb.note("QA Issue: %s [%s]" % (error_msg, error_class))
     return True
 
+def package_qa_add_message(messages, section, message):
+    if section not in messages:
+        messages[section] = message
+    else:
+        messages[section] = messages[section] + "\n" + message
+
 QAPATHTEST[libexec] = "package_qa_check_libexec"
 def package_qa_check_libexec(path,name, d, elf, messages):
 
@@ -198,7 +204,7 @@ def package_qa_check_libexec(path,name, d, elf, messages):
         return True
 
     if 'libexec' in path.split(os.path.sep):
-        messages["libexec"] = "%s: %s is using libexec please relocate to %s" % (name, package_qa_clean_path(path, d), libexec)
+        package_qa_add_message(messages, "libexec", "%s: %s is using libexec please relocate to %s" % (name, package_qa_clean_path(path, d), libexec))
         return False
 
     return True


This will obviously be a bit more work to make the patch but will IMO
result in a cleaner piece of code as an end result.

Cheers,

Richard