[OSSTEST PATCH 00/10] Bisection fix and reporting improvements

[OSSTEST PATCH 00/10] Bisection fix and reporting improvements

[Ian Jackson]

Here are:
 6 tiny patches which, together, centralise the handling of --max-flight=
 1 tiny patch to introduce --excluded-flights=F1,F2...
 1 fix to cs-bisection-step (relevant for qemu-mainline right now)
 2 small patches to slightly improve job history

[OSSTEST PATCH 01/10] Flight restriction: Move $maxflight to Osstest::Executive

[Ian Jackson]

We are going to move the interpretation of --max-flight into
Osstest::Executive.  Start this by making $maxflight shared between
Executive and the main program.  Later, we will make it no longer
exported.

There are a number of places in Executive where a `my' variable
$maxflight shadows the newly moved global.  These will be got rid of
later, too.

No functional change.

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

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index ecd38c3..ec41ca1 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -46,6 +46,7 @@ BEGIN {
     @ISA         = qw(Exporter);
     @EXPORT      = qw(get_harness_rev grabrepolock_reexec
                       findtask @all_lock_tables
+                      $maxflight
                       report_run_getinfo report_altcolour
                       report_blessingscond report_find_push_age_info
                       tcpconnect_queuedaemon plan_search
@@ -193,6 +194,8 @@ sub opendb ($) {
 
 #---------- history reporting ----------
 
+our $maxflight;
+
 our $green=  '#008800';
 our $red=    '#ff8888';
 our $yellow= '#ffff00';
diff --git a/sg-report-flight b/sg-report-flight
index e6ca38c..018e8e2 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -45,8 +45,6 @@ our (@includebeginfiles,@includefiles);
 open DEBUG, ">/dev/null";
 open MRO, ">/dev/null";
 
-our $maxflight;
-
 while (@ARGV && $ARGV[0] =~ m/^-/) {
     $_= shift @ARGV;
     last if m/^--?$/;
diff --git a/sg-report-host-history b/sg-report-host-history
index 8e0e2aa..be51483 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -31,7 +31,6 @@ our $limit= 200;
 our $flightlimit;
 our $htmlout = ".";
 our @blessings;
-our $maxflight;
 
 open DEBUG, ">/dev/null";
 
diff --git a/sg-report-job-history b/sg-report-job-history
index e20c376..358c949 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -29,7 +29,6 @@ use Osstest::Executive;
 our (@blessings,@branches);
 our $limit= 100;
 our $htmlout;
-our $maxflight;
 our $flight;
 our $job;
 
-- 
1.7.10.4

Re: [OSSTEST PATCH 01/10] Flight restriction: Move $maxflight to Osstest::Executive

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> We are going to move the interpretation of --max-flight into
> Osstest::Executive.  Start this by making $maxflight shared between
> Executive and the main program.  Later, we will make it no longer
> exported.
> 
> There are a number of places in Executive where a `my' variable
> $maxflight shadows the newly moved global.  These will be got rid of
> later, too.
> 
> No functional change.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

[OSSTEST PATCH 02/10] Flight restriction: Introduce restrictflight_arg and restrictflight_cond

[Ian Jackson]

Provide the new functions which we will eventually use everywhere.

* Introduce restrictflight_arg which can be used in an argument loop.
* Introduce restrictflight_cond which generates a SQL condition.
* Have restrictflight_cond's value include surrounding ( ) for safety.
* Use the above in sg-report-host-history, renaming $maxflightcond.

