[Insight-developers] Learning from my mistakes (an adventure in git)

[Brad King]

On 08/02/2010 10:51 AM, Gaëtan Lehmann wrote:
> Really useful for sure!

Thanks.  We went through several iterations of local hook installation
enforcement in VTK before arriving at this solution.  The earlier
attempts generated a CMake-time error so even non-developers could
not build without installing the hooks.  This is much simpler.

> Would it be possible to fetch the hooks automatically (at least try  
> to, if the network in not available at this time)?

Perhaps, but IMO it is more trouble than it's worth.  The goal is to
make it hard to commit accidentally without the local hooks installed.
Any automatic steps to enforce this should have as few failure points
as possible.

Git intentionally does not allow clones to set up hooks automatically,
citing security reasons.  Therefore the earliest step we can control
is when CMake runs.

The "hooks" branch is shared by many Kitware-hosted projects.  To
avoid duplication we do not want to copy hooks into every project's
source tree.  Therefore we cannot install all the hooks by copying
them at CMake configuration time.  We also want to avoid blowing
away any local modifications the user has made to the local hooks
so we cannot just overwrite them with new ones every time CMake runs.

We *cannot* run Git during CMake configuration because we might not
know where to find the Git executable or which one was used if multiple
are installed (MSysGit and Cygwin git do not interact very well in a
single work tree).  Instead we write a small .git/hooks/pre-commit
file as a bootstrapping step.  If the user never does development then
it has no effect, but when the user tries to commit then it blocks
the commit and prints the instructions.

The only place we could try to run git to automatically install the
hooks is in the bootstrapping pre-commit hook.  Inside this script we
can just run "git" and do not need to find the executable.  However,
installing the real hooks involves replacing pre-commit while it is
open (running), which may not work on Windows.  If it fails the hooks
directory may be left in a confusing state.  It is much simpler to
give the user one-time cut-and-paste instructions as we do now, and
it only happens on the first *commit* anyway.

> I don't see any style check for now. Is it the expected behavior?

Yes.  We cannot run style checks on the server anymore, at least not
without adding quite a bit of customized infrastructure code.  We
cannot enforce style in the local hooks unless we know where to find
a local installation of KWStyle.  These hooks should be kept as
simple as possible to ensure they run properly everywhere.  Also,
I frequently commit work-in-progress changes locally as a backup
of my work tree.  I would not want this blocked by style checks.

IMO the style checks should be *tests* just like any other.  If the
style is bad the test fails.  If you push a change that introduces
a test failure, then shame on you ;)

-Brad