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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 28, 2007, 23:42 UTC
Message-ID: <7vve7m0wfo.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071128230355.GB13964@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 23 quoted lines

> 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?