| Message ID | 20190616201338.4593-1-thomas.petazzoni@bootlin.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [buildroot-test] web/import.inc.php: account for failures that contain an images/ path | expand |
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 { >
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
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
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
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 {
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(-)