Im letzten Artikel ging es um Code-Reviews mit dem Review Board. Ich habe die Vorteile von Code-Reviews und einen möglichen Review-Prozess beschrieben. Außerdem habe ich erwähnt, dass SVN-Commits bei uns automatisch ein Review auslösen. Hier möchte ich die Details zu dieser Integration verraten.

Die Integration in das SVN haben wir mit einem Post-Commit-Hook gelöst, der das Command-Line-Tool post-review startet. Der Hook ist auf zwei Skripte aufgeteilt. Das folgende Shellskript organisiert das Logging und ruft das Pythonskript postcommit-review.py auf:

#!/bin/sh
REPOSITORY="$1"
REV="$2"
LOG=/var/log/post-commit.log

echo `date` "$REPOS" "$REV" >> $LOG

# note the & after the stdout/stderr redirection which starts the process in the background - 
#        we don't want that the commiter has to wait
/path/to/postcommit-review.py "$REPOS" "$REV" >> $LOG 2>> $LOG &

exit 0

Es ist auch möglich den Commit fehlschlagen zu lassen, wenn die Anlage im Review Board schiefgeht. Dazu darf postcommit-review.py nicht im Hintergrund gestartet werden, sondern mit postcommit-review.py .. || exit 1

Die Logdatei wird mit logrotate regelmäßig rotiert, um den Speicherverbrauch der Logs zu limitieren.

Das Python-Skript postcommit-review.py informiert das Review Board über den neuen Commit undbasiert auf https://github.com/reviewboard/reviewboard/blob/master/contrib/tools/svn…. Für das Skript gibt es einen eigenen User im Review Board, der ‘submit as’-Rechte hat, damit er im Namen des Committers den Review-Request erstellen kann. Für alle Commits ohne Angabe eines Reviewers werden Review-Requests in der Gruppe noreview erstellt, falls sie doch jemand ansehen möchte. Falls die Anlage des Review-Requests nicht klappt, wird ein zweiter Versuch ohne Daten aus der Commit-Message gemacht und der Review-Request in der Gruppe error erstellt. Diese Gruppe wurde zuvor im Review Board erstellt.

#!/usr/bin/env python
#
# This script should be invoked from the subversion post-commit hook like this:
#
# REPOS="$1"
# REV="$2"
# /usr/bin/python /some/path/svn-hook-postcommit-review "$REPOS" "$REV" || exit 1
#
# Searches the commit message for text in the form of:
# reviewer:[REVIEW-GROUP-NAME]
# updatereview:[REVIEW-NR]
#
# The log message is interpreted for review request parameters:
# summary = up to first period+space, first new-line, or 250 chars
# description = entire log message
# existing review updated if log message includes 'updatereview:[0-9]+'
# bugs added to review if log message includes 'bug:[0-9]+'
#
# By default, the review request is created out of a diff between the current
# revision (M) and the previous revision (M-1).
#
# To limit the diff to changes in a certain path (e.g. a branch), include
# 'base path:"<path>"' in the log message. The path must be relative to
# the root of the repository and be surrounded by single or double quotes.
#
# An example commit message is:
# Changed blah and foo to do this or that, bug:1234. reviewer:christian
#
# This would create a review of the currently commited revision for the 
# review board user 'christian'. It would place the entire log message in the 
# review summary and description, and put bug id 1234 in the bugs field.
#
# This script may only be run from outside a working copy.
#

#
# User configurable variables
#

# Path to post-review script
POSTREVIEW_PATH = "/usr/local/bin/"
# Username and password for Review Board user that will be connecting
# to create all review requests. This user must have 'submit as'
# privileges, since it will submit requests in the name of svn committers.
USERNAME = 'commit-hook'
PASSWORD = 'xxxxxxxxxxxxxxxxx'

# If true, runs post-review in debug mode and outputs its diff
DEBUG = False

#
# end user configurable variables
#

import sys
import os
import subprocess
import re
import svn.fs
import svn.core
import svn.repos

# starts a sub process
def execute(command, env=None, ignore_errors=False):
    """
    Utility function to execute a command and return the output.
    Derived from Review Board's post-review script.
    """
    if env:
        env.update(os.environ)
    else:
        env = os.environ

    p = subprocess.Popen(command,
                         stdin = subprocess.PIPE,
                         stdout = subprocess.PIPE,
                         stderr = subprocess.STDOUT,
                         shell = False,
                         close_fds = sys.platform.startswith('win'),
                         universal_newlines = True,
                         env = env)
    data = p.stdout.read()
    rc = p.wait()
    if rc and not ignore_errors:
        sys.stderr.write('Failed to execute command: %s\n%s\n' % (command, data))
    return rc

