Better source: svn client-side precommit hooks

Dieser Post wurde aus meiner alten WordPress-Installation importiert. Sollte es Darstellungsprobleme, falsche Links oder fehlende Bilder geben, bitte einfach hier einen Kommentar hinterlassen. Danke.

Every single line of source code is being tested very deeply before being committed, isn't it? Well, maybe in a perfect world but reality has very little space for testing your source and simple typos often break stuff unnecessarily. Developers usually don't like testing at all, they want to develop new stuff and finish annoying tasks as fast as possible.

Automation may help (and I believe that commit-time is way too late for finding bad style or bugs) but svn doesn't support client-side hooks and server-side hooks may return "error" or "ok", but no "warnings".

There are a lot of Perl test tools which do syntax checks, print out compile-time warnings or look for bad style (like Perl critic). I like warnings::unused and wrote a tool for checking "use"-lines (compare modules used in the code and the one given in "use" lines) - but forget to run them on most commits.

I created a small client-side pre-commit hook for svn. Well, svn doesn't support any client-side hooks but my script provides a workaround: My script is called "svn" and lives in some place which is set in $PATH before the /usr/bin. Try it in your shell:

$ echo $PATH/home/sebastian/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
My personal bin directory (which is short for binary, not waste-bin) is the first item in my search path, a script places there is preferred over /usr/bin. /usr/local/bin would be a better choice if all users on a computer should use it.

Here is the script:


use strict;use warnings;

use Cwd qw(abs_path);use File::Slurp;use File::Temp;use File::Spec ();

A Perl script header, nothing special here except maybe the $|=1 line which outputs each print's contents even if there is no \n at the end of the line.
my @cmdline = @ARGV;exec '/usr/bin/svn', @ARGV unless (shift @cmdline) =~ /^com/; # Pass any non-commit arg directly
I want to capture commit calls only, all others switch over to the real svn client by calling /usr/bin/svn with all arguments provided by the user.

svn accepts many arguments but my script ignores them - because they don't matter for committing:

my @svnfiles;while (@cmdline) {        my $arg = shift @cmdline;        # Remove parameters for arguments        shift @cmdline if $arg =~ /^\-\-(?:depth|targets|message|file|editor\-cmd|encoding|with\-revprop|changelist|keep\-changelists|username|password|config\-dir|config\-option)$/;        shift @cmdline if $arg =~ /^\-[mf]$/;        # Skip named arguments        next if $arg =~ /^\-/;        push @svnfiles,$arg;}
All non-file arguments are ignored, everything else is collected into the array @svnfiles.

The script should check all Perl files, but what does all Perl files mean? svn accepts many wildcard formats and would only commit changed files. The svn tool is used to discover what is changed and will be committed:

open my $filelist,'-|','/usr/bin/svn status '.join(' ', map { "'$_'"; } @svnfiles);
All file-arguments are escaped in " chars (to support spaces in filenames and other special chars) and then passed to the svn status command.
local $ENV{PERL5LIB} = "/path/to/my/libs:lib";
All checks need to find files included via require, do or use.
my @files;my @failedfiles;my $other_files = 0;for my $file (map { if (/^D/) { (); } else { /^.{7} (\S.*?)\s*$/; } } <$filelist>) {
Deleted files (with a D as the status char) are ignored: Why should something be checked if it will be deleted anyway?
 if ($file !~ /\.(?:p[ml]|t)$/) { # No tests for non-perl files and deletes  $other_files = 1;  next; }
All non-Perl files are skipped (everything not in *.pl *.pm and *.t) but the script remembers that at least on other file was seen.
 print "\n\e[1;34m========== $file ==========\n";
The filename is shown right before the first checks start: The user should see which file got the errors (if there are any).
 my $perlcerr = File::Temp->new; # System retuns true if the called script returned false! my $result = system 'perl -c "'.$file.'" 2>'.$perlcerr->filename;
The syntax checker is started and it's result is redirected into a temporary file.
 my $report = read_file($perlcerr->filename); if ($result) {  print "\e[1;31m$report\e[0m";  push @failedfiles, $file;  next; }
Files failing the basic syntax check don't get any further checks. The following checks are more warning-checks than error checks and most of them require a valid Perl script to run. The \e[1;31m switches text to light red on ANSI terminals supporting ANSI.
 print "\e[1;32m$report\e[0m";
The report should be "Syntax check ok" usually.
 system '',$file; system 'perlcritic',$file; is my script for checking if all modules actually used within a source file are loaded with use. It's not 100% perfect but does a good job anyway.You should create your personal Perl::Critic config file to get Critic hints appropriate to your system and coding style.
 my $testfile = abs_path($file); if ($testfile =~ s/\/lib\//\/test\//) {  $testfile .= '.t';  $testfile = File::Spec->abs2rel($testfile);  system 'prove "'.$testfile.'"'   if -e $testfile; }
This projects test files life in a test/ directory next to lib. Run all tests for the current file (if there are any). prove will handle directories containing multiple test scripts and plain files as test scripts.

push @files, $file;}

Remember the file.

Critic, Unit-Tests and "use"-Checks aren't critical for me: They should be fixed but the checks might get false positives or fail due to any other reason, their results are shown, but ignored.

if (!@files and !$other_files) { print "No files left to commit\n"; exit 1;}
Warn and exit if no file passed the syntax check and no non-Perl files have been detected.
if (@files or @failedfiles) { # Skip question if we only have other_files
Non-Perl files are untested at all, no need to ask for confirmation if no test could fail because no test was done.
 if (@failedfiles) {  print "\e[31;1mFiles with syntax errors:\n* ".join("\n* ",@failedfiles)."\e[0m\n"; } else {  print "\e[32;1mAll Perl files passed syntax check.\n\e[0m\n"; } print "Continue commit (y/N)?"; my $YN = <STDIN>; exit unless $YN =~ /^y/i;
Show a short summary for the tests and ask the user if the commit process should continue.
 for my $file (@files) {  printf '%-110s'."\r","Tidy $file";  system 'perltidy',$file; } printf '%s'," \r";}
Run Perl's tidy for all files to clean up source for the local regulations.
exec '/usr/bin/svn', @ARGV;
Finally run the real svn tool to do the commit. All command line arguments are passed including any additional parameters.

Download the complete svn wrapper tool here.

It should be easily adaptable for git or other languages.


6 Kommentare. Schreib was dazu

  1. Ian Norton

    Hi Sebastian,

    Thanks for sharing this, excellent suggestions :)

  2. [Imported from blog]

  3. [Imported from blog]

  4. Hi Sebastian - thanks for the article. It is indeed frustrating that svn precommit hooks don't allow you to output any warnings, so for that a client-side hook is the only option.

    I do something similar with tidyall - specifically, the -s or --svn option will run checks against any uncommitted files according to svn. But your script is of course more accurate, in that it only checks the files currently being committed.

  5. [Imported from blog]

  6. [Imported from blog]

Schreib was dazu

Die folgenden HTML-Tags sind erlaubt:<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>