On Wed, Dec 17, 2014 at 1:32 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> From: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
> diff --git a/send-pack.c b/send-pack.c
> index 77e4201..a696d5c 100644
> --- a/send-pack.c
> +++ b/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;
error() returns -1, so:
return error(...);
Also, why split the string literal like that? It's not warranted by
the indentation level. Instead:
return error("atomic push failed for ref %s. status: %d\n",
failing_ref->name, failing_ref->status);
> +}
> +
> int send_pack(struct send_pack_args *args,
> int fd[], struct child_process *conn,
> struct ref *remote_refs,
> @@ -373,9 +397,21 @@ int send_pack(struct send_pack_args *args,
> * the pack data.
> */
> for (ref = remote_refs; ref; ref = ref->next) {
> - if (check_to_send_update(ref, args) < 0)
> + switch (check_to_send_update(ref, args)) {
> + case 0: /* no error */
> + break;
> + case -CHECK_REF_STATUS_REJECTED:
I realize that Junio said that this could wait for cleanup patch by
someone later, but I'd argue that defining these constants now with
their proper negative values would be beneficial:
First, it is a potential maintenance burden that each person working
on the code later must remember to negate the constants each time they
are used. Forgetting to do so will lead to incorrect behavior.
Second, negating these constants at point-of-use implies incorrectly
that there is some valid use-case for the non-negated forms. It's
misleading and confusing.
> + /*
> + * When we know the server would reject a ref update if
> + * we were to send it and we're trying to send the refs
> + * atomically, abort the whole operation.
> + */
> + if (use_atomic)
> + return atomic_push_failure(args, remote_refs, ref);
> + /* Fallthrough for non atomic case. */
> + default:
> continue;
> -
> + }
> if (!ref->deletion)
> need_pack_data = 1;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html