Re: [PATCH 6/7] push.c: add an --atomic argument

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

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

> I'd like to discuss the big picture around this feature. I don't think
> that any of these questions are blockers, with the possible exception of
> the question of whether "--atomic" should fall back to non-atomic if the
> server doesn't support atomic pushes.
>
> 1. Should "--atomic" someday become the default?
>
> You seem to imply that "--atomic" might become the default sometime in
> the future. (I realize that this patch series does not propose to change
> the default but let's talk about it anyway.) In the real world, the most
> common reason for an "--atomic" push to fail would be that somebody else
> has pushed to a branch since our last update, resulting in a non-ff
> error. Would I find out about such an error before or after I have
> transferred my objects to the server?

That question is pretty much rhetorical, as certain rejections you
cannot fundamentally implement without having the data at hand.

> If I only find out at the end of the transfer, then it could be a pretty
> frustrating experience pushing a lot of references to a server over a
> slow connection.

We'd like to have a long overdue protocol update for fetch and push
soonish anyawy (perhaps in the first half of 2015) and part of that
should include unified logic for common ancestor negotiation between
fetch and push [*1*].  We should be able to ease that with an
optimization similar to quickfetch done on the fetch side once that
happens.

> Even *if* "--atomic" becomes the default, we would certainly want to
> support a "--no-atomic" (or "--non-atomic"?) option to get the old
> behavior. It might be a good idea to add that option now, so that
> forward-looking script writers can start explicitly choosing "--atomic"
> vs. "--no-atomic".

Perhaps.  But on the other hand, pushing multiple refs at the same
time is a sign that they are related and need to go together.  The
reason why one but not others fails would be an indication that
there is somebody else pushing into the same repository and the
pusher and the other party are stepping on each other's toes, which
should be resolved primarily by inter-developer communication, not
with "--no-atomic" workaround.

> 2. Is this an option that users will want to specify via the command line?
>
> For scripts that want to insist on "atomic" updates, it is no problem to
> specify "--atomic" on the command line.
>
> But supposing that "--atomic" is a good default for some people, it
> would be awkward for them to have to specify it on every "git push"
> invocation. It therefore might be nice to have a configuration setting
> to choose whether "--atomic" is the default.
>
> Also (see above) it might be useful to set "--atomic" only for
> particular servers (for example, only for those to which you have a fast
> connection). This suggests that the "atomic/non-atomic" configuration
> should be settable on a per-remote basis.

I think you are hinting to have remote.atomicPush = {yes,no} that is
weaker than remote.$nick.atomicPush = {yes,no} or something like
that.  I agree that would be a good direction to go.

> 3. What should happen if the server doesn't support atomic pushes?

If you asked for atomic push explicitly and if the server cannot
support it, push should fail.

If the only reason we are doing atomic is because in some future we
flipped the default (i.e. no remote.*.atomicPush or --atomic option
from the command line), then it might be OK to continue with a
warning.

I however think that the automatic demotion is too much complexity
for such a simple option.  "Please be atomic if you can but I'd take
non-atomic one if you do not want to give me atomic one" that is
responded by "I'd do non-atomic then, as you are perfectly happy
without" is not very useful---such a pusher should just say "I
accept non-atomic", which is what "--no-atomic" is for.


[Footnotes]

*1* Two other big ones are syntax change to have an explicit
    extension packets instead of hiding the capability after NUL,
    and resolving the "who speaks first" issue.