factor out watches and w-i-p handling to separate worker

originally, the idea was to have one gerrit-bot instance per worker, but
that would result in an (even greater) flood of comments. so all
functionality that didn't fit into sanitize-commit was added to
gerrit-bot. at this time it's getting messy, so clean it up by making it
a plain launcher/dispatcher again, with the ability to run multiple
workers on a single commit.

Change-Id: I8c04f449726793b99e9f1ac5c6db330a1f17389f
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

2 files changed, 173 insertions, 85 deletions

diff --git a/git-hooks/gerrit-bot b/git-hooks/gerrit-bot
index 05c444f..2df6d20 100755
--- a/git-hooks/gerrit-bot
+++ b/git-hooks/gerrit-bot
@@ -38,26 +38,20 @@ use Gerrit::REST;
 #     Base dir for local GIT clones of the projects.
 #   gitdofetch
 #     Need to fetch the repos or are they local?
-#   worker
-#     The worker is run in a local bare clone of the inspected repository.
-#     The magic string @SHA@ is replaced by the commit to be checked.
+#   workers
+#     A space-separated list of workers.
+#     They are run in a local bare clone of the inspected repository.
+#     There must be a workers.<worker> config entry for each worker
+#     (note that these entries are NOT namespaced with the instance).
+#     The magic string @SHA1@ is replaced by the commit to be checked;
+#     @PROJ@ by the gerrit project the worker is run for.
 #     It is expected to dump a Gerrit ReviewInput JSON entity to stdout.
+#     The output from all workers is merged.
 #   excluded (optional)
 #     Space-separated list of exclusions of the form <project> or
 #     <project>:<branch>.
 #   maintainers (optional)
 #     Space-separated list of reviewers to add on "special occasions".
-#   watches (optional)
-#     Space-separated list of path watches. Each watch requires an own
-#     section named watches.<name> with the following keys:
-#       projects (default ".*")
-#         Regular expression specifying the projects to watch.
-#       files
-#         Regular expression specifying the filepaths to watch.
-#       message (optional)
-#         The message to post when this watch triggers.
-#       invite (optional)
-#         Space-separated list of reviewers to add when this watch triggers.
 #   verbose (default 0)
 #     Print progress/result messages.
 
@@ -98,25 +92,11 @@ my $REST_USER = getcfg 'restuser', undef;
 my $REST_PASS = getcfg 'restpass', undef;
 my $GIT_BASEDIR = getcfg 'gitbasedir';
 my $GIT_DO_FETCH = getcfg 'gitdofetch';
-my $WORKER = getcfg 'worker';
+my $WORKERS = getcfg 'workers';
 my %EXCLUDED_PROJECTS = map { $_ => 1 } split(/\s+/, getcfg('excluded', ""));
 my @MAINTAINERS = split(/\s+/, getcfg('maintainers', ""));
-my @WATCHES = split(/\s+/, getcfg('watches', ""));
 my $verbose = getcfg 'verbose', 0;
 
-my (%watch_projects, %watch_files, %watch_messages, %watch_invites);
-for my $w (@WATCHES) {
-  my $p = $config{'watches.'.$w.'.projects'};
-  $watch_projects{$w} = defined($p) ? qr/^$p$/ : undef;
-  my $f = $config{'watches.'.$w.'.files'};
-  die "watches.$w.files not set.\n" if (!defined($f));
-  $watch_files{$w} = qr/^$f$/;
-  my $m = $config{'watches.'.$w.'.message'};
-  $watch_messages{$w} = defined($m) ? $m."\n\n" : "";
-  my $i = $config{'watches.'.$w.'.invite'};
-  $watch_invites{$w} = defined($i) ? [ split(/\s+/, $i) ] : [];
-}
-
 my @gerrit = ("ssh", $GERRIT_HOST, "gerrit");
 my @gerrit_actor = ("ssh", $GERRIT_ACTOR_HOST, "gerrit");
 my $gerrit_rest = Gerrit::REST->new($REST_HOST, $REST_USER, $REST_PASS);
@@ -132,6 +112,34 @@ sub printerr($)
   print STDERR $msg.".\n";
 }
 
