[Buildroot] [PATCH buildroot-test] web/import.inc.php: account for failures that contain an images/ path

[Buildroot] [PATCH buildroot-test] web/import.inc.php: account for failures that contain an images/ path

[Thomas Petazzoni]

With the testing of BR2_REPRODUCIBLE=y recently added and its
generation of a tar filesystem, we are now testing the tar filesystem
generation logic, and this reveals some interesting breakage, such as:

  http://autobuild.buildroot.net/results/fe3/fe378bca29c86a681ba9ad40386cb89248195c50/build-end.log

However, the "reason" deduced from the build log by the PHP logic is a
bit crappy, it's
"/home/br-user/autobuild/run/instance-0/output/images/rootfs.tar".

This commit adjusts the ugly PHP code with even more ugliness to take
into account the fact that a path may look like
foo/thing/build/<pkg>-<version>/.stamp_something or like
foo/thing/images/rootfs.tar.

The reasons extracted for the two previous examples would be
"<pkg>-<version>" (like is done today) and "rootfs.tar".

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 web/import.inc.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/web/import.inc.php b/web/import.inc.php
index 5b5462e..956c6d8 100644
--- a/web/import.inc.php
+++ b/web/import.inc.php
@@ -231,7 +231,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 -3 " . $thisbuildfinaldir . "build-end.log | grep -v '\[_all\]' | grep 'make.*: \*\*\*' | sed 's,.*\[\([^\]*\)\] Error.*,\\1,' | sed 's,.*/\(images\|build\)/\([^/]*\).*,\\2,'", $tmp);
 	if (trim($tmp[0]))
 	  $reason = $tmp[0];
 	else {
-- 
2.21.0

[Buildroot] [PATCH buildroot-test] web/import.inc.php: account for failures that contain an images/ path

[Arnout Vandecappelle]

On 16/06/2019 22:13, Thomas Petazzoni wrote:
> With the testing of BR2_REPRODUCIBLE=y recently added and its
> generation of a tar filesystem, we are now testing the tar filesystem
> generation logic, and this reveals some interesting breakage, such as:
> 
>   http://autobuild.buildroot.net/results/fe3/fe378bca29c86a681ba9ad40386cb89248195c50/build-end.log
> 
> However, the "reason" deduced from the build log by the PHP logic is a
> bit crappy, it's
> "/home/br-user/autobuild/run/instance-0/output/images/rootfs.tar".
> 
> This commit adjusts the ugly PHP code with even more ugliness to take

 This sounds like a great idea :-)

 Note that the Python code already has similar logic to extract the reason. Why
don't we save that logic as part of the tarball in a "reason" file, and extract
it from there?

 I know someone who has been working on the autobuild_run script lately who
could certainly do that :-)

 That said, we still have to support the legacy database which doesn't have that
"reason" file. So it's still useful to apply this patch.

 Regards,
 Arnout

> into account the fact that a path may look like
> foo/thing/build/<pkg>-<version>/.stamp_something or like
> foo/thing/images/rootfs.tar.
> 
> The reasons extracted for the two previous examples would be
> "<pkg>-<version>" (like is done today) and "rootfs.tar".
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  web/import.inc.php | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/web/import.inc.php b/web/import.inc.php
> index 5b5462e..956c6d8 100644
> --- a/web/import.inc.php
> +++ b/web/import.inc.php
> @@ -231,7 +231,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 -3 " . $thisbuildfinaldir . "build-end.log | grep -v '\[_all\]' | grep 'make.*: \*\*\*' | sed 's,.*\[\([^\]*\)\] Error.*,\\1,' | sed 's,.*/\(images\|build\)/\([^/]*\).*,\\2,'", $tmp);
>  	if (trim($tmp[0]))
>  	  $reason = $tmp[0];
>  	else {
> 

[Buildroot] [PATCH buildroot-test] web/import.inc.php: account for failures that contain an images/ path

[Thomas Petazzoni]

Hello,

On Mon, 17 Jun 2019 21:39:22 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

> > This commit adjusts the ugly PHP code with even more ugliness to take  
> 
>  This sounds like a great idea :-)

