Re: [PATCH v2 06/11] serve: advertise trace2 SID in v2 capabilities

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 4, 2020, 21:11 UTC
Message-ID: <xmqqtuu4ltd9.fsf@gitster.c.googlers.com>
In-Reply-To: <9573428cc02111d3141ec2481bf679b31a5139e2.1604355792.git.steadmon@google.com>

Josh Steadmon <steadmon@google.com> writes:

Show 34 quoted lines

> When trace2 is enabled and trace2.advertiseSID is true, advertise the
> server's trace2 session ID for all protocol v2 connections via the new
> trace2-sid capability.
>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
>  serve.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/serve.c b/serve.c
> index f6341206c4..b40d7aad34 100644
> --- a/serve.c
> +++ b/serve.c
> @@ -8,6 +8,8 @@
>  #include "serve.h"
>  #include "upload-pack.h"
>  
> +static int advertise_trace2_sid;
> +
>  static int always_advertise(struct repository *r,
>  			    struct strbuf *value)
>  {
> @@ -30,6 +32,15 @@ static int object_format_advertise(struct repository *r,
>  	return 1;
>  }
>  
> +static int trace2_advertise(struct repository *r, struct strbuf *value)
> +{
> +	if (!advertise_trace2_sid || !trace2_is_enabled())
> +		return 0;
> +	if (value)
> +		strbuf_addstr(value, trace2_session_id());
> +	return 1;
> +}

To both 05/11 and 06/11, the same "if the user instructs us to advertise, shouldn't we arrange the thing we were told to advertise to be automatically available by at least allocating a session ID, if not enabling the entire tracing?" applies.

Show 20 quoted lines

>  struct protocol_capability {
>  	/*
>  	 * The name of the capability.  The server uses this name when
> @@ -66,6 +77,7 @@ static struct protocol_capability capabilities[] = {
>  	{ "fetch", upload_pack_advertise, upload_pack_v2 },
>  	{ "server-option", always_advertise, NULL },
>  	{ "object-format", object_format_advertise, NULL },
> +	{ "trace2-sid", trace2_advertise, NULL },
>  };
>  
>  static void advertise_capabilities(void)
> @@ -261,6 +273,8 @@ static int process_request(void)
>  /* Main serve loop for protocol version 2 */
>  void serve(struct serve_options *options)
>  {
> +	git_config_get_bool("trace2.advertisesid", &advertise_trace2_sid);
> +
>  	if (options->advertise_capabilities || !options->stateless_rpc) {
>  		/* serve by default supports v2 */
>  		packet_write_fmt(1, "version 2\n");