Re: [yocto-autobuilder] [RFC] Add support for generate bitbake error reports

["Aníbal Limón" <anibal.limon@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 7447 bytes --]



On 06/15/2016 08:19 AM, Joshua G Lock wrote:
> On Tue, 2016-06-14 at 12:03 -0500, Aníbal Limón wrote:
>>
>> On 06/14/2016 10:57 AM, Aníbal Limón wrote:
>>>
>>>
>>> On 06/14/2016 10:09 AM, Lock, Joshua G wrote:
>>>> On Thu, 2016-06-09 at 16:23 -0500, Aníbal Limón wrote:
>>>>> Hi folks,
>>>>>
>>>>> Currently we support to send error reports to
>>>>> errors.yoctoproject.org
>>>>> about failing tasks on bitbake using SendErrorReport buildstep
>>>>> but we
>>>>> have a lack of bitbake related errors like exceptions.
>>>>>
>>>>> A bug exist to implement this support into Error report web
>>>>> [1], i'm
>>>>> working on it but for generate bitbake error reports there's a
>>>>> need
>>>>> of
>>>>> some process monitoring the bitbake output in this case the
>>>>> Yocto
>>>>> Autobuilder.
>>>>>
>>>>> This email is for review my current implementation for generate
>>>>> bitbake
>>>>> error reports in the Autobuilder [2] next i'll try to explain
>>>>> how it
>>>>> works.
>>>>>
>>>>> I aadded a BitbakeShellCommand [3] class for use in the
>>>>> buildsteps
>>>>> that
>>>>> executes bitbake the main objective of this class is to have
>>>>> common
>>>>> operations to be made in bitbake commands like create error
>>>>> reports
>>>>> if
>>>>> fails.
>>>>>
>>>>> For create error reports this class add an stdio observer to
>>>>> look at
>>>>> bitbake output and if bitbake fails review the bitbake output
>>>>> for
>>>>> identify if the failure is an non-related task error [4]. If
>>>>> the
>>>>> observer found bitbake error creates an Error report with the
>>>>> information in the master controller.
>>>>>
>>>>> In order to send the bitbake error to Error report web the
>>>>> controller
>>>>> transfer the report to the worker using a new DownloadDirectory
>>>>> implementation that i made [5], the SendErrorReport buildstep
>>>>> works
>>>>> on
>>>>> the worker side so it's easy to transfer the reports from
>>>>> master to
>>>>> worker instead of send it by master.
>>>>>
>>>>> Finally to complete with the job of have the bitbake errors
>>>>> reports
>>>>> the
>>>>> Error report web changes need  (i'm working on it) to be
>>>>> integrated
>>>>> first in order to don't break anything.
>>>>>
>>>>> Please review it and provide me feedback.
>>>>
>>>> This would've been much easier to review as a series of patches.
>>>>
>>>> After a quick read via the gitweb the series as a whole looks
>>>> good.
>>>>
>>>> A couple of review comments:
>>>>
>>>> In a04b41d37c29191318386455d8d958ff815a3a10 "lib/buildsteps.py:
>>>> Add
>>>> BitbakeLogLineObserver for BitbakeShellCommands." you have a
>>>> comment
>>>>
>>>> "discard line that are not errors and line that is recipe task
>>>> errors"
>>>>
>>>> but the lines are not actually discarded, afaict  from a cursory
>>>> read
>>>> through they aren't used in the rest of the series
>>>> unless errors['bitbake'] == True? 
>>>>
>>>> Could we move the error['log'].append() to after the if statement
>>>> which
>>>> checks whether this is a bitbake error?
>>>
>>> Yes and i'll update the commit message to be more consistent.
>>>
>>>>
>>>> Minor nit, in caf472bc696053227825c5a102feef3e17574ba2
>>>> "lib/buildsteps:
>>>> BitbakeShellCommand add support for create error reports"
>>>> in get_error_report_bitbake_dir() you use both " and ' for
>>>> strings.
>>>>
>>>> Same in 40279597615c49bc4f4f067cbab937584b626671
>>>
>>> I'll fix the typos for only use "".
>>>
>>> 	alimon
>>>
>> I updated the branch with the changes suggested now at,
>>
>> http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder/log/?h=co
>> ntrib/alimon/devel
> 
> Is that the right location? For example I still see log lines being
> appended to error['log'] *before* the check to discard lines in:
> 
> http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder/commit/?h=c
> ontrib/alimon/devel&id=742e5bf33934c4ec9de9136a854e190aaf8ba0b3
> 
> +    def _handleError(self, line):
> +        if not hasattr(self.step, 'errors'):
> +            self.step.errors = {}
> +            self.step.errors['bitbake'] = False
> +            self.step.errors['log'] = []
> +
> +        # save all the log for be able to get report variables like
> +        # machine, target, distro, etc
> +        self.step.errors['log'].append(line)
> +
> +        # discard line that are not errors and line that
> +        # is recipe task errors
> +        if (not self.rexp_error.match(line)) or \
> +                self.rexp_pnpv_error.match(line) or \
> +                self.rexp_task_error.match(line) or \
> +                self.rexp_log_error.match(line):
> +            return
> +
> +        # if not match recipe task type is a bitbake exception/error
> +        self.step.errors['bitbake'] = True
> 
> whereas I think we only need to save log lines when it's a bitbake
> error, i.e:
> 
> +    def _handleError(self, line):
> +        if not hasattr(self.step, 'errors'):
> +            self.step.errors = {}
> +            self.step.errors['bitbake'] = False
> +            self.step.errors['log'] = []
> +
> +        # discard line that are not errors and line that
> +        # is recipe task errors
> +        if (not self.rexp_error.match(line)) or \
> +                self.rexp_pnpv_error.match(line) or \
> +                self.rexp_task_error.match(line) or \
> +                self.rexp_log_error.match(line):
> +            return
> +
> +        # save all the log for be able to get report variables like
> +  
>       # machine, target, distro, etc
> +        self.step.errors['log'].ap
> pend(line)
> +
> +        # if not match recipe task type is a bitbake exception/error
> +        self.step.errors['bitbake'] = True
> 

I tried to explain this in the comment above append log, we need the
full log for get information about machine. distro, etc also if a line
don't match but then is discover that is a bitbake error we  want to
have the full log not discarding recipe/task errors because some times
the context about bitbake error/exception matters.


> Further 
> http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder/commit/?h=c
> ontrib/alimon/devel&id=2523c869855d8e2393b9dd7142c0992908444584
> 
> still has typo's in the commit title and message.
> 
> Finally, please don't end the subject line of commit messages with a
> period.
> 

I'll fix the commit msg.

	alimon

> Regards,
> 
> Joshua
> 
>> 	alimon
>>
>>>>
>>>> Regards,
>>>>
>>>> Joshua
>>>>
>>>>>
>>>>> Cheers,
>>>>> 	alimon
>>>>>
>>>>> [1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=7583
>>>>> [2]
>>>>> http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder/log
>>>>> /?h=co
>>>>> ntrib/alimon/devel
>>>>> [3]
>>>>> http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder/tre
>>>>> e/lib/
>>>>> python2.7/site-
>>>>> packages/autobuilder/lib/buildsteps.py?h=contrib/alimon/devel#n
>>>>> 92
>>>>> [4]
>>>>> http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder/tre
>>>>> e/lib/
>>>>> python2.7/site-
>>>>> packages/autobuilder/lib/buildsteps.py?h=contrib/alimon/devel#n
>>>>> 53
>>>>> [5]
>>>>> http://git.yoctoproject.org/cgit/cgit.cgi/yocto-autobuilder/com
>>>>> mit/?h
>>>>> =contrib/alimon/devel&id=4022920bb0e56d1eef3dfe7c5e9b4699f801cd
>>>>> f1
>>>
>>>
>>>
>>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]