Jiang Xin <worldhello.net@xxxxxxxxx> writes:
> diff --git a/transport-helper.c b/transport-helper.c
> index 20a7185ec4..ab3b52eb14 100644
> --- a/transport-helper.c
> +++ b/transport-helper.c
> @@ -894,6 +894,21 @@ static int push_refs_with_push(struct transport *transport,
> case REF_STATUS_REJECT_STALE:
> case REF_STATUS_REJECT_ALREADY_EXISTS:
> if (atomic) {
> + /* Mark other refs as failed */
> + for (ref = remote_refs; ref; ref = ref->next) {
> + if (!ref->peer_ref && !mirror)
> + continue;
> +
> + switch (ref->status) {
> + case REF_STATUS_NONE:
> + case REF_STATUS_OK:
> + case REF_STATUS_EXPECTING_REPORT:
> + ref->status = REF_STATUS_ATOMIC_PUSH_FAILED;
> + continue;
> + default:
> + break; /* do nothing */
> + }
> + }
OK, so this is more in line with the check done in send_pack() that
fails the push before we even send any pack data. I wonder if it is
worth considering to move the logic of this loop into a helper
function so that this and the other one in 2/3 can call it and stay
in sync, something along the lines of
void reject_push_to_other_refs(struct ref *ref, int mirror_mode)
{
for (; ref; ref = ref->next) {
... one iteration of the above loop ...
}
}
Then the above part would become
case REF_STATUS_REJECT_FOO:
if (atomic)
reject_push_to_other_refs(remote_refs, mirror);
and the part modified by 2/3 would also become
static int atomic_push_failure(...)
{
reject_push_to_other_refs(remote_refs, args->send_mirror);
return error("atomic push failed ...");
}
I am not sure if it is better to keep 2/3 and 3/3 separate or make
them into a single step, but perhaps it is because I am not getting
the true reason why you made them separate in the first place.
Thanks.