+sub merge_hashes($$);
+
+sub merge_hashes($$)
+{
+  my ($base, $ext) = @_;
+
+  for my $k (keys %$ext) {
+    my $br = \$$base{$k};
+    my $ev = $$ext{$k};
+    if (!defined($$br)) {
+      $$br = $ev;
+    } else {
+      my $bt = ref($$br);
+      my $et = ref($ev);
+      die "Values for '$k' have different types ($bt, $et).\n" if ($bt ne $et);
+      if ($et eq "HASH") {
+        merge_hashes($$br, $ev);
+      } elsif ($et eq "ARRAY") {
+        push @$$br, @$ev;
+      } elsif ($et eq "") {
+        $$br = $$br."\n\n".$ev;
+      } else {
+        die "Values for '$k' have unsupported type $bt.\n";
+      }
+    }
+  }
+}
+
 sub process_commit($$$$$)
 {
   my ($number, $project, $branch, $ref, $rev) = @_;
@@ -143,12 +151,10 @@ sub process_commit($$$$$)
   my $orig_project = $project;
   $project =~ s,/$,,; # XXX Workaround QTQAINFRA-381
   my $verdict = {};
-  my $message = "";
-  my $iswip = 0;
   my @invite;
   if (defined($EXCLUDED_PROJECTS{$project}) || defined($EXCLUDED_PROJECTS{$project.":".$branch})) {
     $verbose and print "===== ".strftime("%c", localtime(time()))." ===== excluding commit ".$ref." in ".$project."\n";
-    $message = "(skipped)";
+    $$verdict{message} = "(skipped)";
     $$verdict{labels}{'Sanity-Review'} = 1;
   } else {
     $verbose and print "===== ".strftime("%c", localtime(time()))." ===== processing commit ".$ref." in ".$project."\n";
@@ -183,59 +189,36 @@ sub process_commit($$$$$)
       $verbose and print "===== ".strftime("%c", localtime(time()))." ===== fetched change\n";
     }
 
-    my $subject;
-
-    my @watches;
-    for my $w (@WATCHES) {
-      my $wp = $watch_projects{$w};
-      push @watches, $w if (!defined($wp) || $project =~ $wp);
-    }
-    if (@watches) {
-      my @touched = `git show --pretty=\%s --name-only --ignore-submodules -C $rev`;
-      chop(@touched);
-      $subject = shift @touched;
-      shift @touched;  # empty line
-      for my $w (@watches) {
-        for my $file (@touched) {
-          if ($file =~ $watch_files{$w}) {
-            $message .= $watch_messages{$w};
-            push @invite, @{$watch_invites{$w}};
-            last;
-          }
-        }
+    for my $w (split(/ /, $WORKERS)) {
+      my $worker = $config{'workers.'.$w};
+      die "workers.$worker not set.\n" if (!defined($worker));
+      $worker =~ s/\@PROJ\@/$project/g;
+      $worker =~ s/\@SHA1\@/$rev/g;
+      my $output;
+      open VERDICT, $worker." 2>&1 |" or die "cannot run worker '$w': ".$!;
+      {
+        local $/;
+        $output = <VERDICT>;
       }
-    }
-
-    if (!defined($subject)) {
-        $subject = `git show --pretty=\%s -s $rev`;
-        chop($subject);
-    }
-    if ($subject =~ /^[<[]?[Ww]ip\b|\bWIP\b|\*{3}|^(?:squash|fixup)! |^(.)\1*$/) {
-        $iswip = 1;
-        $message = "Apparently pushing a Work In Progress\n\n".$message;
-    }
-
-    my $worker = $WORKER;
-    $worker =~ s/\@SHA1\@/$rev/g;
-    my $output;
-    open VERDICT, $worker." 2>&1 |" or die "cannot run worker: ".$!;
-    {
-      local $/;
-      $output = <VERDICT>;
-    }
-    close VERDICT;
-    die "Worker for commit ".$ref." in ".$project." crashed with signal ".$?.".\n" if ($? & 127);
-    die "Worker returned non-zero exit code for commit ".$ref." in ".$project.".\n" if ($?);
-    if (length($output) > 50000) {
-      $message = "**** Worker produced an unreasonable amount of output. You should ask the bot maintainers for advice.";
-      push @invite, @MAINTAINERS;
-    } else {
+      close VERDICT;
       while ($output =~ s/^([^\{][^\n]*\n)//s) {
-        print STDERR "Non-JSON output from worker: $1\n";
+        print STDERR "Non-JSON output from worker '$w': $1\n";
       }
-      $verdict = decode_json($output);
-      defined($verdict) or die "cannot decode verdict as JSON\n";
-      $$verdict{labels}{'Code-Review'} = ($iswip ? -2 : 0);
+      die "Worker '$w' for commit ".$ref." in ".$project." crashed with signal ".$?.".\n" if ($? & 127);
+      die "Worker '$w' returned non-zero exit code for commit ".$ref." in ".$project.".\n" if ($?);
+      my $vd = {};
+      if (length($output) > 50000) {
+        $$vd{message} = "**** Worker '$w' produced an unreasonable amount of output. You should ask the bot maintainers for advice.";
+        push @invite, @MAINTAINERS;
+      } else {
+        $vd = decode_json($output);
+        defined($vd) or die "cannot decode verdict from worker '$w' as JSON\n";
+        if (defined($$vd{invite})) {
+          push @invite, @{$$vd{invite}};
+          delete $$vd{invite};
+        }
+      }
+      merge_hashes($verdict, $vd);
     }
   }
   if (@invite) {
@@ -246,9 +229,6 @@ sub process_commit($$$$$)
       $verbose and print "Invited @invite to ".$rev." (".$project."/".$ref.")\n";
     }
   }
-  if (defined($$verdict{message}) || length($message)) {
-    $$verdict{message} = $message.($$verdict{message} || "");
-  }
   eval {
     $gerrit_rest->POST("/changes/$number/revisions/$rev/review", $verdict);
   };
diff --git a/git-hooks/gerrit-bot-extras b/git-hooks/gerrit-bot-extras
new file mode 100755
index 0000000..fad85da
--- /dev/null
+++ b/git-hooks/gerrit-bot-extras
@@ -0,0 +1,108 @@
+#! /usr/bin/perl
+
+# Copyright (C) 2017 The Qt Company Ltd.
+# Contact: http://www.qt.io/licensing/
+#
+# You may use this file under the terms of the 3-clause BSD license.
+# See the file LICENSE from this package for details.
+#
+
+use strict;
+use warnings;
+use JSON;
+
+# Usage: $0 <project> <SHA1> [instance]
+# - default instance is 'sanitybot'
+# - configure git: git config --global <instance>.<option> <value>
+# Valid options are:
+#   watches (optional)
+#     Space-separated list of path watches. Each watch requires an own
+#     section named watches.<name> with the following keys (note that
+#     these sections are NOT namespaced with the instance):
+#       projects (default ".*")
+#         Regular expression specifying the projects to watch.
+#       files
+#         Regular expression specifying the filepaths to watch.
+#       message (optional)
+#         The message to post when this watch triggers.
+#       invite (optional)
+#         Space-separated list of reviewers to add when this watch triggers.
+
+die "usage: $0 <project> <SHA1> [instance]\n" if ($#ARGV < 1);
+my $project = shift @ARGV;
+my $rev = shift @ARGV;
+
+my $instance = 'sanitybot';
+$instance = $ARGV[0] if ($#ARGV > -1);
+
+# Doing this is less expensive than calling git repeatedly.
+my %config = ();
+for (`git config -l`) {
+    /^([^=]+)=(.*$)/;
+    $config{$1} = $2;
+}
+
+sub getcfg($;$)
+{
+    my ($key, $def) = @_;
+    my $fkey = $instance.'.'.$key;
+    if (defined $config{$fkey}) {
+        return $config{$fkey};
+    } elsif (@_ > 1) {
+        return $def;
+    } else {
+        die $fkey." not set.\n";
+    }
+}
+
+my @messages = ();
+my %verdict = ();
+my @invite = ();
+
+my (@watches, %watch_files, %watch_messages, %watch_invites);
+for my $w (split(/\s+/, getcfg('watches', ""))) {
+    my $p = $config{'watches.'.$w.'.projects'};
+    next if (defined($p) && $project !~ qr/^$p$/);
+    my $f = $config{'watches.'.$w.'.files'};
+    die "watches.$w.files not set.\n" if (!defined($f));
+    $watch_files{$w} = qr/^$f$/;
+    $watch_messages{$w} = $config{'watches.'.$w.'.message'};
+    my $i = $config{'watches.'.$w.'.invite'};
+    $watch_invites{$w} = defined($i) ? [ split(/\s+/, $i) ] : [];
+    push @watches, $w;
+}
+
+my $subject;
+
+if (@watches) {
+    my @touched = `git show --pretty=\%s --name-only --ignore-submodules -C $rev`;
+    chop(@touched);
+    $subject = shift @touched;
+    shift @touched;  # empty line
+    for my $w (@watches) {
+        for my $file (@touched) {
+            if ($file =~ $watch_files{$w}) {
+                push @messages, $watch_messages{$w} if (defined($watch_messages{$w}));
+                push @invite, @{$watch_invites{$w}};
+                last;
+            }
+        }
+    }
+}
+
+if (!defined($subject)) {
+    $subject = `git show --pretty=\%s -s $rev`;
+    chop($subject);
+}
+
+my $cr = 0;
+if ($subject =~ /^[<[]?[Ww]ip\b|\bWIP\b|\*{3}|^(?:squash|fixup)! |^(.)\1*$/) {
+    unshift @messages, "Apparently pushing a Work In Progress";
+    $cr = -2;
+}
+$verdict{labels}{'Code-Review'} = $cr;
+
+$verdict{message} = join("\n\n", @messages) if (@messages);
+$verdict{invite} = \@invite if (@invite);
+
+print encode_json(\%verdict)."\n";