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

2013-07-15 Thread Duy Nguyen
On Mon, Jul 15, 2013 at 8:01 AM, Duy Nguyen pclo...@gmail.com wrote:
 Also, the sender may have cloned from the receiver (fully) and then
 fetched a different history shallowly from elsewhere.  The receiver
 may have no commit on that history, including the shallow-bottom.


 Hmm.. right. And the receiver needs to setup proper graft to seal the
 shallow bottom. So it's really not safe to do pushing from a shallow
 repo without 52fed6e

Because this makes pushing from a shallow repo fall into mostly
works category, I withdraw this patch.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-15 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Mon, Jul 15, 2013 at 8:01 AM, Duy Nguyen pclo...@gmail.com wrote:
 Also, the sender may have cloned from the receiver (fully) and then
 fetched a different history shallowly from elsewhere.  The receiver
 may have no commit on that history, including the shallow-bottom.


 Hmm.. right. And the receiver needs to setup proper graft to seal the
 shallow bottom. So it's really not safe to do pushing from a shallow
 repo without 52fed6e

 Because this makes pushing from a shallow repo fall into mostly
 works category, I withdraw this patch.

Yeah, reliably fails when it shouldn't may be safer than before
that patch, but the fact that you cannot push from a shallow one
still remains, so we probably shouldn't tell the users to use it.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-14 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Sun, Jul 14, 2013 at 4:25 AM, Jonathan Nieder jrnie...@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.

The subset assumption does not necessarily hold, does it?  The
receiver may have rewound its tips and pruned since then.

Also, the sender may have cloned from the receiver (fully) and then
fetched a different history shallowly from elsewhere.  The receiver
may have no commit on that history, including the shallow-bottom.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-14 Thread Duy Nguyen
On Mon, Jul 15, 2013 at 1:52 AM, Junio C Hamano gits...@pobox.com wrote:
 Duy Nguyen pclo...@gmail.com writes:

 On Sun, Jul 14, 2013 at 4:25 AM, Jonathan Nieder jrnie...@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.

 The subset assumption does not necessarily hold, does it?  The
 receiver may have rewound its tips and pruned since then.

 Also, the sender may have cloned from the receiver (fully) and then
 fetched a different history shallowly from elsewhere.  The receiver
 may have no commit on that history, including the shallow-bottom.


Hmm.. right. And the receiver needs to setup proper graft to seal the
shallow bottom. So it's really not safe to do pushing from a shallow
repo without 52fed6e
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-13 Thread Jonathan Nieder
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?

[...]
 --- 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.

Aside from that nit,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-13 Thread Duy Nguyen
On Sun, Jul 14, 2013 at 4:25 AM, Jonathan Nieder jrnie...@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 jrnie...@gmail.com

 Thanks,
 Jonathan
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-12 Thread Duy Nguyen
On Fri, Jul 12, 2013 at 12:54 PM, Junio C Hamano gits...@pobox.com wrote:
 Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 + 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.

 Ahh, sorry for the noise.  You still say you cannot push _into_ it.

Yes, I actually removed that check in the first iteration of this
patch, reasoning that it can't cause any harm and it mostly works. But
mostly is not good enough. I will try remove it later if I can make
it always work by extending git protocol a bit (I think full clone
pushing to shallow one can always work. Shallow pushing to shallow,
probably no such guarantee)
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-11 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 The document says one cannot push from a shallow clone. But that is
 not true (maybe it was at some point in the past). The client does not
 stop such a push nor does it give any indication to the receiver that
 this is a shallow push. If the receiver accepts it, it's in.

 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.

Err, what are these lines in builtin/receive-pack.c doing, then?

int cmd_receive_pack(int argc, const char **argv, const char *prefix)
{
...
setup_path();

if (!enter_repo(dir, 0))
die('%s' does not appear to be a git repository, dir);

if (is_repository_shallow())
die(attempt to push into a shallow repository);
...

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-07-11 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 + 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.

Ahh, sorry for the noise.  You still say you cannot push _into_ it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html