Re: [PATCH v2 04/11] serve: provide "receive" function for object-format capability

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 15, 2021, 16:54 UTC
Message-ID: <xmqqlf3xye1r.fsf@gitster.g>
In-Reply-To: <YUE1fy0eA0wQnn3f@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 8 quoted lines

> There should be no change of behavior here, except for two
> broken-protocol cases:
>
>   - if the client sends multiple conflicting object-format capabilities
>     (which they should not), we'll now choose the last one rather than
>     the first. We could also detect and complain about the duplicates
>     quite easily now, which we could not before, but I didn't do so
>     here.

I'd imagine that it would have been easy to retain the "use the first one" behaviour if we wanted to; I do not think this behaviour change really matters in practice, so it's OK.

>   - if the client sends a bogus "object-format" with no equals sign,
>     we'll now say so, rather than "unknown object format: ''"

OK.

Show 32 quoted lines

> @@ -228,22 +241,6 @@ static int has_capability(const struct strvec *keys, const char *capability,
>  	return 0;
>  }
>  
> -static void check_algorithm(struct repository *r, struct strvec *keys)
> -{
> -	int client = GIT_HASH_SHA1, server = hash_algo_by_ptr(r->hash_algo);
> -	const char *algo_name;
> -
> -	if (has_capability(keys, "object-format", &algo_name)) {
> -		client = hash_algo_by_name(algo_name);
> -		if (client == GIT_HASH_UNKNOWN)
> -			die("unknown object format '%s'", algo_name);
> -	}
> -
> -	if (client != server)
> -		die("mismatched object format: server %s; client %s\n",
> -		    r->hash_algo->name, hash_algos[client].name);
> -}
> -
>  enum request_state {
>  	PROCESS_REQUEST_KEYS,
>  	PROCESS_REQUEST_DONE,
> @@ -317,7 +314,10 @@ static int process_request(void)
>  	if (!command)
>  		die("no command requested");
>  
> -	check_algorithm(the_repository, &keys);
> +	if (client_hash_algo != hash_algo_by_ptr(the_repository->hash_algo))
> +		die("mismatched object format: server %s; client %s\n",
> +		    the_repository->hash_algo->name,
> +		    hash_algos[client_hash_algo].name);

OK.