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

PATCH In review Robert Yang 2 participants 3 messages Jan 15, 2016

lore

Share & tools

lore

Robert Yang

author

2016-01-15 02:15 UTC #0 lore raw

cover insane.bbclass: make package_qa_walk() can print all the messages

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(-)

Signature · 1 line

1.7.9.5

Robert Yang

author

2016-01-15 02:15 UTC #1 lore raw

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

* 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 <[email protected]>

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

meta/classes/insane.bbclass +17 -3

index bf6a598..bb58f79 100644
@@ -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

Signature · 1 line

1.7.9.5

Richard Purdie

2016-01-15 12:22 UTC #2 lore raw

Quoted 12 lines · 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.

Quoted 6 lines

[YOCTO #8436]
Signed-off-by Robert Yang <[email protected]>
---
 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:

meta/classes/insane.bbclass +7 -1

index acf8639..03028f8 100644
@@ -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