[PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars

[PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars

[Ian Campbell]

This is useful if you have flights which refer to other flights.

This is probably split into more patches than necessary, but it makes it a
bit easier to read (and made it a bit easier for me to figure out the
correct path to success).

The final patch in particular may not be very "Perl-ish". Suggestions
welcome.

Ian.

[PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step

[Ian Campbell]

This will make it easier to collect more rows.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 mg-show-flight-runvars | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index c800110..820c577 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -46,6 +46,7 @@ die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 our ($flight) = @ARGV;
 
 our @cols = qw(job name val);
+our @rows;
 
 $flight =~ m/^\d+/ or $flight = "'$flight'";
 my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
@@ -62,6 +63,10 @@ $colws[1] += length $synthsufx;
 while (my (@row) = $q->fetchrow_array()) {
     my $synth = shift @row;
     $row[1] .= $synthsufx if $synth && $synth ne 'f'; # sqlite3 is typeless
-    printf "%-*s %-*s %-*s\n", map { $colws[$_], $row[$_] } qw(0 1 2)
+    push @rows, \@row;
+}
+
+foreach my $row (@rows) {
+    printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
         or die $!;
 }
-- 
2.6.1

Re: [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step"):
> This will make it easier to collect more rows.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub

[Ian Campbell]

This will make it easier to collect more rows.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 mg-show-flight-runvars | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index 820c577..0995430 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -43,29 +43,35 @@ for (;;) {
 
 die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 
-our ($flight) = @ARGV;
 
 our @cols = qw(job name val);
 our @rows;
+our @colws;
 
-$flight =~ m/^\d+/ or $flight = "'$flight'";
-my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
+sub collect ($) {
+    my ($flight) = @_;
 
-my @colws = $dbh_tests->selectrow_array
-    ("SELECT ".(join ',', map { "max(length($_))" } @cols)." $qfrom");
+    $flight =~ m/^\d+/ or $flight = "'$flight'";
+    my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
 
-my $q = $dbh_tests->prepare
-    ("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, job");
-$q->execute();
+    @colws = $dbh_tests->selectrow_array
+	("SELECT ".(join ',', map { "max(length($_))" } @cols)." $qfrom");
 
-$colws[1] += length $synthsufx;
+    my $q = $dbh_tests->prepare
+	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, job");
+    $q->execute();
 
-while (my (@row) = $q->fetchrow_array()) {
-    my $synth = shift @row;
-    $row[1] .= $synthsufx if $synth && $synth ne 'f'; # sqlite3 is typeless
-    push @rows, \@row;
+    while (my (@row) = $q->fetchrow_array()) {
+	my $synth = shift @row;
+	$row[1] .= $synthsufx if $synth && $synth ne 'f'; # sqlite3 is typeless
+	push @rows, \@row;
+    }
 }
 
+$colws[1] += length $synthsufx;
+
+collect($ARGV[0]);
+
 foreach my $row (@rows) {
     printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
         or die $!;
-- 
2.6.1

Re: [PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub"):
> This will make it easier to collect more rows.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL

[Ian Campbell]

This will work even once @rows is not all collected by the same SQL
statement.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 mg-show-flight-runvars | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index 0995430..0cf4e13 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -46,7 +46,6 @@ die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 
 our @cols = qw(job name val);
 our @rows;
-our @colws;
 
 sub collect ($) {
     my ($flight) = @_;
@@ -54,9 +53,6 @@ sub collect ($) {
     $flight =~ m/^\d+/ or $flight = "'$flight'";
     my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
 
-    @colws = $dbh_tests->selectrow_array
-	("SELECT ".(join ',', map { "max(length($_))" } @cols)." $qfrom");
-
     my $q = $dbh_tests->prepare
 	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, job");
     $q->execute();
@@ -68,10 +64,15 @@ sub collect ($) {
     }
 }
 
-$colws[1] += length $synthsufx;
-
 collect($ARGV[0]);
 
+our @colws;
+sub max ($$) { $_[$_[0] < $_[1]] }
+foreach my $row (@rows) {
+    @colws = map { max($colws[$_]//0,length($row->[$_])) } qw(0 1 2)
+}
+$colws[1] += length $synthsufx;
+
 foreach my $row (@rows) {
     printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
         or die $!;
-- 
2.6.1

Re: [PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL"):
> This will work even once @rows is not all collected by the same SQL
> statement.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse)

[Ian Campbell]

Adds a new -r (==recurse) option which for now only adds "$flight." to
the job name, i.e. nothing is recursive yet.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 mg-show-flight-runvars | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index 0cf4e13..f96539f 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -27,6 +27,7 @@ csreadconfig();
 
 my $synthcond = '(NOT synth)';
 my $synthsufx = '';
+my $recurse = 0;
 
 for (;;) {
     last unless @ARGV;
@@ -36,6 +37,8 @@ for (;;) {
     if (m/^-a$/) {
 	$synthcond = '(1=1)';
 	$synthsufx = '~';
+    } elsif (m/^-r$/) {
+	$recurse = 1;
     } else {
 	die "$_ ?";
     }
@@ -59,6 +62,7 @@ sub collect ($) {
 
     while (my (@row) = $q->fetchrow_array()) {
 	my $synth = shift @row;
+	$row[0] = "$flight.$row[0]" if $recurse;
 	$row[1] .= $synthsufx if $synth && $synth ne 'f'; # sqlite3 is typeless
 	push @rows, \@row;
     }
-- 
2.6.1

Re: [PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse)

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse)"):
> Adds a new -r (==recurse) option which for now only adds "$flight." to
> the job name, i.e. nothing is recursive yet.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

[PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Campbell]

By looping over @rows looking for buildjobs runvars and adding those
jobs to the output until nothing changes.

The output is resorted by runvar name which is the desired default
behaviour. As usual can be piped to sort(1) to sort by flight+job.

Note that synth runvars (if requests) are now sorted in with the
regular ones, whereas previously they were listed last. Retaining the
old behaviour would be too complex I feel.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
 mg-show-flight-runvars | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index f96539f..0bfbc6f 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -46,15 +46,17 @@ for (;;) {
 
 die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 
-
 our @cols = qw(job name val);
 our @rows;
+our %jobs;
+
+sub collect ($;$) {
+    my ($flight,$jobcond) = @_;
 
-sub collect ($) {
-    my ($flight) = @_;
+    $jobcond //= "TRUE";
 
     $flight =~ m/^\d+/ or $flight = "'$flight'";
-    my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
+    my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond AND $jobcond";
 
     my $q = $dbh_tests->prepare
 	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, job");
@@ -65,11 +67,33 @@ sub collect ($) {
 	$row[0] = "$flight.$row[0]" if $recurse;
 	$row[1] .= $synthsufx if $synth && $synth ne 'f'; # sqlite3 is typeless
 	push @rows, \@row;
+	$jobs{$row[0]} = 1;
     }
 }
 
 collect($ARGV[0]);
 
+foreach my $row (@rows) {
+    next unless $row->[1] =~ m/^(?:.*_)?([^_]*)buildjob$/;
+    next if $jobs{$row->[2]};
+
+    # parse this flight and job, which must be in $flight.$job
+    # form if $recurse is true (see collect())
+    my ($tflight, $tjob) = flight_otherjob(undef, $row->[0]);
+    die "$row->[1]" unless $tflight;
+
+    # parse the buildjob reference and recurse. might be a job in
+    # this flight, in which case we still recurse since it might
+    # be a chain from a non-top-level job which hasn't been
+    # included yet. %jobs will prevent us from duplicating or
+    # infinite loops.
+    my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
+    collect($oflight, "job = '$ojob'");
+
+    # collect() appends to @rows, so we don't need any special
+    # handling to pickup anything which was newly added.
+}
+
 our @colws;
 sub max ($$) { $_[$_[0] < $_[1]] }
 foreach my $row (@rows) {
@@ -77,7 +101,8 @@ foreach my $row (@rows) {
 }
 $colws[1] += length $synthsufx;
 
-foreach my $row (@rows) {
+# Sort by runvar name, then (flight+)job.
+foreach my $row (sort { $a->[1] cmp $b->[1]//$a->[0] cmp $b->[1] } @rows) {
     printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
         or die $!;
 }
-- 
2.6.1

Re: [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request"):
> By looping over @rows looking for buildjobs runvars and adding those
> jobs to the output until nothing changes.
...
> Note that synth runvars (if requests) are now sorted in with the
> regular ones, whereas previously they were listed last. Retaining the
> old behaviour would be too complex I feel.

...
> -sub collect ($) {
> -    my ($flight) = @_;
> +    $jobcond //= "TRUE";
...
>      my $q = $dbh_tests->prepare
>  	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, job");

You probably want to drop the `synth' from the ORDER here, even if you
take my suggestion below.  The sort is still a good idea here because
it ensures that the output is deterministic.

> +    my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
> +    collect($oflight, "job = '$ojob'");

SQL injection vulnerability, I think.  (There are lots of places
where jobs are treated this way, but they come from the command line
or similar, or have been checked against some regexp.)

I think you should use the $jobcond @jobcondparams pattern.


> -foreach my $row (@rows) {
> +# Sort by runvar name, then (flight+)job.
> +foreach my $row (sort { $a->[1] cmp $b->[1]//$a->[0] cmp $b->[1] } @rows) {

If you retain it this way, put spaces round your //.

But maybe you should use a Schwartzian transform instead.

   my $xform = sub { [ ($->[1] =~ m/\~$/)." $_->[1] $_->[0]", $_ ]; };
   foreach my $row (map { $_->[1] } sort { $xform->($a) cmp
       $xform->($b) etc.

This makes the synth sorting easy too.

Ian.

Re: [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Campbell]

On Mon, 2016-02-01 at 15:19 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH OSSTEST v1 5/5] mg-show-flight-runvars:
> recurse on buildjobs upon request"):
> > By looping over @rows looking for buildjobs runvars and adding those
> > jobs to the output until nothing changes.
> ...
> > Note that synth runvars (if requests) are now sorted in with the
> > regular ones, whereas previously they were listed last. Retaining the
> > old behaviour would be too complex I feel.
> 
> ...
> > -sub collect ($) {
> > -    my ($flight) = @_;
> > +    $jobcond //= "TRUE";
> ...
> >      my $q = $dbh_tests->prepare
> >  	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth,
> > name, job");
> 
> You probably want to drop the `synth' from the ORDER here, even if you
> take my suggestion below.  The sort is still a good idea here because
> it ensures that the output is deterministic.

OK

> > +    my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
> > +    collect($oflight, "job = '$ojob'");
> 
> SQL injection vulnerability, I think.  (There are lots of places
> where jobs are treated this way, but they come from the command line
> or similar, or have been checked against some regexp.)
> 
> I think you should use the $jobcond @jobcondparams pattern.

Indeed. Fixed.

> -foreach my $row (@rows) {
> > +# Sort by runvar name, then (flight+)job.
> > +foreach my $row (sort { $a->[1] cmp $b->[1]//$a->[0] cmp $b->[1] }
> > @rows) {
> 
> If you retain it this way, put spaces round your //.
> 
> But maybe you should use a Schwartzian transform instead.
> 
>    my $xform = sub { [ ($->[1] =~ m/\~$/)." $_->[1] $_->[0]", $_ ]; };

>    foreach my $row (map { $_->[1] } sort { $xform->($a) cmp
>        $xform->($b) etc.

ITIYM:

   foreach my $row (map { $_->[1] }
                    sort { $a->[0] cmp $b->[0] }
                    map { $xform->($_) }
                    @rows) {
        ...
   }

Since doing the xform-> in the sort defeats the purpose (or I don't
properly understand the Schwartzian transform)

> This makes the synth sorting easy too.

Right.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Jackson]

Ian Campbell writes ("Re: [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request"):
> ITIYM:
> 
>    foreach my $row (map { $_->[1] }
>                     sort { $a->[0] cmp $b->[0] }
>                     map { $xform->($_) }
>                     @rows) {
>         ...
>    }
> 
> Since doing the xform-> in the sort defeats the purpose (or I don't
> properly understand the Schwartzian transform)

Yes.  Actually, if you do that then you don't need to break out
$xform->().  Sorry, my previous message said `Schwartzian transform'
but uses a slightly different (and arguably inferior) idiom.

Ian.

[PATCH OSSTEST 5/5 v2] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Campbell]

By looping over @rows looking for buildjobs runvars and adding those
jobs to the output until nothing changes.

The output is resorted by runvar name which is the desired default
behaviour. As usual can be piped to sort(1) to sort by flight+job.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - Use $jobcond,@jobconfparams to avoid SQL injection
 - Only recurse if the option was given
 - Drop synth from ORDER BY
 - Use a Schwatzian transform for the sort, at the same time allowing
   retention of the sorting of synth runvars last.
---
 mg-show-flight-runvars | 44 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index f96539f..faa0ea1 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -46,30 +46,56 @@ for (;;) {
 
 die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 
-
 our @cols = qw(job name val);
 our @rows;
+our %jobs;
+
+sub collect ($;$@) {
+    my ($flight,$jobcond,@jobcondparams) = @_;
 
-sub collect ($) {
-    my ($flight) = @_;
+    $jobcond //= "TRUE";
 
     $flight =~ m/^\d+/ or $flight = "'$flight'";
-    my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
+    my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond AND $jobcond";
 
     my $q = $dbh_tests->prepare
-	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, job");
-    $q->execute();
+	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY name, job");
+    $q->execute(@jobcondparams);
 
     while (my (@row) = $q->fetchrow_array()) {
 	my $synth = shift @row;
 	$row[0] = "$flight.$row[0]" if $recurse;
 	$row[1] .= $synthsufx if $synth && $synth ne 'f'; # sqlite3 is typeless
 	push @rows, \@row;
+	$jobs{$row[0]} = 1;
     }
 }
 
 collect($ARGV[0]);
 
+if ($recurse) {
+    foreach my $row (@rows) {
+	next unless $row->[1] =~ m/^(?:.*_)?([^_]*)buildjob$/;
+	next if $jobs{$row->[2]};
+
+	# parse this flight and job, which must be in $flight.$job
+	# form if $recurse is true (see collect())
+	my ($tflight, $tjob) = flight_otherjob(undef, $row->[0]);
+	die "$row->[1]" unless $tflight;
+
+	# parse the buildjob reference and recurse. might be a job in
+	# this flight, in which case we still recurse since it might
+	# be a chain from a non-top-level job which hasn't been
+	# included yet. %jobs will prevent us from duplicating or
+	# infinite loops.
+	my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
+	collect($oflight, "job = ?", $ojob);
+
+	# collect() appends to @rows, so we don't need any special
+	# handling to pickup anything which was newly added.
+    }
+}
+
 our @colws;
 sub max ($$) { $_[$_[0] < $_[1]] }
 foreach my $row (@rows) {
@@ -77,7 +103,11 @@ foreach my $row (@rows) {
 }
 $colws[1] += length $synthsufx;
 
-foreach my $row (@rows) {
+# Sort by runvar name, then (flight+)job, synth runvars come last.
+foreach my $row (map { $_->[0] }
+		 sort { $a->[1] cmp $b->[1] }
+		 map { [ $_, ($_->[1] =~ m/~$/)." $_->[1] $_->[0]" ] }
+		 @rows) {
     printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
         or die $!;
 }
-- 
2.6.1

Re: [PATCH OSSTEST 5/5 v2] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 5/5 v2] mg-show-flight-runvars: recurse on buildjobs upon request"):
> By looping over @rows looking for buildjobs runvars and adding those
> jobs to the output until nothing changes.
> 
> The output is resorted by runvar name which is the desired default
> behaviour. As usual can be piped to sort(1) to sort by flight+job.
...
> +if ($recurse) {
> +    foreach my $row (@rows) {
> +	next unless $row->[1] =~ m/^(?:.*_)?([^_]*)buildjob$/;
> +	next if $jobs{$row->[2]};

This does the deduping based on $row->[2] which is the runvar value.
But the same job might be referred to by different runvar values.  I
think this means that you might report the same job twice.

If
   1234.foo  buildjob   wombat
   1234.bar  buildjob   1234.wombat
then you dump 1234.wombat twice.

> +	my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);

Also the place you set $jobs{} is somewhat wrong.  If there is a job
with no rows you can process it twice (harmlessly, I think, as it
happens).


So I think you want
        next if $jobs{$oflight,$ojob}++;

> +	collect($oflight, "job = ?", $ojob);
> +
> +	# collect() appends to @rows, so we don't need any special
> +	# handling to pickup anything which was newly added.

I don't think this is true.  Or at least, it happens to be true, but
my copy of the FM says:

       If any part of LIST is an array, "foreach" will get very
       confused if you add or remove elements within the loop body,
       for example with "splice".  So don't do that.

AIUI you're avoiding actual recursion, inside collect()'s row loop, to
avoid trying to have one SQL query for each nesting level.  It might
be worth mentioning that somewhere.

Maybe you should have the foreach use an explicit row index.

Ian.

[PATCH OSSTEST 5/5 v3] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Campbell]

By looping over @rows looking for buildjobs runvars and adding those
jobs to the output until nothing changes.

The output is resorted by runvar name which is the desired default
behaviour. As usual can be piped to sort(1) to sort by flight+job.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2:
 - Use $jobcond,@jobconfparams to avoid SQL injection
 - Only recurse if the option was given
 - Drop synth from ORDER BY
 - Use a Schwatzian transform for the sort, at the same time allowing
   retention of the sorting of synth runvars last.
v3:
 - Fixup detection of already handle jobs to cope properly with jobs
   with now rows.
 - Iterate over @rows using an index var
---
 mg-show-flight-runvars | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/mg-show-flight-runvars b/mg-show-flight-runvars
index f96539f..c847456 100755
--- a/mg-show-flight-runvars
+++ b/mg-show-flight-runvars
@@ -46,19 +46,21 @@ for (;;) {
 
 die unless @ARGV==1 && $ARGV[0] =~ m/^\w+$/;
 
-
 our @cols = qw(job name val);
 our @rows;
+our %jobs;
+
+sub collect ($;$@) {
+    my ($flight,$jobcond,@jobcondparams) = @_;
 
-sub collect ($) {
-    my ($flight) = @_;
+    $jobcond //= "TRUE";
 
     $flight =~ m/^\d+/ or $flight = "'$flight'";
-    my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond";
+    my $qfrom = "FROM runvars WHERE flight=$flight AND $synthcond AND $jobcond";
 
     my $q = $dbh_tests->prepare
-	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY synth, name, job");
-    $q->execute();
+	("SELECT synth, ".(join ',', @cols)." $qfrom ORDER BY name, job");
+    $q->execute(@jobcondparams);
 
     while (my (@row) = $q->fetchrow_array()) {
 	my $synth = shift @row;
@@ -70,6 +72,34 @@ sub collect ($) {
 
 collect($ARGV[0]);
 
+if ($recurse) {
+    # collect() appends to @rows, avoiding the need to recurse
+    # there. However this means we can't use foreach (since that is
+    # not guaranteed to work if the array is mutated under its feet),
+    # instead use an index.
+    for ( my $rowidx = 0; $rowidx < @rows ; $rowidx++ ) {
+	my $row = $rows[$rowidx];
+
+	next unless $row->[1] =~ m/^(?:.*_)?([^_]*)buildjob$/;
+
+	# parse this flight and job, which must be in $flight.$job
+	# form if $recurse is true (see collect())
+	my ($tflight, $tjob) = flight_otherjob(undef, $row->[0]);
+	die "$row->[1]" unless $tflight;
+
+	# parse the buildjob reference and recurse. might be a job in
+	# this flight, in which case we still recurse since it might
+	# be a chain from a non-top-level job which hasn't been
+	# included yet. %jobs will prevent us from duplicating or
+	# infinite loops.
+	my ($oflight, $ojob) = flight_otherjob($tflight, $row->[2]);
+
+	next if $jobs{$oflight,$ojob}++;
+
+	collect($oflight, "job = ?", $ojob);
+    }
+}
+
 our @colws;
 sub max ($$) { $_[$_[0] < $_[1]] }
 foreach my $row (@rows) {
@@ -77,7 +107,11 @@ foreach my $row (@rows) {
 }
 $colws[1] += length $synthsufx;
 
-foreach my $row (@rows) {
+# Sort by runvar name, then (flight+)job, synth runvars come last.
+foreach my $row (map { $_->[0] }
+		 sort { $a->[1] cmp $b->[1] }
+		 map { [ $_, ($_->[1] =~ m/~$/)." $_->[1] $_->[0]" ] }
+		 @rows) {
     printf "%-*s %-*s %-*s\n", map { $colws[$_], $row->[$_] } qw(0 1 2)
         or die $!;
 }
-- 
2.6.1

Re: [PATCH OSSTEST 5/5 v3] mg-show-flight-runvars: recurse on buildjobs upon request

[Ian Jackson]

Ian Campbell writes ("[PATCH OSSTEST 5/5 v3] mg-show-flight-runvars: recurse on buildjobs upon request"):
> By looping over @rows looking for buildjobs runvars and adding those
> jobs to the output until nothing changes.
> 
> The output is resorted by runvar name which is the desired default
> behaviour. As usual can be piped to sort(1) to sort by flight+job.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>