[OSSTEST PATCH 00/13] Linux 4.1, and reporting improvements

[OSSTEST PATCH 00/13] Linux 4.1, and reporting improvements

[Ian Jackson]

Most of this series is improvements to html and email output.  It
includes the push gate age query library function I offered to Ian
Campbell.

Also thrown in is a patch ot test the Linux 4.1 stable branch.

Ian.

[OSSTEST PATCH 01/13] ts-leak-check: Report leaked loop block devices

[Ian Jackson]

These show up as leaked processes `[loop1]' etc. too, but that is an
implementation detail, and it is clearer to show them separately.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 ts-leak-check |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ts-leak-check b/ts-leak-check
index fdade36..8a97971 100755
--- a/ts-leak-check
+++ b/ts-leak-check
@@ -153,10 +153,19 @@ sub inventory_domains () {
     }
 }
 
+sub inventory_losetup () {
+    my $loops= target_cmd_output_root($ho, "losetup -a");
+    foreach (split /\n/, $loops) {
+	m,(/dev/loop\d+): , or die "$_ ?";
+	item('loop-device', $1, $_, 1);
+    }
+}
+
 sub inventory () {
     inventory_domains();
     inventory_processes();
     inventory_xenstore();
+    inventory_losetup();
     inventory_files('/tmp /var/run /var/tmp /var/lib/xen /var/core');
 }
 
-- 
1.7.10.4

Re: [OSSTEST PATCH 01/13] ts-leak-check: Report leaked loop block devices

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> These show up as leaked processes `[loop1]' etc. too, but that is an
> implementation detail, and it is clearer to show them separately.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 02/13] Test Linux 4.1

[Ian Jackson]

The kernel.org stable git branch layout is the same for 4.1 as for
3.x, so simply extend the pattern, and add the branch to BRANCHES.

DEPLOYMENT NOTE: After this patch is acked, but just before it is
pushed to pretest, run (this version of)
  ./mg-branch-setup linux-4.1 bisect b953c0d234bc72e8489d3bf51a276c5c4ec85345
to create the working trees on the osstest VM and push the base of
v4.1 to the tested output branch.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 ap-common       |    2 +-
 cr-for-branches |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ap-common b/ap-common
index de7bb1d..1877662 100644
--- a/ap-common
+++ b/ap-common
@@ -116,7 +116,7 @@ info_linux_tree () {
 		: ${TREE_LINUX_THIS:=git://kernel.ubuntu.com/ubuntu/linux.git}
 		: ${TAG_LINUX_THIS:=$1.y}
 		;;
-	linux-3.*)
+	linux-3.*|linux-4.*)
 		: ${TREE_LINUX_THIS:=${KERNEL_SCM}/stable/linux-stable.git}
 		: ${TAG_LINUX_THIS:=$1.y}
 		;;
diff --git a/cr-for-branches b/cr-for-branches
index fd2b9c0..07650c1 100755
--- a/cr-for-branches
+++ b/cr-for-branches
@@ -31,7 +31,7 @@ scriptoptions="$1"; shift
 LOGFILE=tmp/cr-for-branches.log
 export LOGFILE
 
-: ${BRANCHES:=osstest xen-4.0-testing xen-4.1-testing xen-4.2-testing xen-4.3-testing xen-4.4-testing xen-4.5-testing xen-unstable qemu-mainline qemu-upstream-unstable qemu-upstream-4.2-testing qemu-upstream-4.3-testing qemu-upstream-4.4-testing qemu-upstream-4.5-testing linux-3.18 linux-3.16 linux-3.14 linux-3.10 linux-3.4 linux-arm-xen seabios ovmf ${EXTRA_BRANCHES}}
+: ${BRANCHES:=osstest xen-4.0-testing xen-4.1-testing xen-4.2-testing xen-4.3-testing xen-4.4-testing xen-4.5-testing xen-unstable qemu-mainline qemu-upstream-unstable qemu-upstream-4.2-testing qemu-upstream-4.3-testing qemu-upstream-4.4-testing qemu-upstream-4.5-testing linux-4.1 linux-3.18 linux-3.16 linux-3.14 linux-3.10 linux-3.4 linux-arm-xen seabios ovmf ${EXTRA_BRANCHES}}
 export BRANCHES
 
 fetchwlem=$wlem
-- 
1.7.10.4

Re: [OSSTEST PATCH 02/13] Test Linux 4.1

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> The kernel.org stable git branch layout is the same for 4.1 as for
> 3.x, so simply extend the pattern, and add the branch to BRANCHES.
> 
> DEPLOYMENT NOTE: After this patch is acked, but just before it is
> pushed to pretest, run (this version of)
>   ./mg-branch-setup linux-4.1 bisect b953c0d234bc72e8489d3bf51a276c5c4ec85345
> to create the working trees on the osstest VM and push the base of
> v4.1 to the tested output branch.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

However, I think it is inevitable there will eventually be linux-5.*. By
moving this case to the end of the switch I think it could safely become
linux-* i.e. the default/fallback is to assume a stable kernel.org tree.
Everything else is a special case anyway.

BTW, I can't see a reason why linux-3.0 is a separate case any more...

Ian

[OSSTEST PATCH 03/13] Reporting: sg-report-flight: Fix undefined value in $worstrow[]

[Ian Jackson]

