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

2 messages, 2 authors, 2021-03-04 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2021-03-04 00:26:55

Jonathan Tan [off-list ref] 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.

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

From: Jonathan Tan <hidden>
Date: 2021-03-04 01:54:13

Jonathan Tan [off-list ref] writes:
quoted
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).
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help