diff options
author | Oswald Buddenhagen <oswald.buddenhagen@qt.io> | 2017-02-13 21:16:02 +0100 |
---|---|---|
committer | Oswald Buddenhagen <oswald.buddenhagen@qt.io> | 2017-06-12 10:17:09 +0000 |
commit | 2792c18e1d33594da05914d9d521adc2cc641ad9 (patch) | |
tree | 7100e93a723b4749d5020d3fed9e6db7bc00d257 /git-hooks/gerrit-bot | |
parent | 59e404b2f2628ca3f4e8104ec55c7409f9db90f3 (diff) |
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>
Diffstat (limited to 'git-hooks/gerrit-bot')
-rwxr-xr-x | git-hooks/gerrit-bot | 150 |
1 files changed, 65 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); }; |