Patchwork [5/9] Move check that all installed files are shipped into insane.bbclass

login
register
mail settings
Submitter lumag
Date Dec. 13, 2011, 4:19 p.m.
Message ID <1323793193-31090-5-git-send-email-dbaryshkov@gmail.com>
Download mbox | patch
Permalink /patch/16823/
State New
Headers show

Comments

lumag - Dec. 13, 2011, 4:19 p.m.
Checking that all installed files are shipped is in reality a QA check.
It would benefit from mechanisms like ERROR_QA/WARNING_QA. So move it
into insane.bbclass. If some of the files are installed but should not
be shipped for some reasons, one can add them to the variable
IGNORE_UNSHIPPED_FILES.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 meta/classes/insane.bbclass  |   49 ++++++++++++++++++++++++++++++++++++++++-
 meta/classes/package.bbclass |   15 ------------
 2 files changed, 47 insertions(+), 17 deletions(-)
Phil Blundell - Dec. 13, 2011, 4:27 p.m.
On Tue, 2011-12-13 at 20:19 +0400, Dmitry Eremin-Solenikov wrote:
> +def packages_qa_unshipped_files(d):
> +    skip = (d.getVar('INSANE_SKIP_${PN}', True) or "").split()
> +    if "unshipped" in skip:
> +        return False

	[...]

> +    if not packages_qa_unshipped_files(d):
> +        shipped_sane = False
>  
> -    if not walk_sane or not rdepends_sane:
> +    if not walk_sane or not rdepends_sane or not shipped_sane:
>          bb.fatal("QA run found fatal errors. Please consider fixing them.")

Is that really what you want?

p.
lumag - Dec. 13, 2011, 4:52 p.m.
On 12/13/2011 08:27 PM, Phil Blundell wrote:
> On Tue, 2011-12-13 at 20:19 +0400, Dmitry Eremin-Solenikov wrote:
>> +def packages_qa_unshipped_files(d):
>> +    skip = (d.getVar('INSANE_SKIP_${PN}', True) or "").split()
>> +    if "unshipped" in skip:
>> +        return False

Hmm. This really should be "return True". Should I send the fixed 
version of this patch or this change will be picked by appliers?

>
> 	[...]
>
>> +    if not packages_qa_unshipped_files(d):
>> +        shipped_sane = False
>>
>> -    if not walk_sane or not rdepends_sane:
>> +    if not walk_sane or not rdepends_sane or not shipped_sane:
>>           bb.fatal("QA run found fatal errors. Please consider fixing them.")
>
> Is that really what you want?
>
> p.
Richard Purdie - Dec. 15, 2011, 2:55 p.m.
On Tue, 2011-12-13 at 20:19 +0400, Dmitry Eremin-Solenikov wrote:
> Checking that all installed files are shipped is in reality a QA check.
> It would benefit from mechanisms like ERROR_QA/WARNING_QA. So move it
> into insane.bbclass. If some of the files are installed but should not
> be shipped for some reasons, one can add them to the variable
> IGNORE_UNSHIPPED_FILES.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  meta/classes/insane.bbclass  |   49 ++++++++++++++++++++++++++++++++++++++++-
>  meta/classes/package.bbclass |   15 ------------
>  2 files changed, 47 insertions(+), 17 deletions(-)

I like the patch and have been thinking we should do something like
this. One minor comment below.

> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 5726e69..7a8643a 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -100,7 +100,7 @@ def package_qa_get_machine_dict():
>  
> 
>  # Currently not being used by default "desktop"
> -WARN_QA ?= "ldflags useless-rpaths rpaths"
> +WARN_QA ?= "ldflags useless-rpaths rpaths unshipped"
>  ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch la2 pkgconfig la perms"
>  
>  def package_qa_clean_path(path,d):
> @@ -485,6 +485,48 @@ def package_qa_check_rdepends(pkg, pkgdest, skip, d):
>  
>      return sane
>  
> +IGNORE_UNSHIPPED_FILES ??= ""
> +
> +def packages_qa_unshipped_files(d):
> +    skip = (d.getVar('INSANE_SKIP_${PN}', True) or "").split()
> +    if "unshipped" in skip:
> +        return False

