Re: [PATCH 2/5] send-pack.c: add an --atomic-push command line argument
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:03:17
Stefan Beller [off-list ref] 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?
quoted hunk
@@ -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;
quoted hunk
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.
quoted hunk
@@ -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;