June 2, 2011

Running php linter before pushing changes to a git repository

A problem

Have you ever reviewed code that contained parse errors and wasn’t even proper PHP? I have and that’s why I have finally decided to stop that. Well, I have not decided to stop reviewing code, I have decided to prevent people committing PHP code which contain parse errors. With git hooks it’s fairly easy.

One way with pre-commit hook

One way to do it would be with pre-commit hook. Travis Swicegood explained it very well in his blog post. One problem with that is that every developer would have to deploy that hook locally in every repository. So that was really not an option for us.

Or another with pre-receive hook

What I wanted to do was to prevent people pushing broken code to git repositories stored on our git server (sort of in-house github). So I would deploy a hook on the server and the check would be done in there. pre-receive hook seems to be an ideal place for that. You can find our current version of the hook below:

Installation

To get it installed simply copy pre-receive hook into hooks subdirectory of a repository on a git server and make the file executable.

Internals

So how does it work? It’s actually pretty simple. If the hook exists with a non zero error code the push will fail, otherwise it will succeed. So all we have to do is to get a list of files changed in a given changeset and run php linter against them.

The hook accepts 3 parameters:

  • old revision id (ie. 325fbce03ba542c37c8529f36bf66998594e8384)
  • new revision id (ie. 58127dc11f4fd0c433cc6485a10925be5b56e1bb)
  • full reference name (ie. refs/heads/master)

Running git diff with –name-only option will give us list of changed files. But we cannot simply run php linter at this point as you have to remember that on git server there is just bare repository, without working directory tree. So we have to fetch the files straight from git context. We can fetch an object contents from index using git cat-file command, but it requires a unique object identifier to return file contents, so first we need to get this first. git ls-tree command is just what we need. Running it with new revision id and file name, we will get all the information we need!

#> git ls-tree 58127dc11f4fd0c433cc6485a10925be5b56e1bb Test/Application/WebTest.test.php
100644 blob 30a5e1914fe075db11c31780454fcc9d1d83aa9d	Test/Application/WebTest.test.php

Now, when we run git cat-file with the object type we got back (blob in case of files) and object id, we will get contents of that file, that we can finally pipe into php linter process. If the process returns any non-zero error code we display an appropriate message and stop script execution with a non-zero code:

smarek@28:~/git/modules/Sandbox (master)$ git push origin master
Counting objects: 14, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (8/8), done.
Writing objects: 100% (9/9), 857 bytes, done.
Total 9 (delta 2), reused 0 (delta 0)

Running php linter...
Error parsing file 'Test/Application/WebTest.test.php'

error: hooks/pre-receive exited with error code 255
To git:Sandbox.git
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'git:Sandbox.git'

otherwise we just stop the script with a zero code!

smarek@28:~/git/modules/Sandbox (master)$ git push origin master
Counting objects: 8, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 658 bytes, done.
Total 6 (delta 0), reused 0 (delta 0)

Running php linter...
No errors detected

To git:Sandbox.git
90f43b9..36582e1  master -> master

No more parse errors! FTW!

2 Comments

  • Thanks for your great script.

    We’ve found a little glitch, when there are deletes php files in git diff.

    You can solve it with adding –diff-filter:
    exec(“git diff –name-only –diff-filter=ACMRTUXB $params[0] $params[1] 2> /dev/null”, $diff, $return);

Leave a Reply

Your email address will not be published. Required fields are marked *