The branch master has been updated via 8909d2161299bafbdb0690cdf8b677c0ce7c2f34 (commit) via d3d425aad09ae79bdef29405f7110f06beb3e500 (commit) from c524254ebfa2a527cd7d6b025bea0bee397af9d0 (commit)
- Log ----------------------------------------------------------------- commit 8909d2161299bafbdb0690cdf8b677c0ce7c2f34 Author: Richard Levitte <rich...@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 <rich...@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 <m...@openssl.org>', - "caswell" => 'Matt Caswell <m...@openssl.org>', - "mark" => 'Mark J. Cox <m...@openssl.org>', - "cox" => 'Mark J. Cox <m...@openssl.org>', - "viktor" => 'Viktor Dukhovni <vik...@openssl.org>', - "steve" => 'Stephen Henson <st...@openssl.org>', - "henson" => 'Stephen Henson <st...@openssl.org>', - "tim" => 'Tim Hudson <t...@openssl.org>', - "tjh" => 'Tim Hudson <t...@openssl.org>', - "lutz" => 'Lutz Jänicke <jaeni...@openssl.org>', - "jaenicke" => 'Lutz Jänicke <jaeni...@openssl.org>', - "emilia" => 'Emilia Käsper <emi...@openssl.org>', - "ekasper" => 'Emilia Käsper <emi...@openssl.org>', - "ben" => 'Ben Laurie <b...@openssl.org>', - "stevem" => 'Steve Marquess <marqu...@openssl.org>', - "marquess" => 'Steve Marquess <marqu...@openssl.org>', - "bodo" => 'Bodo Möller <b...@openssl.org>', - "andy" => 'Andy Polyakov <ap...@openssl.org>', - "appro" => 'Andy Polyakov <ap...@openssl.org>', - "kurt" => 'Kurt Roeckx <k...@openssl.org>', - "rich" => 'Rich Salz <rs...@openssl.org>', - "rsalz" => 'Rich Salz <rs...@openssl.org>', - "geoff" => 'Geoff Thorpe <ge...@openssl.org>', - "richard" => 'Richard Levitte <levi...@openssl.org>', - "levitte" => 'Richard Levitte <levi...@openssl.org>', - - "kaduk" => 'Ben Kaduk <ka...@mit.edu>', - "pauli" => 'Paul Dale <paul.d...@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 ".@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"; } _____ openssl-commits mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-commits