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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 12, 2015, 21:57 UTC
Message-ID: <xmqq8uh77jct.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1420687404-13997-9-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 21 quoted lines

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

Show 21 quoted lines

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