Subject: Re: [PATCHv12 06/10] receive-pack.c: negotiate atomic push support

From: Junio C Hamano <[email-masked]>
Date: Thu, 08 Jan 2015 15:51:23 -0800

Stefan Beller <[email-masked]> writes:

> From: Ronnie Sahlberg <[email-masked]>
>
> This adds the atomic protocol option to allow
> receive-pack to inform the client that it has
> atomic push capability.
>
> This commit makes the functionality introduced
> in the previous commits go live for the serving
> side. The changes in documentation reflect the
> protocol capabilities of the server.
>
> Signed-off-by: Stefan Beller <[email-masked]>
> ---
>
> Notes:
>     v10, v11, v12:
>     * no changes
>     
>     v9:
>      This was once part of "[PATCH 1/7] receive-pack.c:
>      add protocol support to negotiate atomic-push"
>      but now it only touches the receive-pack.c part
>      and doesn't bother with the send-pack part any more.
>      That is done in a later patch, when send-pack actually
>      learns all the things it needs to know about the
>      atomic push option.
>     
>      We can configure the remote if it wants to advertise
>      atomicity!
>     
>     All previous notes were lost due to my glorious
>     capability to operate git rebase.

The list archive remembers if you really care ;-)

I ran out of time and concentration for today to read it through at
this step; among things I saw, nothing looked wrong so far, and at
this step everything looks ready to be tested almost.

Looking good.

>
>  Documentation/technical/protocol-capabilities.txt | 13 +++++++++++--
>  builtin/receive-pack.c                            | 11 +++++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
>
> 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 362d33f..4c069c5 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -37,6 +37,7 @@ static int receive_fsck_objects = -1;
>  static int transfer_fsck_objects = -1;
>  static int receive_unpack_limit = -1;
>  static int transfer_unpack_limit = -1;
> +static int advertise_atomic_push = 1;
>  static int unpack_limit = 100;
>  static int report_status;
>  static int use_sideband;
> @@ -159,6 +160,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
>  		return 0;
>  	}
>  
> +	if (strcmp(var, "receive.advertiseatomic") == 0) {
> +		advertise_atomic_push = git_config_bool(var, value);
> +		return 0;
> +	}
> +
>  	return git_default_config(var, value, cb);
>  }
>  
> @@ -174,6 +180,8 @@ static void show_ref(const char *path, const unsigned char *sha1)
>  
>  		strbuf_addstr(&cap,
>  			      "report-status delete-refs side-band-64k quiet");
> +		if (advertise_atomic_push)
> +			strbuf_addstr(&cap, " atomic");
>  		if (prefer_ofs_delta)
>  			strbuf_addstr(&cap, " ofs-delta");
>  		if (push_cert_nonce)
> @@ -1263,6 +1271,9 @@ 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 (advertise_atomic_push
> +			    && parse_feature_request(feature_list, "atomic"))
> +				use_atomic = 1;
>  		}
>  
>  		if (!strcmp(line, "push-cert")) {