Warum SVN Pre Commit Hooks böse sind!

Immer mehr Projekte setzen auf SVN Pre Commit Hooks. Auch liest man immer öfters, dass dies sozusagen zum guten Ton gehört. Aber das wäre nicht der Blog, der er ist, wenn ich nicht wieder was zu motzen hätte. Dass diese Hooks eine nette Idee sind bezweifel ich ja gar nicht, aber man kann sie auch falsch einsetzen. Und genau darüber möchte ich heute versuchen zu reden.

Erst mal für alle die es nicht wissen, bei diesen Hooks geht es darum bei einem Einchecken in das Repository gewisse Eigenschaften zu testen. Falls diese nicht eingehalten wurden, verhindert das System das Einchecken. Wunderbare Sache, wenn man dieses Feature in kleinen Dosen anwendet. Zu einem Problem wird es aber, wenn man es übertreibt. Ich gebe mal drei Beispiel, ihr könnt euch ja mal aussuchen, welche davon euch gefallen und welche nicht.

php -l
Ich hatte ja schon über den Syntax Check von PHP geschrieben. Dieses Tool in einen Hook eingebaut checked mir jedes mal vor einem Commit, ob mein Code syntaktisch korrekt ist.
PHP_CodeSniffer
Auch ein Tool, dessen Einsatz ich oft genug empfohlen habe. Der Sniffer in Kombination mit SVN verhindert, dass unsauberer Code in mein Repository gelangt. So kann ich von ausgehen, dass meine Code Formatierung und diverse Metriken immer dem Standard entsprechen.
Commit Eigenschaften
Immer wieder gerne in der Prüfung: Entspricht meine Commit Message unseren Standards. Ist zum Beispiel das die Bug-ID enthalten oder ähnliches.

Und bei welchen würdet ihr sagen, dass es eine gute Idee ist? Bei mir ist es wie folgt. 1) Sehr gut. 3) Ist ok. 2) NEIN!

Ich höre immer öfters, dass Leute ihre Codeformatierung an einen Hook hängen. Aber stellt man sich mal die Situation vor, dass die Webseite wegen eines kleinen Fehlers down ist. Er wird sofort gefunden und gefixed. Jede Sekunde kann Geld kosten. Was machen wir also? Möglichst schnell comitten damit der Fehler aus dem Produktivsystem raus ist. Sollte es jetzt in diesem Workflow eine Instanz geben, die mir verbietet einzuchecken, nur weil ich Tabs statt Spaces verwendet habe? Ich glaube nicht. Ich hoffe, ihr seht wo ich hin will. Ich finde es immer unschön einen Workflow zu unterbrechen, nur weil etwas „unschön“ ist. Der php -l Check hingegen ist etwas anderes. Alles was diesen Test nicht besteht ist definitiv kaputt.

Was die Commit Eigenschaften angeht, da bin ich auch erst nach Diskussion abgewichen, dass ich es für eine doofe Idee halte. Aber so viel Zeit muss sein, damit man, falls beim Hotfix etwas schief geht noch Anhaltspunkte hat, was da denn jetzt nun passiert ist. Soviel Zeit muss also sein, um es später dokumentiert zu haben, was passiert ist.

Aber nur weil ich den CodeSniffer nicht an dieser Stelle einsetzen würde, würde ich trotzdem sagen, dass er sehr sinnvoll ist – habe ja schließlich genügend drüber geschrieben. Nur sollte man dieses Tool in seiner Continuous Integration Lösung verorten. Ich bekomme also alle meine Informationen, die ich brauche, um meinen COde sauber zu halten und verändere oder blockiere den Workflow nicht, da alles wunderbar parallel und unabhängig abläuft.

Das ganze kann natürlich bei Projekten ganz anders sein, deren Ausfall egal ist. Dort kann man so Spielereien einbauen, denn es ist sicher ein schönes Gefühl zu wissen, dass der Code sauber ist und es gar nicht möglich ist „schlechten“ Code einzubauen. Was ich also sagen wollte mit meiner provokanten Überschrift: Ich würde den Workflow des Entwicklers nicht mit unkritischen Dingen beeinträchtigen. Sollte man aber diese Tests nicht im Commit machen, dann unbedingt im Continuous Integration, denn ansonsten sind die Tests wertlos. Ich hoffe mal, dass das genügend Diskussionsstoff war.

PS: Der Artikel ist übrigens absichtlich ein wenig überspitzt, damit wir ein wenig diskutieren können.

23 Comments

Dann stell ich doch gleich mal die erste spitze Frage 😉 Jemand der sich so ein System hinstellt .. hat doch mindestens ein Test-/Staging-System auf dem er Änderungen vorher sichtet, bevor er sie aufs Produktiv-System stellt? :o)

Was ich hingegen gerne mal verwende ist der post-commit hook, der automatisch die letzte Revision aufs Staging-System ausspielt – sowas von bequem :>

@Steffkes: Staging Systeme sind genau so selten wie ein Continuous Integration System habe ich leider das Gefühl. Aber auch der Einsatz einer Staging Instanz ändert meiner Meinung nach nicht meine Aussage von oben 🙂

