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

[Junio C Hamano <gitster@pobox.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.