Re: [PATCH 3/7] send-pack.c: add --atomic command line argument

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Dec 22, 2014, 22:58 UTC
Message-ID: <CAPig+cQCLWy8rEqMCMxkvz+jAYwOA2tZE1LagkB5vY-60pe1mQ@mail.gmail.com>
In-Reply-To: <1419017941-7090-4-git-send-email-sbeller@google.com>

On Fri, Dec 19, 2014 at 2:38 PM, Stefan Beller <sbeller@google.com> wrote:

Show 39 quoted lines

> From: Ronnie Sahlberg <sahlberg@google.com>
>
> This adds support to send-pack to negotiate and use atomic pushes
> iff the server supports it. Atomic pushes are activated by a new command
> line flag --atomic.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index b564a77..b961e5a 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -282,6 +282,30 @@ free_return:
>         return update_seen;
>  }
>
> +
> +static int atomic_push_failure(struct send_pack_args *args,
> +                              struct ref *remote_refs,
> +                              struct ref *failing_ref)
> +{
> +       struct ref *ref;
> +       /* Mark other refs as failed */
> +       for (ref = remote_refs; ref; ref = ref->next) {
> +               if (!ref->peer_ref && !args->send_mirror)
> +                       continue;
> +
> +               switch (ref->status) {
> +               case REF_STATUS_EXPECTING_REPORT:
> +                       ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
> +                       continue;
> +               default:
> +                       ; /* do nothing */
> +               }
> +       }
> +       error("atomic push failed for ref %s. status: %d\n",
> +             failing_ref->name, failing_ref->status);
> +       return -1;

Not itself worth a re-send, but if you do re-send for some other reason...

    return error(...);

would be more idiomatic (as mentioned in the previous review).

Show 5 quoted lines

> +}
> +
>  int send_pack(struct send_pack_args *args,
>               int fd[], struct child_process *conn,
>               struct ref *remote_refs,