diff --git a/cgi-bin/show_log.pl b/cgi-bin/show_log.pl index 810c3a5..0804b79 100755 --- a/cgi-bin/show_log.pl +++ b/cgi-bin/show_log.pl @@ -18,6 +18,7 @@ BEGIN } use lib "$ENV{BFConfDir}/perl5"; use BFUtils; +use PatchStackLog; use DBI; use Template; @@ -32,8 +33,11 @@ BEGIN check_email_only(); -my $template_opts = { INCLUDE_PATH => $template_dir, EVAL_PERL => 1, - VARIABLES => { livery => livery() } }; +my $template_opts = { + INCLUDE_PATH => $template_dir, + EVAL_PERL => 1, + VARIABLES => { livery => livery() } +}; my $template = Template->new($template_opts); die "no dbname" unless $dbname; @@ -58,7 +62,7 @@ BEGIN my ($stage_times, $run_time); my $other_branches; my ($changed_this_run_logs, $changed_since_success_logs); -my ($patch_stack, $patch_stack_diff); +my ($patch_stack, $patch_stack_diff); use vars qw($info_row); @@ -120,7 +124,7 @@ BEGIN my $row = $sth->fetchrow_arrayref; $branch = $row->[5]; $git_head_ref = $row->[9]; - $run_time = $row->[10]; + $run_time = $row->[10]; $sth->finish; my $last_build_row; @@ -149,9 +153,9 @@ BEGIN $changed_since_success = $row->[4]; my $log_file_names = $row->[6]; $scm = $row->[7]; - $scm ||= 'cvs'; # legacy scripts + $scm ||= 'cvs'; # legacy scripts $scmurl = $row->[8]; - $scmurl = undef unless $scmurl && $scmurl =~ /^http/; # slight sanity check + $scmurl = undef unless $scmurl && $scmurl =~ /^http/; # slight sanity check $scmurl = 'http://git.postgresql.org/gitweb?p=postgresql.git;a=commit;h=' if ($scmurl && $scmurl eq 'http://git.postgresql.org/git/postgresql.git'); $log_file_names =~ s/^\{(.*)\}$/$1/ if $log_file_names; @@ -160,9 +164,8 @@ BEGIN if (grep { $_ eq 'patch_stack.log' } @log_file_names) { - my ($ptext) = - $db->selectrow_array($patch_stack_log_statement, undef, - $system, $logdate); + my ($ptext) = $db->selectrow_array($patch_stack_log_statement, + undef, $system, $logdate); $patch_stack = parse_patch_stack_log($ptext) if $ptext; } @@ -225,7 +228,7 @@ BEGIN }; $stage_times = $db->selectall_hashref($stage_times_query, 'log_stage', undef, - $system, $logdate); + $system, $logdate); unless ($run_time) { my $run_time_query = q{ @@ -252,7 +255,7 @@ BEGIN my @log_pieces; my @log_piece_names; -my @pieces = split (/$log_marker (.*?) $log_marker\r?\n/, $log); +my @pieces = split(/$log_marker (.*?) $log_marker\r?\n/, $log); if ($log =~ /^$log_marker/) { $log = ""; @@ -260,12 +263,13 @@ BEGIN elsif (@pieces) { $log = shift(@pieces); + # skip useless preliminary make output if ($log =~ /.*?\n(([A-Za-z]{3} \d\d \d\d:\d\d:\d\d )?(echo "\+\+\+))/s) { - my $pos = $-[1]; - my $good = substr($log,$pos); - my $head = substr($log,0,$pos); + my $pos = $-[1]; + my $good = substr($log, $pos); + my $head = substr($log, 0, $pos); $head =~ s/.*ake.*?Nothing to be done for.*?\n//s; $log = $head . $good; } @@ -273,7 +277,7 @@ BEGIN while (@pieces) { push(@log_piece_names, shift(@pieces)); - push(@log_pieces, shift(@pieces)); + push(@log_pieces, shift(@pieces)); } for (@log_piece_names) { @@ -300,9 +304,9 @@ BEGIN urldt => $logdate, log_file_names => \@log_file_names, conf => $conf, - log => $log, - log_pieces => \@log_pieces, - log_piece_names => \@log_piece_names, + log => $log, + log_pieces => \@log_pieces, + log_piece_names => \@log_piece_names, changed_this_run => $changed_this_run, changed_since_success => $changed_since_success, changed_this_run_logs => $changed_this_run_logs, @@ -321,52 +325,6 @@ BEGIN ########################################################## -# Parse the structured patch_stack.log artifact written by -# PGBuild::Modules::PatchStack (client-side): a few "key: value" header -# lines followed by one "filenamesubject" line per patch in the -# series. Returns a hashref { id, source, status, patches => [ {name, -# subject}, ... ] }, or undef if the text doesn't look like this format. -sub parse_patch_stack_log -{ - my $text = shift; - return unless defined $text && $text ne ''; - - my %info; - my @patches; - foreach my $line (split(/\n/, $text)) - { - if ($line =~ /^patch_stack_(id|source|status):\s?(.*)$/) - { - $info{$1} = $2; - } - elsif ($line =~ /^([^\t]+)\t(.*)$/) - { - push(@patches, { name => $1, subject => $2 }); - } - } - $info{patches} = \@patches; - return \%info; -} - -# Compare two parsed patch_stack.log structures and report which patch -# filenames were added/removed, but only when the series identity -# actually differs -- avoids noise from e.g. a patch's subject line -# changing while the tree SHA (and hence the filenames) stayed the same. -sub diff_patch_stack -{ - my ($cur, $prev) = @_; - return if ($cur->{id} // '') eq ($prev->{id} // ''); - - my %cur_names = map { $_->{name} => 1 } @{ $cur->{patches} }; - my %prev_names = map { $_->{name} => 1 } @{ $prev->{patches} }; - - my @added = sort grep { !$prev_names{$_} } keys %cur_names; - my @removed = sort grep { !$cur_names{$_} } keys %prev_names; - - return unless @added || @removed; - return { added => \@added, removed => \@removed }; -} - sub process_changed { @@ -386,7 +344,7 @@ sub process_changed ## no critic (RegularExpressions::ProhibitUnusedCapture) next if ($scm eq 'cvs' and (!m!^(pgsql|master|REL\d_\d_STABLE)/!)); push(@changed_rows, [ $1, $3 ]) if (m!(^\S+)(\s+)(\S+)!); - $commits{$3} = 1 if $scm eq 'git'; + $commits{$3} = 1 if $scm eq 'git'; } if ($git_from && $git_to) { diff --git a/perl5/PatchStackLog.pm b/perl5/PatchStackLog.pm new file mode 100644 index 0000000..5a0b6db --- /dev/null +++ b/perl5/PatchStackLog.pm @@ -0,0 +1,131 @@ +package PatchStackLog; + +use strict; +use warnings; + +## no critic (ProhibitAutomaticExportation) +use Exporter qw(import); +our (@EXPORT, @EXPORT_OK, %EXPORT_TAGS); +@EXPORT = qw( parse_patch_stack_log diff_patch_stack ); + +# Parse the structured patch_stack.log artifact written by +# PGBuild::Modules::PatchStack (client-side): "key: value" header lines +# followed by one line per patch, in series order. +# +# Format 1, written by clients before the blob-SHA change, has two +# tab-separated fields per patch line: name and subject. Format 2 +# announces itself with a "patch_stack_format: 2" header and has three: +# name, blob SHA, subject. The SHA is what lets diff_patch_stack() below +# report a patch whose content changed under an unchanged filename, +# which format 1 gives no way to detect. +# +# Header lines carry no tab, and the parser depends on that in the +# other direction: it is why a server that predates a given key skips +# it rather than mistaking it for a patch row. Any key added later must +# keep that property. +sub parse_patch_stack_log +{ + my $text = shift; + return unless defined $text && $text ne ''; + + my @lines = split(/\n/, $text); + + # Settle the format before interpreting any patch line; the marker + # is written first but nothing requires it to stay that way. + my $format = 1; + foreach my $line (@lines) + { + $format = $1 if $line =~ /^patch_stack_format:\s?(\d+)/; + } + + my %info = (format => $format); + my @patches; + + foreach my $line (@lines) + { + if ($line =~ /^patch_stack_(id|commit|source|status):\s?(.*)$/) + { + $info{$1} = $2; + next; + } + + # Any other patch_stack_* key, known or not, is not a patch row. + next if $line =~ /^patch_stack_\w+:/; + next unless $line =~ /\t/; + + if ($format >= 2) + { + my ($name, $sha, $subject) = split(/\t/, $line, 3); + + # The limit of 3 keeps a tab inside the subject intact. A + # format-2 line short of a field is read the old way rather + # than yielding a silently undefined SHA. + if (defined $subject) + { + push(@patches, + { name => $name, sha => $sha, subject => $subject }); + next; + } + } + + my ($name, $subject) = split(/\t/, $line, 2); + push(@patches, { name => $name, sha => undef, subject => $subject }); + } + + $info{patches} = \@patches; + return \%info; +} + +# Compare two parsed patch_stack.log structures. Reports patch filenames +# added and removed and -- when both runs carry blob SHAs -- filenames +# whose content changed. Returns undef when nothing moved. +# +# The identity short-circuit keeps out noise from e.g. a subject line +# changing while the series itself is untouched. +# +# When either side predates format 2 there are no SHAs to compare, so +# modifications are simply not reported. That under-reports across a +# client upgrade rather than claiming a change that may not have +# happened. +# +# A filename listed twice in one series collapses in these hashes and is +# not distinguished. That limitation predates this code and is unchanged. +sub diff_patch_stack +{ + my ($cur, $prev) = @_; + + my $cur_id = defined $cur->{id} ? $cur->{id} : ''; + my $prev_id = defined $prev->{id} ? $prev->{id} : ''; + return if $cur_id eq $prev_id; + + my %cur_sha = map { $_->{name} => $_->{sha} } @{ $cur->{patches} }; + my %prev_sha = map { $_->{name} => $_->{sha} } @{ $prev->{patches} }; + + my @added = sort grep { !exists $prev_sha{$_} } keys %cur_sha; + my @removed = sort grep { !exists $cur_sha{$_} } keys %prev_sha; + + my @modified; + foreach my $name (sort keys %cur_sha) + { + next unless exists $prev_sha{$name}; + next unless defined $cur_sha{$name} && defined $prev_sha{$name}; + next if $cur_sha{$name} eq $prev_sha{$name}; + push( + @modified, + { + name => $name, + from => $prev_sha{$name}, + to => $cur_sha{$name} + } + ); + } + + return unless @added || @removed || @modified; + return { + added => \@added, + removed => \@removed, + modified => \@modified + }; +} + +1; diff --git a/templates/log.tt b/templates/log.tt index 2e977f8..eeb8e47 100644 --- a/templates/log.tt +++ b/templates/log.tt @@ -155,12 +155,17 @@ not recorded [% IF patch_stack %]