Falls ich so einen Hook überhaupt einrichten würde wär auf jedenfall eine: „Wenn die Logmessage ‚mist mist mist der server steht!!!‘ enthält dann ignorier den Check“ Abfrage dabei um genau das Problem zumindest schon mal zu unterbinden.

Vielleicht mit einem sinnvolleren String 😉

Im großen und ganzen denke ich es liegt hauptsächlich daran was das Team will, wenn es für die Leute einfacher ist das gleich beim Commit gesagt zu bekommen (oder man den CI Output erst mal lang ignoriert) kann das schon der einfachere Weg sein, immerhin ist der Editor noch offen und man muss nicht nachdenken.

Aber ich halte es damit wie mit phpdocumentor Parser Fehlern, die muss man auch nicht _sofort_ beheben und wenn man zumindest einmal pro Woche (je nach Projekt und Commithäufigkeit) alle „Build“ Errors behebt reicht das.

Staging Systeme helfen eigentlich nur, wenn sie auch wie ein Live-System benutzt werden – also täglich damit gearbeitet wird.

Was mich mal interessieren würde, wie man Continuous Integration System mal richtig aufsetzt. Ich habe zu hause mal versucht, phpUnderControl für ein Projekt aufzusetzen und habe jetzt so ne halbe Installation hinbekommen, die danach schreit, neu aufgesetzt zu werden 😉 Hat dazu mal jemand Informationen, wie man das richtig aufsetzt? Quasi ein Howto für Einsteiger 😉

@Dennis: Ich kann mich ja mal dran versuchen. Wir setzen zwar bald Bamboo ein, aber phpUnderControl ist meinen Erinnerungen nach ziemlich einfach aufzusetzen. Ich schau mal was ich machen kann.

@Nils: joa, sooo schwierig scheint esn icht zu sein, aber irgendwie macht sich keiner mal die Mühe, die Schritte auszuformulieren, so dass auch ein nicht versierter Entwickler in dem Bereich das mal aufsetzen kann. Man kann ja nicht einfach mal mittendrin 2 oder 3 Schritte nicht erwähnen und meinen, das kriegt dann schon jeder hin, denn daran scheitert es bei mir immer wieder, dass irgendwo ein Schritt ausgelassen wurde in der Beschreibung *hmpf*

@Dennis: Klingt einleuchtend. Vielleicht machen wir das einfach mal zusammen 🙂

@Nils und Dennis

Nachdem ich neulich mit einer ähnlichen Problematik zu „kämpfen“ hatte, wäre ich gar nicht traurig darüber, eine Art Zusammenfassung dann irgendwo als Artikel zu finden. 🙂

Da schliesse ich mich dem Benjamin an. 😉

@benjamin, nils, dennis and all
ich habe so eine Anleitung geschrieben.
http://www.cds-spremberg.de/blog/installation-phpundercontrol-from-scratch/

Die ist bereits mehrfach nachgeprüft/nachgebaut worden. Ich muss heute auf einem Debian 5 eine weitere Installation starten. Alle neuen Erkenntnisse werde ich einarbeiten, freue mich aber auch über Hinweise.

@Artikel Thema
Interessant sind die post-commit Hooks. Meist leider nur als Email Schleuder „missbraucht“. Der Code ist ordentlich eingecheckt und das Deployment auf Testsysteme kann starten.

Habe in den letzten Tagen Beispiele gebraucht und leider nichts gefunden, dass auf gebranchte Verwaltungen eingeht – also habe ich meine Skripte in mühevoller (Commit)Arbeit erstellt. Für einen Artikel darüber kann ich gerne Material liefern.

Den CodeSniffer im Pre-Commit kann enorm nützlich sein, aber auch das Gegenteil bewirken. Es hängt von den Regeln ab und ob der existierende Code diese erfüllt.

Wenn der existierende Code die Regeln erfüllt, stellt der Pre-Commit-Hook sicher, das dies so bleibt. Hier gelten ähnliche Regeln wie für Tests, Zeit die jetzt investiert wird, spart man später. Da die Änderungen an Quellcode atomar und unmittelbar sein sollten, hat der Programmierer benötigten Dateien in diesem Moment noch in Bearbeitung. Wird er später über Änderungen benachrichtigt (Post-Commit-Hook) ist er höchstwahrscheinlich schon mit einem anderen Problem beschäftigt und muss erst wieder „umschalten“.

Neue Hooks sollte man eventuell erst im Post-Commit-Hook und in externen Scripten einführen. So kann ein „QA-Beauftragter“ erstmal dafür sorgen, dass der bestehende Code die Bedingungen erfüllt.

@jensk: Das hab ich auch schon gefunden, aber ich finde es weniger gut, wenn das ganze Beispiel-Projekt modifiziert wird. Zudem ist das run-Script auch nicht so das Wahre, da habe ich mittlerweile ein wesentlich besseres bekommen über den #phpunit IRC Channel.