The fallback entry in the worstrow search, used when a job has not
been run at all, was wrong.  Also, fix the doc comment which induced
the mistake.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-report-flight |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sg-report-flight b/sg-report-flight
index 6481521..117b609 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -965,7 +965,7 @@ END
 
     my $cell_html = sub {
 	my ($ei, $core_only) = @_;
-	# => ($h, $priority);
+	# => ($h, $priority, $ch);
 	my $s= $ei->{Step};
 	my $sum_core= $ei->{SummaryCore};
 	$sum_core= $ei->{Summary} if !defined $sum_core;
@@ -998,7 +998,7 @@ END
     my @worstrow1;
     my @worstrow2;
     foreach my $col (@cols) {
-	my @worst=(' bgcolor="#444444">',0);
+	my @worst=('',0,'bgcolor="#444444"','');
 	foreach my $rowix (0..$#{ $fi->{GridRows} }) {
             my $ei= $fi->{GridGrid}{$col}[$rowix];
 	    next unless $ei;
-- 
1.7.10.4

Re: [OSSTEST PATCH 03/13] Reporting: sg-report-flight: Fix undefined value in $worstrow[]

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> The fallback entry in the worstrow search, used when a job has not
> been run at all, was wrong.  Also, fix the doc comment which induced
> the mistake.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  sg-report-flight |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sg-report-flight b/sg-report-flight
> index 6481521..117b609 100755
> --- a/sg-report-flight
> +++ b/sg-report-flight
> @@ -965,7 +965,7 @@ END
>  
>      my $cell_html = sub {
>  	my ($ei, $core_only) = @_;
> -	# => ($h, $priority);
> +	# => ($h, $priority, $ch);

This is a 3-tuple.

>  	my $s= $ei->{Step};
>  	my $sum_core= $ei->{SummaryCore};
>  	$sum_core= $ei->{Summary} if !defined $sum_core;
> @@ -998,7 +998,7 @@ END
>      my @worstrow1;
>      my @worstrow2;
>      foreach my $col (@cols) {
> -	my @worst=(' bgcolor="#444444">',0);
> +	my @worst=('',0,'bgcolor="#444444"','');

But this is a 4-tuple. Is there still a disconnect?

Ah no, out of context here there is a @worst=@this followed by a "push
@worst". So this change is correct, but perhaps a comment would help
clarify? e.g

# Result of $cell_html + Encoded string of failing stepid

?

Ian.

Re: [OSSTEST PATCH 03/13] Reporting: sg-report-flight: Fix undefined value in $worstrow[]

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 03/13] Reporting: sg-report-flight: Fix undefined value in $worstrow[]"):
> On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
...
> > diff --git a/sg-report-flight b/sg-report-flight
> > index 6481521..117b609 100755
> > --- a/sg-report-flight
> > +++ b/sg-report-flight
> > @@ -965,7 +965,7 @@ END
> >  
> >      my $cell_html = sub {
> >  	my ($ei, $core_only) = @_;
> > -	# => ($h, $priority);
> > +	# => ($h, $priority, $ch);
> 
> This is a 3-tuple.
> 
> >  	my $s= $ei->{Step};
> >  	my $sum_core= $ei->{SummaryCore};
> >  	$sum_core= $ei->{Summary} if !defined $sum_core;
> > @@ -998,7 +998,7 @@ END
> >      my @worstrow1;
> >      my @worstrow2;
> >      foreach my $col (@cols) {
> > -	my @worst=(' bgcolor="#444444">',0);
> > +	my @worst=('',0,'bgcolor="#444444"','');
> 
> But this is a 4-tuple. Is there still a disconnect?
> 
> Ah no, out of context here there is a @worst=@this followed by a "push
> @worst". So this change is correct, but perhaps a comment would help
> clarify? e.g
> 
> # Result of $cell_html + Encoded string of failing stepid
> 
> ?

Good idea.  I have added a comment, thanks.

Ian.

[OSSTEST PATCH 04/13] Reporting: In report_run_getinfo, abstract code for returning

[Ian Jackson]

This will allow us to change the function's API in one place (plus all
the call sites).

No functional change in this patch.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 6c16fdd..2389eb2 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -202,8 +202,14 @@ sub report_run_getinfo ($) {
     #    flight job status
     my ($f) = @_;
     my $status= $f->{status};
+
+    my $single = sub {
+	my ($summary, $colour) = @_;
+	return { Summary => $summary, Colour => $colour };
+    };
+
     if ($status eq 'pass') {
-        return { Summary => "($status)", Colour => $green };
+        return $single->("($status)", $green);
     } elsif ($status eq 'fail' or $status eq 'broken') {
 	my $failcolour = $status eq 'fail' ? $red : $yellow;
 	our $failstepq //= db_prepare(<<END);
@@ -216,19 +222,18 @@ END
         $failstepq->execute($f->{flight}, $f->{job});
         my $fs= $failstepq->fetchrow_hashref();
         if (!defined $fs) {
-            return { Summary => "(unknown)", Colour => $yellow };
+            return $single->("(unknown)", $yellow);
         } elsif ($fs->{status} eq 'fail') {
-            return { Summary => "$fs->{testid}", Colour => $failcolour };
+            return $single->("$fs->{testid}", $failcolour);
         } elsif ($fs->{status} eq 'broken') {
-            return { Summary => "$fs->{testid} broken", Colour => $yellow };
+            return $single->("$fs->{testid} broken", $yellow);
         } else {
-            return { Summary => "$fs->{testid} $fs->{status}",
-                     Colour => $failcolour };
+            return $single->("$fs->{testid} $fs->{status}", $failcolour);
         }
     } elsif ($status eq 'blocked') {
-        return { Summary => "blocked", Colour => $purple },
+        return $single->("blocked", $purple),
     } else {
-        return { Summary => "($f->{status})", Colour => $yellow };
+        return $single->("($f->{status})", $yellow);
     }
 }
 
-- 
1.7.10.4

Re: [OSSTEST PATCH 04/13] Reporting: In report_run_getinfo, abstract code for returning

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> This will allow us to change the function's API in one place (plus all
> the call sites).
> 
> No functional change in this patch.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 05/13] Reporting: report_run_getinfo produces ColourAttr

[Ian Jackson]

This includes the bgcolour attribute name and the quotes.  This will
make it possible for this function to sometimes not set a background
for the whole table cell in the future (which will be part of putting
multiple step results in each cell).

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm   |    5 ++++-
 sg-report-host-history |    2 +-
 sg-report-job-history  |    3 +--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 2389eb2..f53d271 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -205,7 +205,10 @@ sub report_run_getinfo ($) {
 
     my $single = sub {
 	my ($summary, $colour) = @_;
-	return { Summary => $summary, Colour => $colour };
+	return {
+	    Summary => $summary,
+	    ColourAttr => " bgcolor=\"$colour\"",
+	};
     };
 
     if ($status eq 'pass') {
diff --git a/sg-report-host-history b/sg-report-host-history
index f4e73e9..9e3edf7 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -242,7 +242,7 @@ END
 	print H "<td><a href=\"$url\">$jr->{job}</td>\n";
 
 	my $ri = report_run_getinfo({ %$jr, %$ir });
-	print H "<td bgcolor=\"$ri->{Colour}\">$ri->{Summary}</td>\n";
+	print H "<td $ri->{ColourAttr}>$ri->{Summary}</td>\n";
 
 	print H "</tr>\n\n";
 	$alternate ^= 1;
diff --git a/sg-report-job-history b/sg-report-job-history
index 900d998..aa33f94 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -224,7 +224,6 @@ END
         my @last_revs;
         my @alt_revs= ('0')x $#rev_grid_cols;
         foreach my $r (@test_rows) {
-            my $colour= "bgcolor=\"$r->{Colour}\"";
             my $altcolour= report_altcolour($alternate);
             print H "<tr $altcolour>";
 	    my $started = $r->{Flight}{started};
@@ -236,7 +235,7 @@ END
             print H "<td><a href=\"$url\">$flt</a></td>\n";
             print H "<td>".encode_entities($r->{Flight}{branch})."</td>\n";
             $url= "$c{ReportHtmlPubBaseUrl}/$flt/".encode_entities($j)."/";
-            print H "<td $colour><a href=\"$url\">".
+            print H "<td $r->{ColourAttr}><a href=\"$url\">".
                 encode_entities($r->{Summary})."</a></td>\n";
             my $lastrev;
             my $hosts = join ", ", map { $_ // "-" } @{ $r->{Hosts} };
-- 
1.7.10.4

Re: [OSSTEST PATCH 05/13] Reporting: report_run_getinfo produces ColourAttr

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> This includes the bgcolour attribute name and the quotes.  This will
> make it possible for this function to sometimes not set a background
> for the whole table cell in the future (which will be part of putting
> multiple step results in each cell).
> 
> No functional change.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  Osstest/Executive.pm   |    5 ++++-
>  sg-report-host-history |    2 +-
>  sg-report-job-history  |    3 +--
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
> index 2389eb2..f53d271 100644
> --- a/Osstest/Executive.pm
> +++ b/Osstest/Executive.pm
> @@ -205,7 +205,10 @@ sub report_run_getinfo ($) {
>  
>      my $single = sub {
>  	my ($summary, $colour) = @_;
> -	return { Summary => $summary, Colour => $colour };
> +	return {
> +	    Summary => $summary,
> +	    ColourAttr => " bgcolor=\"$colour\"",

FWIW all the uses have a space before there $ri->{ColourAttr}, so
strictly you don't need it here, I think.

Nonetheless:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Re: [OSSTEST PATCH 05/13] Reporting: report_run_getinfo produces ColourAttr

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 05/13] Reporting: report_run_getinfo produces ColourAttr"):
> On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
...
> > -	return { Summary => $summary, Colour => $colour };
> > +	return {
> > +	    Summary => $summary,
> > +	    ColourAttr => " bgcolor=\"$colour\"",
> 
> FWIW all the uses have a space before there $ri->{ColourAttr}, so
> strictly you don't need it here, I think.

Ah yes.

> Nonetheless:
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks.  I will remove that space and keep your ack.

Ian.

Re: [OSSTEST PATCH 05/13] Reporting: report_run_getinfo produces ColourAttr

[Ian Campbell]

On Tue, 2015-06-30 at 10:25 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 05/13] Reporting: report_run_getinfo produces ColourAttr"):
> > On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> ...
> > > -	return { Summary => $summary, Colour => $colour };
> > > +	return {
> > > +	    Summary => $summary,
> > > +	    ColourAttr => " bgcolor=\"$colour\"",
> > 
> > FWIW all the uses have a space before there $ri->{ColourAttr}, so
> > strictly you don't need it here, I think.
> 
> Ah yes.
> 
> > Nonetheless:
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Thanks.  I will remove that space and keep your ack.

Sure. I spotted another later in the series which I commented on too. 

[OSSTEST PATCH 06/13] Reporting: sg-report-job-history: Honour --limit

[Ian Jackson]

The global $limit was erroneously shadowed by a local in
processjobbranch.  Fix this, and move the erroneously hardcoded value
to the global default.

No functional change if --limit was not specified, which it is not for
any of our runs with the in-tree configs.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-report-job-history |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sg-report-job-history b/sg-report-job-history
index aa33f94..82713d3 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -28,7 +28,7 @@ use Osstest::TestSupport;
 use Osstest::Executive;
 
 our (@blessings,@branches);
-our $limit= 500;
+our $limit= 100;
 our $htmlout;
 
 open DEBUG, ">/dev/null";
@@ -123,7 +123,6 @@ sub processjobbranch ($$) {
         $cond .= " AND branch = ?";
         push @params, $bra;
     }
-    my $limit= 100;
     my $offset= $limit-1;
 
     my $fromstuff= <<END;
-- 
1.7.10.4

Re: [OSSTEST PATCH 06/13] Reporting: sg-report-job-history: Honour --limit

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> The global $limit was erroneously shadowed by a local in
> processjobbranch.  Fix this, and move the erroneously hardcoded value
> to the global default.
> 
> No functional change if --limit was not specified, which it is not for
> any of our runs with the in-tree configs.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 07/13] Reporting: report_run_getinfo produces Content

[Ian Jackson]

Previously we produced Summary, and the call sites were confused about
whether it had already been html-escaped.  Now we produce something
which is explicitly already html.

No functional change except that some sg-report-host-history output is
escaped as it ought to have been (but this is only relevant if the
database contains strange things which none of ours do).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm   |    3 ++-
 sg-report-host-history |    2 +-
 sg-report-job-history  |    2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index f53d271..9df4d91 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -32,6 +32,7 @@ use IO::Handle;
 use JSON;
 use File::Basename;
 use IO::Socket::INET;
+use HTML::Entities;
 #use Data::Dumper;
 
 use Osstest;
@@ -206,7 +207,7 @@ sub report_run_getinfo ($) {
     my $single = sub {
 	my ($summary, $colour) = @_;
 	return {
-	    Summary => $summary,
+	    Content => encode_entities($summary),
 	    ColourAttr => " bgcolor=\"$colour\"",
 	};
     };
diff --git a/sg-report-host-history b/sg-report-host-history
index 9e3edf7..d4e5dea 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -242,7 +242,7 @@ END
 	print H "<td><a href=\"$url\">$jr->{job}</td>\n";
 
 	my $ri = report_run_getinfo({ %$jr, %$ir });
-	print H "<td $ri->{ColourAttr}>$ri->{Summary}</td>\n";
+	print H "<td $ri->{ColourAttr}>$ri->{Content}</td>\n";
 
 	print H "</tr>\n\n";
 	$alternate ^= 1;
diff --git a/sg-report-job-history b/sg-report-job-history
index 82713d3..a932c78 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -235,7 +235,7 @@ END
             print H "<td>".encode_entities($r->{Flight}{branch})."</td>\n";
             $url= "$c{ReportHtmlPubBaseUrl}/$flt/".encode_entities($j)."/";
             print H "<td $r->{ColourAttr}><a href=\"$url\">".
-                encode_entities($r->{Summary})."</a></td>\n";
+                $r->{Content}."</a></td>\n";
             my $lastrev;
             my $hosts = join ", ", map { $_ // "-" } @{ $r->{Hosts} };
             print H "<td>".encode_entities($hosts)."</td>\n";
-- 
1.7.10.4

Re: [OSSTEST PATCH 07/13] Reporting: report_run_getinfo produces Content

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> Previously we produced Summary, and the call sites were confused about
> whether it had already been html-escaped.  Now we produce something
> which is explicitly already html.
> 
> No functional change except that some sg-report-host-history output is
> escaped as it ought to have been (but this is only relevant if the
> database contains strange things which none of ours do).
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond

[Ian Jackson]

The returned SQL is a self-contained expression, and does not require
additional bind parameters.  To spot SQL quoting problems, we die if
a blessing is not reasonable.

The use sites of the $blessingcond in sg-report-flight are adjusted to
no longer pass @blessings to execute.

No overall functional change with reasonable blessings.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm |   13 +++++++++++++
 sg-report-flight     |   10 +++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 9df4d91..ba668bc 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -47,6 +47,7 @@ BEGIN {
     @EXPORT      = qw(get_harness_rev grabrepolock_reexec
                       findtask @all_lock_tables
                       report_run_getinfo report_altcolour
+                      report_blessingscond
                       tcpconnect_queuedaemon plan_search
                       alloc_resources alloc_resources_rollback_begin_work
                       resource_check_allocated resource_shared_mark_ready
@@ -246,6 +247,18 @@ sub report_altcolour ($) {
     return "bgcolor=\"#".(qw(d0d0d0 ffffff))[$bool]."\"";
 }
 
+sub report_blessingscond ($$) {
+    my ($blessings, $maxflight) = @_;
+    my $blessingscond= '('.join(' OR ', map {
+	die if m/[^-_.0-9a-z]/;
+	"blessing='$_'"
+				} @$blessings).')';
+    if (defined $maxflight) {
+	$blessingscond= "( flight <= $maxflight AND $blessingscond )";
+    }
+    return $blessingscond;
+}
+
 #---------- host (and other resource) allocation ----------
 
 our $taskid;
diff --git a/sg-report-flight b/sg-report-flight
index 117b609..c1661ec 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -127,11 +127,7 @@ our $cw= 79;
 our $tl= 20;
 our $htmlleaf= "info.html";
 
-our $blessingscond= '('.join(' OR ', map { "blessing=?" } @blessings).')';
-
-if (defined $maxflight) {
-    $blessingscond= "( flight <= $maxflight AND $blessingscond )";
-}
+our $blessingscond= report_blessingscond(\@blessings, $maxflight);
 
 sub displayflightnum ($) {
     my ($flight) = @_;
@@ -202,7 +198,7 @@ END
       ORDER BY blessing ASC, flight DESC
 END
     $flightsq= db_prepare($flightsq);
-    $flightsq->execute(@flightsq_params, @blessings);
+    $flightsq->execute(@flightsq_params);
 
     my $buildflightsq= db_prepare(<<END);
         SELECT val FROM runvars
@@ -691,7 +687,7 @@ END
             next;
         }
 
-        $anypassq->execute($j->{job}, $s->{testid}, @blessings);
+        $anypassq->execute($j->{job}, $s->{testid});
         if (!$anypassq->fetchrow_hashref()) {
             print MRO "never-passed $j->{job} $s->{testid} $st\n";
             print DEBUG " never passed\n";
-- 
1.7.10.4

Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> The returned SQL is a self-contained expression, and does not require
> additional bind parameters.  To spot SQL quoting problems, we die if
> a blessing is not reasonable.
> 
> The use sites of the $blessingcond in sg-report-flight are adjusted to
> no longer pass @blessings to execute.
> 
> No overall functional change with reasonable blessings.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

BTW, I noticed that your valid blessing regex here includes "-", which
is not allowed elsewhere, which breaks "./standalone make-flight -f
$branch $branch" for our common branch names meaning the flight name
needs some munging. A minor inconvenience, but if this check were moved
to a centralised helper it could be easily fixed...

Or at least this was once the case, I can't see the problematic check
right now and haven't actually tried this for a while. Perhaps it got
fixed already and I didn't notice?

> ---
>  Osstest/Executive.pm |   13 +++++++++++++
>  sg-report-flight     |   10 +++-------
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
> index 9df4d91..ba668bc 100644
> --- a/Osstest/Executive.pm
> +++ b/Osstest/Executive.pm
> @@ -47,6 +47,7 @@ BEGIN {
>      @EXPORT      = qw(get_harness_rev grabrepolock_reexec
>                        findtask @all_lock_tables
>                        report_run_getinfo report_altcolour
> +                      report_blessingscond
>                        tcpconnect_queuedaemon plan_search
>                        alloc_resources alloc_resources_rollback_begin_work
>                        resource_check_allocated resource_shared_mark_ready
> @@ -246,6 +247,18 @@ sub report_altcolour ($) {
>      return "bgcolor=\"#".(qw(d0d0d0 ffffff))[$bool]."\"";
>  }
>  
> +sub report_blessingscond ($$) {
> +    my ($blessings, $maxflight) = @_;
> +    my $blessingscond= '('.join(' OR ', map {
> +	die if m/[^-_.0-9a-z]/;
> +	"blessing='$_'"
> +				} @$blessings).')';
> +    if (defined $maxflight) {
> +	$blessingscond= "( flight <= $maxflight AND $blessingscond )";
> +    }
> +    return $blessingscond;
> +}
> +
>  #---------- host (and other resource) allocation ----------
>  
>  our $taskid;
> diff --git a/sg-report-flight b/sg-report-flight
> index 117b609..c1661ec 100755
> --- a/sg-report-flight
> +++ b/sg-report-flight
> @@ -127,11 +127,7 @@ our $cw= 79;
>  our $tl= 20;
>  our $htmlleaf= "info.html";
>  
> -our $blessingscond= '('.join(' OR ', map { "blessing=?" } @blessings).')';
> -
> -if (defined $maxflight) {
> -    $blessingscond= "( flight <= $maxflight AND $blessingscond )";
> -}
> +our $blessingscond= report_blessingscond(\@blessings, $maxflight);
>  
>  sub displayflightnum ($) {
>      my ($flight) = @_;
> @@ -202,7 +198,7 @@ END
>        ORDER BY blessing ASC, flight DESC
>  END
>      $flightsq= db_prepare($flightsq);
> -    $flightsq->execute(@flightsq_params, @blessings);
> +    $flightsq->execute(@flightsq_params);
>  
>      my $buildflightsq= db_prepare(<<END);
>          SELECT val FROM runvars
> @@ -691,7 +687,7 @@ END
>              next;
>          }
>  
> -        $anypassq->execute($j->{job}, $s->{testid}, @blessings);
> +        $anypassq->execute($j->{job}, $s->{testid});
>          if (!$anypassq->fetchrow_hashref()) {
>              print MRO "never-passed $j->{job} $s->{testid} $st\n";
>              print DEBUG " never passed\n";

Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond"):
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks.

> BTW, I noticed that your valid blessing regex here includes "-", which
> is not allowed elsewhere, which breaks "./standalone make-flight -f
> $branch $branch" for our common branch names meaning the flight name
> needs some munging. A minor inconvenience, but if this check were moved
> to a centralised helper it could be easily fixed...

I guess.  Do we have anywhere else that validates a blessing name ?
This particular bit of code is just intended a safety catch to stop
the SQL going wrong.

IMO `-' is valid in blessings.  Consider `commission-oseleta'.

> Or at least this was once the case, I can't see the problematic check
> right now and haven't actually tried this for a while. Perhaps it got
> fixed already and I didn't notice?

I'm not sure what check exactly you are referring to...

Ian.

Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond

[Ian Campbell]

On Tue, 2015-06-30 at 10:31 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond"):
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Thanks.
> 
> > BTW, I noticed that your valid blessing regex here includes "-", which
> > is not allowed elsewhere, which breaks "./standalone make-flight -f
> > $branch $branch" for our common branch names meaning the flight name
> > needs some munging. A minor inconvenience, but if this check were moved
> > to a centralised helper it could be easily fixed...
> 
> I guess.  Do we have anywhere else that validates a blessing name ?
> This particular bit of code is just intended a safety catch to stop
> the SQL going wrong.
> 
> IMO `-' is valid in blessings.  Consider `commission-oseleta'.

I'm being very confused -- I was thinking of flight names, not blessing
names.

> > Or at least this was once the case, I can't see the problematic check
> > right now and haven't actually tried this for a while. Perhaps it got
> > fixed already and I didn't notice?
> 
> I'm not sure what check exactly you are referring to...

Neither was I right then (above and beyond s/blessing/flight/
confusion).

I've just done "./standalone -f xen-unstable xen-unstable" and it seems
to have worked but:

$ ./mg-show-flight-runvars xen-unstable
Died at ./mg-show-flight-runvars line 42.

Which is from:
        die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;

I also notice that standalone-generate-flight-runvars is laundering out
the - too.

Ian.

Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond"):
> On Tue, 2015-06-30 at 10:31 +0100, Ian Jackson wrote:
> > IMO `-' is valid in blessings.  Consider `commission-oseleta'.
> 
> I'm being very confused -- I was thinking of flight names, not blessing
> names.

Ah.  Yes, I think `-' isn't legal in flight names.  Even that letters
are permitted is a concession :-).

> I've just done "./standalone -f xen-unstable xen-unstable" and it seems
> to have worked but:
> 
> $ ./mg-show-flight-runvars xen-unstable
> Died at ./mg-show-flight-runvars line 42.
> 
> Which is from:
>         die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
> 
> I also notice that standalone-generate-flight-runvars is laundering out
> the - too.

Perhaps having a central regexp for flight names would be a good
thing, yes, but I don't think it's relevant for this patch.  I suggest
we cross that bridge next time we have to introduce another ad-hoc
check for a flight name.

Ian.

Re: [OSSTEST PATCH 08/13] Reporting: Break out report_blessingscond

[Ian Campbell]

On Tue, 2015-06-30 at 10:54 +0100, Ian Jackson wrote:
> Perhaps having a central regexp for flight names would be a good
> thing, yes, but I don't think it's relevant for this patch.

Right yes, I was confusing blessings and flights when I first piped up,
sorry.

>   I suggest
> we cross that bridge next time we have to introduce another ad-hoc
> check for a flight name.

Ack.

[OSSTEST PATCH 09/13] Reporting sg-report-job-history: support --max-flight

[Ian Jackson]

Using report_blessingscond instead of the open-coded map.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-report-job-history |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sg-report-job-history b/sg-report-job-history
index a932c78..31b8d37 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -30,6 +30,7 @@ use Osstest::Executive;
 our (@blessings,@branches);
 our $limit= 100;
 our $htmlout;
+our $maxflight;
 
 open DEBUG, ">/dev/null";
 
@@ -40,6 +41,8 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
         $$1= $2;
     } elsif (m/^--(limit)\=([1-9]\d*)$/) {
         $$1= $2;
+    } elsif (m/^--max-flight\=([1-9]\d*)$/) {
+	$maxflight= $1;
     } elsif (m/^--html-dir=(.*)$/) {
         $htmlout= $1;
     } elsif (m/^--branches?=(.*)$/) {
@@ -116,9 +119,9 @@ sub processjobbranch ($$) {
     my @rev_grid_cols;
     my @test_rows;
 
-    my $blessingscond= '('.join(' OR ', map { "blessing=?" } @blessings).')';
+    my $blessingscond= report_blessingscond(\@blessings, $maxflight);
     my $cond = "job = ? AND $blessingscond";
-    my (@params) = ($j, @blessings);
+    my (@params) = ($j);
     if (defined $bra) {
         $cond .= " AND branch = ?";
         push @params, $bra;
-- 
1.7.10.4

Re: [OSSTEST PATCH 09/13] Reporting sg-report-job-history: support --max-flight

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> Using report_blessingscond instead of the open-coded map.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 10/13] Reporting: sg-report-host-history: Support --max-flight

[Ian Jackson]

We can't use report_blessingscond because we handle blessings
differently in sg-report-host-history (since we want the history of
even adhoc etc. flights).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-report-host-history |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sg-report-host-history b/sg-report-host-history
index d4e5dea..f33fafa 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -31,6 +31,7 @@ our $limit= 200;
 our $flightlimit;
 our $htmlout = ".";
 our @blessings;
+our $maxflight;
 
 open DEBUG, ">/dev/null";
 
@@ -44,6 +45,8 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
         $$1= $2;
     } elsif (m/^--flight-limit\=([1-9]\d*)$/) {
 	$flightlimit= $1;
+    } elsif (m/^--max-flight\=([1-9]\d*)$/) {
+	$maxflight= $1;
     } elsif (m/^--blessings?=(.*)$/) {
         push @blessings, split ',', $1;
     } elsif (m/^--html-dir=(.*)$/) {
@@ -59,6 +62,7 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
 
 @ARGV or die $!;
 
+our $maxflightcond = defined($maxflight) ? "flight <= $maxflight" : "TRUE";
 our $flightcond;
 
 sub computeflightsrange () {
@@ -85,6 +89,7 @@ END
 	  FROM (
 	    SELECT flight
 	      FROM flights
+             WHERE $maxflightcond
 	     ORDER BY flight DESC
 	     LIMIT $flightlimit
 	  ) f
@@ -116,6 +121,7 @@ sub mainquery () {
 	 WHERE $namecond
 	   AND ($valcond)
 	   AND $flightcond
+           AND $maxflightcond
 	 ORDER BY flight DESC
 	 LIMIT ($limit * 3 + 100) * ?
 END
-- 
1.7.10.4

Re: [OSSTEST PATCH 10/13] Reporting: sg-report-host-history: Support --max-flight

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> We can't use report_blessingscond because we handle blessings
> differently in sg-report-host-history (since we want the history of
> even adhoc etc. flights).
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 11/13] Reporting: In history tables, show multiple failure steps

[Ian Jackson]

In report_run_getinfo, used by sg-report-job-history and
sg-report-host-history, make provision for reporting multiple failed
steps.

The failed steps are put side by side (in a single table cell).  The
cell background is set by the job status, but the individual steps may
vary their colour, as seen in this example:
  http://xenbits.xen.org/people/iwj/2015/linux-next.html

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm |   32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index ba668bc..9f90f67 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -222,19 +222,31 @@ sub report_run_getinfo ($) {
 	     WHERE flight=? AND job=?
 	       AND status!='pass'
 	  ORDER BY stepno
-	     LIMIT 1
 END
         $failstepq->execute($f->{flight}, $f->{job});
-        my $fs= $failstepq->fetchrow_hashref();
-        if (!defined $fs) {
-            return $single->("(unknown)", $yellow);
-        } elsif ($fs->{status} eq 'fail') {
-            return $single->("$fs->{testid}", $failcolour);
-        } elsif ($fs->{status} eq 'broken') {
-            return $single->("$fs->{testid} broken", $yellow);
-        } else {
-            return $single->("$fs->{testid} $fs->{status}", $failcolour);
+	my @content;
+	while (my $fs = $failstepq->fetchrow_hashref()) {
+	    my $summary = $fs->{testid};
+	    my $colour;
+	    if ($fs->{status} eq 'fail') {
+		$colour = $red;
+	    } elsif ($fs->{status} eq 'broken') {
+		$summary .= " broken";
+		$colour = $yellow;
+	    } else {
+		$summary .= " $fs->{status}";
+		$colour = $failcolour;
+	    }
+	    push @content, "<span style=\"background-color: $colour\">".
+		encode_entities($summary)."</span>";
         }
+	if (!@content) {
+	    return $single->("(unknown)", $yellow);
+	}
+	return {
+	    Content => (join " | ", @content),
+	    ColourAttr => " bgcolor=\"$failcolour\"",
+	};
     } elsif ($status eq 'blocked') {
         return $single->("blocked", $purple),
     } else {
-- 
1.7.10.4

Re: [OSSTEST PATCH 11/13] Reporting: In history tables, show multiple failure steps

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> In report_run_getinfo, used by sg-report-job-history and
> sg-report-host-history, make provision for reporting multiple failed
> steps.
> 
> The failed steps are put side by side (in a single table cell).  The
> cell background is set by the job status, but the individual steps may
> vary their colour, as seen in this example:
>   http://xenbits.xen.org/people/iwj/2015/linux-next.html
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> +	    ColourAttr => " bgcolor=\"$failcolour\"",

Another possibly unnecessary space, FWIW.


> +	};
>      } elsif ($status eq 'blocked') {
>          return $single->("blocked", $purple),
>      } else {

[OSSTEST PATCH 12/13] Reporting: Provide report_find_push_age_info

[Ian Jackson]

This utility function provides information about how far behind the
push gate is right now.

I have tested it with this rune in the Cambridge instance:

  perl -we 'use Osstest::Executive; use Osstest; use Data::Dumper; open DEBUG, ">&STDERR" or die $!; csreadconfig(); print Dumper report_find_push_age_info([qw(real)], undef, [qw(osstest linux-next)], "osstest", "280a18b2b2612174e473a1c1a137ddd47c49aab9", "cc8b79ce9586d2b0fbddbd4260e876eab1d408d4")'

Currently it produces this output:

$VAR1 = {
          'CountTip' => '4',
          'FirstTip' => {
                          'flight' => 37635,
                          'intended' => 'real',
                          'blessing' => 'real',
                          'started' => 1435460756,
                          'branch' => 'osstest'
                        },
          'LastTip' => {
                         'flight' => 37638,
                         'intended' => 'real',
                         'blessing' => 'real',
                         'started' => 1435538150,
                         'branch' => 'osstest'
                       },
          'Basis' => {
                       'flight' => 37629,
                       'intended' => 'real',
                       'blessing' => 'real',
                       'started' => 1435181770,
                       'branch' => 'osstest'
                     },
          'FirstAfterBasis' => {
                                 'flight' => 37630,
                                 'intended' => 'real',
                                 'blessing' => 'real',
                                 'started' => 1435314944,
                                 'branch' => 'osstest'
                               },
          'CountAfterBasis' => '9'
        };

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm |  142 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 141 insertions(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 9f90f67..cf15de4 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -47,7 +47,7 @@ BEGIN {
     @EXPORT      = qw(get_harness_rev grabrepolock_reexec
                       findtask @all_lock_tables
                       report_run_getinfo report_altcolour
-                      report_blessingscond
+                      report_blessingscond report_find_push_age_info
                       tcpconnect_queuedaemon plan_search
                       alloc_resources alloc_resources_rollback_begin_work
                       resource_check_allocated resource_shared_mark_ready
@@ -271,6 +271,146 @@ sub report_blessingscond ($$) {
     return $blessingscond;
 }
 
+sub report__find_test ($$$$$$$$) {
+    my ($blessings, $maxflight, $branches, $tree,
+	$revision, $selection, $extracond, $sortlimit) = @_;
+    # Reports information about a flight which tried to test $revision
+    # of $tree.  ($revision may be undef);
+
+    my @params;
+
+    my $querytext = <<END;
+        SELECT $selection
+	 FROM flights f
+	WHERE
+END
+
+    if (defined $revision) {
+	if ($tree eq 'osstest') {
+	    $querytext .= <<END;
+		EXISTS (
+		   SELECT 1
+		    FROM flights_harness_touched t
+		   WHERE t.harness=?
+		     AND t.flight=f.flight
+		 )
+END
+            push @params, $revision;
+	} else {
+	    $querytext .= <<END;
+		EXISTS (
+		   SELECT 1
+		    FROM runvars
+		   WHERE name=?
+		     AND val=?
+		     AND r.flight=f.flight
+		 )
+END
+            push @params, "revision_$tree", $revision;
+        }
+    } else {
+	$querytext .= <<END;
+	    TRUE
+END
+    }
+
+    my $blessingscond = report_blessingscond($blessings,$maxflight);
+    $querytext .= <<END;
+	  AND $blessingscond
+END
+
+    my $branchescond = join ' OR ', map { "branch=?" } @$branches;
+    $querytext .= <<END;
+	  AND ($branchescond)
+END
+    push @params, @$branches;
+
+    $querytext .= $extracond;
+    $querytext .= $sortlimit;
+
+    my $query = db_prepare($querytext);
+    $query->execute(@params);
+
+    my $row = $query->fetchrow_hashref();
+    $query->finish();
+    return $row;
+}
+
+sub report_find_push_age_info ($$$$$$) {
+    my ($blessings, $maxflight, $branches, $tree,
+	$basis_revision, $tip_revision) = @_;
+    # Reports information about tests of $tree.
+    # (Subject to @$blessings, $maxflight, @$branches)
+    # Returns {
+    #    Basis           =>  row for last test of basis
+    #    FirstAfterBasis =>  row for first test after basis
+    #    FirstTip        =>  row for first test of tip (after Basis)
+    #    LastTip         =>  row for last test of tip (after Basis)
+    #    CountAfterBasis =>  count of runs strictly after Basis
+    #    CountTip        =>  count of runs on Tip
+    #  }
+    # where
+    #  row for ... is from fetchrow_hashref of SELECT * FROM flights
+    #                 (or undef if no such thing exists)
+    #  Count       is a scalar integer.
+    #
+    # Only flights which specified the exact revision specified
+    # are considered (not ones which specified a tag, for example).
+
+    my $findtest = sub {
+	my ($revision,$selection,$extracond,$sortlimit) = @_;
+	report__find_test($blessings,$maxflight,$branches,$tree,
+			 $revision,$selection,$extracond,$sortlimit);
+    };
+
+    my $findcount = sub {
+	my ($revision,$extracond,$sortlimit) = @_;
+	my $row = $findtest->($revision, 'COUNT(*) AS count',
+			      $extracond, $sortlimit);
+	return $row->{count} // die "$revision $extracond $sortlimit ?";
+    };
+
+    my $out = { };
+    $out->{Basis} = $findtest->($basis_revision, '*', '', <<END);
+        ORDER BY flight DESC
+        LIMIT 1
+END
+
+    my $afterbasis = $out->{Basis} ? <<END : '';
+        AND flight > $out->{Basis}{flight}
+END
+
+    $out->{FirstAfterBasis} = $findtest->(undef, '*', $afterbasis, <<END)
+        ORDER BY flight ASC
+	LIMIT 1
+END
+        if $afterbasis;
+
+    $out->{FirstTip} = $findtest->($tip_revision, '*', $afterbasis, <<END);
+        ORDER BY flight ASC
+        LIMIT 1
+END
+
+    my $likelytip = $out->{FirstTip} ? <<END : '';
+        AND flight >= $out->{FirstTip}{flight}
+END
+
+    $out->{LastTip} = $findtest->($tip_revision, '*', $likelytip, <<END)
+        ORDER BY flight DESC
+        LIMIT 1
+END
+        if $out->{FirstTip};
+
+    $out->{CountAfterBasis} = $findcount->(undef, $afterbasis, '')
+        if $afterbasis;
+
+    $out->{CountTip} =
+	$out->{FirstTip} ? $findcount->($tip_revision, $likelytip, '')
+	: 0;
+
+    return $out;
+}
+
 #---------- host (and other resource) allocation ----------
 
 our $taskid;
-- 
1.7.10.4

Re: [OSSTEST PATCH 12/13] Reporting: Provide report_find_push_age_info

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> This utility function provides information about how far behind the
> push gate is right now.
> 
> I have tested it with this rune in the Cambridge instance:
> 
>   perl -we 'use Osstest::Executive; use Osstest; use Data::Dumper; open DEBUG, ">&STDERR" or die $!; csreadconfig(); print Dumper report_find_push_age_info([qw(real)], undef, [qw(osstest linux-next)], "osstest", "280a18b2b2612174e473a1c1a137ddd47c49aab9", "cc8b79ce9586d2b0fbddbd4260e876eab1d408d4")'
> 
> Currently it produces this output:
> 
> $VAR1 = {
>           'CountTip' => '4',
>           'FirstTip' => {
>                           'flight' => 37635,
>                           'intended' => 'real',
>                           'blessing' => 'real',
>                           'started' => 1435460756,
>                           'branch' => 'osstest'
>                         },
>           'LastTip' => {
>                          'flight' => 37638,
>                          'intended' => 'real',
>                          'blessing' => 'real',
>                          'started' => 1435538150,
>                          'branch' => 'osstest'
>                        },
>           'Basis' => {
>                        'flight' => 37629,
>                        'intended' => 'real',
>                        'blessing' => 'real',
>                        'started' => 1435181770,
>                        'branch' => 'osstest'
>                      },
>           'FirstAfterBasis' => {
>                                  'flight' => 37630,
>                                  'intended' => 'real',
>                                  'blessing' => 'real',
>                                  'started' => 1435314944,
>                                  'branch' => 'osstest'
>                                },
>           'CountAfterBasis' => '9'
>         };
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

I've applied this and had a go, but not yet attempted to integrate into
the mg-all-branches-status script I posted. But still:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

[OSSTEST PATCH 13/13] Reporting: sg-report-flight: Put overall pushgate stats in email

[Ian Jackson]

This adds something like this (would have appeared in 37638):

  Last test of basis    37629  2015-06-24 21:36:10 Z    4 days
  Testing same since    37635  2015-06-28 03:05:56 Z    1 days    4 attempts
  Failing since         37630  2015-06-26 10:35:44 Z    3 days    9 attempts

or this (would have appeared in 37629):

  Last test of basis    37617  2015-06-20 04:06:01 Z    9 days
  Testing same since    37628  2015-06-24 13:35:23 Z    5 days    2 attempts

(Both examples from the Cambridge instance.)

This is added just after the versions are reported.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 sg-report-flight |   41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/sg-report-flight b/sg-report-flight
index c1661ec..5ca004a 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -499,6 +499,45 @@ sub printversions ($) {
     }
 }
 
+sub print_pushgate_summary () {
+    my @thistree = sort keys %{ $specver{'this'} };
+    my @thattree = sort keys %{ $specver{'that'} };
+    if (!(@thistree==1 && @thattree==1 && $thistree[0] eq $thattree[0])) {
+	print DEBUG "NO PUSHGATE SUMMARY (@thistree) != (@thattree)";
+    }
+    my $tree = $thistree[0];
+
+    my $info = report_find_push_age_info(
+	[ $blessings[0] ], $maxflight, [ $branch ],
+	$tree, $specver{that}{$tree}, $specver{this}{$tree}
+	);
+    print "\n";
+    my $now = time;
+    my $pinfo = sub {
+	my ($what, $flightkey, $countkey) = @_;
+	my $f = $info->{$flightkey};
+	my $count = $info->{$countkey};
+	printf "%-20s", $what;
+	if ($f) {
+	    printf(" %6d  %s %4d days",
+		   $f->{flight},
+		   show_abs_time($f->{started}),
+		   floor(($now - $f->{started}) / 86400));
+	} else {
+	    printf(" %34s     ","(not found)");
+	}
+	if (defined $count) {
+	    printf(" %4d attempts", $count);
+	}
+	print "\n";
+    };
+    $pinfo->('Last test of basis', "Basis",'');
+    $pinfo->('Testing same since', "FirstTip","CountTip");
+    $pinfo->('Failing since', "FirstAfterBasis","CountAfterBasis")
+        unless ($info->{FirstAfterBasis}{flight} // 'NONE')
+	    eq ($info->{FirstTip}{flight}        // 'NONE');
+}
+
 sub printout {
     my ($r, @failures) = @_;
     print <<END or die $!;
@@ -579,6 +618,8 @@ END
         printversions('that');
     }
 
+    print_pushgate_summary();
+
     if (@includefiles) {
         print "\n","-"x60, "\n" or die $!;
     }
-- 
1.7.10.4

Re: [OSSTEST PATCH 13/13] Reporting: sg-report-flight: Put overall pushgate stats in email

[Ian Campbell]

On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> This adds something like this (would have appeared in 37638):
> 
>   Last test of basis    37629  2015-06-24 21:36:10 Z    4 days
>   Testing same since    37635  2015-06-28 03:05:56 Z    1 days    4 attempts
>   Failing since         37630  2015-06-26 10:35:44 Z    3 days    9 attempts

Chronologically shouldn't the second and third lines be the other way
around?

> 
> or this (would have appeared in 37629):
> 
>   Last test of basis    37617  2015-06-20 04:06:01 Z    9 days
>   Testing same since    37628  2015-06-24 13:35:23 Z    5 days    2 attempts
> 
> (Both examples from the Cambridge instance.)
> 
> This is added just after the versions are reported.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  sg-report-flight |   41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/sg-report-flight b/sg-report-flight
> index c1661ec..5ca004a 100755
> --- a/sg-report-flight
> +++ b/sg-report-flight
> @@ -499,6 +499,45 @@ sub printversions ($) {
>      }
>  }
>  
> +sub print_pushgate_summary () {
> +    my @thistree = sort keys %{ $specver{'this'} };
> +    my @thattree = sort keys %{ $specver{'that'} };
> +    if (!(@thistree==1 && @thattree==1 && $thistree[0] eq $thattree[0])) {
> +	print DEBUG "NO PUSHGATE SUMMARY (@thistree) != (@thattree)";
> +    }
> +    my $tree = $thistree[0];
> +
> +    my $info = report_find_push_age_info(
> +	[ $blessings[0] ], $maxflight, [ $branch ],
> +	$tree, $specver{that}{$tree}, $specver{this}{$tree}
> +	);
> +    print "\n";
> +    my $now = time;
> +    my $pinfo = sub {
> +	my ($what, $flightkey, $countkey) = @_;
> +	my $f = $info->{$flightkey};
> +	my $count = $info->{$countkey};
> +	printf "%-20s", $what;
> +	if ($f) {
> +	    printf(" %6d  %s %4d days",
> +		   $f->{flight},
> +		   show_abs_time($f->{started}),
> +		   floor(($now - $f->{started}) / 86400));
> +	} else {
> +	    printf(" %34s     ","(not found)");
> +	}
> +	if (defined $count) {
> +	    printf(" %4d attempts", $count);
> +	}
> +	print "\n";
> +    };
> +    $pinfo->('Last test of basis', "Basis",'');
> +    $pinfo->('Testing same since', "FirstTip","CountTip");
> +    $pinfo->('Failing since', "FirstAfterBasis","CountAfterBasis")
> +        unless ($info->{FirstAfterBasis}{flight} // 'NONE')
> +	    eq ($info->{FirstTip}{flight}        // 'NONE');
> +}
> +
>  sub printout {
>      my ($r, @failures) = @_;
>      print <<END or die $!;
> @@ -579,6 +618,8 @@ END
>          printversions('that');
>      }
>  
> +    print_pushgate_summary();
> +
>      if (@includefiles) {
>          print "\n","-"x60, "\n" or die $!;
>      }

Re: [OSSTEST PATCH 13/13] Reporting: sg-report-flight: Put overall pushgate stats in email

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 13/13] Reporting: sg-report-flight: Put overall pushgate stats in email"):
> On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> > This adds something like this (would have appeared in 37638):
> > 
> >   Last test of basis    37629  2015-06-24 21:36:10 Z    4 days
> >   Testing same since    37635  2015-06-28 03:05:56 Z    1 days    4 attempts
> >   Failing since         37630  2015-06-26 10:35:44 Z    3 days    9 attempts
> 
> Chronologically shouldn't the second and third lines be the other way
> around?

I guess.  I will swap them.

Ian.

Re: [OSSTEST PATCH 13/13] Reporting: sg-report-flight: Put overall pushgate stats in email

[Ian Campbell]

On Tue, 2015-06-30 at 10:50 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 13/13] Reporting: sg-report-flight: Put overall pushgate stats in email"):
> > On Mon, 2015-06-29 at 18:14 +0100, Ian Jackson wrote:
> > > This adds something like this (would have appeared in 37638):
> > > 
> > >   Last test of basis    37629  2015-06-24 21:36:10 Z    4 days
> > >   Testing same since    37635  2015-06-28 03:05:56 Z    1 days    4 attempts
> > >   Failing since         37630  2015-06-26 10:35:44 Z    3 days    9 attempts
> > 
> > Chronologically shouldn't the second and third lines be the other way
> > around?
> 
> I guess.  I will swap them.

Acked-by: Ian Campbell <ian.campbell@citrix.com>