Patch stack

-

Source: [% patch_stack.source | html %] — Status: [% patch_stack.status | html %]

+

Source: [% patch_stack.source | html %] +[%- IF patch_stack.id %] — Stack: [% patch_stack.id.substr(0,10) | html %][% END %] +[%- IF patch_stack.commit %] (patches commit [% patch_stack.commit.substr(0,10) | html %])[% END %] + — Status: [% patch_stack.status | html %]

[% IF patch_stack.patches.size > 0 %] +[%- has_sha = 0 %] +[%- FOREACH p IN patch_stack.patches %][% IF p.sha %][% has_sha = 1 %][% END %][% END %] - +[% IF has_sha %][% END %] [% FOREACH p IN patch_stack.patches %] - +[% IF has_sha %][% END %] [% END %]
PatchSubject
PatchBlobSubject
[% p.name | html %][% p.subject | html %]
[% p.name | html %][% p.sha ? p.sha.substr(0,7) : '' | html %][% p.subject | html %]
[% ELSE %] @@ -173,6 +178,8 @@ not recorded [% END %] [% FOREACH r IN patch_stack_diff.removed %]- [% r | html %] [% END %] +[% FOREACH m IN patch_stack_diff.modified %]~ [% m.name | html %] ([% m.from.substr(0,7) | html %] -> [% m.to.substr(0,7) | html %]) +[% END %] [% END %] [% END %]