Patchwork [17/17] package.bbclass: make unshipped files a fatal error

login
register
mail settings
Submitter lumag
Date Sept. 21, 2011, 6:40 p.m.
Message ID <1316630404-10336-17-git-send-email-dbaryshkov@gmail.com>
Download mbox | patch
Permalink /patch/11907/
State New, archived
Headers show

Comments

lumag - Sept. 21, 2011, 6:40 p.m.
I belive that "files were installed but not shipped in any package"
message should become a fatal error. While it's true that sometimes some
files are generated by do_install task, which aren't necessary for our
target systems. However in generic I think this is a very important and
usually overlooked QA sign. E.g. recently I've found/fixed some bugs in
eglibc packaging only due to making this a fatal error. Sometimes new
versions of a software add new files, but then they are sometimes
ignored, as this messages can be easily missed. You can come with more
examples if you'd like to.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 meta/classes/package.bbclass |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Richard Purdie - Sept. 22, 2011, 5:48 a.m.
On Wed, 2011-09-21 at 22:40 +0400, Dmitry Eremin-Solenikov wrote:
> I belive that "files were installed but not shipped in any package"
> message should become a fatal error. While it's true that sometimes some
> files are generated by do_install task, which aren't necessary for our
> target systems. However in generic I think this is a very important and
> usually overlooked QA sign. E.g. recently I've found/fixed some bugs in
> eglibc packaging only due to making this a fatal error. Sometimes new
> versions of a software add new files, but then they are sometimes
> ignored, as this messages can be easily missed. You can come with more
> examples if you'd like to.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  meta/classes/package.bbclass |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index a9c510d..f4a535e 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -948,6 +948,7 @@ python populate_packages () {
>  		bb.warn("For recipe %s, the following files were installed but not shipped in any package:" % pn)
>  		for f in unshipped:
>  			bb.warn("  " + f)
> +		bb.fatal("Unshipped files found")
>  
>  	bb.build.exec_func("package_name_hook", d)

One of thee post-release things I want to sort out is the QA warnings
and I add this warning under that category. We introduced the ideas of
warnings to error and warn on into the QA class already and I'd like to
be able to control this warning in that way using the same variables.

Also, rather than being bb.fatal, this should be bb.error. An Error
message will allow the build to complete but will set the bitbake exit
code to show when errors have occurred. This means errors still light
the autobuilder up red when they occur so we're usually pretty good
about noticing and removing error messages.

Cheers,

Richard
lumag - Sept. 22, 2011, 6:36 a.m.
On 9/22/11, Richard Purdie <richard.purdie@linuxfoundation.org> wrote:
> On Wed, 2011-09-21 at 22:40 +0400, Dmitry Eremin-Solenikov wrote:
>> I belive that "files were installed but not shipped in any package"
>> message should become a fatal error. While it's true that sometimes some
>> files are generated by do_install task, which aren't necessary for our
>> target systems. However in generic I think this is a very important and
>> usually overlooked QA sign. E.g. recently I've found/fixed some bugs in
>> eglibc packaging only due to making this a fatal error. Sometimes new
>> versions of a software add new files, but then they are sometimes
>> ignored, as this messages can be easily missed. You can come with more
>> examples if you'd like to.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> ---
>>  meta/classes/package.bbclass |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
>> index a9c510d..f4a535e 100644
>> --- a/meta/classes/package.bbclass
>> +++ b/meta/classes/package.bbclass
>> @@ -948,6 +948,7 @@ python populate_packages () {
>>  		bb.warn("For recipe %s, the following files were installed but not
>> shipped in any package:" % pn)
>>  		for f in unshipped:
>>  			bb.warn("  " + f)
>> +		bb.fatal("Unshipped files found")
>>
>>  	bb.build.exec_func("package_name_hook", d)
>
> One of thee post-release things I want to sort out is the QA warnings
> and I add this warning under that category. We introduced the ideas of
> warnings to error and warn on into the QA class already and I'd like to
> be able to control this warning in that way using the same variables.

Nice idea! On the rights of a feature request:
It would be nice to allow user to:
1) specify the severity of the QA problems (warn, error, fatal) on a
global + per-qa-check level
2) specify an override for some of the checks, so that the packager
can specify that
e.g. QA check for .so links in a non-dev packages should be ignored
for cgroups-pam package for /lib/security/pam*.so lib. (I get the idea
from Debian lintian overrides. Hope
I was able to write what I mean).

>
> Also, rather than being bb.fatal, this should be bb.error. An Error
> message will allow the build to complete but will set the bitbake exit
> code to show when errors have occurred. This means errors still light
> the autobuilder up red when they occur so we're usually pretty good
> about noticing and removing error messages.

Hmmm. If it's a bb.error, is it logged somewhere except in the task log file?
Because even if I see a red state in my oestats installation (BTW:
what is the current way to track build statuses?), I don't think I'll
be able to dig through multi-page oestats task output for some big
image looking for erroneous-but-not-failed items.
Paul Eggleton - Sept. 22, 2011, 9:45 a.m.
On Thursday 22 September 2011 07:36:23 Dmitry Eremin-Solenikov wrote:
> Nice idea! On the rights of a feature request:
> It would be nice to allow user to:
> 1) specify the severity of the QA problems (warn, error, fatal) on a
> global + per-qa-check level

To affect the severity of warnings on a global basis you should be able to use 
ERROR_QA / WARN_QA, although I have not tried this.

> 2) specify an override for some of the checks, so that the packager
> can specify that
> e.g. QA check for .so links in a non-dev packages should be ignored
> for cgroups-pam package for /lib/security/pam*.so lib.

For per-package, you can set for example INSANE_SKIP_${PN} += "dev-so" to 
disable the .so links in non-dev package QA warning.

More details (e.g. the name for each QA check) can be found in insane.bbclass.

Cheers,
Paul
lumag - Sept. 22, 2011, 9:50 a.m.
Hello,

On 09/22/2011 01:45 PM, Paul Eggleton wrote:
> On Thursday 22 September 2011 07:36:23 Dmitry Eremin-Solenikov wrote:
>> Nice idea! On the rights of a feature request:
>> It would be nice to allow user to:
>> 1) specify the severity of the QA problems (warn, error, fatal) on a
>> global + per-qa-check level
>
> To affect the severity of warnings on a global basis you should be able to use
> ERROR_QA / WARN_QA, although I have not tried this.
>
>> 2) specify an override for some of the checks, so that the packager
>> can specify that
>> e.g. QA check for .so links in a non-dev packages should be ignored
>> for cgroups-pam package for /lib/security/pam*.so lib.
>
> For per-package, you can set for example INSANE_SKIP_${PN} += "dev-so" to
> disable the .so links in non-dev package QA warning.
>
> More details (e.g. the name for each QA check) can be found in insane.bbclass.

This is a too big handle. Ideally I'd like to be able to whitelist on 
the file level, rather than on the package/test level.

Patch

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index a9c510d..f4a535e 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -948,6 +948,7 @@  python populate_packages () {
 		bb.warn("For recipe %s, the following files were installed but not shipped in any package:" % pn)
 		for f in unshipped:
 			bb.warn("  " + f)
+		bb.fatal("Unshipped files found")
 
 	bb.build.exec_func("package_name_hook", d)