[OSSTEST PATCH 1/6] Other-revision-jobs: Provide central test

[OSSTEST PATCH 1/6] Other-revision-jobs: Provide central test

[Ian Jackson]

Since 75fbbc19 "Arrange to test migration from the previous Xen
version", some flights have contained additional jobs build-*-prev,
which build a different revision of xen.git.

However, this violates an existing assumption in several of the
automatic archaeologists, namely that a flight should contain only
runvars referring to a single revision of a tree.

We will need to adjust all the places where this assumption is baked
in.  The question arises, as to how the code in general is supposed to
know.  There are many possible schemes, but almost all of them would
involve some kind of schema change and/or would be violated by
now-recorded history.

For now we adopt the following rule: the job name tells you.  That is,
revision runvars in jobs with certain job names are disregarded.  We
call non-disregarded jobs `main-revision jobs', since they use the
`main' revisions of everything, and others `other-revision jobs'.

We provide a single function in Osstest.pm which takes as argument a
SQL expression string representing a job name, and returns a SQL
expression string evaluating to a boolean, specifying whether the job
is a main revision job.  This can be used in queries.

In subsequent patches I will go through all plausibly-relevant output
from
  git-grep 'revision_\|revision\\\\_'
and update each piece in turn.

There are obviously-irrelevant hits in TestSupport (build_clone and
store_vcs_revision) and in BuildSupport.

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

