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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2021, 16:27 UTC
Message-ID: <xmqq5z2z51nb.fsf@gitster.c.googlers.com>
In-Reply-To: <5c4295e46b7f11eb8acc0024e87935e7@oschina.cn>

"lilinchao@oschina.cn" <lilinchao@oschina.cn> writes:

> Sorry, you may forget there is a re-read git-config under these lines

Ahhhh, thanks. You're right that "clone" is "special" in that way and requires unusual order---I forgot about that.

Show 20 quoted lines

>>This is only for cloning from a local repository, no?  IOW, path at
>>this point may even be "git://github.com/git/git/" and checking with
>>access() does not make sense.
>>
>>Ah, it is even worse.  get_repo_path() can return NULL, so mkpath()
>>will crash in such a case.  This must be at least
>>
>>	if (path && !access(mkpath("%s/shallow", path), F_OK))
>>	is_shallow = 1;
>> ...
>>
>>So, I think the above two hunks are making the code worse.  If we
>>are to detect and reject cloning from the shallow repository when
>>going through the transport layer (i.e. "--no-local" or cloning from
>>"git://github.com/git/git", or "https://github.com/git/git", if it
>>were a shallow repository), that must be handled separately.
>> 
> Sorry, I made the question simple. Reject cloning a shallow repository
> should apply to all four type transport protocols. There still a bunch of
> work to be done.

Sorry, but I didn't realize that this is just a work-in-progress that shows an early "only local transport is covered" preview. I think, modulo that the access(mkpath()) thing should be in the "if (is_local)" block, the patch covers the case of local transport well enough. I think by "four types of transport" you mean ssh://, git:// https:// etc., but they all go through the same transport API, so hopefully once one of them, say, git://, is covered, that would take us a long way.

Thanks.