Re: [PATCH] upload-pack: add a trigger for post-upload-pack hook

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> +static void run_post_upload_pack_hook(int create_full_pack)
>> +{
>> +	const char *fetch_type;
>> +	fetch_type = (create_full_pack) ? "clone" : "fetch";
>> +	run_hook(get_index_file(), "post-upload-pack", fetch_type);
>> +}
>
> Does it really need an index file? This operation in question seems to
> be totally disconnected from the index (and indeed, most bare
> repositories won't even have one). Probably it should pass NULL as the
> initial argument to run_hook.

Very good point; a bare repository does not have to have (and typically
shouldn't have) the index, and a bare repository is what upload-pack
typically serves.

A short-and-sweet:

	run_hook(NULL, "post-upload-pack",
        	 create_full_pack ? "clone" : "fetch,
                 NULL);

would be sufficient.  Notice that run_hook() is variadic and its argument
list needs to be terminated with NULL (iow, the original patch is buggy
and risks reading random places on the stack---I would recommend against
using it on your production site yet).

> Is there any other information that might be useful to other non-GitHub
> users of the hook? The only thing I can think of is the list of refs
> that were fetched.

I do not think that information is available.  "want" will tell you what
object they want, but that does not necessarily uniquey translate to a
ref.

If we are allowed to talk about asking for the moon, and if one of the
primary reason for this new hook is statistics, it would be useful to see
the number of bytes given, where the fetch-pack came from, and if we are
using git-daemon virtual hosting which of our domain served the request.