Re: [PATCHv2 5/6] push.c: add an --atomic-push argument

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 16, 2014, 19:33 UTC
Message-ID: <CAPig+cR5x=GsZ6Gg_i==o0WKgqU1PP9uQ6QAgNkF8SBOvRCJ5g@mail.gmail.com>
In-Reply-To: <1418755747-22506-5-git-send-email-sbeller@google.com>

On Tue, Dec 16, 2014 at 1:49 PM, Stefan Beller <sbeller@google.com> wrote:

Show 23 quoted lines

> From: Ronnie Sahlberg <sahlberg@google.com>
>
> Add a command line argument to the git push command to request atomic
> pushes.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Notes:
>     Changes v1 -> v2
>         It's --atomic now! (dropping the -push)
>
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 21b3f29..3feacc5 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-push] [-n | --dry-run] [--receive-pack=<git-receive-pack>]

s/atomic-push/atomic/

Show 9 quoted lines

>            [--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 the an atomic transaction on the server side if available.

s/the an/an/

Show 15 quoted lines

> +       Either all refs are updated, or on error, no refs are updated.
> +       If the server does not support atomic pushes the push will fail.
> +
>  --receive-pack=<git-receive-pack>::
>  --exec=<git-receive-pack>::
>         Path to the 'git-receive-pack' program on the remote
> diff --git a/builtin/push.c b/builtin/push.c
> index a076b19..fe0b8cc 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -518,6 +518,8 @@ int cmd_push(int argc, const char **argv, const char *prefix)
>                 OPT_BIT(0, "follow-tags", &flags, N_("push missing but relevant tags"),
>                         TRANSPORT_PUSH_FOLLOW_TAGS),
>                 OPT_BIT(0, "signed", &flags, N_("GPG sign the push"), TRANSPORT_PUSH_CERT),
> +               OPT_BIT(0, "atomic", &flags, N_("use a single atomic transaction at the serverside, if available"),

"single atomic" sounds awfully redundant.

"serverside" is odd. Perhaps "server side" or merely "remote" or "remote side".

Show 25 quoted lines

> +                       TRANSPORT_ATOMIC_PUSH),
>                 OPT_END()
>         };
>
> diff --git a/transport.c b/transport.c
> index c67feee..5b29514 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -830,6 +830,7 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
>         args.dry_run = !!(flags & TRANSPORT_PUSH_DRY_RUN);
>         args.porcelain = !!(flags & TRANSPORT_PUSH_PORCELAIN);
>         args.push_cert = !!(flags & TRANSPORT_PUSH_CERT);
> +       args.atomic = !!(flags & TRANSPORT_ATOMIC_PUSH);
>         args.url = transport->url;
>
>         ret = send_pack(&args, data->fd, data->conn, remote_refs,
> diff --git a/transport.h b/transport.h
> index 3e0091e..25fa1da 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -125,6 +125,7 @@ struct transport {
>  #define TRANSPORT_PUSH_NO_HOOK 512
>  #define TRANSPORT_PUSH_FOLLOW_TAGS 1024
>  #define TRANSPORT_PUSH_CERT 2048
> +#define TRANSPORT_ATOMIC_PUSH 4096

For consistency with existing names, should this be TRANSPORT_PUSH_ATOMIC?

>  #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
>  #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - gettext_width(x)), (x)
> --
> 2.2.0.31.gad78000.dirty