Merge lp:~stefan.goetz-deactivatedaccount/hipl/commitguard into lp:hipl

[Stefan Götz]

Adds a bazaar pre-commit hook for checking file contents before a commit and to aborting the commit if necessary. Also includes a few guards that enforce some aspects of HIPL code style policy.

[Diego Biurrun (diego-biurrun)]

 review needs-fixing

On Thu, Jan 19, 2012 at 07:36:24AM +0000, Stefan Götz wrote:
> Stefan Götz has proposed merging lp:~stefan.goetz/hipl/commitguard into lp:hipl.

Looks mostly good to me. I don't speak Python, so this review is
necessarily superficial.

> --- .commitguards/author-attribution.guard 1970-01-01 00:00:00 +0000
> +++ .commitguards/author-attribution.guard 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,4 @@
> +[guard]
> +files=\.(c|h|am|ac|in|py|sh)$
> +actions=amr
> +command=if egrep -q "\*.*@author\b" "{}"; then echo "{}: files should not contain author attributions"; false; fi

Do we really need to restrict this to certain file types?

> --- .commitguards/copyright-guard-space.py 1970-01-01 00:00:00 +0000
> +++ .commitguards/copyright-guard-space.py 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,68 @@
> +#!/usr/bin/env python
> +# -*- coding: latin-1 -*-

Please use UTF-8, this is the 21st century (for some time already).
I'm also not sure if you need to tell emacs about this, the file
should be ASCII anyway.

> --- .commitguards/copyright-year.guard 1970-01-01 00:00:00 +0000
> +++ .commitguards/copyright-year.guard 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,4 @@
> +[guard]
> +files=\.(c|h|am|ac|in|py|sh)$
> +actions=amr
> +command=CR_TOKEN=" Copyright \(c\) "; CR_HOLDER=" Aalto "; Y=$(date +%Y); if ! egrep "$CR_TOKEN.*$CR_HOLDER" "{}" | egrep -q "\b$Y\b"; then echo "{}: The copyright statement is not up-to-date"; sed -r "/ Copyright \(c\) / { /\b$Y\b/ ! s/ ([0-9, -]+) ([a-zA-Z]+)/ $Y \2/ }" "{}" | diff -u "{}" - >&2; false; fi
>
> === added file 'tools/bazaar/plugins/commitguard.py'
> --- tools/bazaar/plugins/commitguard.py 1970-01-01 00:00:00 +0000
> +++ tools/bazaar/plugins/commitguard.py 2012-01-19 07:35:28 +0000
> @@ -0,0 +1,353 @@
> +#!/usr/bin/env python
> +# -*- coding: latin-1 -*-

ditto

