Re: [PATCH v3] Allow update hooks to update refs on their own

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

Jeff King <peff@peff.net> writes:

> On Wed, Nov 28, 2007 at 02:14:03PM -0800, Steven Grimm wrote:
>
>> @@ -177,7 +179,16 @@ static int receive_status(int in, struct ref *refs)
>>  
>>  		line[strlen(line)-1] = '\0';
>>  		refname = line + 3;
>> -		msg = strchr(refname, ' ');
>> +		newsha1_hex = strchr(refname, ' ');
>> +		if (newsha1_hex) {
>> +			*newsha1_hex++ = '\0';
>> +			if (get_sha1_hex(newsha1_hex, newsha1)) {
>> +				fprintf(stderr, "protocol error: bad sha1 %s\n",
>> +					newsha1_hex);
>> +				newsha1_hex = NULL;
>> +			}
>> +		}
>> +		msg = strchr(newsha1_hex, ' ');
>>  		if (msg)
>>  			*msg++ = '\0';
>
> Doesn't this always put the first "word" of a response into newsha1_hex?
> We want to do this only for 'ok' responses; 'ng' responses are already
> using that space as part of the error message.

I do not think reporting back the rewritten object name makes much sense
nor adds any value; it won't be useful information until you fetch the
object.

I do not think reporting back _anything_ other than "ok" adds much value
at all.  Sure, if the update hook did something funky you would get such
a report, but the situation is not any different if some warm body is
sitting on the other end and building on top of what you pushed
immediately he sees any push into the repository, and in such a case
your git-push would not get any such reporting anyway.

We do not even have to worry about this reporting at all if we do not
allow munging the refs in the update hook.  In a sense, this patch is
creating a problem that does not need to be solved.  Perhaps modifying
update hook to allow so makes it possible to munge refs while holding a
lock, but is it really worth this hassle?  Isn't there a better way, I
wonder?