Re: [PATCH] git push --track

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 15, 2010, 18:16 UTC
Message-ID: <7vmy0f1bf6.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100115140048.GB30986@rm.endoftheinternet.org>

Rudolf Polzer <divVerent@alientrap.org> writes:

Show 20 quoted lines

> On Thu, Jan 14, 2010 at 09:47:22PM -0800, Junio C Hamano wrote:
>> I have a feeling that it is more appropriate to have the additional code
>> in transport_push(), which gets ls-remote information, runs match_refs()
>> and finally calls transport->push_refs().  I think the extra branch
>> configuration would fit better inside the if block immediately after all
>> that happens, i.e.
>> 
>> 	if (!(flags & TRANSPORT_PUSH_DRY_RUN)) {
>> 		struct ref *ref;
>> 		for (ref = remote_refs; ref; ref = ref->next)
>> 			update_tracking_ref(transport->remote, ref, verbose);
>> +		if (flags & TRANSPORT_PUSH_RECONFIGURE_FORK)
>> +			configure_forked_branch(...);
>> 	}
>> 
>> in transport.c
>
> I thought about this place when making my patch, but didn't put it there
> because this function is not called in the rsync protocol (which defines
> transport->push).

That's not a very good reasoning. Instead of punishing well behaved transport that defines push_ref, punish _only_ the transports that does not define it (see the paragraph at the end of this message).

> But well. Why bother with this, if this feature was rejected before already
> anyway.

Read the thread Nana quoted for you again; I nor anybody ever _rejected_ the ultimate goal, even though I said that the justifications were not sufficiently convincing for the previous implementation attempts.

I think Ilari's patch is done right and can be extended by anybody who cares about rsync transport to call an extra ls-remote in the "does this one lack push_ref but know how to push" codepath.