| Message ID | 1454336912-28503-5-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
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.
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.
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 $!; }
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(-)