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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 12, 2015, 23:29 UTC
Message-ID: <CAPig+cQv6Dr7z56qXiqzZLmuBt=_TWcPSpQSTyLv0euT-k+umA@mail.gmail.com>
In-Reply-To: <1420687404-13997-7-git-send-email-sbeller@google.com>

On Wed, Jan 7, 2015 at 10:23 PM, Stefan Beller <sbeller@google.com> wrote:

Show 25 quoted lines

> From: Ronnie Sahlberg <sahlberg@google.com>
>
> This adds the atomic protocol option to allow
> receive-pack to inform the client that it has
> atomic push capability.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> Notes:
>     v9:
>      We can configure the remote if it wants to advertise
>      atomicity!
> 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
> @@ -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;
> +       }

Is this needed only to support the tests you add in t5543, or do you intend it to be useful to end users?

If it is for end users, then it deserves to be documented. Is also deserves proper mention and justification in the commit message.

If it's only intended to assist automated testing, then perhaps control it via an environment variable rather than a configuration option. (See, for instance, GIT_TEST_SPLIT_INDEX or GIT_USE_LOOKUP as precedent.)

Show 25 quoted lines

> +
>         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")) {
> --
> 2.2.1.62.g3f15098