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

[Jonathan Tan <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?

Yes.

> 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)?

Reading the documentation, the mere presence should be enough. Yes, I
think upload-pack will spend unnecessary cycles if the channel is
terminated halfway (and I don't know if we can prevent spending these
cycles, since I/O can be buffered). I think it should be possible for
the client to cut off when it sees shallow-info (besides the possible
wastage of cycles and I/O on the server's end).

Having said that, I think this different from the ls-remote case. There,
the server is awaiting another request from the user before sending more
information, but here, the server intends to send everything at once.