diff --git a/Osstest.pm b/Osstest.pm
index 8f97dd2..55e1aa1 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -35,6 +35,7 @@ BEGIN {
                       getmethod
                       postfork
                       flight_otherjob
+                      main_revision_job_cond
                       $dbh_tests db_retry db_retry_retry db_retry_abort
                       db_begin_work db_prepare
                       ensuredir get_filecontents_core_quiet system_checked
@@ -317,6 +318,11 @@ sub flight_otherjob ($$) {
            die "$otherflightjob ?";
 }
 
+sub main_revision_job_cond ($) {
+    my ($jobfield) = @_;
+    return "(($jobfield) NOT LIKE 'build-%-prev')";
+}
+
 sub get_filecontents_core_quiet ($) { # ENOENT => undef
     my ($path) = @_;
     if (!open GFC, '<', $path) {
-- 
1.7.10.4

[OSSTEST PATCH 2/6] Other-revision-jobs: Update report__find_test

[Ian Jackson]

This is straightforward.  We simply don't look at the revision runvars
in non-main-revision jobs, when searching for suitable flights.

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

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index 97723be..bf968c8 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -336,6 +336,7 @@ END
 		   WHERE name=?
 		     AND val=?
 		     AND r.flight=f.flight
+                     AND ${\ main_revision_job_cond('r.job') }
 		 )
 END
             push @params, "revision_$tree", $revision;
-- 
1.7.10.4

[OSSTEST PATCH 3/6] Other-revision-jobs: Update sg-check-tested

[Ian Jackson]

These changes are obvious.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
 sg-check-tested |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/sg-check-tested b/sg-check-tested
index 8cdc8a9..e503cf0 100755
--- a/sg-check-tested
+++ b/sg-check-tested
@@ -58,12 +58,14 @@ END
                WHERE (name = ? OR name = ?)
                AND   val != ''
                AND   NOT (val = ? OR val LIKE ?)
+               AND   ${\ main_revision_job_cond('r.job') }
                AND   r.flight = flights.flight)
 END
              EXISTS
              (SELECT *
                FROM runvars r
                WHERE name = ?
+               AND   ${\ main_revision_job_cond('r.job') }
                AND   r.flight = flights.flight
                AND   val != '')
 END
@@ -160,6 +162,7 @@ END
 		SELECT DISTINCT val
 		  FROM runvars
 		 WHERE flight=?
+                 AND   ${\ main_revision_job_cond('job') }
 		 AND   (name=? OR name=?)
 END
                                    $flight,
-- 
1.7.10.4

[OSSTEST PATCH 4/6] Other-revision-jobs: Update sg-report-flight and -job-history

[Ian Jackson]

We need adjust only the regression analysis.

The other occurrences of special treatment for revision fields are for
reporting output, and are in the context of a specific job.

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

diff --git a/sg-report-flight b/sg-report-flight
index 78c91da..a88379a 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -211,6 +211,7 @@ END
             SELECT flight, job, val FROM runvars
                 WHERE flight=?
 		  AND name=?
+                  AND ${\ main_revision_job_cond('job') }
 		GROUP BY flight, job, val
 END
     $revisionsq= db_prepare($revisionsq);
@@ -455,6 +456,9 @@ END
                   AND name like 'built_revision_%'
                 ORDER BY name
 END
+        # We report in jobtext revisions in non-main-revision jobs, too.
+        # (Although, at the time of writing this comment, the jobtexts
+        # seem not to ever be printed in the report...)
         $revh->execute();
         while (my $r= $revh->fetchrow_hashref()) {
             my $br= $r->{name};
diff --git a/sg-report-job-history b/sg-report-job-history
index 9a6e17b..0e2a3f9 100755
--- a/sg-report-job-history
+++ b/sg-report-job-history
@@ -93,6 +93,7 @@ our $revisionsq= db_prepare(<<END);
          WHERE flight=? AND job=?
            AND name LIKE E'built\\_revision\\_\%'
 END
+# (We report on non-main-revision jobs just as for main-revision ones.)
 
 sub add_revisions ($$$$) {
     my ($revmap, $flightnum, $j, $sfx) = @_;
-- 
1.7.10.4

[OSSTEST PATCH 5/6] Other-revision-jobs: Provide other_revision_job_suffix

[Ian Jackson]

This is a string, a function of the job name, that identifies the
class of `other revisions'.  It is empty for main-revision jobs
and currently there is only `<delimiter>prev' for build-*-prev.

We are going to use this in the bisector.

Reimplement main_revision_job_cond in terms of this.  No functional
change, except that the SQL optimiser may have more work to do.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest.pm |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Osstest.pm b/Osstest.pm
index 55e1aa1..1fe16fc 100644
--- a/Osstest.pm
+++ b/Osstest.pm
@@ -35,7 +35,7 @@ BEGIN {
                       getmethod
                       postfork
                       flight_otherjob
-                      main_revision_job_cond
+                      main_revision_job_cond other_revision_job_suffix
                       $dbh_tests db_retry db_retry_retry db_retry_abort
                       db_begin_work db_prepare
                       ensuredir get_filecontents_core_quiet system_checked
@@ -318,9 +318,19 @@ sub flight_otherjob ($$) {
            die "$otherflightjob ?";
 }
 
+sub other_revision_job_suffix ($$) {
+    my ($jobfield, $separator) = @_;
+    return <<END
+      (CASE
+       WHEN ($jobfield) LIKE 'build-%-prev' THEN '${separator}prev'
+       ELSE                                      ''
+       END)
+END
+}
+
 sub main_revision_job_cond ($) {
     my ($jobfield) = @_;
-    return "(($jobfield) NOT LIKE 'build-%-prev')";
+    return "(${\ other_revision_job_suffix($jobfield,'x') } = '')";
 }
 
 sub get_filecontents_core_quiet ($) { # ENOENT => undef
-- 
1.7.10.4

[OSSTEST PATCH 6/6] Other-revision-jobs: Update cs-bisection-step

[Ian Jackson]

This is rather more subtle.  We want to be able to bisect over all the
relevant inputs.

What we actually want to do if one of the *prev* tests fail is to
treat the "previous Xen branch" as a separate "tree" when bisecting,
so each revision tuple has both "current" and "old" Xen versions.
That way if the stable-4.x branch has broken forward migration, we
will report it properly.

Indeed, this needs to be extended not just to the Xen revision, but
all the inputs to the *prev* build.

We achieve this with new concept `other-revision job suffix',
introduced in the previous patch.  The bisector now works internally
always with tree names which are `<tree>[ <suffix>]' (delimited by a
space).  (Henceforth, we'll call `[ <suffix>]' the `othrev'.)

That is, all the revisions specified in prev build jobs are treated as
revisions of different trees to the revisions of apparently-same trees
in non-prev jobs.

The specific changes needed to cs-bisection-step are very small.  We
only need to adjust the code which reads and writes the database:

* When we do the cross join on urls and revisions which generates the
  rev tuple for a particular flight, also have the database compute
  the othrev for each tree.  Then, print the othrev in the debug
  output, and append it to the tree name.

  That resulting name is used everywhere:

  It affects `mixed revision' detection, so we consider build-*-prev
  jobs with differing revisions to problematic, or main-revision build
  jobs with differing revisions, but we treat each category of build
  job separately so the fact that the prev and main build jobs have
  different revisions is fine.

  The name is used for the key that is returned from flight_rmap.
  Thence it is used for the Name in @treeinfos, and therefore the
  results from flight_rtuple will be terms of this decorated tree
  namespace.

* When we are preparing a new job to go, we need to (effectively) undo
  this transformation.  The query which finds the `tree_' variables
  for a particular tree name is arranged to take an additional
  parameter, which is the othrev.  If the othrev does not match the
  job, the name is not returned in the results.

  Actually, because both the job and the othrev are query parameters,
  what happens is either that they match (ie, the othrev in the tree
  name from @treeinfos is indeed the othrev for the job we are
  constructing) in which case we process the variable as before; or
  they don't match, in which case the query contains contradictory
  conditions in its AND clauses, and returns no rows.

  So the ultimate effect is that we process each Name from @treeinfos
  only if it is for the this kind of job.  This slightly convoluted
  implementation arises from the fact that the job-to-othrev mapping
  is implemented as SQL, so we need to ask the database.

There is no need to change any of the output processing and reporting,
because "<tree> prev" is a perfectly good thing to print in all the
relevant contexts.

And there is no need to change how we drive adhoc-revtuple-generator,
because we do not pass it tree names at all, only urls.

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

diff --git a/cs-bisection-step b/cs-bisection-step
index 57ecac4..42bf20b 100755
--- a/cs-bisection-step
+++ b/cs-bisection-step
@@ -108,7 +108,7 @@ our ($branch,$job,$testid) = @ARGV;
 
 our ($latest_flight, $hosts, $basispass_flight);
 our (@treeinfos);
-# $treeinfos[]{Name}
+# $treeinfos[]{Name}     Name might be  "<treename> <other-job-revision-spec>"
 # $treeinfos[]{Url}
 
 our $restrictflight_cond= restrictflight_cond();
@@ -208,6 +208,7 @@ END
         SELECT url.val AS uval,
 	       rev.val AS rval,
 	       url.job AS job,
+               ${\ other_revision_job_suffix('url.job',' ') } AS othrev,
 	       url.name AS longname
 
 	    FROM tmp_build_info AS rev
@@ -232,7 +233,8 @@ END
         $row->{longname} =~ m/^tree_/ or die "$row->{longname} ?";
         my $name= $'; #'
         print DEBUG " $flight.$row->{job} uval=$row->{uval}".
-            " rval=$row->{rval} name=$name\n";
+            " rval=$row->{rval} name=$name othrev=\`$row->{othrev}'\n";
+	$name .= $row->{othrev};
         my $rev= $row->{rval};
         next unless length $rev;
         $rev =~ s/\+//g;
@@ -1069,14 +1071,21 @@ END
         SELECT name FROM runvars
           WHERE  flight=? AND job=?
             AND  name = ?
+            AND  ${\ other_revision_job_suffix('job',' ') } = ?
 END
     foreach (my $i=0; $i<@treeinfos; $i++) {
         my $name= $treeinfos[$i]{Name};
+	my $othrev = $name =~ s{ (.+)$}{} ? $1 : '';
         my $treevar= 'tree_'.$name;
-        $treeq->execute($copyflight, $popjob, $treevar);
+        $treeq->execute($copyflight, $popjob, $treevar, $othrev);
         my ($treerow) = $treeq->fetchrow_array();
         $treeq->finish();
         next unless defined $treerow;
+	# Effect of the check on other_revison_job_suffix is that
+	# we don't see a runvar row if the othrev from the treeinfo
+	# does not correspond to that of the job - ie, the treeinfo
+	# does not apply to this job.
+
         my $revname= "revision_$name";
         my $revval= $trevisions[$i];
 
-- 
1.7.10.4

Re: [OSSTEST PATCH 1/6] Other-revision-jobs: Provide central test

[Ian Jackson]

Ian Jackson writes ("[OSSTEST PATCH 1/6] Other-revision-jobs: Provide central test"):
> Since 75fbbc19 "Arrange to test migration from the previous Xen
> version", some flights have contained additional jobs build-*-prev,
> which build a different revision of xen.git.

I have force pushed all these.  They wouldn't be really tested by the
push gate anyway.  I have aborted the current osstest self-gate
flight.  I'll rebase that branch and resubmit it.

Ian.