[OSSTEST PATCH] sg-report-flight: Fix --include (broken by 3d56a191)

[OSSTEST PATCH] sg-report-flight: Fix --include (broken by 3d56a191)

[Ian Jackson]

In 3d56a191 "sg-report-flight: Make bodyprint[f] defer the output" it
became wrong for anything in the main printout part of
sg-report-flight to write directly to STDOUT.  However, the two (very
similar) bits of code for handling the two kinds of --include still
used cat.

The result is emails looking like this:

  From: osstest service owner <osstest-admin@xenproject.org>
  To: <xen-devel@lists.xensource.com>, <rumpkernel-builds@freelists.org>
  CC: <ian.jackson@eu.citrix.com>
  Subject: [rumpuserxen test] People who touched revisions under test:
    Alice Bob <alice@example.com>
    Ian Jackson <Ian.Jackson@eu.citrix.com>
    Carol David <carol@example.com>
  Date: Fri, 3 Jul 2015 06:50:53 +0000

  flight 59037 rumpuserxen real [real]

Fix this by replacing those two near-identical bits of code with calls
to a single function which slurps the required include file into the
body text accumulation variable.

(There is no need to provide a facility for header includes, since
there is no need for a caller to pass headers through
sg-report-flight: they can simply be written into the output file
before running sg-report-flight, as indeed the string `Subject: ' is.

Despite the fact that --include-begin is used only by cr-daily-branch
which passes it a file `heading', the text is intended for the start
of the body, not the email headers.)

I have run

  ./sg-report-flight --include-begin=header --include=bodyinc --this-xen=e13013dbf1d5997915548a3b5f1c39594d8c1d7b --that-xen=84066dd4ef4bb5983e246c629a26ef4f3394e5d5 --html-dir=$HOME/public_html/d --max-flight=37590 37590

with some suitable lorem ipsum in bodyinc and header and observed:

(a) The bug introduced in 3d56a191 is visible in 3c464fbf.

(b) The output after this patch differs from the output produced by
1f0c168d (the previous osstest production commit) only in addition of
the `Last test of basis' and `Testing same since' information, in the
appropriate place.

Deployment note: Because this patch changes only sg-report-flight, it
is safe to deploy under the feet of a running flight.  I intend to do
this for all the production branches in the colo.

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

diff --git a/sg-report-flight b/sg-report-flight
index 829e0c3..e6ca38c 100755
--- a/sg-report-flight
+++ b/sg-report-flight
@@ -555,6 +555,17 @@ sub print_pushgate_summary () {
     $pinfo->('Testing same since', "FirstTip","CountTip");
 }
 
+sub includes ($) {
+    my ($includefiles) = @_;
+    local $/ = undef;
+    foreach my $include (@$includefiles) {
+	open INCLUDE, $include or die "include $include: $!";
+	$body_text .= <INCLUDE>;
+	die $! if INCLUDE->error;
+	close INCLUDE;
+    }
+}    
+
 sub printout {
     my ($r, @failures) = @_;
     $header_text = '';
@@ -564,10 +575,7 @@ sub printout {
     $header_text .= <<END or die $!;
 $r->{Flight}: $r->{OutcomeSummary}
 END
-    foreach my $include (@includebeginfiles) {
-        STDOUT->flush();
-        system_checked('cat','--',$include);
-    }
+    includes(\@includebeginfiles);
     bodyprint <<END;
 flight $r->{Flight} $branch $r->{FlightInfo}{blessing} [$r->{FlightInfo}{intended}]
 $c{ReportHtmlPubBaseUrl}/$r->{Flight}/
@@ -655,11 +663,7 @@ END
     if (@includefiles) {
         bodyprint "\n","-"x60, "\n";
     }
-    foreach my $include (@includefiles) {
-        STDOUT->flush();
-        system_checked('cat','--',$include);
-        bodyprint "-"x60, "\n";
-    }
+    includes(\@includefiles);
 
     bodyprint "\njobs:\n", $r->{JobsSummary}, "\n";
 
-- 
1.7.10.4

Re: [OSSTEST PATCH] sg-report-flight: Fix --include (broken by 3d56a191)

[Ian Campbell]

On Fri, 2015-07-03 at 11:23 +0100, Ian Jackson wrote:
> In 3d56a191 "sg-report-flight: Make bodyprint[f] defer the output" it
> became wrong for anything in the main printout part of
> sg-report-flight to write directly to STDOUT.  However, the two (very
> similar) bits of code for handling the two kinds of --include still
> used cat.
> 
> The result is emails looking like this:
> 
>   From: osstest service owner <osstest-admin@xenproject.org>
>   To: <xen-devel@lists.xensource.com>, <rumpkernel-builds@freelists.org>
>   CC: <ian.jackson@eu.citrix.com>
>   Subject: [rumpuserxen test] People who touched revisions under test:
>     Alice Bob <alice@example.com>
>     Ian Jackson <Ian.Jackson@eu.citrix.com>
>     Carol David <carol@example.com>
>   Date: Fri, 3 Jul 2015 06:50:53 +0000
> 
>   flight 59037 rumpuserxen real [real]
> 
> Fix this by replacing those two near-identical bits of code with calls
> to a single function which slurps the required include file into the
> body text accumulation variable.
> 
> (There is no need to provide a facility for header includes, since
> there is no need for a caller to pass headers through
> sg-report-flight: they can simply be written into the output file
> before running sg-report-flight, as indeed the string `Subject: ' is.
> 
> Despite the fact that --include-begin is used only by cr-daily-branch
> which passes it a file `heading', the text is intended for the start
> of the body, not the email headers.)
> 
> I have run
> 
>   ./sg-report-flight --include-begin=header --include=bodyinc --this-xen=e13013dbf1d5997915548a3b5f1c39594d8c1d7b --that-xen=84066dd4ef4bb5983e246c629a26ef4f3394e5d5 --html-dir=$HOME/public_html/d --max-flight=37590 37590
> 
> with some suitable lorem ipsum in bodyinc and header and observed:
> 
> (a) The bug introduced in 3d56a191 is visible in 3c464fbf.
> 
> (b) The output after this patch differs from the output produced by
> 1f0c168d (the previous osstest production commit) only in addition of
> the `Last test of basis' and `Testing same since' information, in the
> appropriate place.
> 
> Deployment note: Because this patch changes only sg-report-flight, it
> is safe to deploy under the feet of a running flight.  I intend to do
> this for all the production branches in the colo.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

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