[Mesa-dev] [PATCH mesa 3/5] bin: rewrite get_reviewers as opt-in only

Eric Engestrom eric.engestrom at intel.com
Thu May 31 14:04:33 UTC 2018


Some people have mentioned they don't like the current get_reviewers.pl
script (the one from the kernel) because it is way too greedy in its
search for reviewers.

I tried to modify it to remove the git blame functionality, but I had
a way too hard time figuring out what all this perl black magic does,
and I figured I wouldn't be the only one, so I rewrote it in python
instead, so that more people would be able to maintain it.

I kept only the basic functionality of parsing the REVIEWERS file, and
only supporting the R: and F: tags for now (those were the only ones
used in Mesa anyway). Patches can be passed in as arguments or via
stdin, and `-f` can be used to simply find reviewers for the files
given.

This should be a drop-in replacement for the old script, meant to be
used with git; you can use it automatically by setting:
$ git config sendemail.cccmd bin/get_reviewer.py

To reiterate, this script is now opt-in only, as it only returns
reviewers added in REVIEWERS.

Signed-off-by: Eric Engestrom <eric.engestrom at intel.com>
---
As a quirk that I decided to leave in, "F: /dev/null" can now be used to
subscribe to files being added or deleted, which I'm thinking of adding
to the build systems groups, so that build system reviewers can make
sure their build system was updated accordingly. Thoughts?
---
 bin/get_reviewer.py | 177 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 177 insertions(+)
 create mode 100755 bin/get_reviewer.py

diff --git a/bin/get_reviewer.py b/bin/get_reviewer.py
new file mode 100755
index 00000000000000000000..543086ba21505b81ea0c
--- /dev/null
+++ b/bin/get_reviewer.py
@@ -0,0 +1,177 @@
+#!/usr/bin/env python3
+"""
+Parser for REVIEWERS, designed for use with git.
+
+Use it automatically by configuring git like this:
+  $ git config sendemail.cccmd bin/get_reviewer.py
+
+"""
+
+def parse_options(args):
+    """
+    Takes care of parsing the options and returning a simple dict with
+    one member per option.
+    """
+    import argparse
+    parser = argparse.ArgumentParser(description='''
+        Get the list of reviewers for your changes.
+        If no file is given, reads stdin.
+    ''')
+    parser.add_argument('-f',
+                        action='store_true',
+                        help='''find the reviewers for the files given,
+                                instead of extracting the files from the patches''')
+    parser.add_argument('file',
+                        nargs='*',
+                        default=[],
+                        help='patch, or plain file (see `-f`)')
+    return parser.parse_args(args)
+
+def parse_diff(diff):
+    """
+    Takes a unified diff and returns a list of all the files it
+    modifies.
+    """
+    files = []
+
+    def diff_filename(line):
+        """
+        Extracts the filename (full path) from a diff header
+        """
+        filename = line.split()[1]
+
+        # If the file is being created or deleted, don't strip anything
+        if filename == '/dev/null':
+            return filename
+
+        # Strip a/... or b/... prefix
+        return '/'.join(filename.split('/')[1:])
+
+    for line in diff.splitlines():
+        if line == '---':
+            # Not part of the diff itself; ignore
+            pass
+        # Keep both old and new filenames, in case of files being added
+        # or deleted
+        elif line.startswith('---') or line.startswith('+++'):
+            files.append(diff_filename(line))
+
+    # Deduplicate the list
+    return set(files)
+
+def canonicalize_globs(dirty_globs):
+    """
+    Canonicalize glob pattern from REVIEWERS to make them ready for
+    python's glob() consumption
+    """
+    import os.path
+    cleaned_globs = []
+    for path in dirty_globs:
+        if '*' not in path:
+            # Make sure dirs end with /* to match all it contents
+            if os.path.isdir(path):
+                if path.endswith('/'):
+                    path += '*'
+                else:
+                    path += '/*'
+            else:
+                # Any full path must exist (ie. dir or file)
+                assert os.path.exists(path)
+        cleaned_globs.append(path)
+    return cleaned_globs
+
+def parse_reviewers():
+    """
+    Parse the REVIEWERS file and return a dict of, per group:
+    - name (first line of the group)
+    - reviewers (Full Name <email at address.tld>)
+    - files (glob-style)
+    """
+    import re
+    reviewers = []
+    with open('REVIEWERS', 'r') as file:
+        lines = file.readlines()
+        header_over = False
+        useful_lines = []
+        for line in lines:
+            line = line.strip()
+            if header_over:
+                useful_lines.append(line)
+            elif re.match(r'^----+$', line):
+                header_over = True
+        groups = '\n'.join(useful_lines).split('\n\n')
+
+        for group in groups:
+            _name = None
+            _reviewers = []
+            _files = []
+            for line in group.splitlines():
+                if not line or line.isspace():
+                    continue
+                elif _name is None:
+                    _name = line
+                    continue
+
+                tag = line[0]
+                value = ' '.join(line.split()[1:])
+                if tag == 'R':
+                    _reviewers.append(value)
+                elif tag == 'F':
+                    _files.append(value)
+                else:
+                    assert False
+
+            _files = canonicalize_globs(_files)
+
+            reviewers.append({
+                'name': _name,
+                'files': _files,
+                'reviewers': _reviewers,
+            })
+    return reviewers
+
+def main(args, stdin):
+    """
+    Entrypoint when invoked as a script. Will print a list (one per
+    line) of registered reviewers for the files given, or for the files
+    touched by the patch piped in.
+    """
+    import fnmatch
+    import os
+
+    options = parse_options(args)
+
+    # With `-f`, any files given are used as is.
+    # Otherwise, files are considered to be patches and parsed as such.
+    # If no argument is given, then stdin is read as a patch.
+    if options.file:
+        assert all(os.path.exists(file) for file in options.file)
+        if options.f:
+            changed_files = options.file
+        else:
+            for filename in options.file:
+                with open(filename, 'r') as file:
+                    changed_files = parse_diff(file.read())
+    else:
+        changed_files = parse_diff(stdin.read())
+
+    review_groups = parse_reviewers()
+
+    # Filter down to the reviewers who care about the files changed
+    reviewers = []
+    for changed_file in changed_files:
+        for group in review_groups:
+            for group_file in group['files']:
+                if fnmatch.fnmatch(changed_file, group_file):
+                    for reviewer in group['reviewers']:
+                        reviewers.append(reviewer)
+
+    #TODO: add `-i` interactive mode
+
+    # Deduplicate list
+    for reviewer in set(reviewers):
+        print(reviewer)
+
+if __name__ == '__main__':
+    import sys
+    main(sys.argv[1:], sys.stdin)
-- 
Cheers,
  Eric



More information about the mesa-dev mailing list