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