Most of us like things done the right way and (more often than not) our way. Nothing can be more infuriating than looking through poorly documented logs or code, and trying to understand poor documentation and what it was meant to get across.
During development we often look to setting up good coding practices that include documenting and commenting what’s going on and in what format we agree to write comments. And in many organizations, this is where keeping those good practices stop. In most organizations I’ve worked for, developers rarely submit good, formatted notes with their code if they submit documentation at all. So here is a quick and simple method to enforce good commenting standards for your organization using SecureCI. If you haven’t downloaded SecureCI, this is your time to try it out!
Example Development Process
When I’m looking through SVN logs for a specific revision I like to know that the information in the log is going to let me know everything I need to know to successfully revert my repository to the correct revision. I want to know that whenever a developer submits code that it can be tracked back to a specific ticket in Trac. The best way to do this is with a simple Pre-Commit Check.
My ideal log message would look something like the following:
Fixes #3146. Should address the XSS vulnerability for this web page.
I really only care to enforce the action and the ticket number reference; this would provide me a reference to the trac ticket number, what was done and still allow the developer enough flexibility to comment appropriately for the circumstance. No need to get too specific and lock yourself into a format that eventually leads to meaningless comments.
Pre-Commit Check Implementation
To perform the Pre-Commit Check we need to build a pre-commit hook for SVN and Trac. We will need a script that will be invoked by SVN to validate that the SVN comments submitted match up to real tickets in Trac. I’ve decided to take an existingscript and modify it, to fit my needs instead of reinventing the wheel.
Start by taking in a couple environment variables that will be passed in from SVN. In this case we are concerned with receiving the path to our Trac environment and the SVN Logs.
# Check that you have all the correct system arguments, and provide user feedback if you don't! if len(sys.argv) != 3: print >> sys.stderr, 'The appropriate usage should be: %s <Your_Trac_Env_Path> <Your_SVN_Log_Message>' % sys.argv[0] sys.exit(1) env_path = sys.argv[1] log = sys.argv[2]
Next we will use a regular expression command to find any references to a Trac ticket in the log message. I’m allowing a lot of action words to address all the different things a developer might do to a ticket with SVN. I’ve also made sure those actions are in various tenses so that I don’t force myself to only use past tense or future tense in my comments. The actions I decided on were ‘close’, ‘fix’, ‘address’, ‘reference’, and ‘see’. NOTE: This would be a good place to edit the phrases to fit your organization. My actions may be too restrictive or too open for your organization.
tickets = []
# Grep through the comment to look for an action and a reference to a ticket. The ticket should be proceeded
# by a '#' sign
for tmp in re.findall('(?:close|closed|closes|fix|fixed|fixes|addresses|addressed|references|refs|re|see)' \
'.?(#[0-9]+(?:(?:[, &]+| *and *)#[0-9]+)*)', log.lower()):
tickets += re.findall('#([0-9]+)', tmp)
# No tickets were mentioned. Provide the user some feedback so they know what went wrong.
if tickets == []:
print >> sys.stderr, 'At least one ticket must be mentioned ' \
'in the log message. Ex: Fixes #3146.'
sys.exit(1)
Next, we will need to go through our Trac tickets and make sure that users can only mention tickets. Even though it may not be necessary I want to know that the tickets being referenced are open and not closed. This is a really good way to reduce errors in commenting (fat fingering) or prevent someone from adding additional code unnecessarily.
env = open_environment(env_path)
db = env.get_db_cnx()
cursor = db.cursor()
cursor.execute("SELECT COUNT(id) FROM ticket WHERE status <> 'closed' AND id IN (%s)" % ','.join(tickets))
row = cursor.fetchone()
# At least one of the tickets mentioned in the log messages has to be open. If the ticket is not mentioned
# provide the user feedback so they know what went wrong.
if not row or row[0] > 1:
print >> sys.stderr, 'You must mention at least one open ticket in the log message.'
sys.exit(1)
else:
sys.exit(0)
All together your script should look something like this. You’ll want to put it in your Trac pre-commit folder (/var/lib/trac/hooks) and name it ‘trac-pre-commit-hook’. NOTE: I had to create a python egg to get this to run in my specific environment. Depending on how you configure you SecureCI Instance, you may not need it.
#!/usr/bin/env python
# -*- coding: utf-8 -*-
import os
import re
import sys
if not 'PYTHON_EGG_CACHE' in os.environ:
os.environ['PYTHON_EGG_CACHE'] = os.path.join(sys.argv[1], '.egg-cache')
from trac.env import open_environment
def main():
# Check that you have all the correct system arguments, and provide user feedback if you don't!
if len(sys.argv) != 3:
print >> sys.stderr, 'The appropriate usage should be: %s <Your_Trac_Env_Path> <Your_SVN_Log_Message>' % sys.argv[0]
sys.exit(1)
env_path = sys.argv[1]
log = sys.argv[2]
tickets = []
# Grep through the comment to look for an action and a reference to a ticket. The ticket should be proceeded
# by a '#' sign
for tmp in re.findall('(?:close|closed|closes|fix|fixed|fixes|addresses|addressed|references|refs|re|see)' \
'.?(#[0-9]+(?:(?:[, &]+| *and *)#[0-9]+)*)', log.lower()):
tickets += re.findall('#([0-9]+)', tmp)
# No tickets were mentioned. Provide the user some feedback so they know what went wrong.
if tickets == []:
print >> sys.stderr, 'At least one ticket must be mentioned in the log message. Ex: Fixes #3146.'
sys.exit(1)
env = open_environment(env_path)
db = env.get_db_cnx()
cursor = db.cursor()
cursor.execute("SELECT COUNT(id) FROM ticket WHERE status <> 'closed' AND id IN (%s)" % ','.join(tickets))
row = cursor.fetchone()
# At least one of the tickets mentioned in the log messages has to be open. If the ticket is not mentioned
# provide the user feedback so they know what went wrong.
if not row or row[0] > 1:
print >> sys.stderr, 'You must mention at least one open ticket in the log message.'
sys.exit(1)
else:
sys.exit(0)
if __name__ == '__main__':
main()
Lastly we will need to have our SVN repository invoke our script. To do that, we will call our Pre-Commit using the following pre-commit script. Note: If you’re using SecureCI out of the box, it should be an easy cut-and-paste job. If not, you may need to alter this script for your specific environment.
REPOS="$1" TXN="$2" TRAC_ENV="/var/lib/trac/secureci" LOG=`/usr/bin/svnlook log -t "$TXN" "$REPOS"` /usr/bin/python /var/lib/trac/secureci/hooks/trac-pre-commit-hook $TRAC_ENV "$LOG" || exit 1 set e exit 0
Place this script in the SVN hook directory (var/lib/svn/repos/secureci/hooks) as ‘pre-commit’, restart your server and your good to go!
What’s Next
Next, I will look into addressing a post-commit check for SecureCI and how it can help you have a cleaner and easier development process.