> +_commitguard_tutorial = """
> +Bazaar plugin for checking code modifications at pre-commit time.
> +
> +About
> +-----
> +
> +Installation
> +------------
> +Copy this file into the bazaar plugin directory (usually .bazaar/plugins/ in

Bazaar

> + The commitguard plugin offers the user to apply such a patch automatically
> + for convenience.

For added convenience the commitguard plugin offers the user the option
of applying such a patch automatically.

Diego

review: Needs Fixing

[Stefan Götz (stefan.goetz-deactivatedaccount)]

> Looks mostly good to me. I don't speak Python, so this review is
> necessarily superficial.

Thanks for the review(s)!

>> --- .commitguards/author-attribution.guard 1970-01-01 00:00:00 +0000
>> +++ .commitguards/author-attribution.guard 2012-01-19 07:35:28 +0000
>> @@ -0,0 +1,4 @@
>> +[guard]
>> +files=\.(c|h|am|ac|in|py|sh)$
>> +actions=amr
>> +command=if egrep -q "\*.*@author\b" "{}"; then echo "{}: files should not contain author attributions"; false; fi
>
> Do we really need to restrict this to certain file types?

Not really, no. I changed it so it applies to all files.

>> --- .commitguards/copyright-guard-space.py 1970-01-01 00:00:00 +0000
>> +++ .commitguards/copyright-guard-space.py 2012-01-19 07:35:28 +0000
>> @@ -0,0 +1,68 @@
>> +#!/usr/bin/env python
>> +# -*- coding: latin-1 -*-
>
> Please use UTF-8, this is the 21st century (for some time already).

You're right. Done (removed).

> I'm also not sure if you need to tell emacs about this, the file
> should be ASCII anyway.

This comment is also obeyed by the python interpreter. In fact, it is
necessary as soon as non-plain-ASCII characters are in a python script.

>> bazaar
> Bazaar
>
>> + The commitguard plugin offers the user to apply such a patch automatically
>> + for convenience.
>
> For added convenience the commitguard plugin offers the user the option
> of applying such a patch automatically.

Done.

Cheers,
 Stefan

[Miika Komu (miika-iki)]

How does the differ from tools/bazaar/plugins/stylecheck.py or what is the relationship with it? And why do we need now two different hooks?

[Stefan Götz (stefan.goetz-deactivatedaccount)]

> How does the differ from tools/bazaar/plugins/stylecheck.py or what is the relationship with it?

stylecheck.py is specifically tailored towards running a code
beautifier, uncrustify in our case.

commitguard.py makes it much simpler to run arbitrary commands at
pre-commit time and adds a little bit of user convenience. This is
useful for checks that uncrustify cannot perform and is very easy to extend.

> And why do we need now two different hooks?

It would be possible to run uncrustify via the commitguard plugin
instead of the stylecheck plugin. However, some stylecheck features make
it much more usable with uncrustify than commitguard. In particular,
uncrustify can operate on multiple files with stylecheck while
commitguard aborts the commit after the first inconsistent file found,
which can be annoying if there several files need uncrustification.

I'd say that both plugins have their use.

Cheers,
 Stefan

[Miika Komu (miika-iki)]

Hi,

On 01/19/2012 06:16 PM, Stefan Götz wrote:
>> How does the differ from tools/bazaar/plugins/stylecheck.py or what is the relationship with it?
>
> stylecheck.py is specifically tailored towards running a code
> beautifier, uncrustify in our case.
>
> commitguard.py makes it much simpler to run arbitrary commands at
> pre-commit time and adds a little bit of user convenience. This is
> useful for checks that uncrustify cannot perform and is very easy to extend.
>
>> And why do we need now two different hooks?
>
> It would be possible to run uncrustify via the commitguard plugin
> instead of the stylecheck plugin. However, some stylecheck features make
> it much more usable with uncrustify than commitguard. In particular,
> uncrustify can operate on multiple files with stylecheck while
> commitguard aborts the commit after the first inconsistent file found,
> which can be annoying if there several files need uncrustification.
>
> I'd say that both plugins have their use.

sorry if I forgot some earlier discussions on the topic but until I see
a clear and concrete plan yet to replace stylecheck with commitguard, I
would suggest to sticking to a single tool (stylecheck.py) instead of
introducing another alternative as an extra maintenance burden.

[Stefan Götz (stefan.goetz-deactivatedaccount)]

> sorry if I forgot some earlier discussions on the topic but until I see
> a clear and concrete plan yet to replace stylecheck with commitguard, I
> would suggest to sticking to a single tool (stylecheck.py) instead of
> introducing another alternative as an extra maintenance burden.

Running uncrustify via commitguard results in a definite and annoying
loss of usability. Thus I strongly recommend keeping stylecheck around.
Otherwise, I'd be worried that users abandon the use of pre-commit hooks
altogether because using uncrustify became more cumbersome.

Note that the commitguard plugin, as is, runs checks that are orthogonal
to what uncrustify does. So this plugin is useful to HIPL developers in
itself. I'm not sure whether this became sufficiently clear.

How about this: I'll create a commit guard that runs uncrustify but
leave it disabled. Should the maintenance burden of having both
commitguard and stylecheck become a real issue, we could always drop
stylecheck at that point. Ok?

Stefan

[Miika Komu (miika-iki)]

Hi,

On 20/01/12 16:52, Stefan Götz wrote:
>> sorry if I forgot some earlier discussions on the topic but until I see
>> a clear and concrete plan yet to replace stylecheck with commitguard, I
>> would suggest to sticking to a single tool (stylecheck.py) instead of
>> introducing another alternative as an extra maintenance burden.
>
> Running uncrustify via commitguard results in a definite and annoying
> loss of usability. Thus I strongly recommend keeping stylecheck around.
> Otherwise, I'd be worried that users abandon the use of pre-commit hooks
> altogether because using uncrustify became more cumbersome.
>
> Note that the commitguard plugin, as is, runs checks that are orthogonal
> to what uncrustify does. So this plugin is useful to HIPL developers in
> itself. I'm not sure whether this became sufficiently clear.

thanks for making this explicit, I understood this now better. I guess
there's more policies coming to the commitguard plugin?

> How about this: I'll create a commit guard that runs uncrustify but
> leave it disabled.

If it's orthogonal instead of redundant, then no need to.

> Should the maintenance burden of having both
> commitguard and stylecheck become a real issue, we could always drop
> stylecheck at that point. Ok?

Ok.

[Miika Komu (miika-iki)]

So go ahead with this one.

review: Approve

[René Hummen (rene-hummen)]

Thanks a lot for the automated commit guards. Very good documentation and extensibility!

I just got a suggested workflow that would allow merging of the stylechecker with this script:
1) Make a local copy of the files touched in the current commit.
2) Apply the commit guards (including uncrustify) one after the other to each of the local copies.
2.1) Tell the user which guard failed on which file.
2.2) Apply an eventual change of a commit guard to the local copy.
3) After all commit guards have been executed (and the corresponding changes have been applied to the local copies incrementally), show the user the diff between the local files and the files in the commit.
3.1) Apply diff, if accepted by the user.

I know that this workflow would require some restructuring of the code. Hence, I don't mind having two scripts for now. However, the benefit would be a single pre-commit hook and the check of all committed files with _all_ hooks in one go. Right now, we need to commit as often as the commit contains "errors" until finally the commit is pushed to the repository.

Additionally, it might be worth having an install_bzr_hooks.sh script that copies the scripts to one of the two valid bazaar plugin locations in order to increase usage and acceptance.

[Diego Biurrun (diego-biurrun)]

On Thu, Jan 19, 2012 at 04:16:56PM +0000, Stefan Götz wrote:
> > And why do we need now two different hooks?
>
> It would be possible to run uncrustify via the commitguard plugin
> instead of the stylecheck plugin. However, some stylecheck features make
> it much more usable with uncrustify than commitguard. In particular,
> uncrustify can operate on multiple files with stylecheck while
> commitguard aborts the commit after the first inconsistent file found,
> which can be annoying if there several files need uncrustification.

This is a considerable shortcoming IMO. Ideally all issues in all
files should be detected and be complained about. Otherwise you have
to run through multiple iterations of commit guard as you fix issue
after issue...

Diego

[Stefan Götz (stefan.goetz-deactivatedaccount)]

Hi Rene!

> I just got a suggested workflow that would allow merging of the stylechecker with this script:
> 1) Make a local copy of the files touched in the current commit.
> 2) Apply the commit guards (including uncrustify) one after the other to each of the local copies.
> 2.1) Tell the user which guard failed on which file.
> 2.2) Apply an eventual change of a commit guard to the local copy.
> 3) After all commit guards have been executed (and the corresponding changes have been applied to the local copies incrementally), show the user the diff between the local files and the files in the commit.
> 3.1) Apply diff, if accepted by the user.

That makes a lot of sense and is certainly more usable than the current
approach. I'll work on it on the usability branch.

> Additionally, it might be worth having an install_bzr_hooks.sh script that copies the scripts to one of the two valid bazaar plugin locations in order to increase usage and acceptance.

Yeah, well, I don't think the difference between a single cp command and a
script will change something about usage and acceptance - so I'll let someone
else write that script :)

Stefan

[Miika Komu (miika-iki)]

I just realized that the only the documentation is in the script itself and there's nothing in the HACKING - is this intentional?

[lp:~stefan.goetz-deactivatedaccount/hipl/commitguard]

6250. By Diego Biurrun on 2012-01-24

Ignore test/check_hipd unit test executable.

6251. By Diego Biurrun on 2012-01-24

hipd: Fix some typos that the DEB package lintian program complained about.

6252. By Diego Biurrun on 2012-01-24

debian: Abort on errors during dnsproxy postinst script.

6253. By Diego Biurrun on 2012-01-24

debian: Register HIPL documentation with doc-base.

6254. By Diego Biurrun on 2012-01-24

debian: Extend package descriptions.

6255. By Diego Biurrun on 2012-01-24

packaging: Add dependency of hipdnsproxy on hipd.

hipdnsproxy reads etc/hip/hosts, which only exists if hipd is installed.

6256. By Diego Biurrun on 2012-01-24

hadb: Vastly simplify match_peer_addr().

6257. By Diego Biurrun on 2012-01-25

Rename nsupdate.pl ---> nsupdate.

It's more sensible to leave out the unnecessary language extension from the
program name. It does not really add information and would have to be
changed in the case the program was rewritten some day.

6258. By Miika Komu on 2012-01-25

A bug fix to hipfw unit tests on 32-bit machines

Fixed bug id #918374 in launchpad. Now two failing hipfw tests are
disabled on 32-bit machines. Tested on both 32-bit and 64-bit machines.

6259. By Stefan Götz on 2012-01-25

Compare host IDs across all address bytes instead of comparing only the first few bytes equal to the width of a pointer.

6260. By Stefan Götz on 2012-01-25

Merge lp:~stefan.goetz/hipl/hidb-db, approved by Rene and Diego

6281. By Stefan Götz on 2012-01-25

Briefly mention the commitguard (and the stylecheck) plugin in doc/HACKING

6282. By Stefan Götz on 2012-01-25

Remove empty leading and trailing comment lines in copyright blocks.

6283. By Stefan Götz on 2012-01-25

Remove GNU-isms from scripts

6284. By Stefan Götz on 2012-01-25

Fix grammar in documentation

6285. By Stefan Götz on 2012-01-25

Place references to shell variables into curly braces to avoid expansion problems

6286. By Stefan Götz on 2012-01-25

Remove '\b' from regular expression for portability

Unmerged revisions

=== added directory '.commitguards'
=== added file '.commitguards/author-attribution.guard'
--- .commitguards/author-attribution.guard	1970-01-01 00:00:00 +0000
+++ .commitguards/author-attribution.guard	2012-01-22 08:57:26 +0000
@@ -0,0 +1,31 @@
+# Copyright (c) 2012 Aalto University and RWTH Aachen University.
+#
+# Permission is hereby granted, free of charge, to any person
+# obtaining a copy of this software and associated documentation
+# files (the "Software"), to deal in the Software without
+# restriction, including without limitation the rights to use,
+# copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following
+# conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+
+[guard]
+files=.*
+actions=amr
+command=
+    if AUTHORS="$(egrep "@author\b" "{}")"; then
+        echo "{}: files should not contain author attributions such as '$(echo "$AUTHORS" | head -n 1)'"
+        false
+    fi

=== added file '.commitguards/copyright-guard-space.guard'
--- .commitguards/copyright-guard-space.guard	1970-01-01 00:00:00 +0000
+++ .commitguards/copyright-guard-space.guard	2012-01-22 08:57:26 +0000
@@ -0,0 +1,27 @@
+# Copyright (c) 2012 Aalto University and RWTH Aachen University.
+#
+# Permission is hereby granted, free of charge, to any person
+# obtaining a copy of this software and associated documentation
+# files (the "Software"), to deal in the Software without
+# restriction, including without limitation the rights to use,
+# copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following
+# conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+
+[guard]
+files=\.(c|h|am|ac|in|py|sh|guard)$
+actions=amr
+command=copyright-guard-space.py "{}"

=== added file '.commitguards/copyright-guard-space.py'
--- .commitguards/copyright-guard-space.py	1970-01-01 00:00:00 +0000
+++ .commitguards/copyright-guard-space.py	2012-01-22 08:57:26 +0000
@@ -0,0 +1,65 @@
+#!/usr/bin/env python
+
+#
+# Copyright (c) 2012 Aalto University and RWTH Aachen University.
+#
+# Permission is hereby granted, free of charge, to any person
+# obtaining a copy of this software and associated documentation
+# files (the "Software"), to deal in the Software without
+# restriction, including without limitation the rights to use,
+# copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following
+# conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+#
+
+#
+# Test whether a copyright comment block is followed by a blank line
+#
+
+import os, re, sys
+
+class CommentStyle:
+    def __init__(self, header, body, footer):
+        self.header = header
+        self.body = body
+        self.footer = footer
+
+
+c_style = CommentStyle(' \* Copyright ', ' \*', ' \*/')
+hash_style = CommentStyle('# Copyright ', '#', '#.*')
+
+styles = {
+    '.c': c_style,
+    '.h': c_style,
+    '.am': hash_style,
+    '.ac': hash_style,
+    '.in': hash_style,
+    '.py': hash_style,
+    '.sh': hash_style,
+}
+
+extension = os.path.splitext(sys.argv[1])[1]
+if extension in styles:
+    style = styles[extension]
+    pattern = re.compile('^' + style.header + '[^\n]*\n(^' + style.body + '[^\n]*\n)+^' + style.footer + '\n\n', re.MULTILINE)
+
+    fh = open(sys.argv[1], 'r')
+    match = pattern.search(fh.read())
+    fh.close()
+
+    if not match:
+        print 'The copyright header in file %s should be followed by an empty line' % (sys.argv[1])
+        sys.exit(1)

=== added file '.commitguards/copyright-year.guard'
--- .commitguards/copyright-year.guard	1970-01-01 00:00:00 +0000
+++ .commitguards/copyright-year.guard	2012-01-22 08:57:26 +0000
@@ -0,0 +1,36 @@
+# Copyright (c) 2012 Aalto University and RWTH Aachen University.
+#
+# Permission is hereby granted, free of charge, to any person
+# obtaining a copy of this software and associated documentation
+# files (the "Software"), to deal in the Software without
+# restriction, including without limitation the rights to use,
+# copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following
+# conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+
+[guard]
+files=\.(c|h|am|ac|in|py|sh|guard)$
+actions=amr
+command=
+    CR_TOKEN=" Copyright \(c\) "
+    CR_HOLDER=" Aalto "
+    Y=$(date +%Y)
+    if ! egrep "$CR_TOKEN.*$CR_HOLDER" "{}" | egrep -q "\b$Y\b"; then
+        echo "{}: The copyright statement is not up-to-date"
+        L=$(( $Y - 1 ))
+        sed -r "/ Copyright \(c\) / { /\b$Y\b/ ! { /-$L / s/-$L /-$Y /; s/ $L / $L-$Y /; /\b$Y\b/ ! s/ ([0-9, -]+) ([a-zA-Z]+)/ \1, $Y \2/ } }" "{}" | diff -u "{}" - >&2
+        false
+    fi

=== added file 'tools/bazaar/plugins/commitguard.py'
--- tools/bazaar/plugins/commitguard.py	1970-01-01 00:00:00 +0000
+++ tools/bazaar/plugins/commitguard.py	2012-01-22 08:57:26 +0000
@@ -0,0 +1,351 @@
+#!/usr/bin/env python
+
+#
+# Copyright (c) 2012 Aalto University and RWTH Aachen University.
+#
+# Permission is hereby granted, free of charge, to any person
+# obtaining a copy of this software and associated documentation
+# files (the "Software"), to deal in the Software without
+# restriction, including without limitation the rights to use,
+# copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following
+# conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+# OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+# HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+# WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+# OTHER DEALINGS IN THE SOFTWARE.
+#
+
+_commitguard_tutorial = """
+Bazaar plugin for checking code modifications at pre-commit time.
+
+About
+-----
+
+Installation
+------------
+Copy this file into the Bazaar plugin directory (usually .bazaar/plugins/ in
+your home directory) or add the directory this file resides in to the
+environment variable BZR_PLUGIN_PATH (export BZR_PLUGIN_PATH="<dir>") before
+running 'bzr commit'.
+
+The pre-commit checks (or 'guards') are described in configuration files with
+the extension .guard. This script looks for such files in two directories:
+  1) $HOME/.bazaar/plugins/commitguards/
+  2) <repository>/.commitguards/
+This allows to set up commit guards both per user and per repository.
+
+Usage
+-----
+This hook is called automatically when running the command 'bzr commit' after
+the commit message has been provided. All available commit guards are then
+applied to the files in the commit. If a file fails a commit guard, the
+commit is aborted with an error message detailing the failure. The commit
+message is printed again for convenience. A commit guard can optionally
+suggest a patch to the file so that it meets the guard's requirements.
+
+At the moment, a commit is aborted on the first failing commit guard.
+
+Commit Guards
+-------------
+Commit guards are described in plain-text configuration files in .ini
+style with the extension .guard. The basic format of these files is
+
+[guard]
+files=<value>
+actions=<value>
+command=<value>
+
+Each guard configuration file provides three pieces of information about a
+commit guard:
+
+1) A set of file-name patterns the guard is to be applied to.
+   The configuration key is 'files'.
+   The configuration value is a regular expression.
+   For example, the entry 'files=\.(c|C)$' makes sure that the guard is applied
+   to all files with the extension .c or .C
+
+2) A set of commit actions specifying if the guard is to be applied to
+   added, modified, and/or renamed files.
+   The configuration key is 'actions'.
+   The configuration value is a string consisting of one or more of the
+   characters a, m, and r, representing added, modified, and renamed files
+   respectively.
+   For example, the entry 'actions=am' makes sure that the guard is applied to
+   new and modified files in a commit but not to renamed files.
+
+3) A command that is executed and which implements the actual guard action.
+   The configuration key is 'command'.
+   The configuration value is a string containing a shell command.
+   Before executing this command, all occurrences of the sub-string {} are
+   replaced with the name of the (added, modified, or renamed) file in the
+   repository and commit to apply the guard to.
+   If the guard considers the given file valid as is, the command should exit
+   with an exit code of 0.
+   In this case, the remaining guards are applied to the remaining files in the
+   commit.
+   If the guard considers a given file as invalid, it should exit with a
+   non-zero exit code.
+   In this case, it may print a message to stdout to inform the user why the
+   commit was aborted.
+   Optionally, the command may print a patch in unified diff format on stderr
+   that, if applied, would bring the file into compliance with the guard's
+   requirements.
+   For added convenience, the commitguard plugin offers the user to apply such
+   a patch straight away.
+   Helper executables can be placed into the same directory as a .guard file.
+   The guard command can then invoke these executables as if they were system
+   commands.
+
+
+
+"""
+
+version_info = (0, 1, 0, 'dev', 0)
+plugin_name = 'commitguard'
+
+from bzrlib import branch, help_topics
+import bzrlib
+import subprocess
+import os
+import os.path
+import sys
+import tempfile
+import re
+import ConfigParser
+
+
+def get_local_file_name(branch, file_name):
+    """Determine the absolute path of the file in a local Bazaar working branch.
+
+    Arguments:
+    branch -- an instance of bzrlib's Branch representing a local working
+    branch. file_name is resolved relative to the base location of branch.
+    file_name -- a string containing the relative path of a file in the file
+    tree of branch.
+
+    This function returns a string that contains the absolute path of the
+    specified file. This file is the actual on-disk working copy and can be
+    accessed and modified. If file_name does not exist in branch or the
+    corresponding local file cannot be found, a RuntimeError is raised.
+
+    """
+    base = branch.base
+    if base.startswith('file://'):
+        base = base[7:]
+    local_file_name = os.path.join(base, file_name)
+    if os.path.exists(local_file_name):
+        return local_file_name
+    else:
+        raise RuntimeError("The file '%s' in branch '%s' could not be found at \
+'%s' as expected!" % (file_name, branch, local_file_name))
+
+
+
+class Guard(object):
+    """A tool that inspects the new state a commit introduces and allows or
+    aborts the commit.
+
+    Each instance of this class wraps a 'guard' tool which may analyse and check
+    the set of modifications of a commit. It may then let the commit pass or
+    prevent it.
+
+    """
+    def __init__(self, cfg_file_name):
+        """Creates and initializes a Guard object and its attributes.
+
+        If the initialization fails, e.g., because a tool is not available or
+        the configuration is invalid, an Exception is raised.
+
+        cfg_file_name -- The path of a guard configuration file from which to
+        initialize this object.
+
+        """
+        self.configure(cfg_file_name)
+
+    def configure(self, cfg_file_name):
+        """Initialize a Guard object from a configuration file.
+
+        cfg_file_name -- The path of a guard configuration file from which to
+        initialize this object.
+
+        """
+        config = ConfigParser.RawConfigParser()
+        config.read(cfg_file_name)
+        self.files = re.compile(config.get('guard', 'files'), re.IGNORECASE)
+        self.actions = config.get('guard', 'actions')
+        self.command = unicode(config.get('guard', 'command'), encoding='utf-8')
+        self.filename = cfg_file_name
+        self.dirname = os.path.dirname(cfg_file_name)
+
+    def applies_to(self, file_name, file_action):
+        """Checks whether this guard applies to a given file or not.
+
+        file_name -- the name of the file in the working repository which is
+        compared against this guard's file name pattern (not case sensitive).
+        file_action -- one of the characters 'a', 'm', or 'r' to indicate
+        whether a commit adds, modifies, or renames the given file. In the case
+        of a renamed file, the file_name parameter is expected to be the new
+        name of the file.
+
+        This function returns True if the given file name and action match this
+        guard and False otherwise.
+
+        """
+        return file_action in self.actions and self.files.search(file_name) is not None
+
+    def run(self, local_file_name, action, msg, diff):
+        """Execute a guard to determine whether a commit may proceed or not.
+
+        Arguments:
+        local_file_name -- the path of a local file to run the guard on. The
+        guard may not modify this file.
+        file_action -- one of the characters 'a', 'm', or 'r' to indicate
+        whether a commit adds, modifies, or renames the given file. In the case
+        of a renamed file, the local_file_name parameter is expected to be the
+        new name of the file.
+        msg -- a file-like object to which the guard may write a message for the
+        user.
+        diff -- a file-like object to which the guard may write a patch in
+        unified diff format which shows how to modify the file contents in order
+        to comply with this guard and let the commit proceed.
+
+        If this function returns true, the commit may proceed. If it returns
+        false, the commit is aborted.
+
+        """
+        if self.applies_to(local_file_name, action):
+            cmd = self.command.replace('{}', local_file_name)
+            env = os.environ
+            if not self.dirname in env['PATH']:
+               env['PATH'] += (':' + self.dirname)
+            p = subprocess.Popen(cmd,
+                                 shell = True,
+                                 stdout = msg,
+                                 stderr = diff,
+                                 env = env)
+            return p.wait() == 0
+        else:
+            return True
+
+
+def create_guards_from_dir(cfgdir, guards):
+    for wroot, wdirs, wfiles in os.walk(cfgdir):
+        for f in wfiles:
+            if f.endswith('.guard'):
+                guards.append(Guard(os.path.join(wroot, f)))
+    return guards
+
+
+def create_guards(branch):
+    guards = []
+    cfgdirs = [os.path.join(os.path.expanduser('~'), '.bazaar/plugins/commitguards')]
+    try:
+        cfgdirs.append(get_local_file_name(branch, '.commitguards'))
+    except:
+        pass
+    for cfgdir in cfgdirs:
+        if os.path.exists(cfgdir):
+            create_guards_from_dir(cfgdir, guards)
+    return guards
+
+
+def get_commit_files(tree_delta):
+    """From all modifications in a commit, retrieve those files which should be
+    run through the guards.
+
+    That is: added, modified, and renamed files, ignoring meta-data changes.
+
+    The return value is a list of tuples. Each tuple consists of a the path name
+    of a file in the Bazaar repository tree as the first element and the
+    character 'a', 'm', or 'r' indicating the action this commit performs on the
+    file.
+
+    """
+    # include added, modified, and renamed, skip removed and changed
+    files = [(path, 'a') for path, file_id, kind in tree_delta.added if kind == 'file']
+    files.extend([(path, 'm') for (path, file_id, kind, text_modified, _) in
+        tree_delta.modified if kind == 'file'])
+    files.extend([(newpath, 'r') for (oldpath, newpath, file_id, kind, text_modified, _) in
+        tree_delta.renamed if kind == 'file'])
+    return files
+
+
+def get_commit_message(local, master, revid):
+    """Returns the commit message of a branch revision."""
+    branch = local or master
+    revision = branch.repository.get_revision(revid)
+    return revision.message
+
+
+def apply_patch(diff_file):
+    """Apply a patch to a local Bazaar branch.
+
+    diff_file -- a file-like object from which the patch can be read. The patch
+    is expected to be in unified diff format.
+
+    If applying the beautification fails, a CalledProcessError is raised.
+
+    """
+    subprocess.check_call(['patch', '-p0', '-i', diff_file.name])
+
+
+def pre_commit_hook(local, master, old_revno, old_revid, future_revno,
+                    future_revid, tree_delta, future_tree):
+    """Run the files modified by this commit through the configured guards and
+    abort the commit if the guards veto it.
+
+    This is the pre-commit hook interface of bzrlib.
+
+    """
+    guards = create_guards(local or master)
+
+    veto = False
+    diff_file = tempfile.NamedTemporaryFile(prefix = plugin_name + "-diff-")
+
+    for tree_file_name, action in get_commit_files(tree_delta):
+        local_file_name = get_local_file_name(local or master, tree_file_name)
+        for guard in guards:
+            if not guard.run(local_file_name, action, sys.stdout, diff_file):
+                veto = True
+                break
+
+        if veto:
+            break
+
+    if veto:
+        diff_file.flush()
+        if diff_file.tell():
+            print "\nThe following patch is suggested so as to comply with the commit guard requirements:\n"
+            diff_file.seek(0)
+            print diff_file.read()
+            print "\nWould you like to apply these changes to your local branch now? [y/N] ",
+            reply = sys.stdin.readline()
+            if reply.strip() == 'y':
+                apply_patch(diff_file)
+                print "Changes successfully applied.\n"
+        diff_file.close()
+
+        msg_file = tempfile.NamedTemporaryFile(prefix = 'commit-msg-%d-' % future_revno, delete = False)
+        msg_file.write(get_commit_message(local, master, future_revid))
+        msg_file.close()
+
+        raise bzrlib.errors.BzrError("This commit has been aborted. The original commit message, stored in %s, was:\n--\n%s\n--" % (msg_file.name, get_commit_message(local, master, future_revid)))
+    diff_file.close()
+
+
+help_topics.topic_registry.register(plugin_name + '-tutorial',
+                                    _commitguard_tutorial,
+                                    'How to use the plugin ' + plugin_name)
+
+branch.Branch.hooks.install_named_hook('pre_commit', pre_commit_hook,
+                                       plugin_name)