Patchwork [RFC,v2] package.bbclass: enable the use of package_qa_handle_error

login
register
mail settings
Submitter Saul Wold
Date July 3, 2012, 12:40 a.m.
Message ID <1341276037-32076-1-git-send-email-sgw@linux.intel.com>
Download mbox | patch
Permalink /patch/31009/
State New
Headers show

Comments

Saul Wold - July 3, 2012, 12:40 a.m.
This will allow the reporting of these errors as either WARNINGs (default)
or ERRORs if installed_vs_shipped is added to the ERROR_QA of the policy
file (such as a <distro_name>.conf file.

V2: found the code I had intended to send instead of that other junk,
    was just not watching what I pushed on that one, sorry. (this is edit in
    no in the actual commit message)

Signed-off-by: Saul Wold <sgw@linux.intel.com>
---
 meta/classes/package.bbclass |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Chris Larson - July 3, 2012, 1:55 p.m.
On Mon, Jul 2, 2012 at 5:40 PM, Saul Wold <sgw@linux.intel.com> wrote:
> This will allow the reporting of these errors as either WARNINGs (default)
> or ERRORs if installed_vs_shipped is added to the ERROR_QA of the policy
> file (such as a <distro_name>.conf file.
>
> V2: found the code I had intended to send instead of that other junk,
>     was just not watching what I pushed on that one, sorry. (this is edit in
>     no in the actual commit message)
>
> Signed-off-by: Saul Wold <sgw@linux.intel.com>
> ---
>  meta/classes/package.bbclass |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> index 0b98c6b..ff2ec96 100644
> --- a/meta/classes/package.bbclass
> +++ b/meta/classes/package.bbclass
> @@ -988,9 +988,14 @@ python populate_packages () {
>                                 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)
> +               msg = pn + ": Files/directories were installed but not shipped"
> +               skip = (d.getVar('INSANE_SKIP_' + pn, True) or "").split()
> +               if "installed_vs_shipped" in skip:
> +                       bb.note("Package %s skipping QA tests: installed_vs_shipped" % pn)
> +               else:
> +                       package_qa_handle_error("installed_vs_shipped", msg, d)
> +                       for f in unshipped:
> +                               package_qa_handle_error("installed_vs_shipped", "  " + f, d)

Hmm, I wonder if this is best, or if it should assemble a single
message with newlines separating the files. *thinks*
Saul Wold - July 3, 2012, 5:12 p.m.
On 07/03/2012 06:55 AM, Chris Larson wrote:
> On Mon, Jul 2, 2012 at 5:40 PM, Saul Wold<sgw@linux.intel.com>  wrote:
>> This will allow the reporting of these errors as either WARNINGs (default)
>> or ERRORs if installed_vs_shipped is added to the ERROR_QA of the policy
>> file (such as a<distro_name>.conf file.
>>
>> V2: found the code I had intended to send instead of that other junk,
>>      was just not watching what I pushed on that one, sorry. (this is edit in
>>      no in the actual commit message)
>>
>> Signed-off-by: Saul Wold<sgw@linux.intel.com>
>> ---
>>   meta/classes/package.bbclass |   11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
>> index 0b98c6b..ff2ec96 100644
>> --- a/meta/classes/package.bbclass
>> +++ b/meta/classes/package.bbclass
>> @@ -988,9 +988,14 @@ python populate_packages () {
>>                                  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)
>> +               msg = pn + ": Files/directories were installed but not shipped"
>> +               skip = (d.getVar('INSANE_SKIP_' + pn, True) or "").split()
>> +               if "installed_vs_shipped" in skip:
>> +                       bb.note("Package %s skipping QA tests: installed_vs_shipped" % pn)
>> +               else:
>> +                       package_qa_handle_error("installed_vs_shipped", msg, d)
>> +                       for f in unshipped:
>> +                               package_qa_handle_error("installed_vs_shipped", "  " + f, d)
>
> Hmm, I wonder if this is best, or if it should assemble a single
> message with newlines separating the files. *thinks*

So that would cause only 1 ERROR or WARNING count, vs N ERRORs or 
WARNIGS in the final count for every file that is listed, I think it's 
good to have the larger count it signal's something went wrong if that 
count increases greatly when a recipe is changed.

Could work either way, but I like the exaggerated count.

Sau!
Khem Raj - July 3, 2012, 7:04 p.m.
-Khem

On Jul 3, 2012, at 10:12 AM, Saul Wold <sgw@linux.intel.com> wrote:

> On 07/03/2012 06:55 AM, Chris Larson wrote:
>> On Mon, Jul 2, 2012 at 5:40 PM, Saul Wold<sgw@linux.intel.com>  wrote:
>>> This will allow the reporting of these errors as either WARNINGs (default)
>>> or ERRORs if installed_vs_shipped is added to the ERROR_QA of the policy
>>> file (such as a<distro_name>.conf file.
>>> 
>>> V2: found the code I had intended to send instead of that other junk,
>>>     was just not watching what I pushed on that one, sorry. (this is edit in
>>>     no in the actual commit message)
>>> 
>>> Signed-off-by: Saul Wold<sgw@linux.intel.com>
>>> ---
>>>  meta/classes/package.bbclass |   11 ++++++++---
>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
>>> index 0b98c6b..ff2ec96 100644
>>> --- a/meta/classes/package.bbclass
>>> +++ b/meta/classes/package.bbclass
>>> @@ -988,9 +988,14 @@ python populate_packages () {
>>>                                 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)
>>> +               msg = pn + ": Files/directories were installed but not shipped"
>>> +               skip = (d.getVar('INSANE_SKIP_' + pn, True) or "").split()
>>> +               if "installed_vs_shipped" in skip:
>>> +                       bb.note("Package %s skipping QA tests: installed_vs_shipped" % pn)
>>> +               else:
>>> +                       package_qa_handle_error("installed_vs_shipped", msg, d)
>>> +                       for f in unshipped:
>>> +                               package_qa_handle_error("installed_vs_shipped", "  " + f, d)
>> 
>> Hmm, I wonder if this is best, or if it should assemble a single
>> message with newlines separating the files. *thinks*
> 
> So that would cause only 1 ERROR or WARNING count, vs N ERRORs or WARNIGS in the final count for every file that is listed, I think it's good to have the larger count it signal's something went wrong if that count increases greatly when a recipe is changed.

If its flagged as error then count really doesn't matter and if it is warning then user doesnt care 
But reporting all together may be better

> 
> Could work either way, but I like the exaggerated count.
> 
> Sau!
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Chris Larson - July 3, 2012, 7:46 p.m.
On Tue, Jul 3, 2012 at 12:04 PM, Khem Raj <raj.khem@gmail.com> wrote:
>
>
> -Khem
>
> On Jul 3, 2012, at 10:12 AM, Saul Wold <sgw@linux.intel.com> wrote:
>
>> On 07/03/2012 06:55 AM, Chris Larson wrote:
>>> On Mon, Jul 2, 2012 at 5:40 PM, Saul Wold<sgw@linux.intel.com>  wrote:
>>>> This will allow the reporting of these errors as either WARNINGs (default)
>>>> or ERRORs if installed_vs_shipped is added to the ERROR_QA of the policy
>>>> file (such as a<distro_name>.conf file.
>>>>
>>>> V2: found the code I had intended to send instead of that other junk,
>>>>     was just not watching what I pushed on that one, sorry. (this is edit in
>>>>     no in the actual commit message)
>>>>
>>>> Signed-off-by: Saul Wold<sgw@linux.intel.com>
>>>> ---
>>>>  meta/classes/package.bbclass |   11 ++++++++---
>>>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
>>>> index 0b98c6b..ff2ec96 100644
>>>> --- a/meta/classes/package.bbclass
>>>> +++ b/meta/classes/package.bbclass
>>>> @@ -988,9 +988,14 @@ python populate_packages () {
>>>>                                 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)
>>>> +               msg = pn + ": Files/directories were installed but not shipped"
>>>> +               skip = (d.getVar('INSANE_SKIP_' + pn, True) or "").split()
>>>> +               if "installed_vs_shipped" in skip:
>>>> +                       bb.note("Package %s skipping QA tests: installed_vs_shipped" % pn)
>>>> +               else:
>>>> +                       package_qa_handle_error("installed_vs_shipped", msg, d)
>>>> +                       for f in unshipped:
>>>> +                               package_qa_handle_error("installed_vs_shipped", "  " + f, d)
>>>
>>> Hmm, I wonder if this is best, or if it should assemble a single
>>> message with newlines separating the files. *thinks*
>>
>> So that would cause only 1 ERROR or WARNING count, vs N ERRORs or WARNIGS in the final count for every file that is listed, I think it's good to have the larger count it signal's something went wrong if that count increases greatly when a recipe is changed.
>
> If its flagged as error then count really doesn't matter and if it is warning then user doesnt care
> But reporting all together may be better
>
>>
>> Could work either way, but I like the exaggerated count.

This is true, but conceptually it's a single message. Further, those
messages are tightly bound to the previous message in the multiple
message case, which means there's a particular context involved which
isn't reflected in the messages. They can and will be intertwined with
other messages from other tasks. That isn't the case if it's just one.

Patch

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 0b98c6b..ff2ec96 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -988,9 +988,14 @@  python populate_packages () {
 				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)
+		msg = pn + ": Files/directories were installed but not shipped" 
+		skip = (d.getVar('INSANE_SKIP_' + pn, True) or "").split()
+		if "installed_vs_shipped" in skip:
+			bb.note("Package %s skipping QA tests: installed_vs_shipped" % pn)
+		else:
+			package_qa_handle_error("installed_vs_shipped", msg, d)
+			for f in unshipped:
+				package_qa_handle_error("installed_vs_shipped", "  " + f, d)
 
 	bb.build.exec_func("package_name_hook", d)