Don't let addrev count the author as a reviewer

As per the latest policy change the author is no longer counted as a
reviewer, so we ensure addrev disallows this.

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/tools/pull/105)

diff --git a/review-tools/gitaddrev b/review-tools/gitaddrev
index cda9299f8305eac22579845e28e21d7dde18b6ca..c6ba49a388324936f61708ee910ecbaff854050d 100755 (executable)
--- a/review-tools/gitaddrev
+++ b/review-tools/gitaddrev
@@ -28,14 +28,16 @@ my $query = OpenSSL::Query->new();
 my @reviewers;
 my @nocla_reviewers;
 my @unknown_reviewers;
-my $skip_reviewer;
 my $otccount = 0;
+my $author = $ENV{GIT_AUTHOR_EMAIL};
+my $authorrev = $query->find_person_tag($author, 'rev');
+
 sub try_add_reviewer {
     my $id = shift;
     my $rc = undef;
     my $id2 = $id =~ /^\@(.*)$/ ? { github => $1 } : $id;
     my $rev = $query->find_person_tag($id2, 'rev');
-    if ($rev) {
+    if ($rev && (!defined $authorrev || $rev ne $authorrev)) {
        my $cla = $query->has_cla(lc $rev);
        if ($cla) {
            unless (grep {$_ eq $rev} @reviewers) {
@@ -64,6 +66,7 @@ foreach (@ARGV) {
        foreach ($query->list_people()) {
            my $email_id = (grep { ref($_) eq "" && $_ =~ m|\@| } @$_)[0];
            my $rev = $query->find_person_tag($email_id, 'rev');
+           next unless defined $rev;
            my $otc = $query->is_member_of($email_id, 'otc');
            next unless $query->has_cla(lc $rev);
            next unless $query->is_member_of($email_id, 'commit') || $otc;
@@ -108,41 +111,19 @@ foreach (@ARGV) {
 }
 
 my @commit_message = map { (my $x = $_) =~ s|\R$||; $x } <STDIN>;
-my $author = $ENV{GIT_AUTHOR_EMAIL};
 my $trivial = !! grep(/^CLA:\s*Trivial\s*$/i, @commit_message);
 
-# If the author is a registered committer, that identity passes as a reviewer
-# too.  There is a twist, though...  see next comment
-if (my $rev = try_add_reviewer($author)) {
-
-    # So here's the deal: We added the commit author because we need to keep
-    # count of the total amount of reviewers, which includes the commit author
-    # if it's a registered committer.  However, the author can "reviewed
-    # themselves", so no Reviewed-by: should be added for that identity.
-    # However, we still need to check the @reviewers count below, so the
-    # solution is to record this rev tag separately and remove it from
-    # @reviewers after the count check.
-    $skip_reviewer = $rev;
-
-} else {
-
-    # In case the author is unknown to our databases or is lacking a CLA,
-    # we need to be extra careful to check if this is supposed to be a
-    # trivial commit.
-
-    # Note: it really should be enough to check if $author is unknown, since
-    # the databases are supposed to be consistent with each other.  However,
-    # let's be cautious and check both, in case someone has been registered
-    # as a known identity without having a CLA in place.
-    die "Commit author ",$author," has no CLA, and this is a non-trivial commit\n"
-       if !$trivial && grep { $_ eq $author } (@nocla_reviewers);
-
-    # Now that that's cleared, remove the author from anything that could cause
-    # more unnecessary errors (false positives).
-    @nocla_reviewers = grep { $_ ne $author } @nocla_reviewers;
-    @unknown_reviewers = grep { $_ ne $author } @unknown_reviewers;
-    $skip_reviewer = $author;
-}
+# Note: it really should be enough to check if $author is unknown, since
+# the databases are supposed to be consistent with each other.  However,
+# let's be cautious and check both, in case someone has been registered
+# as a known identity without having a CLA in place.
+die "Commit author ",$author," has no CLA, and this is a non-trivial commit\n"
+    if !$trivial && grep { $_ eq $author } @nocla_reviewers;
+
+# Now that that's cleared, remove the author from anything that could cause
+# more unnecessary errors (false positives).
+@nocla_reviewers = grep { $_ ne $author } @nocla_reviewers;
+@unknown_reviewers = grep { $_ ne $author } @unknown_reviewers;
 
 if (@unknown_reviewers) {
     die "Unknown reviewers: ", join(", ", @unknown_reviewers), "\n";
@@ -159,11 +140,6 @@ if (scalar @reviewers < 2) {
 if ($otccount < 1) {
     die "At least one of the reviewers must be an OTC member\n";
 }
-if ($skip_reviewer) {
-    @reviewers = grep { !m/$skip_reviewer/i } @reviewers;
-    @nocla_reviewers = grep { !m/$skip_reviewer/i } @nocla_reviewers;
-    @unknown_reviewers = grep { !m/$skip_reviewer/i } @unknown_reviewers;
-}
 
 print STDERR "Going with these reviewers:\n  ", join("\n  ", @reviewers), "\n"
     if $verbose;