How about just check for unshipped in QA_WARN and QA_ERROR here and if
not in either, just return? You cover handling ignoring files just
below.

> +    seen = d.getVar('IGNORE_UNSHIPPED_FILES', True).split()
> +    unshipped = []
> +    dvar = d.getVar('PKGD', True)
> +    destvar = d.getVar('PKGDEST', True)
> +    packages = d.getVar('PACKAGES', True).split()
> +    for p in packages:

Cheers,

Richard

Patch

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 5726e69..7a8643a 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -100,7 +100,7 @@  def package_qa_get_machine_dict():
 
 
 # Currently not being used by default "desktop"
-WARN_QA ?= "ldflags useless-rpaths rpaths"
+WARN_QA ?= "ldflags useless-rpaths rpaths unshipped"
 ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch la2 pkgconfig la perms"
 
 def package_qa_clean_path(path,d):
@@ -485,6 +485,48 @@  def package_qa_check_rdepends(pkg, pkgdest, skip, d):
 
     return sane
 
+IGNORE_UNSHIPPED_FILES ??= ""
+
+def packages_qa_unshipped_files(d):
+    skip = (d.getVar('INSANE_SKIP_${PN}', True) or "").split()
+    if "unshipped" in skip:
+        return False
+
+    seen = d.getVar('IGNORE_UNSHIPPED_FILES', True).split()
+    unshipped = []
+    dvar = d.getVar('PKGD', True)
+    destvar = d.getVar('PKGDEST', True)
+    packages = d.getVar('PACKAGES', True).split()
+    for p in packages:
+        pdir = os.path.join(destvar, p)
+        for root, dirs, files in os.walk(pdir):
+            dir = root[len(pdir):]
+            if not dir:
+                dir = os.sep
+            for f in (files + dirs):
+                path = os.path.join(dir, f)
+                if path not in seen:
+                    seen.append(path)
+
+    for root, dirs, files in os.walk(dvar):
+        dir = root[len(dvar):]
+        if not dir:
+            dir = os.sep
+        for f in (files + dirs):
+            path = os.path.join(dir, f)
+            if path not in seen:
+                unshipped.append(path)
+
+    pn = d.getVar('PN', True)
+
+    if unshipped == []:
+        return True
+
+    ret = package_qa_handle_error("unshipped", "For recipe %s, the following files/directories were installed but not shipped in any package:" % pn, d)
+    for f in unshipped:
+        package_qa_handle_error("unshipped", f, d)
+    return ret
+
 # The PACKAGE FUNC to scan each package
 python do_package_qa () {
     bb.note("DO PACKAGE QA")
@@ -522,6 +564,7 @@  python do_package_qa () {
     g = globals()
     walk_sane = True
     rdepends_sane = True
+    shipped_sane = True
     for package in packages.split():
         skip = (d.getVar('INSANE_SKIP_' + package, True) or "").split()
         if skip:
@@ -546,8 +589,10 @@  python do_package_qa () {
         if not package_qa_check_rdepends(package, pkgdest, skip, d):
             rdepends_sane = False
 
+    if not packages_qa_unshipped_files(d):
+        shipped_sane = False
 
-    if not walk_sane or not rdepends_sane:
+    if not walk_sane or not rdepends_sane or not shipped_sane:
         bb.fatal("QA run found fatal errors. Please consider fixing them.")
     bb.note("DONE with PACKAGE QA")
 }
diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 39c1d4b..fbea9c6 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -957,21 +957,6 @@  python populate_packages () {
 		del localdata
 	os.chdir(workdir)
 
-	unshipped = []
-	for root, dirs, files in os.walk(dvar):
-		dir = root[len(dvar):]
-		if not dir:
-			dir = os.sep
-		for f in (files + dirs):
-			path = os.path.join(dir, f)
-			if ('.' + path) not in seen:
-				unshipped.append(path)
-
-	if unshipped != []:
-		bb.warn("For recipe %s, the following files/directories were installed but not shipped in any package:" % pn)
-		for f in unshipped:
-			bb.warn("  " + f)
-
 	bb.build.exec_func("package_name_hook", d)
 
 	for pkg in package_list: