Re: [PATCH 1/2] fetch-pack: call prepare_shallow_info only if v0

[Jeff King <peff@peff.net>]

On Mon, Mar 25, 2019 at 01:43:22PM -0700, Jonathan Tan wrote:

> In fetch_pack(), be clearer that there is no shallow information before
> the fetch when v2 is used - memset the struct shallow_info to 0 instead
> of calling prepare_shallow_info().
> 
> This patch is in preparation for a future patch in which a v2 fetch
> might call prepare_shallow_info() after shallow info has been retrieved
> during the fetch, so I needed to ensure that prepare_shallow_info() is
> not called before the fetch.

Makes sense.

I wondered here:

> diff --git a/fetch-pack.c b/fetch-pack.c
> index e69993b2eb..a0eb268dfc 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1648,13 +1648,17 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
>  		packet_flush(fd[1]);
>  		die(_("no matching remote head"));
>  	}
> -	prepare_shallow_info(&si, shallow);
> -	if (version == protocol_v2)
> +	if (version == protocol_v2) {
> +		if (shallow && shallow->nr)
> +			BUG("Protocol V2 does not provide shallows at this point in the fetch");
> +		memset(&si, 0, sizeof(si));
>  		ref_cpy = do_fetch_pack_v2(args, fd, ref, sought, nr_sought,
>  					   pack_lockfile);

...who actually might set "shallow". Specifically, I was curious whether
a v2 server could erroneously send us shallow lines, which would trigger
our BUG(), when in fact we should be complaining about the server.

But the answer is no, they have no opportunity. AFAICT, this would only
be set by a call to get_remote_heads(), which we do only for v0/v1
protocols. So there's no way to trigger the BUG. Good.

-Peff