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

[Michael Haggerty <mhagger@alum.mit.edu>]

On 12/19/2014 08:39 PM, Stefan Beller wrote:
> Add a command line argument to the git push command to request atomic
> pushes.
> 
> [...]
> 
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 21b3f29..da63bdf 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
>  SYNOPSIS
>  --------
>  [verse]
> -'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
> +'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=<git-receive-pack>]
>  	   [--repo=<repository>] [-f | --force] [--prune] [-v | --verbose]
>  	   [-u | --set-upstream] [--signed]
>  	   [--force-with-lease[=<refname>[:<expect>]]]
> @@ -136,6 +136,11 @@ already exists on the remote side.
>  	logged.  See linkgit:git-receive-pack[1] for the details
>  	on the receiving end.
>  
> +--atomic::
> +	Use an atomic transaction on the remote side if available.
> +	Either all refs are updated, or on error, no refs are updated.
> +	If the server does not support atomic pushes the push will fail.
> +
> [...]

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?

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. After waiting for a long transfer to complete the user
would find out that the push was rejected and everything has to be done
again from scratch. In such cases non-"--atomic" behavior might be
attractive: any references that can be updated should be updated, so
that not *all* of the objects have to be pushed again.

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


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.


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

It seems to me that there are four reasonable behaviors WRT atomic
pushes. I'll give them tentative names for the purposes of this discussion:

"force" -- Insist on an atomic push, and fail if the server does not
support atomic pushes

"true" -- Use atomic push if supported by the server. If not, emit a
warning and fall back to non-atomic.

"auto" -- Use atomic push if supported by the server. If not, silently
fall back to non-atomic.

"false" -- Push non-atomically regardless of whether the server supports
atomic pushes.

To make it practical to set "atomic" as a universal default, we will
have to be able to deal with servers that don't (yet) support atomic
pushes. Therefore, we would want to use either "true" or "auto" (as
opposed to "force") as the default.

Even in such a case, it would be nice for the user to be able to
suppress any warnings for servers that don't support atomic updates. So
if "true" becomes the default, then we might want to support "auto" as well.

(One could argue that once "atomic" becomes the default, then anybody
who has to deal with an old server should just set "non-atomic" as the
default for that server. But even aside from the inconvenience to the
user, this is not a good alternative. A user who made that change
wouldn't benefit from atomic pushes once the server is upgraded to
support them.)

The command-line "--atomic" option is proposed to request "force"
behavior. On the one hand that makes sense; the user has explicitly
requested an atomic push, so the command should fail if it is not
possible. But on the other hand, if a particular server doesn't (yet)
support atomic pushes, what recourse does the user have but to run the
push again non-atomically? So it might be more expedient for the
"--atomic" option to be equivalent to "true" instead of "force". I don't
have a strong opinion on this either way.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu