[jc: I've CC'ed Jonathan Tan, who is much more knowledgeable than I
am on the transport layer issues, to sanity check my assumption]
"Li Linchao via GitGitGadget" [off-list ref] writes:
quoted hunk
@@ -1363,6 +1384,12 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
goto cleanup;
}
+ if (reject_shallow) {
+ if (local_shallow || is_repository_shallow(the_repository)) {
This may reject to clone from a shallow repository, but at this
point the bulk of the tranfer from the origin repository has already
happened, no? Rejecting after transferring many megabytes feels a
bit too late. That is one of the reasons why I kept hinting that
the transport layer needs to be taught an option to reject talking
to a shallow counterpart if we want to add this feature [*1*].
Also, wouldn't "clone --depth=1 --reject-shallow" from a repository
that is not shallow make the_repository a shallow one at this point
and makes it fail? If the goal of the --reject-shallow option were
to make sure the resulting repository is not shallow, then that is a
technically correct implementation (even though it is wasteful to
transfer a full tree worth of megabytes and then abort), but is the
feature is explained to reject cloning from a shallow one, then
users would be suprised to see ...
+ die(_("source repository is shallow, reject to clone."));
... this message, when cloning from well known publich repositories
that are not shallow.
I think cloning with --depth=<n> when the source repository is deep
enough, should be allowed, so the cleanest solution for the latter
may be to notice the combination of options that make the resulting
repository shallow (I mentioned --depth=<n>, but there may be others)
and the --reject-shallow option and error out before even talking
to the other side at the time we parse the command line.
Thanks.
[Footnote]
*1* Looking at Documentation/technical/pack-protocol.txt, "git
fetch" seem to learn if the repository is shallow immediately
upon contacting "upload-pack" during the Reference Discovery
phase (we'd see 'shallow' packets if they are shallow). I
suspect that the right solution would be to teach the codepath
on the "git fetch" side that accepts, parses, and acts on this
packet to optionally stop communication and error out when the
caller asks not to talk with a shallow repository.
This may reject to clone from a shallow repository, but at this
point the bulk of the tranfer from the origin repository has already
happened, no? Rejecting after transferring many megabytes feels a
bit too late. That is one of the reasons why I kept hinting that
the transport layer needs to be taught an option to reject talking
to a shallow counterpart if we want to add this feature [*1*].
Extending the transport layer in this way might not be too difficult in
the case of native (SSH, git://) protocols and using protocol v0, since
handshake() in transport.c (called indirectly from
transport_get_remote_refs()) writes shallow information to a data
structure that we could potentially expose for the caller to use (before
it calls transport_fetch_refs(). I couldn't see how remote-using
protocols (e.g. HTTP) communicate shallow information, though
(remote-curl.c seems to just keep it for itself), so that will be a more
difficult task. And of course there's the matter of protocol v2, which I
discuss below.
[Footnote]
*1* Looking at Documentation/technical/pack-protocol.txt, "git
fetch" seem to learn if the repository is shallow immediately
upon contacting "upload-pack" during the Reference Discovery
phase (we'd see 'shallow' packets if they are shallow). I
suspect that the right solution would be to teach the codepath
on the "git fetch" side that accepts, parses, and acts on this
packet to optionally stop communication and error out when the
caller asks not to talk with a shallow repository.
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. It may be possible to stop packfile download
(saving bandwidth on the client side, at least) once such information is
returned, though.
--------------
lilinchao
quoted
This may reject to clone from a shallow repository, but at this
point the bulk of the tranfer from the origin repository has already
happened, no? Rejecting after transferring many megabytes feels a
bit too late. That is one of the reasons why I kept hinting that
the transport layer needs to be taught an option to reject talking
to a shallow counterpart if we want to add this feature [*1*].
Extending the transport layer in this way might not be too difficult in
the case of native (SSH, git://) protocols and using protocol v0, since
handshake() in transport.c (called indirectly from
transport_get_remote_refs()) writes shallow information to a data
structure that we could potentially expose for the caller to use (before
it calls transport_fetch_refs(). I couldn't see how remote-using
protocols (e.g. HTTP) communicate shallow information, though
(remote-curl.c seems to just keep it for itself), so that will be a more
difficult task. And of course there's the matter of protocol v2, which I
discuss below.
These discussions were based on PATCH_v4, which is quiet immature then,
In the latest PATCH_v5, for the case of native protocols(ssh, git, file://), they
will eventually call do_fetch_pack_v2() if it's protocol v2, and then will call
receive_shallow_info() for the case FETCH_GET_PACK, so this is the place
I made the change.
As for HTTPs protocl, as long as it's still smart protocol, which means do not
fallback to dumb protocol, it will also call do_fetch_pack_v2(), and go to
"check shallow info" trigger in receive_shallow_info().
So, base on PATCH_V5, I have tested protocol v2, which goes like this:
First, I created a shallow repo on gitlab and gitee respectively.
Then tried to clone them in my terminal, (in order not to look too verbose,
I ommited the result when GIT_TRACE_PACKET is on).
$ ./git-clone --reject-shallow https://gitlab.com/Cactusinhand/rugged.git
Cloning into 'rugged'...
fatal: source repository is shallow, reject to clone.
$ ./git-clone --reject-shallow ssh://gitlab.com/Cactusinhand/rugged.git
Cloning into 'rugged'...
fatal: source repository is shallow, reject to clone.
$ ./git-clone --reject-shallow git@gitlab.com:Cactusinhand/rugged.git
Cloning into 'rugged'...
fatal: source repository is shallow, reject to clone.
$ ./git-clone --reject-shallow https://gitee.com/cactusinhand/rugged.git
Cloning into 'rugged'...
fatal: source repository is shallow, reject to clone.
$ ./git-clone --reject-shallow ssh://gitee.com/cactusinhand/rugged.git
Cloning into 'rugged'...
fatal: source repository is shallow, reject to clone.
$ ./git-clone --reject-shallow git@gitee.com:cactusinhand/rugged.git
Cloning into 'rugged'...
fatal: source repository is shallow, reject to clone.
I haven't tested protocol v1, but I've made the change in do_fetch_pack(),
which is for protocol version 0 and version 1.
I hope you can review the latest patch, and give me some suggestions.
Thanks!
quoted
[Footnote]
*1* Looking at Documentation/technical/pack-protocol.txt, "git
fetch" seem to learn if the repository is shallow immediately
upon contacting "upload-pack" during the Reference Discovery
phase (we'd see 'shallow' packets if they are shallow). I
suspect that the right solution would be to teach the codepath
on the "git fetch" side that accepts, parses, and acts on this
packet to optionally stop communication and error out when the
caller asks not to talk with a shallow repository.
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. It may be possible to stop packfile download
(saving bandwidth on the client side, at least) once such information is
returned, though.