Re: [PATCH 4/4] fetch-pack: mention server version with verbose output

[Jeff King <peff@peff.net>]

On Mon, Aug 13, 2012 at 12:07:35PM -0700, Junio C Hamano wrote:

>  * And this is your 4 adjusted for the previous one, releaving the
>    caller from having to figure out where the capability string
>    ends.
> [...]
> @@ -829,8 +831,15 @@ static struct ref *do_fetch_pack(int fd[2],
>  			fprintf(stderr, "Server supports ofs-delta\n");
>  	} else
>  		prefer_ofs_delta = 0;
> -	if (server_supports("agent"))
> +
> +	if ((agent_feature = server_feature("agent", &agent_len)) != NULL &&
> +	    5 < agent_len && agent_feature[5] == '=') {
>  		agent_supported = 1;
> +		if (args.verbose) {
> +			fprintf(stderr, "Server version is %.*s\n",
> +				agent_len - 6, agent_feature + 6);
> +		}
> +	}

Yeah, this is exactly the kind of ugliness I was trying to avoid with my
allocating wrapper. Still, there is only one call site, so I do not care
overly much (and I as I've already said, I'm lukewarm on the final two
patches, anyway).

There is one difference in your code and mine. With mine, the server can
say simply "agent" to tell the client that it understands the extension
but does not wish to disclose its version. That might be considered
unfriendly (why would the client show theirs if the server is not
willing to do the same?), but it may be a practical decision (e.g.,
security policies may say that servers are higher-risk targets[1]).
Of course, a server can also say "agent=git/none-of-your-business"; this
is just a syntactic question.

-Peff

[1] I think you and I both agreed earlier that the "sharing versions is
    a security risk" line of argument is not that compelling, but that
    does not mean it is not made all the time.

-Peff