Subject: Re: [PATCHv12 08/10] send-pack.c: add --atomic command line argument

From: Junio C Hamano <[email-masked]>
Date: Mon, 12 Jan 2015 13:57:22 -0800

Stefan Beller <[email-masked]> writes:

> +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 */
> +		}
> +	}
> +	return error("atomic push failed for ref %s. status: %d\n",
> +		     failing_ref->name, failing_ref->status);
> +}

Given remote_refs, this will mark all refs that are still in the
"expecting" state as "failed".

> @@ -363,9 +396,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:
> +			/*
> +			 * 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;
> -
> +		}

And this loop stops when it sees one that would certainly get
rejected, letting the other function mark "expecting" ones as "would
fail".

We could have skipped refs that are OK (e.g. REF_STATUS_UPTODATE)
before we saw the first rejection, but it is OK not to go back and
mark them as "would fail", because the other side will not be seeing
our attempt to update these refs anyway.

OK, it makes sense to me.