[openssl-commits] [tools] master update

Richard Levitte levitte at openssl.org
Thu Jun 15 14:05:59 UTC 2017


The branch master has been updated
       via  8909d2161299bafbdb0690cdf8b677c0ce7c2f34 (commit)
       via  d3d425aad09ae79bdef29405f7110f06beb3e500 (commit)
      from  c524254ebfa2a527cd7d6b025bea0bee397af9d0 (commit)


- Log -----------------------------------------------------------------
commit 8909d2161299bafbdb0690cdf8b677c0ce7c2f34
Author: Richard Levitte <richard at levitte.org>
Date:   Thu Jun 15 15:58:40 2017 +0200

    Adapt gitaddrev to use OpenSSL::Query
    
    IMPORTANT NOTE: with this change, it's important to install OpenSSL-Query.
    If you have direct access to the databases, it's also important to install
    the QueryApp libraries.  Finally, you must either defined the environment
    variable BUREAU with the directory where the databases reside as value,
    or defined the two variables CLADB and PERSONBD with the path of the
    respective files as value.  If none of these variables are set, the files
    are assumed to be in the current working directory.
    
    For gitaddrev, the changes are substancial:
    
    - Do CLA checks early
    - Increase the CLA checks
    - Add a check of number of reviewers
    - Add a check of at least one OMC member
    - List *all* unknown reviewers, not just the first
    - With --list, show which ones are OMC members
    
    For ghmerge, the setting of CLADB got removed.

commit d3d425aad09ae79bdef29405f7110f06beb3e500
Author: Richard Levitte <richard at levitte.org>
Date:   Thu Jun 15 15:56:45 2017 +0200

    Let OpenSSL::Query::ClaDB::has_cla massage the input identity
    
    ... exactly like OpenSSL::Query::ClaREST::has_cla does

-----------------------------------------------------------------------

Summary of changes:
 OpenSSL-Query/lib/OpenSSL/Query/ClaREST.pm |   2 +
 review-tools/README                        |   7 +
 review-tools/ghmerge                       |   2 -
 review-tools/gitaddrev                     | 213 ++++++++++++++---------------
 4 files changed, 110 insertions(+), 114 deletions(-)

