{"thread":{"id":"34412","subject":"[PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","startedAt":"2013-07-12T05:37:42Z","lastAt":"2013-07-15T15:35:50Z","messageCount":11,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"223125","messageId":"1373607462-12836-1-git-send-email-pclouds@gmail.com","threadId":"34412","inReplyTo":null,"subject":"[PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-12T05:37:42Z","receivedAt":"2013-07-12T05:37:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The document says one cannot push from a shallow clone. But that is\nnot true (maybe it was at some point in the past). The client does not\nstop such a push nor does it give any indication to the receiver that\nthis is a shallow push. If the receiver accepts it, it's in.\n\nSince 52fed6e (receive-pack: check connectivity before concluding \"git\npush\" - 2011-09-02), receive-pack is prepared to deal with broken\npush, a shallow push can't cause any corruption. Update the document\nto reflect that.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n Documentation/git-clone.txt | 12 +++++++-----\n 1 file changed, 7 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 450f158..85769b8 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -182,11 +182,13 @@ objects from the source repository into a pack in the cloned repository.\n --depth ::\n \tCreate a 'shallow' clone with a history truncated to the\n \tspecified number of revisions. A shallow repository has a\n-\tnumber of limitations (you cannot clone or fetch from\n-\tit, nor push from nor into it), but is adequate if you\n-\tare only interested in the recent history of a large project\n-\twith a long history, and would want to send in fixes\n-\tas patches.\n+\tnumber of limitations (you cannot clone or fetch from it, nor\n+\tpush into it), but is adequate if you are only interested in\n+\tthe recent history of a large project with a long history.\n++\n+Pushing from a shallow clone should be avoided if the git version on\n+the receiver end is older than v1.7.10, or any other git\n+implementation that does not perform connectivity check.\n \n --[no-]single-branch::\n \tClone only the history leading to the tip of a single branch,\n-- \n1.8.2.82.gc24b958\n"},{"id":"223128","messageId":"7v38rkpbmr.fsf@alter.siamese.dyndns.org","threadId":"34412","inReplyTo":"1373607462-12836-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T05:53:32Z","receivedAt":"2013-07-12T05:53:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> The document says one cannot push from a shallow clone. But that is\n> not true (maybe it was at some point in the past). The client does not\n> stop such a push nor does it give any indication to the receiver that\n> this is a shallow push. If the receiver accepts it, it's in.\n>\n> Since 52fed6e (receive-pack: check connectivity before concluding \"git\n> push\" - 2011-09-02), receive-pack is prepared to deal with broken\n> push, a shallow push can't cause any corruption. Update the document\n> to reflect that.\n\nErr, what are these lines in builtin/receive-pack.c doing, then?\n\n int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n {\n ...\n setup_path();\n\n if (!enter_repo(dir, 0))\n die(\"'%s' does not appear to be a git repository\", dir);\n\n if (is_repository_shallow())\n die(\"attempt to push into a shallow repository\");\n ...\n"},{"id":"223129","messageId":"7vy59cnx06.fsf@alter.siamese.dyndns.org","threadId":"34412","inReplyTo":"1373607462-12836-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-12T05:54:49Z","receivedAt":"2013-07-12T05:54:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> +\tnumber of limitations (you cannot clone or fetch from it, nor\n> +\tpush into it), but is adequate if you are only interested in\n> +\tthe recent history of a large project with a long history.\n\nAhh, sorry for the noise. You still say you cannot push _into_ it.\n"},{"id":"223150","messageId":"CACsJy8BnOtgeRrJL5D0HQALstqNzcppabRoE7bYGG37kYz+GeA@mail.gmail.com","threadId":"34412","inReplyTo":"7vy59cnx06.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-12T07:54:35Z","receivedAt":"2013-07-12T07:54:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Jul 12, 2013 at 12:54 PM, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> + number of limitations (you cannot clone or fetch from it, nor\n>> + push into it), but is adequate if you are only interested in\n>> + the recent history of a large project with a long history.\n>\n> Ahh, sorry for the noise. You still say you cannot push _into_ it.\n\nYes, I actually removed that check in the first iteration of this\npatch, reasoning that it can't cause any harm and it mostly works. But\n\"mostly\" is not good enough. I will try remove it later if I can make\nit always work by extending git protocol a bit (I think full clone\npushing to shallow one can always work. Shallow pushing to shallow,\nprobably no such guarantee)\n--\nDuy\n"},{"id":"223256","messageId":"20130713212541.GA10951@google.com","threadId":"34412","inReplyTo":"1373607462-12836-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-07-13T21:25:41Z","receivedAt":"2013-07-13T21:25:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nNguyễn Thái Ngọc Duy wrote:\n\n> Since 52fed6e (receive-pack: check connectivity before concluding \"git\n> push\" - 2011-09-02), receive-pack is prepared to deal with broken\n> push, a shallow push can't cause any corruption. Update the document\n> to reflect that.\n\nHmm, what happens when pushing to servers without that commit? Do you\nthink it should be applied to Debian squeeze for server operators that\nhaven't upgraded yet to the current stable release?\n\n[...]\n> --- a/Documentation/git-clone.txt\n> +++ b/Documentation/git-clone.txt\n> @@ -182,11 +182,13 @@ objects from the source repository into a pack in the cloned repository.\n> --depth ::\n> \tCreate a 'shallow' clone with a history truncated to the\n> \tspecified number of revisions. A shallow repository has a\n> -\tnumber of limitations (you cannot clone or fetch from\n> -\tit, nor push from nor into it), but is adequate if you\n> -\tare only interested in the recent history of a large project\n> -\twith a long history, and would want to send in fixes\n> -\tas patches.\n> +\tnumber of limitations (you cannot clone or fetch from it, nor\n> +\tpush into it), but is adequate if you are only interested in\n> +\tthe recent history of a large project with a long history.\n> ++\n> +Pushing from a shallow clone should be avoided if the git version on\n> +the receiver end is older than v1.7.10, or any other git\n> +implementation that does not perform connectivity check.\n\n\"git name-rev --tags\" tells me 52fed6e was applied during 1.7.8-rc0,\nso it might make sense to s/1.7.10/1.7.8/ here.\n\nAside from that nit,\nReviewed-by: Jonathan Nieder \n\nThanks,\nJonathan\n"},{"id":"223260","messageId":"CACsJy8ATX8aJJ40sF5XSoVpy=X1ZTLj5qpthrTQ52fv40QNn7w@mail.gmail.com","threadId":"34412","inReplyTo":"20130713212541.GA10951@google.com","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-14T02:28:55Z","receivedAt":"2013-07-14T02:28:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Jul 14, 2013 at 4:25 AM, Jonathan Nieder wrote:\n> Hi,\n>\n> Nguyễn Thái Ngọc Duy wrote:\n>\n>> Since 52fed6e (receive-pack: check connectivity before concluding \"git\n>> push\" - 2011-09-02), receive-pack is prepared to deal with broken\n>> push, a shallow push can't cause any corruption. Update the document\n>> to reflect that.\n>\n> Hmm, what happens when pushing to servers without that commit? Do you\n> think it should be applied to Debian squeeze for server operators that\n> haven't upgraded yet to the current stable release?\n\nThis is pushing _from_ a shallow repo, where the sender repo's old\nobjects is a subset of the receiver's. Such a push may be safe even\nwithout the above commit, if I don't miss anything. The commit is in\ncase I do miss something.\n\nThe situation is different when we push _to_ a shallow repo. The\nsender in this case may have more old objects than the receiver. The\nsender may choose to not send such old objects in the push, not\nknowing that the receiver does not have them due to its shallow\nnature. So incomplete push and could only be caught be the commit\nabove.\n\nI don't know about Debian porting strategy, but I think that commit\nshould be back ported to the stable release. There are more git\nclients out there than just C Git and some of them may have bugs. The\ncommit should help protect the server against buggy clients. If you\nback port it, do d21c463 (fetch/receive: remove over-pessimistic\nconnectivity check - 2012-03-15) too to avoid performance regression\nin large pushes/fetches.\n\n>\n> [...]\n>> --- a/Documentation/git-clone.txt\n>> +++ b/Documentation/git-clone.txt\n>> @@ -182,11 +182,13 @@ objects from the source repository into a pack in the cloned repository.\n>> --depth ::\n>> Create a 'shallow' clone with a history truncated to the\n>> specified number of revisions. A shallow repository has a\n>> - number of limitations (you cannot clone or fetch from\n>> - it, nor push from nor into it), but is adequate if you\n>> - are only interested in the recent history of a large project\n>> - with a long history, and would want to send in fixes\n>> - as patches.\n>> + number of limitations (you cannot clone or fetch from it, nor\n>> + push into it), but is adequate if you are only interested in\n>> + the recent history of a large project with a long history.\n>> ++\n>> +Pushing from a shallow clone should be avoided if the git version on\n>> +the receiver end is older than v1.7.10, or any other git\n>> +implementation that does not perform connectivity check.\n>\n> \"git name-rev --tags\" tells me 52fed6e was applied during 1.7.8-rc0,\n> so it might make sense to s/1.7.10/1.7.8/ here.\n\nYeah! I thought \"git tag --contains\" output was sorted and did not\nread past the first few tags. Need to implement version sort for\n--tags, or at least commit date sort..\n\n> Aside from that nit,\n> Reviewed-by: Jonathan Nieder \n>\n> Thanks,\n> Jonathan\n--\nDuy\n"},{"id":"223342","messageId":"7vfvvhgeif.fsf@alter.siamese.dyndns.org","threadId":"34412","inReplyTo":"CACsJy8ATX8aJJ40sF5XSoVpy=X1ZTLj5qpthrTQ52fv40QNn7w@mail.gmail.com","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-14T18:52:56Z","receivedAt":"2013-07-14T18:52:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Sun, Jul 14, 2013 at 4:25 AM, Jonathan Nieder wrote:\n>> Hi,\n>>\n>> Nguyễn Thái Ngọc Duy wrote:\n>>\n>>> Since 52fed6e (receive-pack: check connectivity before concluding \"git\n>>> push\" - 2011-09-02), receive-pack is prepared to deal with broken\n>>> push, a shallow push can't cause any corruption. Update the document\n>>> to reflect that.\n>>\n>> Hmm, what happens when pushing to servers without that commit? Do you\n>> think it should be applied to Debian squeeze for server operators that\n>> haven't upgraded yet to the current stable release?\n>\n> This is pushing _from_ a shallow repo, where the sender repo's old\n> objects is a subset of the receiver's.\n\nThe \"subset\" assumption does not necessarily hold, does it? The\nreceiver may have rewound its tips and pruned since then.\n\nAlso, the sender may have cloned from the receiver (fully) and then\nfetched a different history shallowly from elsewhere. The receiver\nmay have no commit on that history, including the shallow-bottom.\n"},{"id":"223363","messageId":"CACsJy8Bx_o+9S1N_uieQaV8NBbc3T_MR_VCvF+fM==Dy-vt7tw@mail.gmail.com","threadId":"34412","inReplyTo":"7vfvvhgeif.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-15T01:01:40Z","receivedAt":"2013-07-15T01:01:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 15, 2013 at 1:52 AM, Junio C Hamano wrote:\n> Duy Nguyen writes:\n>\n>> On Sun, Jul 14, 2013 at 4:25 AM, Jonathan Nieder wrote:\n>>> Hi,\n>>>\n>>> Nguyễn Thái Ngọc Duy wrote:\n>>>\n>>>> Since 52fed6e (receive-pack: check connectivity before concluding \"git\n>>>> push\" - 2011-09-02), receive-pack is prepared to deal with broken\n>>>> push, a shallow push can't cause any corruption. Update the document\n>>>> to reflect that.\n>>>\n>>> Hmm, what happens when pushing to servers without that commit? Do you\n>>> think it should be applied to Debian squeeze for server operators that\n>>> haven't upgraded yet to the current stable release?\n>>\n>> This is pushing _from_ a shallow repo, where the sender repo's old\n>> objects is a subset of the receiver's.\n>\n> The \"subset\" assumption does not necessarily hold, does it? The\n> receiver may have rewound its tips and pruned since then.\n>\n> Also, the sender may have cloned from the receiver (fully) and then\n> fetched a different history shallowly from elsewhere. The receiver\n> may have no commit on that history, including the shallow-bottom.\n>\n\nHmm.. right. And the receiver needs to setup proper graft to seal the\nshallow bottom. So it's really not safe to do pushing from a shallow\nrepo without 52fed6e\n--\nDuy\n"},{"id":"223383","messageId":"CACsJy8DELmhBwBCk9hmg4EpQxGeCRDPaSEr5rYp=CiMjq8j54Q@mail.gmail.com","threadId":"34412","inReplyTo":"CACsJy8Bx_o+9S1N_uieQaV8NBbc3T_MR_VCvF+fM==Dy-vt7tw@mail.gmail.com","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-15T06:33:49Z","receivedAt":"2013-07-15T06:33:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jul 15, 2013 at 8:01 AM, Duy Nguyen wrote:\n>> Also, the sender may have cloned from the receiver (fully) and then\n>> fetched a different history shallowly from elsewhere. The receiver\n>> may have no commit on that history, including the shallow-bottom.\n>>\n>\n> Hmm.. right. And the receiver needs to setup proper graft to seal the\n> shallow bottom. So it's really not safe to do pushing from a shallow\n> repo without 52fed6e\n\nBecause this makes pushing from a shallow repo fall into \"mostly\nworks\" category, I withdraw this patch.\n--\nDuy\n"},{"id":"223440","messageId":"7voba3dem1.fsf@alter.siamese.dyndns.org","threadId":"34412","inReplyTo":"CACsJy8DELmhBwBCk9hmg4EpQxGeCRDPaSEr5rYp=CiMjq8j54Q@mail.gmail.com","subject":"Re: [PATCH] git-clone.txt: remove the restriction on pushing from a shallow clone","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-15T15:31:18Z","receivedAt":"2013-07-15T15:31:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Mon, Jul 15, 2013 at 8:01 AM, Duy Nguyen wrote:\n>>> Also, the sender may have cloned from the receiver (fully) and then\n>>> fetched a different history shallowly from elsewhere. The receiver\n>>> may have no commit on that history, including the shallow-bottom.\n>>>\n>>\n>> Hmm.. right. And the receiver needs to setup proper graft to seal the\n>> shallow bottom. So it's really not safe to do pushing from a shallow\n>> repo without 52fed6e\n>\n> Because this makes pushing from a shallow repo fall into \"mostly\n> works\" category, I withdraw this patch.\n\nYeah, \"reliably fails when it shouldn't\" may be safer than before\nthat patch, but the fact that you cannot push from a shallow one\nstill remains, so we probably shouldn't tell the users to use it.\n"},{"id":"223441","messageId":"7vk3krdeeh.fsf_-_@alter.siamese.dyndns.org","threadId":"34412","inReplyTo":"CACsJy8DELmhBwBCk9hmg4EpQxGeCRDPaSEr5rYp=CiMjq8j54Q@mail.gmail.com","subject":"[PATCH] Revert \"git-clone.txt: remove the restriction on pushing from a shallow clone\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-15T15:35:50Z","receivedAt":"2013-07-15T15:35:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This reverts commit dacd2bcc414e0b7aac36aaa400da0a743c4741cc.\n\n\"It fails reliably without corrupting the receiving repository when\nit should fail\" may be better than the situation before the receiving\nend was hardened recently, but the fact that sometimes the push does\nnot go through still remains. It is better to advice the users that\nthey cannot push from a shallow repository as a limitation before\nthey decide to use (or not to use) a shallow clone.\n---\n\n * Unfortunately I thought the documentation update was a\n no-brainer and already applied it, so here is a revert.\n\n Documentation/git-clone.txt | 12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 85769b8..450f158 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -182,13 +182,11 @@ objects from the source repository into a pack in the cloned repository.\n --depth ::\n \tCreate a 'shallow' clone with a history truncated to the\n \tspecified number of revisions. A shallow repository has a\n-\tnumber of limitations (you cannot clone or fetch from it, nor\n-\tpush into it), but is adequate if you are only interested in\n-\tthe recent history of a large project with a long history.\n-+\n-Pushing from a shallow clone should be avoided if the git version on\n-the receiver end is older than v1.7.10, or any other git\n-implementation that does not perform connectivity check.\n+\tnumber of limitations (you cannot clone or fetch from\n+\tit, nor push from nor into it), but is adequate if you\n+\tare only interested in the recent history of a large project\n+\twith a long history, and would want to send in fixes\n+\tas patches.\n \n --[no-]single-branch::\n \tClone only the history leading to the tip of a single branch,\n-- \n1.8.3.2-941-gda9c3c8\n"}]}