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

[Bill Lorensen]

I hope we can figure out a way to do style checking as a commit hook.
We have spent many many hours getting the current itk to pass the
style enforced by kwstyle.

A test is not sufficient enough. We have learned that in the past.

This seems like a big step backwards...

Bill

On Mon, Aug 2, 2010 at 11:19 AM, Brad King <brad.king at kitware.com> wrote:
> 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
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>