All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars
@ 2016-02-01 14:28 Ian Campbell
  2016-02-01 14:28 ` [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step Ian Campbell
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Ian Campbell @ 2016-02-01 14:28 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step
  2016-02-01 14:28 [PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars Ian Campbell
@ 2016-02-01 14:28 ` Ian Campbell
  2016-02-01 14:42   ` Ian Jackson
  2016-02-01 14:28 ` [PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub Ian Campbell
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2016-02-01 14:28 UTC (permalink / raw)
  To: ian.jackson, xen-devel; +Cc: 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub
  2016-02-01 14:28 [PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars Ian Campbell
  2016-02-01 14:28 ` [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step Ian Campbell
@ 2016-02-01 14:28 ` Ian Campbell
  2016-02-01 14:55   ` Ian Jackson
  2016-02-01 14:28 ` [PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL Ian Campbell
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2016-02-01 14:28 UTC (permalink / raw)
  To: ian.jackson, xen-devel; +Cc: 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL
  2016-02-01 14:28 [PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars Ian Campbell
  2016-02-01 14:28 ` [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step Ian Campbell
  2016-02-01 14:28 ` [PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub Ian Campbell
@ 2016-02-01 14:28 ` Ian Campbell
  2016-02-01 14:56   ` Ian Jackson
  2016-02-01 14:28 ` [PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse) Ian Campbell
  2016-02-01 14:28 ` [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request Ian Campbell
  4 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2016-02-01 14:28 UTC (permalink / raw)
  To: ian.jackson, xen-devel; +Cc: 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse)
  2016-02-01 14:28 [PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars Ian Campbell
                   ` (2 preceding siblings ...)
  2016-02-01 14:28 ` [PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL Ian Campbell
@ 2016-02-01 14:28 ` Ian Campbell
  2016-02-01 15:19   ` Ian Jackson
  2016-02-01 14:28 ` [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request Ian Campbell
  4 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2016-02-01 14:28 UTC (permalink / raw)
  To: ian.jackson, xen-devel; +Cc: 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-01 14:28 [PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars Ian Campbell
                   ` (3 preceding siblings ...)
  2016-02-01 14:28 ` [PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse) Ian Campbell
@ 2016-02-01 14:28 ` Ian Campbell
  2016-02-01 15:19   ` Ian Jackson
                     ` (2 more replies)
  4 siblings, 3 replies; 17+ messages in thread
From: Ian Campbell @ 2016-02-01 14:28 UTC (permalink / raw)
  To: ian.jackson, xen-devel; +Cc: 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step
  2016-02-01 14:28 ` [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step Ian Campbell
@ 2016-02-01 14:42   ` Ian Jackson
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2016-02-01 14:42 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub
  2016-02-01 14:28 ` [PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub Ian Campbell
@ 2016-02-01 14:55   ` Ian Jackson
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2016-02-01 14:55 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL
  2016-02-01 14:28 ` [PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL Ian Campbell
@ 2016-02-01 14:56   ` Ian Jackson
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2016-02-01 14:56 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-01 14:28 ` [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request Ian Campbell
@ 2016-02-01 15:19   ` Ian Jackson
  2016-02-01 16:16     ` Ian Campbell
  2016-02-03  9:48   ` [PATCH OSSTEST 5/5 v2] " Ian Campbell
  2016-02-03 12:37   ` [PATCH OSSTEST 5/5 v3] " Ian Campbell
  2 siblings, 1 reply; 17+ messages in thread
From: Ian Jackson @ 2016-02-01 15:19 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse)
  2016-02-01 14:28 ` [PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse) Ian Campbell
@ 2016-02-01 15:19   ` Ian Jackson
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2016-02-01 15:19 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-01 15:19   ` Ian Jackson
@ 2016-02-01 16:16     ` Ian Campbell
  2016-02-01 16:53       ` Ian Jackson
  0 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2016-02-01 16:16 UTC (permalink / raw)
  To: Ian Jackson; +Cc: xen-devel

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-01 16:16     ` Ian Campbell
@ 2016-02-01 16:53       ` Ian Jackson
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2016-02-01 16:53 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH OSSTEST 5/5 v2] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-01 14:28 ` [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request Ian Campbell
  2016-02-01 15:19   ` Ian Jackson
@ 2016-02-03  9:48   ` Ian Campbell
  2016-02-03 12:01     ` Ian Jackson
  2016-02-03 12:37   ` [PATCH OSSTEST 5/5 v3] " Ian Campbell
  2 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2016-02-03  9:48 UTC (permalink / raw)
  To: ian.jackson, xen-devel; +Cc: 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST 5/5 v2] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-03  9:48   ` [PATCH OSSTEST 5/5 v2] " Ian Campbell
@ 2016-02-03 12:01     ` Ian Jackson
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2016-02-03 12:01 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH OSSTEST 5/5 v3] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-01 14:28 ` [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request Ian Campbell
  2016-02-01 15:19   ` Ian Jackson
  2016-02-03  9:48   ` [PATCH OSSTEST 5/5 v2] " Ian Campbell
@ 2016-02-03 12:37   ` Ian Campbell
  2016-02-08 16:12     ` Ian Jackson
  2 siblings, 1 reply; 17+ messages in thread
From: Ian Campbell @ 2016-02-03 12:37 UTC (permalink / raw)
  To: ian.jackson, xen-devel; +Cc: 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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH OSSTEST 5/5 v3] mg-show-flight-runvars: recurse on buildjobs upon request
  2016-02-03 12:37   ` [PATCH OSSTEST 5/5 v3] " Ian Campbell
@ 2016-02-08 16:12     ` Ian Jackson
  0 siblings, 0 replies; 17+ messages in thread
From: Ian Jackson @ 2016-02-08 16:12 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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>

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-02-08 16:12 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-01 14:28 [PATCH OSSTEST v1 0/5] mg-show-flight-runvars: recursively follow buildjob runvars Ian Campbell
2016-02-01 14:28 ` [PATCH OSSTEST v1 1/5] mg-show-flight-runvars: collect rows into @rows, output in second step Ian Campbell
2016-02-01 14:42   ` Ian Jackson
2016-02-01 14:28 ` [PATCH OSSTEST v1 2/5] mg-show-flight-runvars: move collection into a sub Ian Campbell
2016-02-01 14:55   ` Ian Jackson
2016-02-01 14:28 ` [PATCH OSSTEST v1 3/5] mg-show-flight-runvars: calculate @colsw from @rows not via SQL Ian Campbell
2016-02-01 14:56   ` Ian Jackson
2016-02-01 14:28 ` [PATCH OSSTEST v1 4/5] mg-show-flight-runvars: include $flight. prefix on job name if -r (recurse) Ian Campbell
2016-02-01 15:19   ` Ian Jackson
2016-02-01 14:28 ` [PATCH OSSTEST v1 5/5] mg-show-flight-runvars: recurse on buildjobs upon request Ian Campbell
2016-02-01 15:19   ` Ian Jackson
2016-02-01 16:16     ` Ian Campbell
2016-02-01 16:53       ` Ian Jackson
2016-02-03  9:48   ` [PATCH OSSTEST 5/5 v2] " Ian Campbell
2016-02-03 12:01     ` Ian Jackson
2016-02-03 12:37   ` [PATCH OSSTEST 5/5 v3] " Ian Campbell
2016-02-08 16:12     ` Ian Jackson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.