No overall functional change.

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

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index ec41ca1..05a7268 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -46,6 +46,7 @@ BEGIN {
     @ISA         = qw(Exporter);
     @EXPORT      = qw(get_harness_rev grabrepolock_reexec
                       findtask @all_lock_tables
+                      restrictflight_arg restrictflight_cond
                       $maxflight
                       report_run_getinfo report_altcolour
                       report_blessingscond report_find_push_age_info
@@ -196,6 +197,20 @@ sub opendb ($) {
 
 our $maxflight;
 
+sub restrictflight_arg ($) {
+    my ($arg) = @_;
+    if ($arg =~ m/^--max-flight\=([1-9]\d*)$/) {
+	$maxflight = $1;
+	return 1;
+    } else {
+	return 0;
+    }
+}
+
+sub restrictflight_cond () {
+    return defined($maxflight) ? "(flight <= $maxflight)" : "TRUE";
+}
+
 our $green=  '#008800';
 our $red=    '#ff8888';
 our $yellow= '#ffff00';
diff --git a/sg-report-host-history b/sg-report-host-history
index be51483..79dc519 100755
--- a/sg-report-host-history
+++ b/sg-report-host-history
@@ -44,8 +44,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 (restrictflight_arg($_)) {
+	# handled by Executive
     } elsif (m/^--blessings?=(.*)$/) {
         push @blessings, split ',', $1;
     } elsif (m/^--html-dir=(.*)$/) {
@@ -61,7 +61,7 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
 
 @ARGV or die $!;
 
-our $maxflightcond = defined($maxflight) ? "flight <= $maxflight" : "TRUE";
+our $restrictflight_cond = restrictflight_cond();
 our $flightcond;
 
 sub computeflightsrange () {
@@ -88,7 +88,7 @@ END
 	  FROM (
 	    SELECT flight
 	      FROM flights
-             WHERE $maxflightcond
+             WHERE $restrictflight_cond
 	     ORDER BY flight DESC
 	     LIMIT $flightlimit
 	  ) f
@@ -120,7 +120,7 @@ sub mainquery () {
 	 WHERE $namecond
 	   AND ($valcond)
 	   AND $flightcond
-           AND $maxflightcond
+           AND $restrictflight_cond
 	 ORDER BY flight DESC
 	 LIMIT ($limit * 3 + 100) * ?
 END
-- 
1.7.10.4

Re: [OSSTEST PATCH 02/10] Flight restriction: Introduce restrictflight_arg and restrictflight_cond

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> Provide the new functions which we will eventually use everywhere.
> 
> * Introduce restrictflight_arg which can be used in an argument loop.
> * Introduce restrictflight_cond which generates a SQL condition.
> * Have restrictflight_cond's value include surrounding ( ) for 
> safety.
> * Use the above in sg-report-host-history, renaming $maxflightcond.
> 
> No overall functional change.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

[OSSTEST PATCH 03/10] Flight restriction: Update cs-bisection-step

[Ian Jackson]

Use restrictflight_arg and restrictflight_cond.

This entails replacing $maxflight_cond (which is empty or contains a
series of texts each starting with AND) with $restrictflight_cond
(which is actually an expression, and might be just "TRUE").

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 cs-bisection-step |   20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/cs-bisection-step b/cs-bisection-step
index bf41f81..90111ff 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -61,7 +61,6 @@ open DEBUG, ">/dev/null" or die $!;
 open SUMMARY, ">/dev/null" or die $!;
 
 our @org_argv= @ARGV;
-our $maxflight_cond= '';
 our $basistemplate;
 
 while (@ARGV && $ARGV[0] =~ m/^\-/) {
@@ -84,8 +83,8 @@ while (@ARGV && $ARGV[0] =~ m/^\-/) {
         $cache_option= 0;
     } elsif (m/^--flight=(\d+)$/) {
         $popflight= $1;
-    } elsif (m/^--max-flight=(\d+)$/) {
-        $maxflight_cond= "AND flights.flight <= $1";
+    } elsif (restrictflight_arg($_)) {
+	# Handled by Executive
     } elsif (m/^--(fail|basis)-flight=(\d+)$/) {
         $specflights{$1}= $2;
     } elsif (m/^--basis-template=(\d+)$/) {
@@ -112,6 +111,7 @@ our (@treeinfos);
 # $treeinfos[]{Name}
 # $treeinfos[]{Url}
 
+our $restrictflight_cond= restrictflight_cond();
 our $blessingscond= '('.join(" OR  ", map { "blessing='$_'" } @blessings).')';
 
 sub psummary ($) {
@@ -313,7 +313,7 @@ sub findbasics () {
                  AND (status = 'fail' OR status = 'pass')
                  AND branch = ?
                  AND $flight_cond{fail}
-                     $maxflight_cond
+                 AND $restrictflight_cond
                ORDER BY notbroken DESC, flights.started DESC
 END
     my $basisq= db_prepare(<<END);
@@ -325,7 +325,7 @@ END
              AND branch = ?
              AND $flight_is_not_broken
              AND $flight_cond{basis}
-                 $maxflight_cond
+             AND $restrictflight_cond
 	   ORDER BY blessing, flights.started DESC
 END
 
@@ -567,7 +567,7 @@ END
             AND flights.started >= $basispass_date
             AND branch = ?
             AND $flight_is_not_broken
-                $maxflight_cond
+            AND $restrictflight_cond
     ");
     $jobq->execute($job,$branch);
     while (my $jobrow= $jobq->fetchrow_hashref()) {
@@ -1106,7 +1106,7 @@ END
             WHERE   j.job=?
             AND     j.status='pass'
             AND     $blessingscond
-                    $maxflight_cond
+            AND     $restrictflight_cond
             AND     0 = ( SELECT count(*)
                           FROM
                               ( SELECT name,val
@@ -1228,7 +1228,8 @@ END
     # one side is NULL (runvar was missing) it still counts.
     my $equalflightsqt = <<END;
         SELECT flight, blessing, started FROM flights
-	    WHERE branch=?  $maxflight_cond
+	    WHERE branch=?
+              AND $restrictflight_cond
               AND blessing = (
 		SELECT intended FROM flights WHERE flight = ?
                 )
@@ -1252,7 +1253,8 @@ END
 	my ($minflight) = $dbh_tests->selectrow_array(<<END, {}, $branch);
             SELECT flight FROM (
 		SELECT flight FROM flights
-		    WHERE branch=? AND $blessingscond $maxflight_cond
+		    WHERE branch=? AND $blessingscond
+		       AND $restrictflight_cond
                        AND flight >= $repro_firstflight
 		    ORDER BY flight DESC
 		    LIMIT 100
-- 
1.7.10.4

Re: [OSSTEST PATCH 03/10] Flight restriction: Update cs-bisection-step

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> Use restrictflight_arg and restrictflight_cond.
> 
> This entails replacing $maxflight_cond (which is empty or contains a
> series of texts each starting with AND) with $restrictflight_cond
> (which is actually an expression, and might be just "TRUE").
> 
> No functional change.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

[OSSTEST PATCH 04/10] Flight restriction: Update sg-report-flight and sg-report-job-history

[Ian Jackson]

Use restrictflight_arg in both these functions.  They still use
$maxflight directly, passing it to report_blessingscond; that will
change in a moment.

No functional change.

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

diff --git a/sg-report-flight b/sg-report-flight
index 018e8e2..bdfbfb6 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -65,8 +65,8 @@ while (@ARGV && $ARGV[0] =~ m/^-/) {
         push @includebeginfiles, $1;
     } elsif (m/^--include=(.*)$/) {
         push @includefiles, $1;
-    } elsif (m/^--max-flight=(\d+)$/) {
-        $maxflight= $1;
+    } elsif (restrictflight_arg($_)) {
+        # Handled by Executive
     } elsif (m/^--allow=(.*)$/) {
         my ($allowfile) = $1;
         if (!open A, '<', $allowfile) {
diff --git a/sg-report-job-history b/sg-report-job-history
index 358c949..ec2776b 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -41,8 +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 (restrictflight_arg($_)) {
+	# Handled by Executive
     } elsif (m/^--html-dir=(.*)$/) {
         $htmlout= $1;
     } elsif (m/^--branches?=(.*)$/) {
-- 
1.7.10.4

Re: [OSSTEST PATCH 04/10] Flight restriction: Update sg-report-flight and sg-report-job-history

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> Use restrictflight_arg in both these functions.  They still use
> $maxflight directly, passing it to report_blessingscond; that will
> change in a moment.
> 
> No functional change.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

[OSSTEST PATCH 05/10] Flight restriction: Make report_blessingscond use implicit $maxflight

[Ian Jackson]

We have $maxflight in Osstest::Executive now, set appropriately.

Use that in report_blessingscond and all its callers including
report_find_push_age_info and hence in mg-all-branch-statuses and
sg-report-flight and sg-report-job-history.

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm   |   21 ++++++++++-----------
 mg-all-branch-statuses |    2 +-
 sg-report-flight       |    4 ++--
 sg-report-job-history  |    2 +-
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 05a7268..a1bc0ad 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -277,20 +277,19 @@ sub report_altcolour ($) {
     return "bgcolor=\"#".(qw(d0d0d0 ffffff))[$bool]."\"";
 }
 
-sub report_blessingscond ($$) {
-    my ($blessings, $maxflight) = @_;
+sub report_blessingscond ($) {
+    my ($blessings) = @_;
+    my $flightcond= restrictflight_cond();
     my $blessingscond= '('.join(' OR ', map {
 	die if m/[^-_.0-9a-z]/;
 	"blessing='$_'"
 				} @$blessings).')';
-    if (defined $maxflight) {
-	$blessingscond= "( flight <= $maxflight AND $blessingscond )";
-    }
+    return "( $flightcond AND $blessingscond )";
     return $blessingscond;
 }
 
-sub report__find_test ($$$$$$$$) {
-    my ($blessings, $maxflight, $branches, $tree,
+sub report__find_test ($$$$$$$) {
+    my ($blessings, $branches, $tree,
 	$revision, $selection, $extracond, $sortlimit) = @_;
     # Reports information about a flight which tried to test $revision
     # of $tree.  ($revision may be undef);
@@ -332,7 +331,7 @@ END
 END
     }
 
-    my $blessingscond = report_blessingscond($blessings,$maxflight);
+    my $blessingscond = report_blessingscond($blessings);
     $querytext .= <<END;
 	  AND $blessingscond
 END
@@ -354,8 +353,8 @@ END
     return $row;
 }
 
-sub report_find_push_age_info ($$$$$$) {
-    my ($blessings, $maxflight, $branches, $tree,
+sub report_find_push_age_info ($$$$$) {
+    my ($blessings, $branches, $tree,
 	$basis_revision, $tip_revision) = @_;
     # Reports information about tests of $tree.
     # (Subject to @$blessings, $maxflight, @$branches)
@@ -377,7 +376,7 @@ sub report_find_push_age_info ($$$$$$) {
 
     my $findtest = sub {
 	my ($revision,$selection,$extracond,$sortlimit) = @_;
-	report__find_test($blessings,$maxflight,$branches,$tree,
+	report__find_test($blessings,$branches,$tree,
 			 $revision,$selection,$extracond,$sortlimit);
     };
 
diff --git a/mg-all-branch-statuses b/mg-all-branch-statuses
index 8b86e3b..bba7216 100755
--- a/mg-all-branch-statuses
+++ b/mg-all-branch-statuses
@@ -53,7 +53,7 @@ gather_info()
 	my ($branch,$tree,$basis,$tip) = @ARGV;
 	print DEBUG "branch=$branch tree=$tree basis=$basis tip=$tip\n";
 	my $info = report_find_push_age_info([qw(real adhoc play)],
-					      undef, [($branch)],
+					      [($branch)],
 					      $tree, $basis, $tip);
 	print DEBUG Dumper $info;
 	my $onevar = sub {
diff --git a/sg-report-flight b/sg-report-flight
index bdfbfb6..78c91da 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -128,7 +128,7 @@ our $cw= 79;
 our $tl= 20;
 our $htmlleaf= "info.html";
 
-our $blessingscond= report_blessingscond(\@blessings, $maxflight);
+our $blessingscond= report_blessingscond(\@blessings);
 
 sub displayflightnum ($) {
     my ($flight) = @_;
@@ -523,7 +523,7 @@ sub print_pushgate_summary () {
     my $tree = $thistree[0];
 
     my $info = report_find_push_age_info(
-	[ $blessings[0] ], $maxflight, [ $branch ],
+	[ $blessings[0] ], [ $branch ],
 	$tree, $specver{that}{$tree}, $specver{this}{$tree}
 	);
     bodyprint "\n";
diff --git a/sg-report-job-history b/sg-report-job-history
index ec2776b..e8cf50f 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -119,7 +119,7 @@ sub processjobbranch ($$) {
     my @rev_grid_cols;
     my @test_rows;
 
-    my $blessingscond= report_blessingscond(\@blessings, $maxflight);
+    my $blessingscond= report_blessingscond(\@blessings);
     my $cond = "job = ? AND $blessingscond";
     my (@params) = ($j);
     if (defined $bra) {
-- 
1.7.10.4

Re: [OSSTEST PATCH 05/10] Flight restriction: Make report_blessingscond use implicit $maxflight

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> We have $maxflight in Osstest::Executive now, set appropriately.
> 
> Use that in report_blessingscond and all its callers including
> report_find_push_age_info and hence in mg-all-branch-statuses and
> sg-report-flight and sg-report-job-history.
> 
> No functional change.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>


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

[OSSTEST PATCH 06/10] Flight restriction: Change implementation of --max-flight

[Ian Jackson]

Abolish $maxflight.  All the users outside Osstest::Executive have
been eliminated, so this is fine.  Replace it with
$restrictflight_cond, which can accumulate multiple conditions.

There is a minor functional change: when multiple --max-flight options
are specified, _all_ of them take effect (effectively using the lowest
value).  That option is not used in production, and I don't expect
people elsewhere to be passing multiple different such options.

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

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index a1bc0ad..b8ab5cd 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -47,7 +47,6 @@ BEGIN {
     @EXPORT      = qw(get_harness_rev grabrepolock_reexec
                       findtask @all_lock_tables
                       restrictflight_arg restrictflight_cond
-                      $maxflight
                       report_run_getinfo report_altcolour
                       report_blessingscond report_find_push_age_info
                       tcpconnect_queuedaemon plan_search
@@ -195,12 +194,12 @@ sub opendb ($) {
 
 #---------- history reporting ----------
 
-our $maxflight;
+our $restrictflight_cond = 'TRUE';
 
 sub restrictflight_arg ($) {
     my ($arg) = @_;
     if ($arg =~ m/^--max-flight\=([1-9]\d*)$/) {
-	$maxflight = $1;
+	$restrictflight_cond .= " AND flight <= $1";
 	return 1;
     } else {
 	return 0;
@@ -208,7 +207,7 @@ sub restrictflight_arg ($) {
 }
 
 sub restrictflight_cond () {
-    return defined($maxflight) ? "(flight <= $maxflight)" : "TRUE";
+    return "($restrictflight_cond)";
 }
 
 our $green=  '#008800';
-- 
1.7.10.4

Re: [OSSTEST PATCH 06/10] Flight restriction: Change implementation of --max-flight

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> Abolish $maxflight.  All the users outside Osstest::Executive have
> been eliminated, so this is fine.  Replace it with
> $restrictflight_cond, which can accumulate multiple conditions.
> 
> There is a minor functional change: when multiple --max-flight 
> options
> are specified, _all_ of them take effect (effectively using the 
> lowest
> value).  That option is not used in production, and I don't expect
> people elsewhere to be passing multiple different such options.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

[OSSTEST PATCH 07/10] Flight restriction: Honour --exclude-flights=F1, F2, ...

[Ian Jackson]

To reproduce a recent bisection problem I needed to exclude not just
all flights after a certain number, but also one earlier flight.  So I
invented this option (and associated yaks).

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

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index b8ab5cd..998a0d0 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -201,6 +201,10 @@ sub restrictflight_arg ($) {
     if ($arg =~ m/^--max-flight\=([1-9]\d*)$/) {
 	$restrictflight_cond .= " AND flight <= $1";
 	return 1;
+    } elsif ($arg =~ m/^--exclude-flights=([0-9,]+)$/) {
+        $restrictflight_cond .= " AND flights.flight != $_"
+	    foreach split /,/, $1;
+	return 1;
     } else {
 	return 0;
     }
-- 
1.7.10.4

Re: [OSSTEST PATCH 07/10] Flight restriction: Honour --exclude-flights=F1, F2, ...

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> To reproduce a recent bisection problem I needed to exclude not just
> all flights after a certain number, but also one earlier flight.  So 
> I
> invented this option (and associated yaks).
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

[OSSTEST PATCH 08/10] cs-bisection-step: Fix memoisation of search_compute_length_at

[Ian Jackson]

There was a half-implemented memoisation.  Memoisation is necessary
because otherwise the algorithm is exponential in the commit history
depth (with base equal to the commit parent fanout).

Sort this out:
 * Break out the actual computation into a ne
   search_compute_length_at_intern
 * Deleting the individual memo assignments, which incidentally
   means we no longer miss an (unimportant) one.
 * Actually having the new memoising function search_compute_length_at
   check $n->{LengthAt} (this is the bugfix).

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 cs-bisection-step |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/cs-bisection-step b/cs-bisection-step
index 90111ff..57ecac4 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -639,8 +639,10 @@ sub consolidateresults () {
 our @interesting_lengths;
 
 sub search_compute_length_at ($);
-sub search_compute_length_at ($) {
+
+sub search_compute_length_at_intern ($) {
     my ($n) = @_;
+
     print DEBUG "CLA $n->{Rtuple} ";
     if (!relevant($n)) {
         print DEBUG "X (irrelevant)\n";
@@ -652,7 +654,6 @@ sub search_compute_length_at ($) {
         if ($n->{Result} ne 'pass') {
             search_compute_length_below($n);
         }
-        $n->{LengthAt}= 0;
         return 0;
     }
     my $res= 1;
@@ -662,10 +663,15 @@ sub search_compute_length_at ($) {
         next if $t eq 'X';
         $res += $t;
     }
-    $n->{LengthAt}= $res;
     return $res;
 }
 
+sub search_compute_length_at ($) {
+    my ($n) = @_;
+    return $n->{LengthAt} if exists $n->{LengthAt};
+    return $n->{LengthAt} = search_compute_length_at_intern($n);
+}
+
 sub search_compute_length_below ($) {
     my ($base) = @_;
     return if $base->{UninterestingFailure};
-- 
1.7.10.4

Re: [OSSTEST PATCH 08/10] cs-bisection-step: Fix memoisation of search_compute_length_at

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> There was a half-implemented memoisation.  Memoisation is necessary
> because otherwise the algorithm is exponential in the commit history
> depth (with base equal to the commit parent fanout).
> 
> Sort this out:
>  * Break out the actual computation into a ne

Some stray characters here?

>    search_compute_length_at_intern
>  * Deleting the individual memo assignments, which incidentally
>    means we no longer miss an (unimportant) one.
>  * Actually having the new memoising function 
> search_compute_length_at
>    check $n->{LengthAt} (this is the bugfix).
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

Re: [OSSTEST PATCH 08/10] cs-bisection-step: Fix memoisation of search_compute_length_at

[Ian Jackson]

Ian Campbell writes ("Re: [OSSTEST PATCH 08/10] cs-bisection-step: Fix memoisation of search_compute_length_at"):
> On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> > There was a half-implemented memoisation.  Memoisation is necessary
> > because otherwise the algorithm is exponential in the commit history
> > depth (with base equal to the commit parent fanout).
> > 
> > Sort this out:
> >  * Break out the actual computation into a ne
> >    search_compute_length_at_intern
> 
> Some stray characters here?

Just a missing `w', fixed.

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

Thanks.

.oO{ git-filter-brach --msg-filter 'cat; echo "Acked-by"' }

Ian.

Re: [OSSTEST PATCH 08/10] cs-bisection-step: Fix memoisation of search_compute_length_at

[Ian Campbell]

On Fri, 2015-07-24 at 11:57 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [OSSTEST PATCH 08/10] cs-bisection-step: 
> Fix memoisation of search_compute_length_at"):
> > On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> > > There was a half-implemented memoisation.  Memoisation is 
> > > necessary
> > > because otherwise the algorithm is exponential in the commit 
> > > history
> > > depth (with base equal to the commit parent fanout).
> > > 
> > > Sort this out:
> > >  * Break out the actual computation into a ne
> > >    search_compute_length_at_intern
> > 
> > Some stray characters here?
> 
> Just a missing `w', fixed.
> 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> Thanks.
> 
> .oO{ git-filter-brach --msg-filter 'cat; echo "Acked-by"' }

$ cat /home/ianc/git-ack-branch
#!/bin/bash

set -ex

: ${BY:=Acked-by: Ian Campbell <ian.campbell@citrix.com>}

git filter-branch  -f --msg-filter "cat ; echo \"${BY}\"" $@
$

[OSSTEST PATCH 09/10] sg-report-job-history: Introduce report_altchangecolour

[Ian Jackson]

No functional change.

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

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 998a0d0..79d433a 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -48,6 +48,7 @@ BEGIN {
                       findtask @all_lock_tables
                       restrictflight_arg restrictflight_cond
                       report_run_getinfo report_altcolour
+                      report_altchangecolour
                       report_blessingscond report_find_push_age_info
                       tcpconnect_queuedaemon plan_search
                       alloc_resources alloc_resources_rollback_begin_work
@@ -280,6 +281,17 @@ sub report_altcolour ($) {
     return "bgcolor=\"#".(qw(d0d0d0 ffffff))[$bool]."\"";
 }
 
+sub report_altchangecolour ($$) {
+    my ($stateref, $thisvalue) = @_;
+    my $state = $$stateref //= { Bool => 0 };
+    my $same =
+	!!defined($thisvalue) == !!defined($state->{LastValue}) &&
+	(!defined $thisvalue || $thisvalue eq $state->{LastValue});
+    $state->{Bool} ^= !$same;
+    $state->{LastValue} = $thisvalue;
+    return report_altcolour($state->{Bool});
+}
+
 sub report_blessingscond ($) {
     my ($blessings) = @_;
     my $flightcond= restrictflight_cond();
diff --git a/sg-report-job-history b/sg-report-job-history
index e8cf50f..11fdaaf 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -231,7 +231,7 @@ END
         print H "</tr>\n";
         my $alternate= 0;
         my @last_revs;
-        my @alt_revs= ('0')x $#rev_grid_cols;
+        my @alt_revs;
         foreach my $r (@test_rows) {
             my $altcolour= report_altcolour($alternate);
             print H "<tr $altcolour>";
@@ -263,12 +263,8 @@ END
                 "</td>\n";
             foreach my $i (0..$#rev_grid_cols) {
                 my $v= $r->{Revisions}[$i];
-                my $same=
-                    !defined($last_revs[$i]) ? !defined($v) :
-                    !defined($v) ? 0 :
-                    $last_revs[$i] eq $v;
-                $alt_revs[$i] ^= !$same;
-                print H "<td ".report_altcolour($alt_revs[$i]).">";
+		my $rev_colour = report_altchangecolour(\$alt_revs[$i], $v);
+                print H "<td $rev_colour>";
                 if (defined $v) {
                     my $vp= $v;
                     if (defined $lastrev && $v eq $lastrev) {
-- 
1.7.10.4

Re: [OSSTEST PATCH 09/10] sg-report-job-history: Introduce report_altchangecolour

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> No functional change.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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

[OSSTEST PATCH 10/10] sg-report-job-history: Colour host column according to whether it changes

[Ian Jackson]

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

diff --git a/sg-report-job-history b/sg-report-job-history
index 11fdaaf..9a6e17b 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -232,6 +232,7 @@ END
         my $alternate= 0;
         my @last_revs;
         my @alt_revs;
+        my $alt_hosts;
         foreach my $r (@test_rows) {
             my $altcolour= report_altcolour($alternate);
             print H "<tr $altcolour>";
@@ -249,7 +250,8 @@ END
                 $r->{Content}."</a></td>\n";
             my $lastrev;
             my $hosts = join ", ", map { $_ // "-" } @{ $r->{Hosts} };
-            print H "<td>".encode_entities($hosts)."</td>\n";
+            my $hosts_colour = report_altchangecolour(\$alt_hosts, $hosts);
+            print H "<td $hosts_colour>".encode_entities($hosts)."</td>\n";
             $osstestverq->execute($r->{Flight}{flight});
             print H
                 "<td>",
-- 
1.7.10.4

Re: [OSSTEST PATCH 10/10] sg-report-job-history: Colour host column according to whether it changes

[Ian Campbell]

On Thu, 2015-07-23 at 19:19 +0100, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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