Eine wirklich saubere Installation wäre wünschenswert.

@dennis
hmm, also ein besseres? Meine Variante startet zuverlässig bei jedem Bootvorgang. Was ist besser?

Du solltest bedenken, dass dort von einer definierten Umgebung ausgegangen wird. Die Pfade und Umgebungsvariablen sind kommentiert und müssen bei abweichender Umgebung natürlich angepasst werden.

Eine sauberere Installation als die von einem frisch installieren Server System kenne ich nicht. Aber ich lerne nun schon so viele Jahre dazu … 😉

In Bezug auf den phpCS-Hook gibt es sowohl für Neatbeans [1], alsauch für Eclipse PDT [2] seit kurzem Hilfen, die vor dem Commit Anzeigen, ob der Code den Coding Guidelines entspricht. Soweit ich weis ist keines der beiden Erweiterungen als ’stable‘ deklariert – ich selber nutze jedoch die Erweiterung für Eclipse PDT produktiv und es funktioniert sehr zuverlässig.

[1] http://www.whitewashing.de/tag/view/CodeSniffer
[2] http://www.phpsrc.org/wiki/

@Bastian: Die PDT Erweiterung nutze ich seuit gestern und ich bin auch wirklich begeistert. Werde auch die Tage was derüber schreiben, da die Doku ein wenig vernachlässigt wurde in dem Projekt.

Pingback: dotfly blog

Wir nutzen die Pre Commit Hooks u.a. für die Sicherstellung der Wahrung von Coding Guidelines – also Punkt 2 aus der Liste.

Die beschriebene Situation (Patch muss schnell auf Live) sollte m.E. nicht dazu führen, dass alle Prozesse ausser Acht gelassen werden – Testing und der „Umweg“ (bzw. normale Weg) über den internen Staging Server gehören also eh dazu, bevor etwas live geht. Dann kommt es zeitlich auch nicht drauf an, ob man sauber formatierten Code schreiben muss oder nicht.

Ein „hot fix“, der wirklich binnen Sekunden auf den Server muss, ist derart selten und darf auch nur in absoluten Ausnahmefällen passieren, sodass dies m.E. kein Argument gegen die SVN Hooks sein kann.

Und wenn es mal unbedingt notwendig sein sollte, so kommt man ja auch an den selbst geschaffenen Regeln vorbei 😉

Insgesamt war die Einführung der SVN Hooks fürs Team bei uns schon eine Umstellung, m.E. steigert das aber die Codequalität und nicht zuletzt die Lesbarkeit enorm.

Gruss,
André

Hallo,
liest hier jemand die „alten“ Artikel und Kommentare?

Ich habe die phpUnderControl Installation nochmal Schritt für Schritt dokumentiert und das Startskript für CruiseControl bei GoogleCode gehostet. Wer es jetzt nicht schafft hat das falsche Betriebssystem 😉

http://www.cds-spremberg.de/blog/phpundercontrol_quick_install_0910/

@jensk: Also ich lese 🙂 Hab aber Cruise Control schon am laufen.

Auch wenn der Beitrag schon recht alt ist will ich noch meinen Senf dazugeben:

Pre-Commit-Hooks sind auf dem Userrechner installierte Programme. Schon daher macht es kaum Sinn, sie so einzusetzen wie es im Blogeintrag angedeutet wird.

Ich habe bei mir nur Hooks installiert, die von uns selbst für unsere Bedürfnisse geschrieben sind und die mich hinweisen, wenn ich WAHRSCHEINLICH etwas nicht einchecken will.
Wenn etwas gefunden wird, wird mir die Zeile angegeben und gefragt, ob ich trotzdem committen will.
Zum Beispiel fragt mich mein Hook, wenn er einen „TODO(LK)“-Eintrag findet.
Automatisch abgebrochen wird da nichts – das hielte ich auch für Unsinn.

@Lars

Nein, in der Regel bezeichnet man mit Pre- oder Post-Commit-Hooks Scripte, die auf dem Server vor oder nach der Annahme des Commits ausgeführt werden. Liegen dann meist (bei Subversion) in /hooks/ im Repository-Konfigurations-Verzeichnis.

Manche Clients ermöglichen auch Scripte, die auf Client-Seite ausgeführt werden, bevor der Commit an den Server gesandt wird, dies ist aber nicht das Szenario auf das sich der Artikel bezieht.

Die Frage ist: Warum gehen Fehler online? Das Problem in den meisten Projekten ist, dass der Releaseprozess manuell durchgeführt werden muss. Manuell bedeutet auch gleichzeitig fehleranfällig und nicht zuverlässig.

Moderne IDEs (Jetbrains PHPStorm, Zend Studio, Eclipse) bieten die Möglichkeit an, Code Sniffer Ergebnisse direkt anzuzeigen. Wenn ich also code schreibe, sollte ich sofort sehen, wenn ich einen Fehler mache.

Für die Projekte, die kein Continous Delivery machen: Commit Hooks besser aus, und über einen automatischen Releaseprozess nachdenken (Siehe Continous Delivery).