Stefan Beller <sbel...@google.com> writes:

> -static int ref_update_to_be_sent(const struct ref *ref, const struct 
> send_pack_args *args)
> +static int ref_update_to_be_sent(const struct ref *ref, const struct 
> send_pack_args *args, int *atomic_push_failed)

Hmph.  Is "atomic push" so special that it deserves a separate
parameter?  When we come up with yet another mode of failure, would
we add another parameter to the callers to this function?

> @@ -203,6 +203,12 @@ static int ref_update_to_be_sent(const struct ref *ref, 
> const struct send_pack_a
>       case REF_STATUS_REJECT_NEEDS_FORCE:
>       case REF_STATUS_REJECT_STALE:
>       case REF_STATUS_REJECT_NODELETE:
> +             if (atomic_push_failed) {
> +                     fprintf(stderr, "Atomic push failed for ref %s. "
> +                             "Status:%d\n", ref->name, ref->status);
> +                     *atomic_push_failed = 1;

All other error message that come from the codepaths around here
seem to avoid uppercase.  Also maybe you want to use error() here?

> +     if (args->use_atomic_push && !atomic_push_supported) {
> +             fprintf(stderr, "Server does not support atomic-push.");
> +             return -1;
> +     }

This check logically belongs to the previous step, no?

> +     atomic_push = atomic_push_supported && args->use_atomic_push;

>  
>       if (status_report)
>               strbuf_addstr(&cap_buf, " report-status");
> @@ -365,7 +376,8 @@ int send_pack(struct send_pack_args *args,
>        * the pack data.
>        */
>       for (ref = remote_refs; ref; ref = ref->next) {
> -             if (!ref_update_to_be_sent(ref, args))
> +             if (!ref_update_to_be_sent(ref, args,
> +                     args->use_atomic_push ? &atomic_push_failed : NULL))
>                       continue;
>  
>               if (!ref->deletion)
> @@ -377,6 +389,23 @@ int send_pack(struct send_pack_args *args,
>                       ref->status = REF_STATUS_EXPECTING_REPORT;
>       }
>  
> +     if (atomic_push_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 */
> +                     }
> +             }
> +             fprintf(stderr, "Atomic push failed.");
> +             return -1;

The same comment as the other fprintf() applies here.

> @@ -728,6 +728,10 @@ static int print_one_push_status(struct ref *ref, const 
> char *dest, int count, i
>                                                ref->deletion ? NULL : 
> ref->peer_ref,
>                                                "remote failed to report 
> status", porcelain);
>               break;
> +     case REF_STATUS_ATOMIC_PUSH_FAILED:
> +             print_ref_status('!', "[rejected]", ref, ref->peer_ref,
> +                                              "atomic-push-failed", 
> porcelain);

Why dashed-words-here?

> +             break;
>       case REF_STATUS_OK:
>               print_ok_ref_status(ref, porcelain);
>               break;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to