Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone

[Duy Nguyen <pclouds@gmail.com>]

On Sun, Jul 14, 2013 at 4:25 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> Since 52fed6e (receive-pack: check connectivity before concluding "git
>> push" - 2011-09-02), receive-pack is prepared to deal with broken
>> push, a shallow push can't cause any corruption. Update the document
>> to reflect that.
>
> Hmm, what happens when pushing to servers without that commit?  Do you
> think it should be applied to Debian squeeze for server operators that
> haven't upgraded yet to the current stable release?

This is pushing _from_ a shallow repo, where the sender repo's old
objects is a subset of the receiver's. Such a push may be safe even
without the above commit, if I don't miss anything. The commit is in
case I do miss something.

The situation is different when we push _to_ a shallow repo. The
sender in this case may have more old objects than the receiver. The
sender may choose to not send such old objects in the push, not
knowing that the receiver does not have them due to its shallow
nature. So incomplete push and could only be caught be the commit
above.

I don't know about Debian porting strategy, but I think that commit
should be back ported to the stable release. There are more git
clients out there than just C Git and some of them may have bugs. The
commit should help protect the server against buggy clients. If you
back port it, do d21c463 (fetch/receive: remove over-pessimistic
connectivity check - 2012-03-15) too to avoid performance regression
in large pushes/fetches.

>
> [...]
>> --- a/Documentation/git-clone.txt
>> +++ b/Documentation/git-clone.txt
>> @@ -182,11 +182,13 @@ objects from the source repository into a pack in the cloned repository.
>>  --depth <depth>::
>>       Create a 'shallow' clone with a history truncated to the
>>       specified number of revisions.  A shallow repository has a
>> -     number of limitations (you cannot clone or fetch from
>> -     it, nor push from nor into it), but is adequate if you
>> -     are only interested in the recent history of a large project
>> -     with a long history, and would want to send in fixes
>> -     as patches.
>> +     number of limitations (you cannot clone or fetch from it, nor
>> +     push into it), but is adequate if you are only interested in
>> +     the recent history of a large project with a long history.
>> ++
>> +Pushing from a shallow clone should be avoided if the git version on
>> +the receiver end is older than v1.7.10, or any other git
>> +implementation that does not perform connectivity check.
>
> "git name-rev --tags" tells me 52fed6e was applied during 1.7.8-rc0,
> so it might make sense to s/1.7.10/1.7.8/ here.

Yeah! I thought "git tag --contains" output was sorted and did not
read past the first few tags. Need to implement version sort for
--tags, or at least commit date sort..

> Aside from that nit,
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
>
> Thanks,
> Jonathan
--
Duy