At 18:08 -0800 28 Dec 2012, Junio C Hamano <gitster@xxxxxxxxx> wrote:
Aaron Schrab <aaron@xxxxxxxxxx> 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?
I'd been trying to keep the changes limited. I'll see about modifying
the existing places that run hooks in v2 of the series.
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.
I think the attraction of the run_hook() API is its simplicity. It's
currently a fairly thin wrapper around the run_command() API. I suspect
that if the run_hook() API were made generic enough to support all of
the existing hook callers it would greatly complicate the existing calls
to run_hook() while not providing much benefit to hook callers which
can't currently use it beyond what run_command() offers.
Since I'm going to be changing the interface for this hook in v2 of the
series so that it will be more complicated than can be readily addressed
with the run_hook() API (and will have use a fixed number of arguments
anyway) I'll be dropping the run_hook_argv() function.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html