diff --git a/OpenSSL-Query/lib/OpenSSL/Query/ClaREST.pm b/OpenSSL-Query/lib/OpenSSL/Query/ClaREST.pm
index a92ef8c..5cb4ae9 100644
--- a/OpenSSL-Query/lib/OpenSSL/Query/ClaREST.pm
+++ b/OpenSSL-Query/lib/OpenSSL/Query/ClaREST.pm
@@ -38,6 +38,8 @@ sub _build__clahandler {
 sub has_cla {
   my $self = shift;
   my $id = shift;
+  if ($id =~ m|<(\S+\@\S+)>|) { $id = $1; }
+  croak "Malformed input ID" unless $id =~ m|^\S+(\@\S+)$|;
 
   my $ua = $self->_clahandler;
   my $json = $ua->get($self->base_url . '/0/HasCLA/'
diff --git a/review-tools/README b/review-tools/README
index d48bdc8..2baca0b 100644
--- a/review-tools/README
+++ b/review-tools/README
@@ -1,3 +1,10 @@
+Environment
+===========
+
+Some of the scripts use the information REST API on https://api.openssl.org.
+If you have direct access to the databases and want to use that instead, set
+the environment variable BUREAU to the directory where they are located.
+
 =================
 addrev is a simple pair of scripts to add or edit reviewers to commits.
 
diff --git a/review-tools/ghmerge b/review-tools/ghmerge
index 16426b3..fc263a6 100755
--- a/review-tools/ghmerge
+++ b/review-tools/ghmerge
@@ -29,8 +29,6 @@ case "$PRNUM" in
         ;;
 esac
 
-test -z "$CLADB" && export CLADB=$HOME/openssl/bureau/cladb.txt
-
 curl -s https://api.github.com/repos/openssl/openssl/pulls/$PRNUM >/tmp/gh$$
 TEAM=$*
 set `python -c '
diff --git a/review-tools/gitaddrev b/review-tools/gitaddrev
index a873296..64976d1 100755
--- a/review-tools/gitaddrev
+++ b/review-tools/gitaddrev
@@ -1,13 +1,21 @@
 #!/usr/bin/perl
 
 use File::Basename;
+use FindBin;
+
+use OpenSSL::Query::REST;
+use Module::Load::Conditional qw(can_load);
+
+can_load(modules => { OpenSSL::Query::DB => undef });
 
 my $rmrev = 0;
 my @reviewers;
+my @nocla_reviewers;
+my @unknown_reviewers;
+my $skip_reviewer;
+my $omccount = 0;
 my @commits;
 my $skip = 0;
-my $my_email;
-my $my_id;
 my $matchstr;
 my $clatype;
 my $found = 0;
@@ -16,57 +24,50 @@ my $refuse = 0;
 my $prnum = 0;
 my $trivial = 0;
 
-%list = (
-	"matt" => 'Matt Caswell <matt at openssl.org>',
-	"caswell" => 'Matt Caswell <matt at openssl.org>',
-	"mark" => 'Mark J. Cox <mark at openssl.org>',
-	"cox" => 'Mark J. Cox <mark at openssl.org>',
-	"viktor" => 'Viktor Dukhovni <viktor at openssl.org>',
-	"steve" => 'Stephen Henson <steve at openssl.org>',
-	"henson" => 'Stephen Henson <steve at openssl.org>',
-	"tim" => 'Tim Hudson <tjh at openssl.org>',
-	"tjh" => 'Tim Hudson <tjh at openssl.org>',
-	"lutz" => 'Lutz Jänicke <jaenicke at openssl.org>',
-	"jaenicke" => 'Lutz Jänicke <jaenicke at openssl.org>',
-	"emilia" => 'Emilia Käsper <emilia at openssl.org>',
-	"ekasper" => 'Emilia Käsper <emilia at openssl.org>',
-	"ben" => 'Ben Laurie <ben at openssl.org>',
-	"stevem" => 'Steve Marquess <marquess at openssl.org>',
-	"marquess" => 'Steve Marquess <marquess at openssl.org>',
-	"bodo" => 'Bodo Möller <bodo at openssl.org>',
-	"andy" => 'Andy Polyakov <appro at openssl.org>',
-	"appro" => 'Andy Polyakov <appro at openssl.org>',
-	"kurt" => 'Kurt Roeckx <kurt at openssl.org>',
-	"rich" => 'Rich Salz <rsalz at openssl.org>',
-	"rsalz" => 'Rich Salz <rsalz at openssl.org>',
-	"geoff" => 'Geoff Thorpe <geoff at openssl.org>',
-	"richard" => 'Richard Levitte <levitte at openssl.org>',
-	"levitte" => 'Richard Levitte <levitte at openssl.org>',
-
-        "kaduk" => 'Ben Kaduk <kaduk at mit.edu>',
-        "pauli" => 'Paul Dale <paul.dale at oracle.com',
-);
-
-my $realname = $0;
-while (my $tmpname = readlink($realname)) {
-    $realname = $tmpname;
-}
-my $clafile = dirname($realname)."/../cladb.txt";
-
-if (defined $ENV{CLADB}) {
-    $clafile = $ENV{CLADB};
-}
+my $query = OpenSSL::Query->new();
 
 foreach (@ARGV) {
     if (/^--list$/) {
-	map { printf "%-15s (%s)\n", $_, $list{$_} } sort keys %list;
-	exit(0);
+	my %list = ();
+	foreach ($query->list_people()) {
+	    my $email_id = (grep { ref($_) eq "" && $_ =~ m|\@| } @$_)[0];
+	    my $rev = $query->find_person_tag($email_id, 'rev');
+	    my $omc = $query->is_member_of($email_id, 'omc');
+	    next unless $query->has_cla($rev);
+	    my @ids = sort grep { $_ =~ m|^[a-z]+$| } map {
+		if (ref($_) eq "HASH") {
+		    values %$_;
+		} else {
+		    $_;
+		}
+	    } @$_;
+	    foreach (@ids) {
+		$list{$_} = { tag => $rev, omc => $omc };
+	    }
+	}
+	foreach (sort { my $res = $list{$a}->{tag} cmp $list{$b}->{tag};
+			$res != 0 ? $res : ($a cmp $b) } keys %list) {
+	    printf "%-15s %-6s (%s)\n",
+		$_, $list{$_}->{omc} ? "[OMC]" : "", $list{$_}->{tag};
+	}
+	exit 0;
     } elsif (/^--reviewer=(.+)$/) {
-        if (!exists $list{$1}) {
-	        print STDERR "Unknown reviewer $1\n";
-	        exit 1;
-        }
-        push @reviewers, $1;
+	my $rev = $query->find_person_tag($1, 'rev');
+	if ($rev) {
+	    my $cla = $query->has_cla($rev);
+	    if ($cla) {
+		unless (grep {$_ eq $rev} @reviewers) {
+		    $omccount++ if $query->is_member_of($1, 'omc');
+		    push @reviewers, $rev;
+		}
+	    } else {
+		push @nocla_reviewers, $1
+		    unless grep {$_ eq $1} @nocla_reviewers;
+	    }
+	} else {
+	    push @unknown_reviewers, $1
+		unless grep {$_ eq $1} @unknown_reviewers;
+	}
     } elsif (/^--prnum=(.+)$/) {
         $prnum = $1;
     } elsif (/^--commit=(.+)$/) {
@@ -74,17 +75,52 @@ foreach (@ARGV) {
         $skip = 1;
     } elsif (/^--rmreviewers$/) {
         $rmrev = 1;
-    } elsif (/^--myemail=((.+)\@.+)$/) {
-        if (!exists $list{$2}) {
-	        print STDERR "Unknown email: $1\n";
-	        exit 1;
-        }
-        $my_email = $1;
-        $my_id = $2;
+    } elsif (/^--myemail=(.+\@.+)$/) {
+	my $rev = $query->find_person_tag($1, 'rev');
+	if ($rev) {
+	    my $cla = $query->has_cla($rev);
+	    if ($cla) {
+		# If author doesn't match us add us as reviewer
+		if ($ENV{GIT_AUTHOR_EMAIL} ne $1) {
+		    unless (grep {$_ eq $rev} @reviewers) {
+			$omccount++ if $query->is_member_of($1, 'omc');
+			push @reviewers, $rev;
+		    }
+		} else {
+		    # Can't review our own commits, setup this one for removal
+		    $skip_reviewer = $rev;
+		    $omccount-- if $query->is_member_of($1, 'omc');
+		}
+	    } else {
+		push @nocla_reviewers, $1
+		    unless grep {$_ eq $1} @nocla_reviewers;
+	    }
+	} else {
+	    push @unknown_reviewers, $1
+		unless grep {$_ eq $1} @unknown_reviewers;
+	}
     } elsif (/^--trivial$/) {
         $trivial = 1;
     }
 }
+if (@unknown_reviewers) {
+    die "Unknown reviewers: ", join(", ", @unknown_reviewers), "\n";
+}
+if (@nocla_reviewers) {
+    die "Reviewers without CLA: ", join(", ", @nocla_reviewers), "\n";
+}
+if (scalar @reviewers < 2) {
+    die "Too few reviewers (total must be at least 2)\n";
+}
+if ($omccount < 1) {
+    die "At least one of the reviewers must be an OMC member\n";
+}
+if ($skip_reviewer) {
+    @reviewers = grep { $_ ne $skip_reviewer } @reviewers;
+    @nocla_reviewers = grep { $_ ne $skip_reviewer } @nocla_reviewers;
+    @unknown_reviewers = grep { $_ ne $skip_reviewer } @unknown_reviewers;
+}
+print STDERR "DEBUG: \@reviewers = ( ", join(", ", @reviewers), " )\n";
 
 if ($skip == 1) {
     my $commit_id = $ENV{GIT_COMMIT};
@@ -103,31 +139,18 @@ if ($skip == 1) {
 }
 
 if (scalar @reviewers == 0 && $rmrev == 0) {
-	print STDERR "No reviewer set!\n";
-	exit 1;
-}
-
-# If author doesn't match us add a reviewer
-if ($ENV{GIT_AUTHOR_EMAIL} ne $my_email) {
-    push @reviewers, $my_id unless grep {$_ eq $my_id} @reviewers;
-} else {
-    # If we are in list delete it: we can't review our own commit
-    @reviewers = grep { $_ ne $my_id } @reviewers;
+    die "No reviewer set!\n";
 }
 
 my $last_line_blank = 0;
 my $have_rev = 0;
-line: while(<STDIN>) {
-    if (/^Reviewed-by:\s*(\S.*\S)\s*$/) {
+while(<STDIN>) {
+    if (/^\(Merged from https:\/\/github\.com\/openssl\/openssl\/pull\//
+	|| /^Reviewed-by:\s*(\S.*\S)\s*$/) {
         next if $rmrev == 1;
         $have_rev = 1;
         # Skip if reviewer already in list
-        my $rtmp;
-        foreach $rtmp (@reviewers) {
-            if ($1 eq $list{$rtmp}) {
-                next line;
-            }
-        }
+        next if $1 && grep { $1 eq $_ } @reviewers;
     }
     print;
     $last_line_blank = ($_ =~ /^\s*$/);
@@ -136,52 +159,18 @@ if ($rmrev == 0) {
     #Add a blank line unless the last one is already blank or a review line
     print "\n" unless $last_line_blank || $have_rev;
     foreach(@reviewers) {
-	    print "Reviewed-by: $list{$_}\n";
+	print "Reviewed-by: $_\n";
     }
     if ($trivial) {
         print "CLA: trivial\n";
     }
 }
 
-print "(Merged from https://github.com/openssl/openssl/pull/$prnum)"
+print "(Merged from https://github.com/openssl/openssl/pull/$prnum)\n"
     if $prnum;
 
 my $email = $ENV{GIT_AUTHOR_EMAIL};
 
-if (!$trivial) {
-    if (open(my $cladb, "<", $clafile)) {
-        my $regex;
-        while(<$cladb>) {
-            /^(#.*|\s*)$/ and next;
-            /^([^\s]+)\s+/ or die "\nSyntax error in cladb.txt";
-            $regex = $1;
-            $regex = lc($regex);
-            # Any emails of the form ". at xyz.com" should not be used for matching
-            next if $regex =~ /^\.\@/;
-            # Any emails of the form "*@xyz.com" should use "*" as a wildcard
-            $regex =~ s/\*/\.\*/g;
-            my $testemail = lc($email);
-            if ($regex =~ /^!(.*)/) {
-                $regex = $1;
-                $refuse++ if ($testemail =~ $regex);
-            } elsif ($testemail =~ $regex) {
-                $num++;
-            }
-        }
-        close $cladb;
-
-        if ($refuse != 0) {
-        die "\nError: Author $email has explicity refused to provide a CLA";
-        } elsif ($num == 0) {
-        warn "\n\nWARNING: CLA for $email not found\n";
-        } elsif ($num != 1) {
-        die "\nError: Multiple matching CLAs for $email in cladb.txt";
-        }
-    } else {
-        warn <<"_____";
-
-WARNING: couldn't open $clafile
-         You may want to set the environment variable CLADB
-_____
-    }
+if (!$trivial && !$query->has_cla(lc $email)) {
+    warn "\n\nWARNING: No CLA found for $email\n";
 }


More information about the openssl-commits mailing list