Integrating Subversion and Review Board

At Google, all production code must be peer reviewed before it is allowed to be submitted to their source control system. Since I'm no longer a Googler, I don't have access to their awesome infrastructure so I had to roll my own. Review Board is a pretty good code review system, and unlike the open source implementation of Google's internal one, you can host Review Board on your own hardware.

We use Subversion as our SCM. Out of the box, Subversion doesn't force code to be reviewed before submission, and I couldn't find any documentation about forcing code reviews through pre-submit hooks. Subversion's hooks are pretty powerful, and Review Board has a decent API, so I wrote my own. Hopefully this is useful to someone out there. It's written in PHP5. Put this in your subversion hooks directory, call it pre-submit, and make it executable.

	#!/usr/local/bin/php
	<?php
	/**
	* Pre-commit Subversion script.
	*
	* Forces the commit message to have a line like
	* review: 42
	* and checks that the review has received a Ship It! from
	* a peer.
	* @author Omni Adams <omni@digitaldarkness.com>
	*/
	/**
	* Review Board server address.
	*/
	define('REVIEW_SERVER', 'http://your-server.com');
	/**
	* Path to svnlook binary.
	*/
	define('SVNLOOK', '/usr/bin/svnlook');
	/**
	* Divider to inject into error messages.
	*/
	define('DIVIDER',
	'********************************************************************************');
	/**
	* Get the name of the user committing the transaction.
	* @param string $transaction
	* @param string $respository
	* @return string Username of the commit author.
	*/
	function getCommitUser($transaction, $repository) {
	$authorOutput = array();
	$authorCommand = SVNLOOK . ' author -t "'
	. $transaction . '" "'
	. $repository . '"';
	exec($authorCommand, $authorOutput);
	$authorOutput = implode(PHP_EOL, $authorOutput);
	return trim($authorOutput);
	}
	/**
	* Check a review to see if it has a ship-it.
	* @param integer $id Review ID to load.
	* @param string $author SVN commit author.
	* @return array(boolean, string) Ship It status, author
	* @throws NotFoundException If we can't find the review.
	* @throws RetryException If RB returned garbage.
	* @throws RequestFailedException If RB is taking a nap.
	*/
	function getReviewStatus($id, $author) {
	$url = REVIEW_SERVER . '/api/json/reviewrequests/'
	. (int)$id . '/reviews/';
	$ch = curl_init();
	curl_setopt($ch, CURLOPT_URL, $url);
	curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
	$result = curl_exec($ch);
	$statusCode = curl_getinfo($ch, CURLINFO_HTTP_CODE);
	if (404 == $statusCode) {
	throw new NotFoundException();
	}
	if (200 != $statusCode) {
	throw new RequestFailedException();
	}
	$result = json_decode($result);
	if (!$result) {
	throw new RetryException();
	}
	if (!property_exists($result, 'reviews')) {
	throw new RetryException();
	}
	$review = $result->reviews[
	count($result->reviews) - 1];
	return array($review->ship_it,
	$review->user->username);
	}
	/**
	* Force the commit message to include the review number.
	*
	* If the commit message does not include a review number,
	* writes an error message to standard error and returns
	* true. Also checks to make sure the review has passed
	* its review.
	* @param string $transaction SVN transaction ID.
	* @param string $repository Full path to the repository.
	* @return boolean True if there was an error.
	*/
	function forceReviewNumberInCommitMessage($transaction,
	$repository, $author) {
	$logOutput = array();
	$logCommand = SVNLOOK . ' log -t "'
	. $transaction . '" "' . $repository . '"';
	exec($logCommand, $logOutput);
	$logOutput = implode(PHP_EOL, $logOutput);
	$logOutput = trim($logOutput);
	// Enforce that a review number is in the commit
	// message.
	$match = array();
	if (!preg_match('/[Rr]eview: [0-9]+/', $logOutput,
	$match)) {
	$message = PHP_EOL . DIVIDER . PHP_EOL . PHP_EOL
	. 'You didn\'t include the review number for '
	. 'this change.'
	. PHP_EOL
	. 'Your commit message MUST include a line '
	. 'with the review' . PHP_EOL
	. 'number like this:' . PHP_EOL . 'review: 42'
	. PHP_EOL;
	file_put_contents('php://stderr', $message);
	return true;
	}
	$match = array_shift($match);
	$match = explode(' ', $match);
	$reviewId = $match[1];
	$reviewStatus = null;
	// Our review board server is a bit flakey, so we may
	// need to retry.
	$retryCount = 0;
	while ($retryCount < 5) {
	try {
	$reviewStatus = getReviewStatus($reviewId,
	$author);
	break;
	} catch (NotFoundException $unused_e) {
	$message = PHP_EOL . DIVIDER . PHP_EOL
	. PHP_EOL
	. 'You put a review number that was not '
	. 'found. Check your review and try '
	. 'again.' . PHP_EOL;
	file_put_contents('php://stderr', $message);
	return true;
	} catch (RetryException $unused_e) {
	file_put_contents('php://stderr',
	'Retrying...' . PHP_EOL);
	$retryCount++;
	} catch (RequestFailedException $unused_e) {
	$message = PHP_EOL . DIVIDER . PHP_EOL
	. PHP_EOL
	. 'The request to the review board '
	. 'failed. Assuming you got a Ship '
	. 'It.' . PHP_EOL;
	file_put_contents('php://stderr', $message);
	return false;
	}
	}
	list($status, $reviewAuthor) = $reviewStatus;
	if (!$status) {
	// If no one has given the review a ship-it, fail
	// immediately.
	$message = PHP_EOL . DIVIDER . PHP_EOL . PHP_EOL
	. 'You included a review number that has not '
	. 'passed its review.'
	. PHP_EOL;
	file_put_contents('php://stderr', $message);
	return true;
	}
	// Make sure the reviewer isn't the same as the
	// author.
	if ($reviewAuthor == $author) {
	$message = PHP_EOL . DIVIDER . PHP_EOL . PHP_EOL
	. 'You can\'t review your own code. Get '
	. 'someone else to do it!'
	. PHP_EOL;
	file_put_contents('php://stderr', $message);
	return true;
	}
	return false;
	}
	/**
	* Not found exception.
	*/
	class NotFoundException extends Exception {}
	/**
	* Request failed exception.
	*/
	class RequestFailedException extends Exception {}
	/**
	* Retry exception.
	*/
	class RetryException extends Exception {}
	$repository = $_SERVER['argv'][1];
	$transaction = $_SERVER['argv'][2];
	$author = getCommitUser($transaction, $repository);
	file_put_contents('php://stderr',
	'Checking the code review status.' . PHP_EOL);
	$errors = forceReviewNumberInCommitMessage($transaction,
	$repository, $author);
	if ($errors) {
	exit(1);
	}
	exit(0);

view raw RB-precommit.php hosted with ❤ by GitHub