The branch master has been updated
via 8909d2161299bafbdb0690cdf8b677c0ce7c2f34 (commit)
via d3d425aad09ae79bdef29405f7110f06beb3e500 (commit)
from c524254ebfa2a527cd7d6b025bea0bee397af9d0 (commit)
- Log -----------------------------------------------------------------
commit 8909d2161299bafbdb0690cdf8b677c0ce7c2f34
Author: Richard Levitte <[email protected]>
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 <[email protected]>
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 <[email protected]>',
- "caswell" => 'Matt Caswell <[email protected]>',
- "mark" => 'Mark J. Cox <[email protected]>',
- "cox" => 'Mark J. Cox <[email protected]>',
- "viktor" => 'Viktor Dukhovni <[email protected]>',
- "steve" => 'Stephen Henson <[email protected]>',
- "henson" => 'Stephen Henson <[email protected]>',
- "tim" => 'Tim Hudson <[email protected]>',
- "tjh" => 'Tim Hudson <[email protected]>',
- "lutz" => 'Lutz Jänicke <[email protected]>',
- "jaenicke" => 'Lutz Jänicke <[email protected]>',
- "emilia" => 'Emilia Käsper <[email protected]>',
- "ekasper" => 'Emilia Käsper <[email protected]>',
- "ben" => 'Ben Laurie <[email protected]>',
- "stevem" => 'Steve Marquess <[email protected]>',
- "marquess" => 'Steve Marquess <[email protected]>',
- "bodo" => 'Bodo Möller <[email protected]>',
- "andy" => 'Andy Polyakov <[email protected]>',
- "appro" => 'Andy Polyakov <[email protected]>',
- "kurt" => 'Kurt Roeckx <[email protected]>',
- "rich" => 'Rich Salz <[email protected]>',
- "rsalz" => 'Rich Salz <[email protected]>',
- "geoff" => 'Geoff Thorpe <[email protected]>',
- "richard" => 'Richard Levitte <[email protected]>',
- "levitte" => 'Richard Levitte <[email protected]>',
-
- "kaduk" => 'Ben Kaduk <[email protected]>',
- "pauli" => 'Paul Dale <[email protected]',
-);
-
-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 "[email protected]" 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
