[vtk-developers] Auto install git hooks

Mon Jul 5 10:48:25 EDT 2010

I pressed the button, and pushed this change into VTK master. I was going to
check the dashboards and see what happens. I can push a very similar change
into ParaView, but it just struck me that this has only been raised on the
VTK developer list.

I tried to make the instructions as explicit as possible for users and
developers. Please let me know if you see any problems.

Marcus
--
Marcus D. Hanwell, Ph.D.
R&D Engineer, Kitware Inc.
(518) 881-4937

On Thu, Jul 1, 2010 at 10:44 AM, Moreland, Kenneth <kmorel at sandia.gov>wrote:

>  To me, the submodule update in a safe area is completely independent of
> installing hooks.  Regardless of whether or not you enable hooks, you have
> to do something to get the internal submodule clones.  Once you’ve done that
> something, the hooks will be available in the VTK submodule.
>
> So the hooks should be locally available for both the ParaView and VTK
> clones even in a safe area.  Why do we need special hook-install handling
> then?
>
> -Ken
>
>
>
> On 7/1/10 8:10 AM, "Marcus D. Hanwell" <marcus.hanwell at kitware.com> wrote:
>
> You are both right, the hooks are already in any clone, and I would use
> origin. If you leave the VTK submodule pointing at our servers, then how do
> you handle git submodule update for ParaView? If you are not handling that,
> then I would assume VTK is on a (no branch) and not pulling master, nor
> being actively developed?
>
> VTK is aware of its inclusion in a ParaView build, and so some special
> handling could be put in place. I think we could actually instruct it to use
> the ParaView hooks by going up another directory. Alternatively you could
> set the flag to disable hooks checks in VTK, but not ParaView. To
> permanently disable the hooks check in a clone you could also touch
> .git/hooks/.git/config, this would make the test pass in that particular
> clone.
>
> Marcus
>
> On Thu, Jul 1, 2010 at 9:46 AM, Stephane PLOIX <stephane.ploix at edf.fr>
> wrote:
>
>
> True if the solution proposed by Marcus is to use the "origin" from git to
> get the hooks. This is not the case for the submodules for instance : by
> default, they point to kitware's native repos, and have to be reconfigured
> to point to the internals clones with commands like
> >>>git config submodule.VTK.url git://my-internal-VTK-clone
> It would be nice if we could simply clone the main paraview repo and get a
> fully functionnal mirror with everything self-contained : submodules, hooks,
> etc... instead of having to have special commands for each and every new git
> functionnality.
>
>
>
> This whole “safe” area business should be a nonissue.  If you have cloned
> the main repository from Kitware, you get a copy of the hooks branch in your
> local repository along with the other branches.  Since the data is taken
> from your local repository, no network access should be necessary and the
> instructions should not change for a safe area, right?
>
> -Ken
>
>
> On 7/1/10 1:46 AM, "Stephane PLOIX" <*stephane.ploix at edf.fr* <
> http://stephane.ploix@edf.fr> > wrote:
>
>
> I am exactly in the case described by Berk, but I would like to push the
> point further. Inside the "safe area", we have replicated all the
> testing/dashboard mechanism, and it would be nice if the proposed mechanism
> could be configured so that the instructions and redirection to download the
> hooks can be modified, instead of only having a solution to turn it off.
> This way, any developer inside the safe area will have to use the hooks as
> they should, without having to go out of the safe area.
>
> Stephane
>
>
>
> *Stephane PLOIX
> Pilote Opérationnel - Visualisation scientifique
> *
> EDF - R&D
> SINETICS
> 1, Av du Général de Gaulle
> 92140 Clamart
> *
> stephane.ploix at edf.fr* <http://stephane.ploix@edf.fr>
> Tél. : +33 (0) 1 47 65 51 10
>  Un geste simple pour l'environnement, n'imprimez ce message que si vous
> en avez l'utilité.
>
>
>
> *
> marcus.hanwell at kitware.com* <http://marcus.hanwell@kitware.com>
> Envoyé par : *vtk-developers-bounces at vtk.org* <
> http://vtk-developers-bounces@vtk.org> 30/06/2010 20:13
> A
>
> *berk.geveci at kitware.com* <http://berk.geveci@kitware.com>
> cc
>
> *vtk-developers at vtk.org* <http://vtk-developers@vtk.org> , *
> kmorel at sandia.gov* <http://kmorel@sandia.gov>
>  Objet
>
> Re: [vtk-developers] Auto install git hooks
>
>
>
>
> On Wed, Jun 30, 2010 at 2:06 PM, Berk Geveci <*berk.geveci at kitware.com* <
> http://berk.geveci@kitware.com> <*mailto:berk.geveci at kitware.com<berk.geveci at kitware.com>
> * <mailto:berk.geveci at kitware.com <berk.geveci at kitware.com>> > > wrote:
> I am joining this argument a bit late so I apologize if I misunderstand.
>
> The solution that Marcus et al. are proposing will have no effect if
> someone is building from a source that does not contain a .git directory,
> correct? If not, it should be. Those have no interest in developing
> VTK/ParaView/ITK/CMake whatever should not clone the git repository.
> Instead, they should download a tarball/zip. Keep in mind that
> github/gitorious etc. all have the ability of generating an archive of the
> repository at any point in time. We should encourage users to use it rather
> than clone the git repo (which requires installing git and all). Similarly,
> if a developer is to send a user a snapshot, that developer should not
> include the .git directory in that snapshot.
>
> If it is not a clone, then no checks are performed. So if the user
> downloads a tarball (or otherwise removes the .git/config in the root source
> directory), then they will never see the error.
>
> My biggest concern is for developers that clone a repo and then take it to
> a safe area to build. At that point, they will not have access to network.
> So we should have a way to bypass anything that requires network access.
>
> There is an option, the can be set in the CMake cache to skip the check. In
> Titan it is called Titan_IGNORE_HOOKS, defaults to off but if set to on
> bypasses the hooks check.
>
> Marcus
>
> On Wed, Jun 30, 2010 at 12:40 PM, Wes Turner <*wes.turner at kitware.com* <
> http://wes.turner@kitware.com> <*mailto:wes.turner at kitware.com<wes.turner at kitware.com>
> * <mailto:wes.turner at kitware.com <wes.turner at kitware.com>> > > wrote:
> I will continue to respectfully disagree.  Not every user has the ability
> to be a developer.  Intentionally breaking the build process for exactly
> those people who will never make a checkin and who are most likely to drop
> use of the toolkit when they it gets too complex or they get annoyed seems
> like a bad idea.
>
> That said, I had my say and will follow the madding crowd. :-)
>
> - Wes
>
>
> On Wed, Jun 30, 2010 at 11:15 AM, Moreland, Kenneth <*kmorel at sandia.gov* <
> http://kmorel@sandia.gov> <*mailto:kmorel at sandia.gov <kmorel at sandia.gov>*<
> mailto:kmorel at sandia.gov <kmorel at sandia.gov>> > > wrote:
>
> If I may, I’ll repeat David’s sentiments a bit more gently.
>
> You would get around this problem if you sent to your customer an archive
> instead of a git url.  Instructing your customer (as you describe him/her)
> to fetch the latest snapshot from the archive is a bad idea all around.
>  First, it adds further complication to the build process (get git and learn
> it well enough to clone a repo), which you said yourself is already too
> onerous.  Second, it invites the inevitable problem that your customer will
> get a version of VTK that is different than yours and with something
> committed that breaks your software.
>
> Will this unnecessarily effect someone somewhere?  I will concede that it
> probably will, but it will be a small percentage of all VTK users and,
> amortized amongst us all, will save much more time than will be lost.
>
> -Ken
>
>
>
> On 6/30/10 8:49 AM, "David Cole" <*david.cole at kitware.com* <
> http://david.cole@kitware.com> <*http://david.cole@kitware.com/* <
> http://david.cole@kitware.com/> > > wrote:
>
> The point here is:
> - anybody using the VTK git repository should have the hooks installed
> locally, so that commits they make have a snowball's chance of being
> merge-able into the main repo
> - if they don't have the hooks installed, then when they run cmake on VTK's
> CMakeLists.txt file, it should:
>   -- automatically install the hooks for you if possible
>     or
>   -- be a fatal configure time error with instructions on how to install
> the hooks
>
> Period. If I were doing this, there wouldn't even be an option to turn it
> off. Marcus is being more than generous here even providing that level of
> control.
>
> If they can't handle that, they shouldn't be using VTK via git. They should
> stick to releases / tarballs that do not have a .git repo in the tree.
>
> There's no such thing as a "user" of a library. They're developers. They
> can handle installing hooks.
>
>
> Just my opinion,
> David C.
>
>
> On Wed, Jun 30, 2010 at 8:57 AM, Wes Turner <*wes.turner at kitware.com* <
> http://wes.turner@kitware.com> <*http://wes.turner@kitware.com/* <
> http://wes.turner@kitware.com/> > > wrote:
> How much more complicated does this make the initial build process for a
> naive user who checked out VTK and just wants to compile/run it?  Every time
> I send code  to a customer, I invariably end up on a t-con where I need to
> walk them through the process of building the toolkits.  This holds even for
> otherwise technologically savvy people.  The frustration level on their end
> can be quite high, even when we prime them for this possibility. Similarly,
> we got blasted on this in a paper submission where the reviewer just wasn't
> willing to try out the Lesion Sizing Toolkit because the install/build
> procedures for ITK/LSTK were too high.  If the intent here is to make anyone
> who downloads VTK via git jump through another set of hoops to get up and
> running, then I think it is a bad idea.  I am happy explaining away
> warnings, however, so making it a warning would be fine.  I would also be
> fine with adding something in for people who claim up front that they want
> to upload back to the repository.
>
> - Wes
>
> On Tue, Jun 29, 2010 at 6:19 PM, Moreland, Kenneth <*kmorel at sandia.gov* <
> http://kmorel@sandia.gov> <*http://kmorel@sandia.gov/* <
> http://kmorel@sandia.gov/> > > wrote:
> That sounds pretty good to me.
>
> -Ken
>
>
>
> On 6/29/10 2:27 PM, "Marcus D. Hanwell" <*marcus.hanwell at kitware.com* <
> http://marcus.hanwell@kitware.com> <*http://marcus.hanwell@kitware.com/* <
> http://marcus.hanwell@kitware.com/> > <*http://marcus.hanwell@kitware.com*<
> http://marcus.hanwell@kitware.com/> <*http://marcus.hanwell@kitware.com/*<
> http://marcus.hanwell@kitware.com/> > > > wrote:
>
> So, I was thinking about this over the weekend, and talked to Brad King.
> There is an environment variable that is set by CTest when it drives the
> build. I can check if that variable is defined, and know that CTest is
> driving the build.
>
> If I did that then when a user first configures VTK CMake will print an
> error with copy/paste instructions to check out the local hooks (along with
> a link to the wiki). They can set a CMake cache variable to ignore the hooks
> to get past it if they wish. If you are not in a git checkout the whole
> process is skipped.
>
> If CTest invoked the configure then the environment variable is set, and so
> the error is not raised - negating the need to set a cache variable on all
> of the dashboard machines. The only assumption is that CTest is driving the
> build, which I think is reasonable. I tested the logic out in Titan and it
> looks good.
>
> Does this cover all of your requirements reasonably? I will be checking
> this code into Titan as I think it is an improvement on what we had there
> too. I would be happy to check the relevant portion into VTK.
>
> Marcus
>
> On Thu, Jun 17, 2010 at 6:33 PM, Moreland, Kenneth <*kmorel at sandia.gov* <
> http://kmorel@sandia.gov> <*http://kmorel@sandia.gov/* <
> http://kmorel@sandia.gov/> > <*http://kmorel@sandia.gov* <
> http://kmorel@sandia.gov/> <*http://kmorel@sandia.gov/* <
> http://kmorel@sandia.gov/> > > > wrote:
>
> In short, a pushurl is not a good indication of whether commits will be
> made or whether those commits will eventually be pushed to the main
> repository.
>
> -Ken
>
>
>
> On 6/17/10 2:46 PM, "Marcus D. Hanwell" <*marcus.hanwell at kitware.com* <
> http://marcus.hanwell@kitware.com> <*http://marcus.hanwell@kitware.com/* <
> http://marcus.hanwell@kitware.com/> > <*http://marcus.hanwell@kitware.com*<
> http://marcus.hanwell@kitware.com/> <*http://marcus.hanwell@kitware.com/*<
> http://marcus.hanwell@kitware.com/> > >  <*
> http://marcus.hanwell@kitware.com* <http://marcus.hanwell@kitware.com/> <*
> http://marcus.hanwell@kitware.com/* <http://marcus.hanwell@kitware.com/> >
> > > wrote:
>
>
> On Thu, Jun 17, 2010 at 4:40 PM, Clinton Stimpson <*clinton at elemtech.com*<
> http://clinton@elemtech.com> <*http://clinton@elemtech.com/* <
> http://clinton@elemtech.com/> > <*http://clinton@elemtech.com* <
> http://clinton@elemtech.com/> <*http://clinton@elemtech.com/* <
> http://clinton@elemtech.com/> > >  <*http://clinton@elemtech.com* <
> http://clinton@elemtech.com/> <*http://clinton@elemtech.com/* <
> http://clinton@elemtech.com/> > > > wrote:
>
> Can you key off the existence of a pushurl?
> But I also wonder how this would keep the hooks updated?
>
> We could possibly be clever and do a little regex to check for the git at form of the url/pushurl. I hadn't considered being that sneaky, but it sound
> like a viable approach and would ease the dashboard pain.
>
> Marcus
>
> On Thursday, June 17, 2010 02:13:37 pm Marcus D. Hanwell wrote:
> > On Thu, Jun 17, 2010 at 4:06 PM, Moreland, Kenneth <*
> kmorel at sandia.gov>wrote*:
> > >  That’s a good point about CMake modifying the source tree, but I think
> > >
> > > this is one of those cases we should let the rule slide.  In this case
> we
> > > are installing what, IMHO, git should be pulling for us.  Although the
> > > Wiki says its optional, it really should be enforced for anyone who
> > > makes any commit to any repository.
> >
> > We came to a similar conclusion in Titan, but I am not sure about letting
> > the rule slide. This is new territory though, and it is just my take
> >
> > > I’m less thrilled about the “error if not installed” option because it
> > > still pushes the responsibility back on every developer.  It could also
> > > wreck havoc on the dashboards as there will be a delay in getting
> someone
> > > to fix the warning.  But if that is the general consensus, it’s way
> > > better than what we have now, which is nothing.  If that is the path we
> > > choose to
> > >
> > > follow, then I would hope that the following could be be features:
> > >    - CMake be very insistent about installing the hooks.  It should not
> > >    be easy to miss or ignore the error.
> > >    - The error should give clear instructions on how to install the
> > >    hooks.
> > >
> > >     It’s annoying to have to find it in the Wiki every time.
> > >
> > >    - The check should also look for any updates to the hooks in
> addition
> > >    to just seeing if they are installed.  One of the problems I run
> into
> > >    is that even though I try to be diligent about installing hooks, I
> > >    miss changes pushed to the repository.
> > >    - The check should turn itself off if not run in a git repository.
>  A
> > >    user who downloaded the source from the web would never be able to
> > >    satisfy the requirement.
> >
> > The checks in Titan have all but the third feature. That would be a
> > valuable general addition though, and I think there is some code floating
> > around that could help us to accomplish this. It would be good to hear
> how
> > others feel about this, but we should certainly be making these things as
> > easy as possible for our developers. I will see what our software process
> > type people think - Brad, Dave, Bill?
> >
> > Marcus
> > --
> > Marcus D. Hanwell, Ph.D.
> > R&D Engineer, Kitware Inc.
> > (518) 881-4937
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100705/61d4ff8c/attachment.html>