Isn't it :-)

>  Note that the Python code already has similar logic to extract the reason. Why
> don't we save that logic as part of the tarball in a "reason" file, and extract
> it from there?

Because history, but indeed it would make a lot more sense to extract
the reason only on the client side, because we anyway extract it
already to be able to extract the relevant part of the build log.

>  That said, we still have to support the legacy database which doesn't have that
> "reason" file. So it's still useful to apply this patch.

There is really no "legacy" to support here. On the server side, when a
result is submitted, the reason is extracted from the build log, and
then stored in the SQL database. Once that is done, when browsing build
results, the reason is always taken from the SQL database.

So server-side, we can entirely drop the logic that calculates the
reason from the build log, and instead replace it with just reading the
reason from an additional "reason" file in the result tarball submitted
by build slaves.

What needs to be done however is a transition period:

 - Adding the "reason" file to the build results on the client side is
   implemented.

 - This change is deployed to all build slaves.

 - Once it's deployed to all build slaves, we can change the server
   side to use the reason file instead of calculating the reason
   manually.

Or we could avoid that transition by using the reason file if
available, and fall back to the old logic if not available.

In the mean time, should I still apply this patch ? :-)

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH buildroot-test] web/import.inc.php: account for failures that contain an images/ path

[Arnout Vandecappelle]

On 18/06/2019 12:18, Thomas Petazzoni wrote:
> Hello,
> 
> On Mon, 17 Jun 2019 21:39:22 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>> This commit adjusts the ugly PHP code with even more ugliness to take  
>>
>>  This sounds like a great idea :-)
> 
> Isn't it :-)
> 
>>  Note that the Python code already has similar logic to extract the reason. Why
>> don't we save that logic as part of the tarball in a "reason" file, and extract
>> it from there?
> 
> Because history, but indeed it would make a lot more sense to extract
> the reason only on the client side, because we anyway extract it
> already to be able to extract the relevant part of the build log.
> 
>>  That said, we still have to support the legacy database which doesn't have that
>> "reason" file. So it's still useful to apply this patch.
> 
> There is really no "legacy" to support here. On the server side, when a
> result is submitted, the reason is extracted from the build log, and
> then stored in the SQL database. Once that is done, when browsing build
> results, the reason is always taken from the SQL database.

 OK, I thought this code was executed on every query. But that would indeed be
crazy.


> So server-side, we can entirely drop the logic that calculates the
> reason from the build log, and instead replace it with just reading the
> reason from an additional "reason" file in the result tarball submitted
> by build slaves.
> 
> What needs to be done however is a transition period:
> 
>  - Adding the "reason" file to the build results on the client side is
>    implemented.
> 
>  - This change is deployed to all build slaves.
> 
>  - Once it's deployed to all build slaves, we can change the server
>    side to use the reason file instead of calculating the reason
>    manually.
> 
> Or we could avoid that transition by using the reason file if
> available, and fall back to the old logic if not available.

 This was the path I was assumed was going to be taken.


> In the mean time, should I still apply this patch ? :-)

 Depends a bit on how long it's going to take Atharva to implement this reason
file. Atharva?

 Regards,
 Arnout

[Buildroot] [PATCH buildroot-test] web/import.inc.php: account for failures that contain an images/ path

[Atharva Lele]

On Tue, Jun 18, 2019 at 3:53 PM Arnout Vandecappelle <arnout@mind.be> wrote:
> Depends a bit on how long it's going to take Atharva to implement this
reason
> file. Atharva?

I will complete the transition to builder-class by end of tomorrow. Then
I'll work on the
reason file implementation. Should be done in a day or two I think. So
should be ready by
the end of 21st June. Does that sound like a reasonable timeframe?

Regards,
Atharva Lele
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20190618/177678d4/attachment.html>