On Mon, Dec 29, 2014 at 11:08 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Mon, Dec 29, 2014 at 9:36 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: >> Subject: receive-pack.c: add documentation for atomic push support > > This patch is doing a lot more than merely adding documentation. It's > also updating send-pack and receive-pack to be able to negotiate the > new protocol capability "atomic". The fact that you removed the actual > advertisement of "atomic" from this patch doesn't turn it into a > documentation-only patch. That's true, though the code itself is dead code and doing nothing in that patch. > > When Michael raised the issue of the server being "broken" due to > advertising a capability which it does not yet implement, his > recommendation[1] was to reorder the patches, not to split out the one > tiny bit (capability advertisement) from the larger change. Was there > an insurmountable conflict which prevented you from reordering the > patches? This is a genuine question since splitting off advertisement > -- and only advertisement -- to a patch later in the series smells > like a least-resistance approach, rather than a proper solution to the > issue Michael raised. Well there was no syntactical problem (i.e. the interactive rebase went flawless), but rather a semantic dependency. The reordered patches would not compile as we'd heavily depend on the use_atomic variable. Of course that could have been introduced where required, but at the time it did not look appealing to me. I'll reword the commit message header to mention the negotiation part as well. > > [1]: http://article.gmane.org/gmane.comp.version-control.git/261793 > >> This documents the protocol option between send-pack and receive-pack to >> * allow receive-pack to inform the client that it has atomic push capability >> * allow send-pack to request atomic push back. >> >> There is currently no setting in send-pack to actually request that atomic >> pushes are to be used yet. This only adds protocol capability not ability >> for the user to activate it. The capability is also not yet advertised >> by receive-pack as git doesn't know how to handle it yet. >> >> Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx> >> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx> >> --- >> diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt >> index 6d5424c..4f8a7bf 100644 >> --- a/Documentation/technical/protocol-capabilities.txt >> +++ b/Documentation/technical/protocol-capabilities.txt >> @@ -18,8 +18,9 @@ was sent. Server MUST NOT ignore capabilities that client requested >> and server advertised. As a consequence of these rules, server MUST >> NOT advertise capabilities it does not understand. >> >> -The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities >> -are sent and recognized by the receive-pack (push to server) process. >> +The 'atomic', 'report-status', 'delete-refs', 'quiet', and 'push-cert' >> +capabilities are sent and recognized by the receive-pack (push to server) >> +process. >> >> The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized >> by both upload-pack and receive-pack protocols. The 'agent' capability >> @@ -244,6 +245,14 @@ respond with the 'quiet' capability to suppress server-side progress >> reporting if the local progress reporting is also being suppressed >> (e.g., via `push -q`, or if stderr does not go to a tty). >> >> +atomic >> +------ >> + >> +If the server sends the 'atomic' capability it is capable of accepting >> +atomic pushes. If the pushing client requests this capability, the server >> +will update the refs in one atomic transaction. Either all refs are >> +updated or none. >> + >> allow-tip-sha1-in-want >> ---------------------- >> >> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c >> index 32fc540..4e8eaf7 100644 >> --- a/builtin/receive-pack.c >> +++ b/builtin/receive-pack.c >> @@ -40,6 +40,7 @@ static int transfer_unpack_limit = -1; >> static int unpack_limit = 100; >> static int report_status; >> static int use_sideband; >> +static int use_atomic; >> static int quiet; >> static int prefer_ofs_delta = 1; >> static int auto_update_server_info; >> @@ -1179,6 +1180,8 @@ static struct command *read_head_info(struct sha1_array *shallow) >> use_sideband = LARGE_PACKET_MAX; >> if (parse_feature_request(feature_list, "quiet")) >> quiet = 1; >> + if (parse_feature_request(feature_list, "atomic")) >> + use_atomic = 1; >> } >> >> if (!strcmp(line, "push-cert")) { >> diff --git a/send-pack.c b/send-pack.c >> index 949cb61..6d0c159 100644 >> --- a/send-pack.c >> +++ b/send-pack.c >> @@ -294,6 +294,8 @@ int send_pack(struct send_pack_args *args, >> int use_sideband = 0; >> int quiet_supported = 0; >> int agent_supported = 0; >> + int use_atomic = 0; >> + int atomic_supported = 0; >> unsigned cmds_sent = 0; >> int ret; >> struct async demux; >> @@ -314,6 +316,8 @@ int send_pack(struct send_pack_args *args, >> agent_supported = 1; >> if (server_supports("no-thin")) >> args->use_thin_pack = 0; >> + if (server_supports("atomic")) >> + atomic_supported = 1; >> if (args->push_cert) { >> int len; >> >> @@ -328,6 +332,10 @@ int send_pack(struct send_pack_args *args, >> "Perhaps you should specify a branch such as 'master'.\n"); >> return 0; >> } >> + if (args->atomic && !atomic_supported) >> + die(_("server does not support --atomic push")); >> + >> + use_atomic = atomic_supported && args->atomic; >> >> if (status_report) >> strbuf_addstr(&cap_buf, " report-status"); >> @@ -335,6 +343,8 @@ int send_pack(struct send_pack_args *args, >> strbuf_addstr(&cap_buf, " side-band-64k"); >> if (quiet_supported && (args->quiet || !args->progress)) >> strbuf_addstr(&cap_buf, " quiet"); >> + if (use_atomic) >> + strbuf_addstr(&cap_buf, " atomic"); >> if (agent_supported) >> strbuf_addf(&cap_buf, " agent=%s", git_user_agent_sanitized()); >> >> diff --git a/send-pack.h b/send-pack.h >> index 5635457..b664648 100644 >> --- a/send-pack.h >> +++ b/send-pack.h >> @@ -13,7 +13,8 @@ struct send_pack_args { >> use_ofs_delta:1, >> dry_run:1, >> push_cert:1, >> - stateless_rpc:1; >> + stateless_rpc:1, >> + atomic:1; >> }; >> >> int send_pack(struct send_pack_args *args, >> -- >> 2.2.1.62.g3f15098 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html