Re: [PATCH v4] builtin/clone.c: add --reject-shallow option

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2021, 23:59 UTC
Message-ID: <xmqqy2f3hjqf.fsf@gitster.c.googlers.com>
In-Reply-To: <20210301220319.3426185-1-jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> This is true with protocol v0, but protocol v2 bundles all shallow
> information (whether coming from the fact that the remote is shallow or
> the fact that the fetcher specified --depth etc.) and sends them
> together with the packfile.

By the above do you mean what happens in FETCH_GET_PACK arm where receive_shallow_info() is called when "shallow-info" header is seen, before the code continues to process wanted-refs, packfile-uris and then finally the packfile?

I do not think it makes much sense to ask any option to make us shallow (like --depth=<n>) while --reject-shallow is in use (after all, if the other side is deep enough to make us <n> commits deep, there is no reason to reject the other side as the source), so your "whether coming from the fact ..." part, while is a valid observation, can be ignored in practice (meaning: it is OK to make "--reject-shallow" be in effect only when we are trying to make a full clone, and reject combinations of it with --depth=<n> and such at the command parsing time).

> It may be possible to stop packfile download (saving bandwidth on
> the client side, at least) once such information is returned,
> though.

Just like "upload-pack" does not get upset by a client that comes only for the initial refs advertisement and disconnects without asking for any "want" (aka "ls-remote"), the server side should be prepared to see if the other side cuts off after seeing the "shallow-info" section header or after seeing the the whole "shallow-info" section, so we should be able to leave early without having to download the bulk data. If the "upload-pack" spends unnecessary cycles when it happens, then we need to fix that. Even if the "fetch" client does not willingly disconnect in the middle, the network disconnect may happen at any point in the exchange, and we'd need to be prepared for it.

Do we need to read and parse the "shallow-info" section, or would the mere presense of the section mean the other side knows this side needs to futz with the .git/shallow information (either because we asked to be made shallow with --depth and the like, or because we tried to clone from them and they are shallow)?

Thanks.