def main():
    if len(sys.argv) != 3:
        sys.stderr.write('Usage: %s <repos> <rev>\n' % sys.argv[0])
        sys.exit(1)

    repos = sys.argv[1]
    rev = sys.argv[2]

    # verify that rev parameter is an int
    try:
        int(rev)
    except ValueError:
        sys.stderr.write("Parameter <rev> must be an int, was given %s\n" % rev)
        sys.exit(1)

    # get the svn file system object
    fs_ptr = svn.repos.svn_repos_fs(svn.repos.svn_repos_open(
            svn.core.svn_path_canonicalize(repos)))

    # get the log message
    log = svn.fs.svn_fs_revision_prop(fs_ptr, int(rev),
                                    svn.core.SVN_PROP_REVISION_LOG)

    # we set this to true if there is no review request
    # in that case we assign the review-request to the noreview-group
    noreview = False

    # error if log message is blank
    emptylog = len(log.strip()) < 1
    if emptylog:
        print 'Log message is empty -> noreview = True'
        noreview = True

    # get the author
    author = svn.fs.svn_fs_revision_prop(fs_ptr, int(rev),
                                       svn.core.SVN_PROP_REVISION_AUTHOR)

    # error if author is blank
    if len(author.strip()) < 1:
        sys.stderr.write("Author is blank, no review request created\n")
        sys.exit(1)

    # check whether to create a review, based on presence of some key words
    if (not 'reviewer:' in log) and (not 'updatereview:' in log):
        print 'No review requested -> noreview = True'
        noreview = True

    # check for update to existing review
    m = re.search(r'updatereview:([0-9]+)', log, re.M | re.I)
    if m:
        reviewid = '--review-request-id=' + m.group(1)
    else:
        reviewid = ''

    # get previous revision number -- either 1 prior, or
    # user-specified number
    m = re.search(r'after(?: )?revision:([0-9]+)', log, re.M | re.I)
    if m:
        prevrev = m.group(1)
    else:
        prevrev = int(rev) - 1

    # check for an explicitly-provided base path (must be contained
    # within quotes)
    m = re.search(r'base ?path:[\'"]([^\'"]+)[\'"]', log, re.M | re.I)
    if m:
        base_path = m.group(1)
    else:
        base_path = ''

    # summary is log up to first period+space / first new line / first 250 chars
    # (whichever comes first)
    if emptylog:
       summary = '--summary=NO COMMIT MESSAGE'
    else:
       summary = '--summary=' + log[:250].splitlines().pop(0).split('. ').pop(0)

    # other parameters for postreview
    repository_url = '--repository-url=file://' + repos
    password = '--password=' + PASSWORD
    username = '--username=' + USERNAME
    description = "--description=(In [%s]) %s" % (rev, log)
    submitas = '--submit-as=' + author
    publish = '-p'
    revision = '--revision-range=%s:%s' % (prevrev, rev)

    # override the settings
    server = '--server=http://my.reviewboard.domain/reviewboard/'
    # check if there is a target group
    m = re.search(r'(?:reviewer):([-a-z,_]+)', log, re.M | re.I)
    if m:
        target = '--target-groups=' + m.group(1)
    else:
        target = ''
    # if there is no review request, put the review request in the group noreview
    if noreview:
        target = '--target-groups=noreview'
    # link to bug
    m = re.search(r'bug[: ]([,0-9]+)', log, re.M | re.I)
    if m:
        bugs = '--bugs-closed=' + m.group(1)
    else:
        bugs = ''

    # common arguments
    args = [repository_url, username, password, publish,
            submitas, revision, base_path, reviewid, server, target]

    # if not updating an existing review, add extra arguments 
    # (we do not 'override' existing reviews)
    if len(reviewid) == 0:
        args += [summary, description, bugs]

    # filter out any potentially blank args, which will confuse post-review
    args = [i for i in args if len(i) > 1]

    if DEBUG:
        args += ['-d', '--output-diff']
        print [os.path.join(POSTREVIEW_PATH, 'post-review')] + args

    # Run Review Board post-review script
    rc = execute([os.path.join(POSTREVIEW_PATH, 'post-review')] + args,
                   env = {'LANG': 'en_US.UTF-8'})

    # if the call failed - for whatever reason (maybe the target group does not exist) -
    # we try to create a default review request for the group 'error'
    if rc:
        print 'first attempt failed, trying again without data from commit message'
        args = [repository_url, username, password, publish,
                submitas, revision, server, "--summary=AUTO-CREATE r%s" % (rev), 
                "--description=revision%s" % (rev), '--target-groups=error']
        rc = execute([os.path.join(POSTREVIEW_PATH, 'post-review')] + args,
                env = {'LANG': 'en_US.UTF-8'})
        if rc:
            print 'second attempt failed too, giving up'
        else:
            print 'second attempt succeeded'



if __name__ == '__main__':
    main()

Alternativ zu post-review kann auch das post-Kommando der RBTools verwendet werden.

Viel Erfolg damit!

Andreas Hubmer
(Software Architect)