Re: [PATCH 1/4] hooks: Add function to check if a hook exists

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 29, 2012, 02:08 UTC
Message-ID: <7vwqw1fw5a.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1356735452-21667-2-git-send-email-aaron@schrab.com>

Aaron Schrab <aaron@schrab.com> writes:

> Create find_hook() function to determine if a given hook exists and is
> executable.  If it is the path to the script will be returned, otherwise
> NULL is returned.

Sounds like a sensible thing to do. To make sure the API is also sensible, all the existing hooks should be updated to use this API, no?

> This is in support for an upcoming run_hook_argv() function which will
> expect the full path to the hook script as the first element in the
> argv_array.

There is currently a public function called run_hook() that squats on the good name with a kludgy API that is too specific to using separate index file. Back when it was a private helper in the implementation of "git commit", it was perfectly fine, but it was exported without giving much thought on the API.

If you are introducing a new run_hook_* function, give it a generic enough API that lets all the existing hook callers to use it. I would imagine that the API requirement may be modelled after run_command() API so that we can pass argv[] and tweak the hook's environ[], as well as feeding its stdin and possibly reading from its stdout. That would be very useful.