Bug 7789

I think ERR can be safely replaced with ERROR:
I don't see any error messages with ERR in bitbake codebase:
$ grep -r ERR bitbake/* |grep print
bitbake/lib/bb/shell.py:            print("ERROR: Command '%s' not known" % old)
bitbake/lib/bb/shell.py:            print("ERROR: No Provider")
bitbake/lib/bb/shell.py:            print("ERROR: Couldn't build '%s'" % names)
bitbake/lib/bb/shell.py:            print("ERROR: Nothing provides '%s'" % name)
bitbake/lib/bb/shell.py:            print("ERROR: Unable to open or parse '%s'" % bf)
bitbake/lib/bb/shell.py:            print("ERROR: Couldn't build '%s'" % name)
bitbake/lib/bb/shell.py:                    print("ERROR: Couldn't open '%s'" % filename)
bitbake/lib/bb/shell.py:                print("SHELL: ERROR: %s/%s already exists" % ( fulldirname, filename ))
bitbake/lib/bb/shell.py:            print("ERROR: Nothing provides '%s'" % params[0])
bitbake/lib/bb/shell.py:            print("ERROR: Nothing provides '%s'" % name)
bitbake/lib/bb/shell.py:            print("ERROR: Sorry, this functionality is currently broken")
bitbake/lib/bb/shell.py:            print("ERROR: Nothing provides '%s'" % name)
bitbake/lib/bb/shell.py:            print("SHELL: ERROR: Nothing provides", preferred)
bitbake/lib/bb/shell.py:        print("ERROR: %s %s" % ( response.status, response.reason ))
bitbake/lib/bb/shell.py:            print("SHELL: ERROR: '%s' command is not a valid command." % command)
bitbake/lib/bb/shell.py:                    print("ERROR: '|' in startup file is not allowed. Ignoring line")

The same with Fail -> 'Failed '

BTW, I also looked under meta for both keywords.
$ grep -r Fail meta*/* |grep print
meta/classes/bugzilla.bbclass:                print >> debug_file, "Failed to reopen the bug #%s" % bug_number
meta/classes/bugzilla.bbclass:                 print >> debug_file, "Failed to attach the build log for bug #%s" % bug_number
meta/classes/bugzilla.bbclass:                 print >> debug_file, "Failed to create a comment the build log for bug #%s" % bug_number
meta/recipes-core/libxml/libxml2/runtest.patch:         fprintf(stderr, "Failed to allocate parser context\n");
meta/recipes-core/libxml/libxml2/runtest.patch:                 fprintf(stderr, "Failed to convert char for Byte 0x%02X\n", i);
meta/recipes-core/libxml/libxml2/runtest.patch:             fprintf(stderr, "Failed to parse char for Byte 0x%02X\n", i);
meta/recipes-core/libxml/libxml2/runtest.patch:         fprintf(stderr, "Failed to allocate parser context\n");
meta/recipes-core/libxml/libxml2/runtest.patch:         fprintf(stderr, "Failed to allocate input buffer\n");
meta/recipes-devtools/qemu/qemu-helper/tunctl.c:    fprintf(stderr, "Failed to open '%s' : ", file);
meta/recipes-devtools/python/python/05-enable-ctypes-cross-build.patch:-                    print "Failed to configure _ctypes module"
meta/recipes-devtools/python/python/05-enable-ctypes-cross-build.patch:+                    print "Failed to configure _ctypes module (res=%d) or missing conffile=%s" % ( res, ffi_configfile )
meta/recipes-devtools/rpm/rpmresolve/rpmresolve.c:                        fprintf(stderr, "Failed to open database %s\n", line);
meta/recipes-devtools/rpm/rpmresolve/rpmresolve.c:            fprintf(stderr, "Failed to open database %s\n", dblistfn);
meta/recipes-devtools/rpm/rpmresolve/rpmresolve.c:                                fprintf(stderr, "Failed to get package origin for %s\n", line);
meta/recipes-devtools/rpm/rpm/debugedit-segv.patch:     fprintf (stderr, "Failed to compute header checksum: %s\n",

$ grep -r Fail bitbake/* |grep print
bitbake/bin/toaster-eventreplay:                print("Failed loading ", line)
bitbake/lib/toaster/bldcontrol/management/commands/checksettings.py:                            print "Failure while trying to import the toaster config file: %s" % e
bitbake/lib/toaster/contrib/tts/launcher.py:            print("Failed while running the test runner: %s", e)
bitbake/lib/toaster/contrib/tts/runner.py:            map(lambda x: config.logger.error("Failed test: %s:\n%s\n" % (pprint.pformat(x[0]), "\n".join(["--  %s" % x for x in eval(pprint.pformat(x[1])).split("\n")]))), result.failures)
bitbake/lib/bb/build.py:            event.fire(TaskFailed(task, logfn, localdata, errprinted), localdata)
bitbake/lib/bb/utils.py:                print("movefile: Failed to move", src, "to", dest, e)
bitbake/lib/bb/utils.py:                print("movefile: Failed to move special file:" + src + "' to '" + dest + "'", a)
bitbake/lib/bb/utils.py:            print("movefile: Failed to chown/chmod/unlink", dest, e)

So, I'm going to change regexp to:
 self.log_check_regex = '(unpacking of archive failed|Cannot find package|exit 1|ERROR: |Failed )'

Does it make sense for you?

OK, my workaround for the "ERR" part of the issue stands therefore - in my codebase, I just replaced "ERR" by "ERROR:", and it worked, but I was not sure if it was covering all the cases or not (and I actually didn't have any errors anymore).

Regarding "Fail", it seems a little bit more tricky... Any opinion?
From the reading of the following list, I might suggest "Failed " as a pattern.
But it's not as obvious as for the "ERROR:" thing - no colon as a clear separator...

bin/toaster-eventreplay:                print("Failed loading ", line)
lib/bb/build.py:            event.fire(TaskFailed(task, logfn, localdata, errprinted), localdata)
lib/bb/utils.py:                print("movefile: Failed to move", src, "to", dest, e)
lib/bb/utils.py:                print("movefile: Failed to move special file:" + src + "' to '" + dest + "'", a)
lib/bb/utils.py:            print("movefile: Failed to chown/chmod/unlink", dest, e)
lib/toaster/bldcontrol/management/commands/checksettings.py:                            print "Failure while trying to import the toaster config file: %s" % e
lib/toaster/contrib/tts/launcher.py:            print("Failed while running the test runner: %s", e)
lib/toaster/contrib/tts/runner.py:            map(lambda x: config.logger.error("Failed test: %s:\n%s\n" % (pprint.pformat(x[0]), "\n".join(["--  %s" % x for x in eval(pprint.pformat(x[1])).split("\n")]))), result.failures)

I realize we've been commenting at the same time...
Yes, I believe the changes proposed do make sense and will solve this problem :)

We should investigate deeper I think. This regexp is unique for RpmRootfs class. It could be so that it looks for the errors produced by smart or rpm and 'Fail' and 'ERR' are valid for those tools.

Are there any "delimiter" after ERR or Fail in smart or rpm?

I think smart is out of scope as it's used to install all sort of packages: rpm, deb, ipk.

Promising results for rpm sources:

$ grep -r Fail . |grep -v /testdata/ |grep -v \.pl:|grep print |grep -v 'Failed '
./5.4.14-r0/rpm-5.4.14/tests/mongo/replica_set.c:                    fprintf( stderr, "Fail.\n" );
./5.4.14-r0/rpm-5.4.14/tests/mongo/replset.c:                    fprintf( stderr, "Fail.\n" );
Binary file ./5.4.14-r0/build/rpmdb/librpmdb_la-fprint.o matches
Binary file ./5.4.14-r0/build/rpmdb/.libs/librpmdb_la-fprint.o matches

Unfortunately we can't use 'Failed ' with the space as it's widely used:
$ grep -r Fail . |grep -v /testdata/ |grep -v \.pl:|grep print |grep -v 'Failed '
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:      fprintf(outfile, "Failed: POSIX code %d: %s\n", rc, buffer);
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:        fprintf(outfile, "**Failed: invalid UTF-8 string cannot be "
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:        fprintf(outfile, "**Failed: character value greater than 0x10ffff "
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:        fprintf(outfile, "**Failed: character value greater than 0xffff "
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:        fprintf(outfile, "**Failed: invalid UTF-8 string cannot be "
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:        fprintf(outfile, "**Failed: character value greater than 0x10ffff "
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:        fprintf(outfile, "**Failed: character value is ill-formed UTF-32\n");
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:      fprintf(outfile, "Failed: %s at offset %d\n", error, erroroffset);
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:        fprintf(outfile, "**Failed: invalid UTF-8 string cannot be used as input in UTF mode\n");
./5.4.14-r0/rpm-5.4.14/pcre/pcretest.c:            fprintf(outfile, "** Failed: character \\x{%x} is greater than "
./5.4.14-r0/rpm-5.4.14/tests/mongo/replica_set.c:                    fprintf( stderr, "Fail.\n" );
./5.4.14-r0/rpm-5.4.14/tests/mongo/replset.c:                    fprintf( stderr, "Fail.\n" );
./5.4.14-r0/rpm-5.4.14/rpmio/rpmaug.c:  rpmaugFprintf(NULL, "Failed\n");
./5.4.14-r0/rpm-5.4.14/rpmio/rpmaug.c:  rpmaugFprintf(NULL, "Failed\n");
./5.4.14-r0/rpm-5.4.14/rpmio/rpmaug.c:  rpmaugFprintf(NULL, "Failed\n");
./5.4.14-r0/rpm-5.4.14/rpmio/rpmaug.c:  rpmaugFprintf(NULL, "Failed\n");
./5.4.14-r0/rpm-5.4.14/rpmio/rpmaug.c:  rpmaugFprintf(NULL, "Failed\n");
./5.4.14-r0/rpm-5.4.14/rpmio/rpmaug.c:          rpmaugFprintf(NULL, "Failed(%d): %s\n", xx, P->av[0]);

Couldn't we use "Fail\.|Failed:|Failed "?
From your log, and the previous ones, it should cover all cases, right?
Also, I didn't see in your log "Failed " as widely used... But I might have missed it.

What we need to take into account is all the logs that can go into log.do_rootfs, since it's the only file that's parsed by this function.
And that's where my limited knowledge of the inner working yocto unfortunately comes into play, I'm not sure where it all comes from...

Unfortunately it will not cover 'Failed(%d)' and 'Failed\n'.

'ERROR: ' is also ok to use I think. There are some corner cases like this for example: 'printf(stderr, "sqlite3_step: ERROR %d -- %s\n", rc, scp->cmd);' which 'ERROR: ' will miss. Howver, current regexp doesn't cover tons of 'Error: ' rpm messages. If we add 'ERROR: ' and 'Error: ' we'd improve overall quality of regexp.

We can also use 'ERROR: |Error: |ERROR |error |'. That should cover pretty much everything. At least much more than current regexp.

> Also, I didn't see in your log "Failed " as widely used... But I might have missed it.
Look at grep command line. 'Failed ' is filtered out. I was looking at the difference between 'Fail' and 'Failed'.

What about "Fail\.|Failed:|Failed\(|Failed\\|Failed "?
I'm a little bit concerned by "Failed ", but if it's not widespread and needs to be considered an error, then it should hopefully do it...

As to those being used in a pathname, apart from the last one maybe, the probability seems low - not impossible, but lower than the original anyway...

(In reply to comment #11)
> > Also, I didn't see in your log "Failed " as widely used... But I might have missed it.
> Look at grep command line. 'Failed ' is filtered out. I was looking at the
> difference between 'Fail' and 'Failed'.

Do the lines with "Failed " make it in log.do_rootfs?

(In reply to comment #10)
> 'ERROR: ' is also ok to use I think. There are some corner cases like this
> for example: 'printf(stderr, "sqlite3_step: ERROR %d -- %s\n", rc,
> scp->cmd);' which 'ERROR: ' will miss. Howver, current regexp doesn't cover
> tons of 'Error: ' rpm messages. If we add 'ERROR: ' and 'Error: ' we'd
> improve overall quality of regexp.
> 
> We can also use 'ERROR: |Error: |ERROR |error |'. That should cover pretty
> much everything. At least much more than current regexp.

Do all the cases make it into log.do_rootfs log file?
Also, I fear we might find "ERROR " and "error " in some pathnames, while it's less likely when followed by a colon I believe.

It's hard to say if all those types of errors go to do_rootfs.log or not.
I agree it can be dangerous in some cases, but how often did you see pathnames with 'error ' or 'ERROR '?

So far my regexp is: '(unpacking of archive failed|Cannot find package|exit 1|ERROR: |Error: |Error |ERROR |Failed |Failed: |Failed$|Failed\d+)'

Does this make sense for you?

A lot of rpm error messages are not covered by this weak regexp:
$ grep -r '_("' * |grep 'fprintf(stderr' | wc -l
177

(In reply to comment #15)
> It's hard to say if all those types of errors go to do_rootfs.log or not.
> I agree it can be dangerous in some cases, but how often did you see
> pathnames with 'error ' or 'ERROR '?
> 
> So far my regexp is: '(unpacking of archive failed|Cannot find package|exit
> 1|ERROR: |Error: |Error |ERROR |Failed |Failed: |Failed$|Failed\d+)'
> 
> Does this make sense for you?

I didn't meet the Failed\d case yet (it's "Failed[0-9]", right?).
But from your log, there are some cases with "Failed(", which doesn't seem caught.

For the "ERROR " and "error ", although it might happen, I agree it can probably be left that way for the moment, waiting for a better solution if any - it's difficult to see a bright one right now, I don't have a better idea so far.

(In reply to comment #16)
> A lot of rpm error messages are not covered by this weak regexp:
> $ grep -r '_("' * |grep 'fprintf(stderr' | wc -l
> 177

Then maybe there should be a better way than just a simple grep put in place to catch those...

OR we could attempt some kind of "standardization" of the error messages format, so they could get recognized easily through a simple regex parsing, with minimizing the risk of the regexp accidentally matching a path name...

But that would be a much bigger scope I believe, and not necessarily an easy one to achieve...

The solution you proposed before should match at least all the parts matched by the current regexp, while lowering the risk of collision with a pathname, so I believe it's an improvement anyway.
Not perfect, but good enough for most cases.
And since we're on the same  line of thought, I don't see a better way either...

Thanks for catching it. I was actually trying to cover this error message: ./5.4.14-r0/rpm-5.4.14/rpmio/rpmaug.c:          rpmaugFprintf(NULL, "Failed(%d): %s\n", xx, P->av[0]);

Updated regexp: '(unpacking of archive failed|Cannot find package|exit 1|ERROR: |Error: |Error |ERROR |Failed |Failed: |Failed$|Failed\(\d+\):)'

Not sure we should go this far though.
May be just case insensitive 'error|failed' is enough to fix this issue? Of course pathnames can contain those, but it's less likely to happen comparing to current regexp.

I think that as long as we can have a delimiter after error|failed that we catch as well, it will reduce the probability more than just "error|failed".
So everything in the new regexp seems fine, except the cases when the delimiter is just a space, in which case it could still happen in a valid pathname...

BUT in a linux environment, not so much actually, which is why I guess it should do the trick.

But I also can understand if you prefer to stay with the simpler regexp, it will in any case be better than the current situation.

In that case, maybe a FAQ somewhere should state that you can actually get some error even if there is no build error, in some cases linked to the pathname, and specify the paths that should be "avoided".

I've sent a patch for review: http://lists.openembedded.org/pipermail/openembedded-core/2015-May/105127.html

Let's see what other people say.

Fixed by this commit in master: http://cgit.openembedded.org/openembedded-core/commit/?id=38871dc0295fb2516e1c8b1dc9948c2d2fc4d2cd&h=master

Thanks a lot! :D

I, of course, had a directory with ERROR in it.

Since any heuristic will fail sometime, it would be more useful to print out the error str it matched in the bb.warn.
basically just add 
bb.warn('log_check: In line: [%s]' % line)

That would have told me immediately it saw ERROR in ~/src/QEMU_ERROR
-b

I, of course, had a directory with ERROR in it.

Since any heuristic will fail sometime, it would be more useful to print out the error str it matched in the bb.warn.
basically just add 
bb.warn('log_check: In line: [%s]' % line)

That would have told me immediately it saw ERROR in ~/src/QEMU_ERROR
-b

Thank you for suggestion!

Sent patch for review: http://lists.openembedded.org/pipermail/openembedded-core/2015-October/111834.html

Patch has been merged to master: http://cgit.openembedded.org/openembedded-core/commit/?id=f9cf31525fc885e1a0f65bd55654631257f87078