| Message ID | 1459468972-31420-2-git-send-email-matt@thewebers.ws |
|---|---|
| State | Accepted |
| Headers | show |
Hello,
On Thu, 31 Mar 2016 19:02:52 -0500, Matt Weber wrote:
> - Sub-make required one additional line tailed
Really? Did you look at my previous review, where I showed the
different examples I could find, and showed that 3 lines were enough?
It's not like I care particularly whether we're tailing 3 or 4 lines,
it's more that I want to understand why in your scenario you would need
4 lines.
Thanks!
Thomas
Thomas, On Mar 31, 2016 8:55 PM, "Thomas Petazzoni" < thomas.petazzoni@free-electrons.com> wrote: > > Hello, > > On Thu, 31 Mar 2016 19:02:52 -0500, Matt Weber wrote: > > - Sub-make required one additional line tailed > > Really? Did you look at my previous review, where I showed the > different examples I could find, and showed that 3 lines were enough? I did and I haven't figured out why yet > > It's not like I care particularly whether we're tailing 3 or 4 lines, > it's more that I want to understand why in your scenario you would need > 4 lines. > > Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Free Electrons > Embedded Linux, Kernel and Android engineering > http://free-electrons.com
Hello, On Thu, 31 Mar 2016 21:37:30 -0500, Matthew Weber wrote: > > Really? Did you look at my previous review, where I showed the > > different examples I could find, and showed that 3 lines were enough? > > I did and I haven't figured out why yet Well, can you show the last 10 lines of your build logs, which require 4 lines for the script to detect the failure reason? Thanks! Thomas
On Thu, Mar 31, 2016 at 10:58 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Hello, > > On Thu, 31 Mar 2016 21:37:30 -0500, Matthew Weber wrote: > >> > Really? Did you look at my previous review, where I showed the >> > different examples I could find, and showed that 3 lines were enough? >> >> I did and I haven't figured out why yet > > Well, can you show the last 10 lines of your build logs, which require > 4 lines for the script to detect the failure reason? > ^ futility/cmd_gbb_utility.c: In function 'do_gbb_utility': futility/cmd_gbb_utility.c:558:5: warning: format '%lli' expects argument of type 'long long int', but argument 3 has type 'off_t {aka long int}' [-Wformat=] "ERROR: can't malloc %" PRIi64 " bytes: %s\n", ^ CC futility/ryu_root_header.o CC firmware/stub/vboot_api_stub_static_sf.o GEN gen/futility_static_cmds.c CC cgpt/cgpt_create.o CC cgpt/cgpt_add.o CC cgpt/cgpt_boot.o CC cgpt/cgpt_show.o CC cgpt/cgpt_repair.o CC cgpt/cgpt_prioritize.o CC cgpt/cgpt_common.o make[2]: *** No rule to make target '/opt/buildroot_testing/buildroot-test/instance-0/output/build/host-vboot-utils-bbdd62f9b030db7ad8eef789aaf58a7ff9a25656/build/host/arch/i686/lib/crossystem_arch.o', needed by '/opt/buildroot_testing/buildroot-test/instance-0/output/build/host-vboot-utils-bbdd62f9b030db7ad8eef789aaf58a7ff9a25656/build/libvboot_util.a'. Stop. make[2]: *** Waiting for unfinished jobs.... CC futility/dump_kernel_config_lib.o package/pkg-generic.mk:195: recipe for target '/opt/buildroot_testing/buildroot-test/instance-0/output/build/host-vboot-utils-bbdd62f9b030db7ad8eef789aaf58a7ff9a25656/.stamp_built' failed make[1]: *** [/opt/buildroot_testing/buildroot-test/instance-0/output/build/host-vboot-utils-bbdd62f9b030db7ad8eef789aaf58a7ff9a25656/.stamp_built] Error 2 Makefile:36: recipe for target '_all' failed make: *** [_all] Error 2 make: Leaving directory '/opt/buildroot_testing/buildroot-test/instance-0/buildroot'
Hello, On Thu, 31 Mar 2016 23:04:57 -0500, Matthew Weber wrote: > make[1]: *** [/opt/buildroot_testing/buildroot-test/instance-0/output/build/host-vboot-utils-bbdd62f9b030db7ad8eef789aaf58a7ff9a25656/.stamp_built] Error 2 > Makefile:36: recipe for target '_all' failed > make: *** [_all] Error 2 > make: Leaving directory '/opt/buildroot_testing/buildroot-test/instance-0/buildroot' Weird, this is this last line that I don't see here: make[1]: *** [/home/thomas/projets/outputs/toto/build/makedevs-buildroot-2016.05-git/.stamp_built] Error 127 Makefile:16: recipe for target '_all' failed make: *** [_all] Error 2 But on the other hand, http://autobuild.buildroot.org/results/b55/b5539c22ee35a36ae3fe4967040ea04b11889a4c/build-end.log has this last line, but doesn't have the "recipe for target ... failed" line. Oh well, I guess I'm just going to take your patch as-is. Maybe some make guru like Arnout will have a good explanation for those differences, but it's not important enough to spend too much time on this. Thanks! Thomas
Hello, On Thu, 31 Mar 2016 19:02:52 -0500, Matt Weber wrote: > - Sub-make required one additional line tailed > - Both autobuild-run regex to truncate end log > and import which sets the failure reason on > the report are updated > > Signed-off-by: Matt Weber <matt@thewebers.ws> > > -- > Changes for v2: > - Added back in : as previous test masked > that is was ok to leave in (Suggested by Thomas) > --- > scripts/autobuild-run | 4 ++-- > web/import.inc.php | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) Applied to buildroot-test, and deployed on autobuild.b.o. Of course, it's up to the people using autobuild-run to update it on their side. Thanks! Thomas
Thomas, Matt, On 2016-09-06 22:04 +0200, Thomas Petazzoni spake thusly: > On Thu, 31 Mar 2016 19:02:52 -0500, Matt Weber wrote: > > - Sub-make required one additional line tailed > > - Both autobuild-run regex to truncate end log > > and import which sets the failure reason on > > the report are updated > > > > Signed-off-by: Matt Weber <matt@thewebers.ws> > > > > -- > > Changes for v2: > > - Added back in : as previous test masked > > that is was ok to leave in (Suggested by Thomas) > > --- > > scripts/autobuild-run | 4 ++-- > > web/import.inc.php | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > Applied to buildroot-test, and deployed on autobuild.b.o. Of course, > it's up to the people using autobuild-run to update it on their side. Done! ;-) Regards, Yann E. MORIN.
Hello, On Tue, 6 Sep 2016 22:04:59 +0200, Thomas Petazzoni wrote: > Hello, > > On Thu, 31 Mar 2016 19:02:52 -0500, Matt Weber wrote: > > - Sub-make required one additional line tailed > > - Both autobuild-run regex to truncate end log > > and import which sets the failure reason on > > the report are updated > > > > Signed-off-by: Matt Weber <matt@thewebers.ws> > > > > -- > > Changes for v2: > > - Added back in : as previous test masked > > that is was ok to leave in (Suggested by Thomas) > > --- > > scripts/autobuild-run | 4 ++-- > > web/import.inc.php | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > Applied to buildroot-test, and deployed on autobuild.b.o. Of course, > it's up to the people using autobuild-run to update it on their side. And I just reverted this commit, as it broke the autobuilders. For a large number of builds, the failure reason was no longer identified properly, and the script was concluding that the failure reason was "all" or "all-recursive" or "lib", i.e the name of a make target, and not the name of the package. Thomas
Thomas, All, On 2016-09-07 09:17 +0200, Thomas Petazzoni spake thusly: > On Tue, 6 Sep 2016 22:04:59 +0200, Thomas Petazzoni wrote: > > Hello, > > > > On Thu, 31 Mar 2016 19:02:52 -0500, Matt Weber wrote: > > > - Sub-make required one additional line tailed > > > - Both autobuild-run regex to truncate end log > > > and import which sets the failure reason on > > > the report are updated > > > > > > Signed-off-by: Matt Weber <matt@thewebers.ws> > > > > > > -- > > > Changes for v2: > > > - Added back in : as previous test masked > > > that is was ok to leave in (Suggested by Thomas) > > > --- > > > scripts/autobuild-run | 4 ++-- > > > web/import.inc.php | 2 +- > > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > Applied to buildroot-test, and deployed on autobuild.b.o. Of course, > > it's up to the people using autobuild-run to update it on their side. > > And I just reverted this commit, as it broke the autobuilders. For a > large number of builds, the failure reason was no longer identified > properly, and the script was concluding that the failure reason was > "all" or "all-recursive" or "lib", i.e the name of a make target, and > not the name of the package. OK, updated and restarted. ;-) Regards, Yann E. MORIN.
diff --git a/scripts/autobuild-run b/scripts/autobuild-run index 1fbf482..f9743a8 100755 --- a/scripts/autobuild-run +++ b/scripts/autobuild-run @@ -665,10 +665,10 @@ def send_results(result, **kwargs): def get_failure_reason(): # Output is a tuple (package, version), or None. lastlines = decode_bytes(subprocess.Popen( - ["tail", "-n", "3", os.path.join(outputdir, "logfile")], + ["tail", "-n", "4", os.path.join(outputdir, "logfile")], stdout=subprocess.PIPE).communicate()[0]).splitlines() - regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/') + regexp = re.compile(r'make.*: \*\*\* .*/(?:build|toolchain)/([^/]*)/') for line in lastlines: m = regexp.search(line) if m: diff --git a/web/import.inc.php b/web/import.inc.php index 243a1f3..e169f6f 100644 --- a/web/import.inc.php +++ b/web/import.inc.php @@ -102,7 +102,7 @@ function import_result($buildid, $filename) $reason = "none"; else { $tmp = Array(); - exec("tail -3 " . $thisbuildfinaldir . "build-end.log | grep -v '\[_all\]' | grep 'make.*: \*\*\*' | sed 's,.*\[\([^\]*\)\] Error.*,\\1,' | sed 's,.*/build/\([^/]*\)/.*,\\1,'", $tmp); + exec("tail -4 " . $thisbuildfinaldir . "build-end.log | grep -v '\[_all\]' | grep 'make.*: \*\*\*' | sed 's,.*\[\([^\]*\)\] Error.*,\\1,' | sed 's,.*/build/\([^/]*\)/.*,\\1,'", $tmp); if (trim($tmp[0])) $reason = $tmp[0]; else
- Sub-make required one additional line tailed - Both autobuild-run regex to truncate end log and import which sets the failure reason on the report are updated Signed-off-by: Matt Weber <matt@thewebers.ws> -- Changes for v2: - Added back in : as previous test masked that is was ok to leave in (Suggested by Thomas) --- scripts/autobuild-run | 4 ++-- web/import.inc.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)