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

From: Junio C Hamano <[email-masked]>
Date: Tue, 30 Dec 2014 08:47:05 -0800

Michael Haggerty <[email-masked]> writes:

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