Re: [OE-core] [PATCH] report-error: make it catch ParseError error

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hi Mingli,

I'm taking this patch for AB testing, however I have a couple remarks
that might turn into a new version of your patch or in a follow-up
cleanup patch, see below.

On Fri, 14 Apr 2023 10:08:38 +0800
"Yu, Mingli" <mingli.yu@eng.windriver.com> wrote:

> From: Mingli Yu <mingli.yu@windriver.com>
> 
> Make the report-error catch ParseError error as below and then
> we can check it directly via error report web.
> 
> ParseError at /build/layers/oe-core/meta/recipes-support/curl/curl_7.88.1.bb:32: unparsed line: 'PACKAGECONFIG[ares] = "--enable-ares,--disable-ares,c-ares,,,threaded-resolver'
> 
> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> ---
>  meta/classes/report-error.bbclass | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/report-error.bbclass b/meta/classes/report-error.bbclass
> index 2b2ad56514..041db44941 100644
> --- a/meta/classes/report-error.bbclass
> +++ b/meta/classes/report-error.bbclass
> @@ -132,6 +132,30 @@ python errorreport_handler () {
>              errorreport_savedata(e, data, "error-report.txt")
>              bb.utils.unlockfile(lock)
>  
> +        elif isinstance(e, bb.event.ParseError):
> +            bb.utils.mkdirhier(logpath)
> +            data = {}
> +            machine = e.data.getVar("MACHINE")
> +            data['machine'] = machine

These two lines could become one line, removing the 'machine' variable:

  data['machine'] = e.data.getVar("MACHINE")

I see you used the same pattern as other if/elif branches in the
surrounding code, so they could also be simplified.

> +            data['build_sys'] = e.data.getVar("BUILD_SYS")
> +            data['nativelsb'] = nativelsb()
> +            data['distro'] = e.data.getVar("DISTRO")
> +            data['target_sys'] = e.data.getVar("TARGET_SYS")
> +            data['failures'] = []
> +            data['component'] = "parse"
> +            data['branch_commit'] = str(oe.buildcfg.detect_branch(e.data)) + ": " + str(oe.buildcfg.detect_revision(e.data))
> +            data['bitbake_version'] = e.data.getVar("BB_VERSION")
> +            data['layer_version'] = get_layers_branch_rev(e.data)
> +            data['local_conf'] = get_conf_data(e, 'local.conf')
> +            data['auto_conf'] = get_conf_data(e, 'auto.conf')
> +            taskdata={}
> +            taskdata['log'] = str(e._msg)
> +            taskdata['task'] = str(e._msg)
> +            data['failures'].append(taskdata)
> +            lock = bb.utils.lockfile(datafile + '.lock')
> +            errorreport_savedata(e, data, "error-report.txt")
> +            bb.utils.unlockfile(lock)

Most of this code is already duplicated in two branches already, and
with your patch there will be three copies. Can the common lines be
moved to a helper function?

Best regards,
Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com