Pre-Commit Hooks and Breaking the Build

One thing I love about git it how you have control over your local environment. Need a pre-commit hook? Just add the damned thing. Need it for Subversion and you're not the admin of the subversion server? Sucks to be you. Fortunately, having a reasonable command line lets you work around this. And I needed to because I broke the build. Twice. In a week. Buying the donuts for this would be slightly less annoying if it weren't for the fact that I get to "enjoy" low-calorie yoghurt bars instead. (Though I'm now below 82 kilos for the first time in a couple of decades).

So no more breaking the build. Right. On my previous team, if I needed a module, I could usually add it. On this team, it's a bit painful, so I just use what I need locally and then break the build when I forget and commit use Test::Most 'die';. So I wrote a little bash scrip to make svn less brain-dead.

#!/bin/bash

if [ "$1" = commit ]; then
    command="ack 'Test::Most|Data::Dumper::Simple' t/ lib/"
    lines=`$command | wc -l`
    if [ $lines > 0 ]; then
      echo $command
      echo `$command`
      echo You suck!
      exit 1
    fi
fi

if [ "$1" = log -o "$1" = annotate ]; then
  /usr/bin/svn "$@" | less
elif [ "$1" = diff ]; then
  /usr/bin/svn --diff-cmd colordiff -x "-u" "$@" | less -R
else
  /usr/bin/svn "$@"
fi

Basically, if I try to commit and I've left offending modules in the code, it tells me I suck and fails. As an added bonus, I realized I no longer need to pipe common commands to less and I get nicely coloured diff output.

This is dangerous, though. I could easily add "run perltidy on my code prior to commit". This sounds great until you realize you've just tidied a Makefile. Oops/ Still, my life is a touch easier now. And I hope that my future "breaking the build" will be in new and creative ways.

My bash isn't very good, so critiques welcome. And don't forget to write tools like this for yourself. We're programmers. Little itches become big problems when we don't scratch them. Make them go away.

And if you're wondering why I'm still using svn, it's because git-svn is a dream when working with our branches, but several git gurus have given up trying to find out why merging back into trunk on our project fails so badly. So I don't do that and still rely on svn for that tiny task.

7 Comments

[ $lines > 0 ] does a string compare. You probably want -gt instead. Also, I would always use $() rather than backticks, and you don't need to quote the "-u" under 'diff' (though you would if you had more options, of course).

Use the -1 (that's a hyphen-one) flag on ack. It stops after the first hit of any kind. Since you're effectively checking for the boolean "Do I have Test::Most or Data::Dumper::Simple", no point in checking the entire tree.

Are you legitimately using Data::Dumper somewhere?

At a previous job, we had a pretty good system with a Subversion commit hook: whenever you checked in a *.pm, the test class associated with that module would get run by the pre-commit hook. If the tests failed, your commit was rejected. This worked pretty well for preventing broken code from getting into the repository, as long as there were tests for the particular module in question.

The downside is that if the tested behavior of a module changed, you had to first commit the new tests, then do a separate commit for the changed module, so the SVN server would run the new tests against it instead of the old ones which would break.

You're running ack twice. I would use something more like:

 

ackout=$(ack ...)

if [ -n "$ackout" ]
then
    echo "You screwed up somewhere:"
    echo "$ackout"
fi

PS: Is there any way to get decent looking code in comments? This is the best I could do, but it's not great.

Untested:

#!/bin/bash
case "$1" in
    commit)
        cmd=( ack -1 'Test::Most|Data::Dumper::Simple' t/ lib/ )
        out=$( "${cmd[@]}" )
        if [ "$out" ] ; then
            echo -E "${cmd[*]}"
            echo -E "$out"
            echo 'You suck!'
            exit 1
        fi
        /usr/bin/svn "$@"
        ;;
    log|annotate) /usr/bin/svn "$@" | less ;;
    diff)         /usr/bin/svn --diff-cmd colordiff -x -u "$@" | less -R ;;
    *)            /usr/bin/svn "$@" ;;
esac

Leave a comment

About Ovid

user-pic Have Perl; Will Travel. Freelance Perl/Testing/Agile consultant. Photo by http://www.circle23.com/. Warning: that site is not safe for work. The photographer is a good friend of mine, though, and it's appropriate to credit his work.