Git: pre-receive hook with PHP_CodeSniffer

Question

Since switching from SVN to Git, we lost the ability to enforce our coding standards through a pre-commit hook on the subversion server.

With Git, you only have pre-commit hooks on the client which cannot be enforced in any way. What makes it worse is that we have developers working with all three main operating systems, thus a pre-commit hook that works on Linux or OS X does not automatically work on Windows.

The way to go is implementing a pre-receive hook on the server, but the solution is not as easy as it seems:

Imagine the developer did 20 commits and wants to push them. All pre-commit and pre-receive hooks I know of (1, 2) just check the single commits, which will ultimately fail and prevent the push. Now the developer fixes the issues and does another commit, and tries to push again. Since the hooks check the single commits, it will fail again.

So we need a pre-receive hook that generates a list of all changed files in all commits that are going to be pushed and runs phpcs on the current state only.

Does such a hook script exist already? Where?

Edit: There seems to be a script that creates that list of files - unfortunately in Python, but that can be ported. I'm still interested in pre-made solutions with PHPCS :)

Karame

Answer 1

We do now employ pre-commit hooks that both check the code and the commit message.

Developers can skip them with -n, but they rarely do and we always have another developer doing QA so things get noticed.

The hook is important because it notices when files are broken, so that broken PHP or JS doesn't get committed at all.

Find them hook code at https://github.com/netresearch/git-client-hooks

We use a central server for development, and our git hooks get automatically installed because we provide a central git repository template that gets used automatically when you git clone or git init.

Karame

Downvoted because this link appears in a comment above already. – Helot

Answer 2

I would rather not wait for a server-side hook to control a push.

You could setup an intermediate repo which would fetch each developer's branches very regularly and audit each new commit, sending an email if a commit fails to meet some pre-defined criteria.

You can also have pre-receive hook on the central repo, but at least the developer would be aware of a potential issue sooner.

Thermotensile

How about rebasing to squash the correction into the offending commit? – Aconcagua

@Benjol: if you have an intermediate repo doing some guarded commit, not squashing said commits allows for a more smaller changeset introduced by said offending commit. – Thermotensile

Answer 3

I don't speek good 'git-anese', but in Mercurial there's a hook option called 'changegroup' that essentially checks the 'top' commit of a group of incoming commits. Maybe someone in the community can tell you if there's an equiv. of 'changegroup' for git?

https://www.mercurial-scm.org/wiki/Hook#The_changegroup_hook

Telemachus

Answer 4

I don't think there's a technical solution here but if you really want to bother people, then integrate phpcs into your CI setup and start opening tickets for it in your issue manager. ;-)

I don't think that's the best idea though because it's really not a technical problem. Your issue is not a pre- or post-commit hook, but that people don't do it and you think you have to force them.

All in all, I understand the importance of a coding standard and I enforce one as well, but there's a social component (or aspect) to it.

It sounds like the people working with you either don't know any better (yet) or are un-willing to learn. So, in case they don't know any better you have to work with them and teach them to adhere to your requirements. That includes teaching them why conventions are important and in the end they need to understand that a feature is not done until everything is green.

Maybe that requires that project management (I gather that's you) breaks down an issue into multiple tasks until they get it:

• feature itself
• phpcs
• documentation
• unit-tests

(In no specific order. ;-))

If they're unwilling to learn you can always take more drastic measure. Like, I'd start off slowly and do a weekly performance review (1-on-1 situation) and reiterate why they don't do it. If that doesn't help -- I guess you catch my drift.

Cladophyll

Downvoted for attempting to sway the OP on what their needs are given they're trying to achieve something specific they already had in place. – Helot

Answer 5

In the Drupal project, we've recently migrated to Git and are looking at similar problems. In our case, we don't want anyone checking in a LICENSE.txt file for a module since our packaging scripts do that automatically. After some back and forth, what we came up with was a receive hook that doesn't reject a bad commit, but every time it detects a bad commit (for some definition of "bad") it automatically files a critical bug in our bug tracker. That way the code is still committed, but the module maintainer and the appropriate webmaster team both get notified immediately that something is amiss and should be fixed. You could just as easily send an email or send a tweet or whatever other notification you want.

Actually we haven't implemented that quite yet, but that's the plan we're working on as soon as our Git implementation team has time. :-)

Basically, there is no good solution for the problem you describe other than rephrasing it; instead of "block detectable violations" it becomes "report detectable violations." I think that's the best you can do.

Dosimeter

Answer 6

use jenkins + gerrit:

http://alblue.bandlem.com/2011/02/gerrit-git-review-with-jenkins-ci.html

if your build fails, the push will be rejected.

http://source.android.com/source/life-of-a-patch.html

Tyrael

Avar

Answer 7

I used this hook: http://criticallog.thornet.net/2011/06/02/running-php-linter-before-pushing-changes-to-a-git-repository/

and modified it to also test code with phpcs.

Might contain some bugs and I have hardcoded drupal code standard but it works! http://pastebin.com/fEmN519B

Withdrew

Answer 8

I don't have an exact answer to this question directly using pre-commit/pre-recieve hooks etc.

I approach this from the other way, running a CI server, (I use jenkins) running phpcs and the checkstyle plug-in for Jenkins.

This allows me to fail the build and email the committer based on the checkstyle report.

Optionally I can set thresholds, so I get an unstable build if there are upto 5 new style violaions but a failure if more than 5 are committed.

Also I can set overall thresholds, so more than 10 violations in the whole project causes a failure and emails the team.

This could be your intermediate server as mentioned above. The post build actions can include a Push to another git repo.

Objectify

We do use that, too. Unfortunately, not all devs care about such mails and want to be forced. That's the way it is :/ – Karame

Answer 9

We use a git wrapper (to replace git-submodules with something more sane for our purposes) and it has the side-effect of automatically setting up pre-commit hooks automatically from a magic directory. Since this is in a business environment, there are no complains (and there is a way to turn that off anyway).

Gonyea

Answer 10

I have experimented with this too. I don't have the code at hand at the moment, but I used one of the hooks (not pre-receive, I think it was the update one) to do a temporary checkout of the new ref. You can speed this up by having a checked out tree which you just have to update and by just doing shallow clones.

This allows access to the whole source tree and you can not only run CS on the files changed with the push, but also run the unit or smoke tests.

I also agree with some of the other comments that these tests should be kept to a minimum, because nothing is more annoying than being blocked by commit hooks. Any further checks should be happening in your CI server or deployment system.

Omnifarious