[vtk-developers] Auto install git hooks
Marcus D. Hanwell
marcus.hanwell at kitware.com
Wed Jun 30 14:12:50 EDT 2010
On Wed, Jun 30, 2010 at 2:06 PM, Berk Geveci <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> 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>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> 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>
>>> 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>
>>> 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> > 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> > 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>
>>> > wrote:
>>>
>>> On Thu, Jun 17, 2010 at 4:40 PM, Clinton Stimpson <clinton at 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
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> **** Kenneth Moreland
>>> *** Sandia National Laboratories
>>> ***********
>>> *** *** *** email: kmorel at sandia.gov <http://kmorel@sandia.gov>
>>> ** *** ** phone: (505) 844-8919
>>> *** web: http://www.cs.unm.edu/~kmorel <
>>> http://www.cs.unm.edu/%7Ekmorel>
>>>
>>>
>>> _______________________________________________
>>> Powered by www.kitware.com <http://www.kitware.com>
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>>
>>>
>>>
>>>
>>>
>>>
>>> **** Kenneth Moreland
>>> *** Sandia National Laboratories
>>> ***********
>>> *** *** *** email: kmorel at sandia.gov
>>> ** *** ** phone: (505) 844-8919
>>> *** web: http://www.cs.unm.edu/~kmorel
>>>
>>>
>>
>>
>> --
>> Wesley D. Turner, Ph.D.
>> Kitware, Inc.
>> Technical Leader
>> 28 Corporate Drive
>> Clifton Park, NY 12065-8662
>> Phone: 518-881-4920
>>
>> _______________________________________________
>> Powered by www.kitware.com
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html
>>
>> Follow this link to subscribe/unsubscribe:
>> http://www.vtk.org/mailman/listinfo/vtk-developers
>>
>>
>>
>
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://www.vtk.org/mailman/listinfo/vtk-developers
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100630/087e9941/attachment.html>
More information about the vtk-developers
mailing list