Re: [PATCH 1/7] receive-pack.c: add protocol support to negotiate atomic-push

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 30, 2014, 16:47 UTC
Message-ID: <xmqqvbktrsom.fsf@gitster.dls.corp.google.com>
In-Reply-To: <549A6C4C.8000905@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

Show 5 quoted lines

> If I understand correctly, after this patch the server advertises the
> "atomic" capability even though it doesn't actually have that ability
> until a later patch. It seems to me that the order of the patches should
> be reversed: don't advertise the capability before it is actually
> implemented. Why? Bisection. Between the two patches the server is buggy.

That is a valid point.  It also reminds us of another thing.

We would need a way to test interoperability among the three new combinations (i.e. new and old receive-pack talking to new and old "git push"). We can control what the sender talks on the wire by giving or not giving "--atomic" option on the command line, but there should be a way for us to control what the receiver talks on the wire, i.e. "receivepack.pushAtomic = false" that tells us not to advertise the "atomic push" capability over the wire, even if you are running the updated "receive-pack" binary.

This will not only for testing. When we discover that "atomic" support is undesirable for whatever reason (e.g. the transaction machinery may consume too many file descriptors without a good reason), we would need a way for users to disable it until the problem is fixed.

And of course the tests should make sure that

 (1) "git push --atomic" that talks with the receiving end that has
     receivepack.pushAtomic set to false behaves as we desire (error
     out? degrade to non-atomic? --- whichever way we decide).

 (2) "git push" that talks with the receiving end with "atomic" enabled
     does not do an atomic push, i.e. try to push two refs, one that
     fast forwards and the other that does not, and see one ref is
     updated while the other ref remains intact and "git push"
     itself reports failured.;

 (3) "git push --atomic" that talks with the receiving end with
     "atomic" enabled does the atomic thing.

among other things.