{"thread":{"id":"32549","subject":"[PATCH] Documentation on depth option in git clone.","startedAt":"2013-01-07T18:06:35Z","lastAt":"2013-07-11T15:49:20Z","messageCount":29,"participants":["Stefan Beller","Jonathan Nieder","Junio C Hamano","Duy Nguyen","Matthijs Kooijman"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"206243","messageId":"1357581996-17505-1-git-send-email-stefanbeller@googlemail.com","threadId":"32549","inReplyTo":null,"subject":"[PATCH] git clone depth of 0 not possible.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-01-07T18:06:35Z","receivedAt":"2013-01-07T18:06:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently it is not possible to have a shallow depth of\njust 0, i.e. only one commit in that repository after cloning.\nThe minimum number of commits is 2, caused by depth=1.\n\nI had no good idea how to add this behavior to git clone as\nthe depth variable in git_transport_options struct (file transport.h)\nuses value 0 for another meaning, so it would have need changes at\nall places, where the transport options depth is being used \n(e.g. fetch)\n\nSo I documented the current behavior, see attached patch.\n\nStefan Beller (1):\n Documentation on depth option in git clone.\n\n Documentation/git-clone.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\n-- \n1.8.1.80.g3e293fb.dirty\n"},{"id":"206239","messageId":"1357581996-17505-2-git-send-email-stefanbeller@googlemail.com","threadId":"32549","inReplyTo":"1357581996-17505-1-git-send-email-stefanbeller@googlemail.com","subject":"[PATCH] Documentation on depth option in git clone.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-01-07T18:06:36Z","receivedAt":"2013-01-07T18:06:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"---\n Documentation/git-clone.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex 7fefdb0..e76aa50 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -186,7 +186,8 @@ objects from the source repository into a pack in the cloned repository.\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+\tas patches. The depth should be at least 1. If it is 0 or\n+\tbelow, the cloned repository will not be shallow.\n \n --single-branch::\n \tClone only the history leading to the tip of a single branch,\n-- \n1.8.1.80.g3e293fb.dirty\n"},{"id":"206265","messageId":"20130108062811.GA3131@elie.Belkin","threadId":"32549","inReplyTo":"1357581996-17505-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-01-08T06:28:11Z","receivedAt":"2013-01-08T06:28:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n> Currently it is not possible to have a shallow depth of\n> just 0, i.e. only one commit in that repository after cloning.\n> The minimum number of commits is 2, caused by depth=1.\n\nSounds buggy. Would anything break if we were to make --depth=1 mean\n\"1 deep, including the tip commit\"?\n"},{"id":"206266","messageId":"7vip78go6b.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"20130108062811.GA3131@elie.Belkin","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-08T06:54:20Z","receivedAt":"2013-01-08T06:54:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Stefan Beller wrote:\n>\n>> Currently it is not possible to have a shallow depth of\n>> just 0, i.e. only one commit in that repository after cloning.\n>> The minimum number of commits is 2, caused by depth=1.\n>\n> Sounds buggy. Would anything break if we were to make --depth=1 mean\n> \"1 deep, including the tip commit\"?\n\nAs long as we do not change the meaning of the \"shallow\" count going\nover the wire (i.e. the number we receive from the user will be\nfudged, so that user's \"depth 1\" that used to mean \"the tip and one\nbehind it\" is expressed as \"depth 2\" at the end-user level, and we\nsend over the wire the number that corresponded to the old \"depth\n1\"), I do not think anything will break, and then --depth=0 may\nmagically start meaning \"only the tip; its immediate parents will\nnot be transferred and recorded as the shallow boundary in the\nreceiving repository\".\n\nI do not mind carrying such a (technially) backward incompatible\nchange in jn/clone-2.0-depth-off-by-one branch, keep it cooking in\n'next' for a while and push it out together with other \"2.0\" topics\nin a future release ;-).\n"},{"id":"206268","messageId":"CACsJy8B0ftDDagTpO4wh-LsBOBy+BhwhV=H-68U246Lq4=Ssfw@mail.gmail.com","threadId":"32549","inReplyTo":"1357581996-17505-1-git-send-email-stefanbeller@googlemail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-08T07:33:40Z","receivedAt":"2013-01-08T07:33:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jan 8, 2013 at 1:06 AM, Stefan Beller\n wrote:\n> Currently it is not possible to have a shallow depth of\n> just 0, i.e. only one commit in that repository after cloning.\n> The minimum number of commits is 2, caused by depth=1.\n>\n> I had no good idea how to add this behavior to git clone as\n> the depth variable in git_transport_options struct (file transport.h)\n> uses value 0 for another meaning, so it would have need changes at\n> all places, where the transport options depth is being used\n> (e.g. fetch)\n>\n> So I documented the current behavior, see attached patch.\n\nIf we choose not to do the off-by-one topic Junio suggested elsewhere\nin the same thread, I think this document patch should be turned into\ncode instead. Just reject --depth=0 with an explanation. Users who are\nhit by this will be caught without the need to read through the\ndocument.\n-- \nDuy\n"},{"id":"206269","messageId":"7vd2xggm8a.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"7vip78go6b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-08T07:36:21Z","receivedAt":"2013-01-08T07:36:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jonathan Nieder writes:\n>\n>> Stefan Beller wrote:\n>>\n>>> Currently it is not possible to have a shallow depth of\n>>> just 0, i.e. only one commit in that repository after cloning.\n>>> The minimum number of commits is 2, caused by depth=1.\n>>\n>> Sounds buggy. Would anything break if we were to make --depth=1 mean\n>> \"1 deep, including the tip commit\"?\n>\n> As long as we do not change the meaning of the \"shallow\" count going\n> over the wire (i.e. the number we receive from the user will be\n> fudged, so that user's \"depth 1\" that used to mean \"the tip and one\n> behind it\" is expressed as \"depth 2\" at the end-user level, and we\n> send over the wire the number that corresponded to the old \"depth\n> 1\"), I do not think anything will break, and then --depth=0 may\n> magically start meaning \"only the tip; its immediate parents will\n> not be transferred and recorded as the shallow boundary in the\n> receiving repository\".\n>\n> I do not mind carrying such a (technially) backward incompatible\n> change in jn/clone-2.0-depth-off-by-one branch, keep it cooking in\n> 'next' for a while and push it out together with other \"2.0\" topics\n> in a future release ;-).\n\nSpeaking of --depth, I think in Git 2.0 we should fix the semantics\nof \"deepening\" done with \"git fetch\".\n\nIts \"--depth\" parameter is used to specify the new depth of the\nhistory that you can tangle from the updated tip of remote tracking\nbranches, and it has a rather unpleasant ramifications.\n\nSuppose you start from \"git clone --depth=1 $there\". You have the\ntoday's snapshot, and one parent behind it. You keep working happily\nwith the code and then realize that you want to know a bit more\nhistory behind the snapshot you started from.\n\n (upstream)\n ---o---o---o---A---B\n\n (you)\n A---B\n\nSo you do:\n\n $ git fetch --depth=3\n\n (upstream)\n ---o---o---o---A---B---C---D---E---F---...---W---X---Y---Z\n\n (you)\n A---B W---X---Y---Z\n\nBut in the meantime, if the upstream accumulated 20+ commits, you\nend up getting the commit at the updated tip of the upstream, and 3\ngenerations of parents behind it. There will be a 10+ commit worth\nof gap between the bottom of the new shallow history and the old tip\nyou have been working on, and the history becomes disjoint.\n\nI think we need a protocol update to fix this; instead of sending\n\"Now I want your tips and N commits behind it, please update my\nshallow bottom accordingly\", which creates the above by giving you Z\nand 3 generations back and updates your cut-off point to W, the\nreceiving end should be able to ask \"I have a shallow history that\ncuts off at these commits. I want to get the history leading up to\nyour tips, and also deepen the history further back from my current\ncut-off points by N commits\", so that you would instead end up with\nsomething like this:\n\n (you)\n o---o---o---A---B---C---D---E---F---...---W---X---Y---Z\n\nThat is, truly \"deepen my history by 3\". We could call that \"git\nfetch --deepen=3\" or something.\n"},{"id":"206270","messageId":"7v8v84gm5v.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"CACsJy8B0ftDDagTpO4wh-LsBOBy+BhwhV=H-68U246Lq4=Ssfw@mail.gmail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-08T07:37:48Z","receivedAt":"2013-01-08T07:37:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> If we choose not to do the off-by-one topic Junio suggested elsewhere\n> in the same thread, I think this document patch should be turned into\n> code instead. Just reject --depth=0 with an explanation. Users who are\n> hit by this will be caught without the need to read through the\n> document.\n\nI thought --depth=0 was a way to explicitly say \"I do not want any\nshallow history\" so far, so we would need to be a bit more careful,\nthough.\n"},{"id":"206271","messageId":"CACsJy8D9+KHT=YfU0+rPCbs+AwxQOpfKzPChDhk8d-MMkRzZug@mail.gmail.com","threadId":"32549","inReplyTo":"7vip78go6b.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-08T07:38:27Z","receivedAt":"2013-01-08T07:38:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jan 8, 2013 at 1:54 PM, Junio C Hamano wrote:\n>> Sounds buggy. Would anything break if we were to make --depth=1 mean\n>> \"1 deep, including the tip commit\"?\n>\n> As long as we do not change the meaning of the \"shallow\" count going\n> over the wire (i.e. the number we receive from the user will be\n> fudged, so that user's \"depth 1\" that used to mean \"the tip and one\n> behind it\" is expressed as \"depth 2\" at the end-user level, and we\n> send over the wire the number that corresponded to the old \"depth\n> 1\"), I do not think anything will break, and then --depth=0 may\n> magically start meaning \"only the tip; its immediate parents will\n> not be transferred and recorded as the shallow boundary in the\n> receiving repository\".\n\nI'd rather we reserve 0 for unlimited fetch, something we haven't done\nso far [1]. And because \"unlimited clone\" with --depth does not make\nsense, --depth=0 should be rejected by git-clone.\n\n[1] If we don't want to break the protocol, we could make depth\n0xffffffff a special value as \"unlimited\" for newer git. Older git\nworks most of the time, until some project exceeds 4G commit depth\nhistory.\n-- \nDuy\n"},{"id":"206274","messageId":"7vvcb8f6aw.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"CACsJy8D9+KHT=YfU0+rPCbs+AwxQOpfKzPChDhk8d-MMkRzZug@mail.gmail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-08T08:05:43Z","receivedAt":"2013-01-08T08:05:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Jan 8, 2013 at 1:54 PM, Junio C Hamano wrote:\n>>> Sounds buggy. Would anything break if we were to make --depth=1 mean\n>>> \"1 deep, including the tip commit\"?\n>>\n>> As long as we do not change the meaning of the \"shallow\" count going\n>> over the wire (i.e. the number we receive from the user will be\n>> fudged, so that user's \"depth 1\" that used to mean \"the tip and one\n>> behind it\" is expressed as \"depth 2\" at the end-user level, and we\n>> send over the wire the number that corresponded to the old \"depth\n>> 1\"), I do not think anything will break, and then --depth=0 may\n>> magically start meaning \"only the tip; its immediate parents will\n>> not be transferred and recorded as the shallow boundary in the\n>> receiving repository\".\n>\n> I'd rather we reserve 0 for unlimited fetch, something we haven't done\n> so far [1]. And because \"unlimited clone\" with --depth does not make\n> sense, --depth=0 should be rejected by git-clone.\n\nI actually was thinking about changing --depth=1 to mean \"the tip,\nwith zero commits behind it\" (and that was consistent with my\ndescription of \"fudging\"), but ended up saying \"--depth=0\" by\nmistake. I too think \"--depth=0\" or \"--depth<0\" does not make\nsense, so we are in agreement.\n\nThanks for a sanity check.\n"},{"id":"206279","messageId":"7vobh0f5nc.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"7vd2xggm8a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-08T08:19:51Z","receivedAt":"2013-01-08T08:19:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I think we need a protocol update to fix this; instead of sending\n> \"Now I want your tips and N commits behind it, please update my\n> shallow bottom accordingly\", which creates the above by giving you Z\n> and 3 generations back and updates your cut-off point to W, the\n> receiving end should be able to ask \"I have a shallow history that\n> cuts off at these commits. I want to get the history leading up to\n> your tips, and also deepen the history further back from my current\n> cut-off points by N commits\", so that you would instead end up with\n> something like this:\n>\n> (you)\n> o---o---o---A---B---C---D---E---F---...---W---X---Y---Z\n>\n> That is, truly \"deepen my history by 3\". We could call that \"git\n> fetch --deepen=3\" or something.\n\nI take that back. If you start from\n\n> (upstream)\n> ---o---o---o---A---B\n>\n> (you)\n> A---B\n\nand you are interested in peeking the history a bit deeper, you\nshould be able to ask \"I have a shallow history that cuts off at\nthese commits. I want my history deepened by N commits. I do not\ncare where your current tips are, by the way.\" with\n\n git fetch --deepen=3 \n\nand end up with\n\n> (you)\n> o---o---o---A---B\n\nwithout getting the new history leading to the updated tip at the\nupstream. If you want the new history leading to the updated tip,\nyou can just say:\n\n git fetch\n\nwithout any --depth nor --deepen option to end up with:\n\n> (you)\n> A---B---C---D---E---F---...---W---X---Y---Z\n\ninstead.\n"},{"id":"206294","messageId":"CACsJy8BJ3eBv-wjq=eTzR4SeEXW2MF5k1w5SFRt7fWRU4vKb_Q@mail.gmail.com","threadId":"32549","inReplyTo":"7vd2xggm8a.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-08T14:28:41Z","receivedAt":"2013-01-08T14:28:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jan 8, 2013 at 2:36 PM, Junio C Hamano wrote:\n> Speaking of --depth, I think in Git 2.0 we should fix the semantics\n> of \"deepening\" done with \"git fetch\".\n\nSpeaking of 2.0, we should support depth per ref. Well we don't have\nto wait until 2.0 because we could just add shallow2 extension to the\npack protocol. We should also apply depth to new refs when fetching\nthem the first time.\n-- \nDuy\n"},{"id":"206296","messageId":"50EC2DE5.2050704@googlemail.com","threadId":"32549","inReplyTo":"CACsJy8BJ3eBv-wjq=eTzR4SeEXW2MF5k1w5SFRt7fWRU4vKb_Q@mail.gmail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-01-08T14:32:05Z","receivedAt":"2013-01-08T14:32:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 01/08/2013 03:28 PM, Duy Nguyen wrote:\n> On Tue, Jan 8, 2013 at 2:36 PM, Junio C Hamano wrote:\n>> Speaking of --depth, I think in Git 2.0 we should fix the semantics\n>> of \"deepening\" done with \"git fetch\".\n> \n> Speaking of 2.0, we should support depth per ref. Well we don't have\n> to wait until 2.0 because we could just add shallow2 extension to the\n> pack protocol. We should also apply depth to new refs when fetching\n> them the first time.\n> \n\nWould this mean I could do something along?\n$ git clone --since v1.8.0 git://github.com/gitster/git.git\n\nSo tags would be allowed as anchors?\n"},{"id":"206297","messageId":"CACsJy8AEb9JsDOZfrXEj3VdMJU4hozjuZHaundQQyDNtaDQeHw@mail.gmail.com","threadId":"32549","inReplyTo":"50EC2DE5.2050704@googlemail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-08T14:45:27Z","receivedAt":"2013-01-08T14:45:27Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Jan 8, 2013 at 9:32 PM, Stefan Beller\n wrote:\n> On 01/08/2013 03:28 PM, Duy Nguyen wrote:\n>> On Tue, Jan 8, 2013 at 2:36 PM, Junio C Hamano wrote:\n>>> Speaking of --depth, I think in Git 2.0 we should fix the semantics\n>>> of \"deepening\" done with \"git fetch\".\n>>\n>> Speaking of 2.0, we should support depth per ref. Well we don't have\n>> to wait until 2.0 because we could just add shallow2 extension to the\n>> pack protocol. We should also apply depth to new refs when fetching\n>> them the first time.\n>>\n>\n> Would this mean I could do something along?\n> $ git clone --since v1.8.0 git://github.com/gitster/git.git\n>\n> So tags would be allowed as anchors?\n\nNo. This is what I had in mind:\n\ngit clone --branch=master --depth=2 git.git # get branch master with depth 2\ngit fetch --depth=10 origin next # get branch next with depth 10\n # master's depth remains 2\ngit fetch origin # get (new) branch 'pu' with default depth 2\n\nBut your case is interesting. We could specify --depth=v1.8.0.. or\neven --depth=v1.8.0~200.. (200 commits before v1.8.0). Somebody may\neven go crazy and make --depth=v1.6.0..v1.8.0 work. --depth is\nprobably not the right name anymore. Any SHA-1 would be allowed as\nanchor. But I think we need to wait for reachability bitmap feature to\ncome first so that we can quickly verify the anchor is reachable from\nthe public refs.\n-- \nDuy\n"},{"id":"206306","messageId":"7vk3rnfv0f.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"50EC2DE5.2050704@googlemail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-08T17:24:16Z","receivedAt":"2013-01-08T17:24:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> On 01/08/2013 03:28 PM, Duy Nguyen wrote:\n>> On Tue, Jan 8, 2013 at 2:36 PM, Junio C Hamano wrote:\n>>> Speaking of --depth, I think in Git 2.0 we should fix the semantics\n>>> of \"deepening\" done with \"git fetch\".\n>> \n>> Speaking of 2.0, we should support depth per ref. Well we don't have\n>> to wait until 2.0 because we could just add shallow2 extension to the\n>> pack protocol. We should also apply depth to new refs when fetching\n>> them the first time.\n>\n> Would this mean I could do something along?\n> $ git clone --since v1.8.0 git://github.com/gitster/git.git\n>\n> So tags would be allowed as anchors?\n\nAs the end-user facing UI, I think it would be much easier to use\nfor users who want to get only the recent part of history that is\nrelevant to their development if you allowed them to ask \"starting\nfrom this one, I do not care anything older than that\" with such an\ninterface. The current \"count how many more generations you want\"\ninterface is crazy in that it forces you to count what you have not\neven seen; I suspect the only reason it was done in such a hacky\nmanner was implementation expediency.\n\nAt the syntax level, however, I do not think we can use --since\nthere, because the keyword has a very different meaning already.\n\nI personally do not think \"depth per ref\" deserves \"it would be nice\nto support in 2.0\", let alone \"2.0 *should* support\", label. Some\nmay find it an interesting mental exercise to think about corner\ncases it will introduce and have to deal with (e.g. you ask 100 from\nmaster and 2 from maint, but maint is behind master by less than\n100---what should happen?), but I do not particularly see any\npractical use cases, and I highly doubt that there is much value in\nbringing in extra complexity such a \"feature\" requires to do it\nright.\n"},{"id":"218636","messageId":"20130528091812.GG25742@login.drsnuggles.stderr.nl","threadId":"32549","inReplyTo":"7vvcb8f6aw.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-05-28T09:18:12Z","receivedAt":"2013-05-28T09:18:12Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hi Junio,\n\nI'm interested in getting a fetch tip commit only feature into git, I'll\nprobably look into creating a patch for this.\n\n> >>> Sounds buggy. Would anything break if we were to make --depth=1 mean\n> >>> \"1 deep, including the tip commit\"?\n> >>\n> >> As long as we do not change the meaning of the \"shallow\" count going\n> >> over the wire (i.e. the number we receive from the user will be\n> >> fudged, so that user's \"depth 1\" that used to mean \"the tip and one\n> >> behind it\" is expressed as \"depth 2\" at the end-user level, and we\n> >> send over the wire the number that corresponded to the old \"depth\n> >> 1\"), I do not think anything will break, and then --depth=0 may\n> >> magically start meaning \"only the tip; its immediate parents will\n> >> not be transferred and recorded as the shallow boundary in the\n> >> receiving repository\".\n> >\n> > I'd rather we reserve 0 for unlimited fetch, something we haven't done\n> > so far [1]. And because \"unlimited clone\" with --depth does not make\n> > sense, --depth=0 should be rejected by git-clone.\n> \n> I actually was thinking about changing --depth=1 to mean \"the tip,\n> with zero commits behind it\" (and that was consistent with my\n> description of \"fudging\"), but ended up saying \"--depth=0\" by\n> mistake. I too think \"--depth=0\" or \"--depth<0\" does not make\n> sense, so we are in agreement.\n\nDid you consider how to implement this? Looking at the code, it seems\nthe \"deepen\" parameter in the wire protocol now means:\n - 0: Do not change anything about the shallowness (i.e., fetch\n everything from the shallow root to the tip).\n - > 0: Create new shallow commits at depth commits below the tip (so\n depth == 1 means tip and one below).\n - INFINITE_DEPTH (0x7fffffff): Remove all shallowness and fetch\n complete history.\n\nGiven this, I'm not sure how one can express \"fetch the tip and nothing\nbelow that\", since depth == 0 already has a different meaning.\n\nOf course, one could using depth == 1 in this case to receive two\ncommits and then drop one, but this would seem a bit pointless to me\n(especially if the commit below the tip is very different from the tip\nleading to a lot of useless data transfer).\n\nOr did I misunderstand something here?\n\nGr.\n\nMatthijs\n"},{"id":"218671","messageId":"CAFzf2Xx2mMO5XJ8n1UsUMMpDvi+KMUt9DpRe80X4zpG=THxSPw@mail.gmail.com","threadId":"32549","inReplyTo":"20130528091812.GG25742@login.drsnuggles.stderr.nl","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-28T16:28:16Z","receivedAt":"2013-05-28T16:28:16Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthijs Kooijman wrote:\n\n> Did you consider how to implement this? Looking at the code, it seems\n> the \"deepen\" parameter in the wire protocol now means:\n> - 0: Do not change anything about the shallowness (i.e., fetch\n> everything from the shallow root to the tip).\n> - > 0: Create new shallow commits at depth commits below the tip (so\n> depth == 1 means tip and one below).\n> - INFINITE_DEPTH (0x7fffffff): Remove all shallowness and fetch\n> complete history.\n>\n> Given this, I'm not sure how one can express \"fetch the tip and nothing\n> below that\", since depth == 0 already has a different meaning.\n\nIf I remember correctly, what we discussed is just changing the\nprotocol to \"5 means a depth of 5\". The client already trusts what the\nserver provides.\n\nThanks and hope that helps,\nJonathan\n"},{"id":"218672","messageId":"CAFzf2XxT5eRNDGT7fEMNMi3aAxsbi4b8aBNx=Nj=b=ziEETm4g@mail.gmail.com","threadId":"32549","inReplyTo":"CAFzf2Xx2mMO5XJ8n1UsUMMpDvi+KMUt9DpRe80X4zpG=THxSPw@mail.gmail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-28T16:31:31Z","receivedAt":"2013-05-28T16:31:31Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> If I remember correctly, what we discussed is just changing the\n> protocol to \"5 means a depth of 5\". The client already trusts what the\n> server provides.\n\n(Or tweaking the protocol by adding a new capability, if unpredictable\nbehavior based on the version of the server won't fly. :))\n\nJonathan\n"},{"id":"218673","messageId":"20130528163416.GK25742@login.drsnuggles.stderr.nl","threadId":"32549","inReplyTo":"CAFzf2Xx2mMO5XJ8n1UsUMMpDvi+KMUt9DpRe80X4zpG=THxSPw@mail.gmail.com","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-05-28T16:34:17Z","receivedAt":"2013-05-28T16:34:17Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hi Jonathan,\n\n> > Did you consider how to implement this? Looking at the code, it seems\n> > the \"deepen\" parameter in the wire protocol now means:\n> > - 0: Do not change anything about the shallowness (i.e., fetch\n> > everything from the shallow root to the tip).\n> > - > 0: Create new shallow commits at depth commits below the tip (so\n> > depth == 1 means tip and one below).\n> > - INFINITE_DEPTH (0x7fffffff): Remove all shallowness and fetch\n> > complete history.\n> >\n> > Given this, I'm not sure how one can express \"fetch the tip and nothing\n> > below that\", since depth == 0 already has a different meaning.\n> \n> If I remember correctly, what we discussed is just changing the\n> protocol to \"5 means a depth of 5\".\n\nThe mail from Junio I replied to said:\n> >> As long as we do not change the meaning of the \"shallow\" count\n> >> going over the wire\n\nWhich seems to conflict with your suggestion. Or are the \"shallow count\"\nand the \"depth\" different things?\n\n> The client already trusts what the server provides.\nIn other words: we won't break existing clients if we suddenly send back\none less commit than before, since the client just sends over what it\nwants and then assumes that whatever it gets back is really what it\nwanted?\n\nGr.\n\nMatthijs\n"},{"id":"218674","messageId":"CAFzf2Xzm4rG0AFEui7iU56HqX0vciVwWTd=Yb+TXLSmBa=Vbjw@mail.gmail.com","threadId":"32549","inReplyTo":"20130528163416.GK25742@login.drsnuggles.stderr.nl","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-28T16:58:00Z","receivedAt":"2013-05-28T16:58:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Matthijs Kooijman wrote:\n\n> In other words: we won't break existing clients if we suddenly send back\n> one less commit than before, since the client just sends over what it\n> wants and then assumes that whatever it gets back is really what it\n> wanted?\n\nYes, depending on your definition of \"break\".\n\nAn advantage of that approach is that old clients would get the new,\nintuitive behavior without upgrading. A disadvantage is that it is a\nconfusing world where the same command produces different effects when\ncontacting different servers.\n"},{"id":"218678","messageId":"7va9nf2fyp.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"20130528091812.GG25742@login.drsnuggles.stderr.nl","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-28T17:04:46Z","receivedAt":"2013-05-28T17:04:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthijs Kooijman writes:\n\n> Did you consider how to implement this? Looking at the code, it seems\n> the \"deepen\" parameter in the wire protocol now means:\n> - 0: Do not change anything about the shallowness (i.e., fetch\n> everything from the shallow root to the tip).\n> - > 0: Create new shallow commits at depth commits below the tip (so\n> depth == 1 means tip and one below).\n> - INFINITE_DEPTH (0x7fffffff): Remove all shallowness and fetch\n> complete history.\n>\n> Given this, I'm not sure how one can express \"fetch the tip and nothing\n> below that\", since depth == 0 already has a different meaning.\n\nDoing it \"correctly\" (in the shorter term) would involve:\n\n - adding a capability on the sending side \"fixed-off-by-one-depth\"\n to the protocol, and teaching the sending side to advertise the\n capability;\n \n - teaching the requestor that got --depth=N from the end user to\n pay attention to the new capability in such a way that:\n\n - when talking to an old sender (i.e. without the off-by-one\n fix), send N-1 for N greater than 1. Punt on N==1;\n\n - when talking to a fixed sender, ask to enable the capability,\n and send N as is (including N==1).\n\n - teaching the sending side to see if the new behaviour to fix\n off-by-one is asked by the requestor, and stop at the correct\n number of commits, not oversending one more. Otherwise retain\n the old behaviour.\n\nIn the longer term, I think we should introduce a better deepening\nmechanism. Cf.\n\n http://thread.gmane.org/gmane.comp.version-control.git/212912/focus=212940\n\n> Of course, one could using depth == 1 in this case to receive two\n> commits and then drop one, but this would seem a bit pointless to me\n> (especially if the commit below the tip is very different from the tip\n> leading to a lot of useless data transfer).\n"},{"id":"218953","messageId":"20130530082322.GW25742@login.drsnuggles.stderr.nl","threadId":"32549","inReplyTo":"7va9nf2fyp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-05-30T08:23:22Z","receivedAt":"2013-05-30T08:23:22Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hi Junio,\n\nOn Tue, May 28, 2013 at 10:04:46AM -0700, Junio C Hamano wrote:\n> Matthijs Kooijman writes:\n> \n> > Did you consider how to implement this? Looking at the code, it seems\n> > the \"deepen\" parameter in the wire protocol now means:\n> > - 0: Do not change anything about the shallowness (i.e., fetch\n> > everything from the shallow root to the tip).\n> > - > 0: Create new shallow commits at depth commits below the tip (so\n> > depth == 1 means tip and one below).\n> > - INFINITE_DEPTH (0x7fffffff): Remove all shallowness and fetch\n> > complete history.\n> >\n> > Given this, I'm not sure how one can express \"fetch the tip and nothing\n> > below that\", since depth == 0 already has a different meaning.\n> \n> Doing it \"correctly\" (in the shorter term) would involve:\n\nGiven below suggestion, I take it you don't like what Jonathan proposed\n(changing the meaning of the deepen parameter in the protocol so that\nthe server effectively decides how to interpret --depth)?\n\n> - adding a capability on the sending side \"fixed-off-by-one-depth\"\n> to the protocol, and teaching the sending side to advertise the\n> capability;\n> \n> - teaching the sending side to see if the new behaviour to fix\n> off-by-one is asked by the requestor, and stop at the correct\n> number of commits, not oversending one more. Otherwise retain\n> the old behaviour.\nWe can implement these two in current git already, since they only\nadd to the protocol, not break it in an incompatible manner, right?\n\n> - teaching the requestor that got --depth=N from the end user to\n> pay attention to the new capability in such a way that:\n> \n> - when talking to an old sender (i.e. without the off-by-one\n> fix), send N-1 for N greater than 1. Punt on N==1;\n> \n> - when talking to a fixed sender, ask to enable the capability,\n> and send N as is (including N==1).\nAnd these should wait for git2, since they change the meaning of the\n--depth parameter? Or is this change ok for current git as well?\n\nWhat do you mean by \"punt\" exactly? Show an error to the user, saying\nonly depth >= 2 is supported?\n\n> In the longer term, I think we should introduce a better deepening\n> mechanism. Cf.\nEven when there will be a better deepening mechanism, the above is still\nuseful (passing --depth=1 serves to get just a single commit without\nhistory, which is a distinct usecase from deepening the history of an\nexisting shallow repository). In other words, I think the \"improved\ndeepening\" and \"fixed depth\" should be complementary features.\n\nGr.\n\nMatthijs\n"},{"id":"219182","messageId":"7vsj10nx4z.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"20130530082322.GW25742@login.drsnuggles.stderr.nl","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-02T19:14:04Z","receivedAt":"2013-06-02T19:14:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthijs Kooijman writes:\n\n>> Doing it \"correctly\" (in the shorter term) would involve:\n>\n> Given below suggestion, I take it you don't like what Jonathan proposed\n> (changing the meaning of the deepen parameter in the protocol so that\n> the server effectively decides how to interpret --depth)?\n\nCorrect.\n\n> We can implement these two in current git already, since they only\n> add to the protocol, not break it in an incompatible manner, right?\n\nCorrect.\n\n>> - teaching the requestor that got --depth=N from the end user to\n>> pay attention to the new capability in such a way that:\n>> \n>> - when talking to an old sender (i.e. without the off-by-one\n>> fix), send N-1 for N greater than 1. Punt on N==1;\n>> \n>> - when talking to a fixed sender, ask to enable the capability,\n>> and send N as is (including N==1).\n> And these should wait for git2, since they change the meaning of the\n> --depth parameter? Or is this change ok for current git as well?\n\nMy suggestion was based on the understanding that everybody agreed\nthat the current behaviour of --depth=1 to have one extra commit\nbehind the shallow \"snapshot\" aka \"poor-man's tarball\", is a *bug*\nto be fixed, so I didn't mean it as a \"backward incompatible change\"\nat all.\n\n> What do you mean by \"punt\" exactly?\n\nAs old senders can only send a history with 2 or more commits deep,\nit would be sensible for the receiver to warn the user that we are\nbuggily asking for one more than the user asked for to the sender,\nand fetch history with two commits. It would be a regression to\nerror it out.\n\n\n>> In the longer term, I think we should introduce a better deepening\n>> mechanism. Cf.\n> Even when there will be a better deepening mechanism, the above is still\n> useful (passing --depth=1 serves to get just a single commit without\n> history, which is a distinct usecase from deepening the history of an\n> existing shallow repository).\n\nCorrect. That is why I said \"in the longer term, we should\nintroduce\". Did I say \"introduce and replace with it\"?\n"},{"id":"222922","messageId":"20130709133542.GJ10217@login.drsnuggles.stderr.nl","threadId":"32549","inReplyTo":"7va9nf2fyp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-07-09T13:35:42Z","receivedAt":"2013-07-09T13:35:42Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hi Junio,\n\n> Doing it \"correctly\" (in the shorter term) would involve:\n> \n> - adding a capability on the sending side \"fixed-off-by-one-depth\"\n> to the protocol, and teaching the sending side to advertise the\n> capability;\n> \n> - teaching the requestor that got --depth=N from the end user to\n> pay attention to the new capability in such a way that:\n> \n> - when talking to an old sender (i.e. without the off-by-one\n> fix), send N-1 for N greater than 1. Punt on N==1;\n> \n> - when talking to a fixed sender, ask to enable the capability,\n> and send N as is (including N==1).\n> \n> - teaching the sending side to see if the new behaviour to fix\n> off-by-one is asked by the requestor, and stop at the correct\n> number of commits, not oversending one more. Otherwise retain\n> the old behaviour.\n\nWhile implementing the above, I noticed my fix now introduced an\noff-by-one error the other way. When investigating, I found this commit:\n\n\tcommit 682c7d2f1a2d1a5443777237450505738af2ff1a\n\tAuthor: Nguyễn Thái Ngọc Duy \n\tDate: Fri Jan 11 16:05:47 2013 +0700\n\n\t upload-pack: fix off-by-one depth calculation in shallow clone\n\n\t get_shallow_commits() is used to determine the cut points at a given\n\t depth (i.e. the number of commits in a chain that the user likes to\n\t get). However we count current depth up to the commit \"commit\" but we\n\t do the cutting at its parents (i.e. current depth + 1). This makes\n\t upload-pack always return one commit more than requested. This patch\n\t fixes it.\n\n\t Signed-off-by: Nguyễn Thái Ngọc Duy \n\t Signed-off-by: Junio C Hamano \n\nWhich actually seems to fix the off-by-one bug that is described in this\nthread, but without going through the hoops of preserving current\nbehaviour for older git versions (that is, it makes behaviour dependent\non server version instead of client version).\n\nDoes this mean the discussion in this thread is meaningless, or is that\ncommit not intended to be the final fix?\n\nIn any case, IIUC that particular patch makes a piece of the existing\ncode dead, which needs to be removed.\n\nGr.\n\nMatthijs\n"},{"id":"223049","messageId":"20130711105733.GG10217@login.drsnuggles.stderr.nl","threadId":"32549","inReplyTo":"20130709133542.GJ10217@login.drsnuggles.stderr.nl","subject":"Re: [PATCH] git clone depth of 0 not possible.","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-07-11T10:57:33Z","receivedAt":"2013-07-11T10:57:33Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hi Junio,\n\n> While implementing the above, I noticed my fix now introduced an\n> off-by-one error the other way. When investigating, I found this commit:\n> \n> \tcommit 682c7d2f1a2d1a5443777237450505738af2ff1a\n> \tAuthor: Nguyễn Thái Ngọc Duy \n> \tDate: Fri Jan 11 16:05:47 2013 +0700\n> \n> \t upload-pack: fix off-by-one depth calculation in shallow clone\n> \n> \t get_shallow_commits() is used to determine the cut points at a given\n> \t depth (i.e. the number of commits in a chain that the user likes to\n> \t get). However we count current depth up to the commit \"commit\" but we\n> \t do the cutting at its parents (i.e. current depth + 1). This makes\n> \t upload-pack always return one commit more than requested. This patch\n> \t fixes it.\n> \n> \t Signed-off-by: Nguyễn Thái Ngọc Duy \n> \t Signed-off-by: Junio C Hamano \n> \n> Which actually seems to fix the off-by-one bug that is described in this\n> thread, but without going through the hoops of preserving current\n> behaviour for older git versions (that is, it makes behaviour dependent\n> on server version instead of client version).\n> \n> Does this mean the discussion in this thread is meaningless, or is that\n> commit not intended to be the final fix?\nLooking more closely, I also see that the above change is already\nreleased in 1.8.2 versions. Given that, I don't think it makes sense to\nto still try to provide this capability to get backward compatible\nbehaviour, since this would cause a off-by-one error the other way when\ntalking to 1.8.2.x servers...\n\nHowever, since I pretty much finished the code for this, I'll send over\nthe patches and let you decide wether to include them or not. If you\nwant to include them but they need to be changed in some way, just let\nme know.\n\nThe first patch of the series should be merged regardless.\n\nGr.\n\nMatthijs\n"},{"id":"223054","messageId":"1373541954-16493-1-git-send-email-matthijs@stdin.nl","threadId":"32549","inReplyTo":"20130711105733.GG10217@login.drsnuggles.stderr.nl","subject":"[PATCH 1/3] upload-pack: Remove a piece of dead code","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-07-11T11:25:52Z","receivedAt":"2013-07-11T11:25:52Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Commit 682c7d2 (upload-pack: fix off-by-one depth calculation in shallow\nclone) introduced a new check in get_shallow_commits to decide when to\nstop traversing the history and mark the current commit as a shallow\nroot.\n\nWith this new check in place, the old check can no longer be true, since\nthe first check always fires first. This commit removes that check,\nmaking the code a bit more simple again.\n\nSigned-off-by: Matthijs Kooijman \n---\n shallow.c | 17 ++++++-----------\n 1 file changed, 6 insertions(+), 11 deletions(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex cbe2526..8a9c96d 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -110,17 +110,12 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,\n \t\t\t\t\tcontinue;\n \t\t\t\t*pointer = cur_depth;\n \t\t\t}\n-\t\t\tif (cur_depth < depth) {\n-\t\t\t\tif (p->next)\n-\t\t\t\t\tadd_object_array(&p->item->object,\n-\t\t\t\t\t\t\tNULL, &stack);\n-\t\t\t\telse {\n-\t\t\t\t\tcommit = p->item;\n-\t\t\t\t\tcur_depth = *(int *)commit->util;\n-\t\t\t\t}\n-\t\t\t} else {\n-\t\t\t\tcommit_list_insert(p->item, &result);\n-\t\t\t\tp->item->object.flags |= shallow_flag;\n+\t\t\tif (p->next)\n+\t\t\t\tadd_object_array(&p->item->object,\n+\t\t\t\t\t\tNULL, &stack);\n+\t\t\telse {\n+\t\t\t\tcommit = p->item;\n+\t\t\t\tcur_depth = *(int *)commit->util;\n \t\t\t}\n \t\t}\n \t}\n-- \n1.8.3.rc1\n"},{"id":"223055","messageId":"1373541954-16493-2-git-send-email-matthijs@stdin.nl","threadId":"32549","inReplyTo":"1373541954-16493-1-git-send-email-matthijs@stdin.nl","subject":"[PATCH 2/3] upload-pack: Introduce new \"fixed-off-by-one-depth\" server feature","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-07-11T11:25:53Z","receivedAt":"2013-07-11T11:25:53Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Commit 682c7d2 (upload-pack: fix off-by-one depth calculation in shallow\nclone) changed the meaning of the fetch depth sent over the wire to mean\nthe total number of commits to return, instead of the number of commits\nbeyond the first. However, when this change is deployed on some servers\nbut not others, this can cause a client to behave differently based on\nthe server version, which is unexpected.\n\nTo prevent this, the new, fixed, depth behaviour is advertised as a server\nfeature and the old behaviour is restored when the feature is not\nrequested by the client.\n\nSigned-off-by: Matthijs Kooijman \n---\n upload-pack.c | 11 +++++++++--\n 1 file changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 127e59a..59f43d1 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -46,6 +46,7 @@ static unsigned int timeout;\n static int use_sideband;\n static int advertise_refs;\n static int stateless_rpc;\n+static int fixed_depth;\n \n static void reset_timeout(void)\n {\n@@ -633,6 +634,8 @@ static void receive_needs(void)\n \t\t\tno_progress = 1;\n \t\tif (parse_feature_request(features, \"include-tag\"))\n \t\t\tuse_include_tag = 1;\n+\t\tif (parse_feature_request(features, \"fixed-off-by-one-depth\"))\n+\t\t\tfixed_depth = 1;\n \n \t\to = parse_object(sha1_buf);\n \t\tif (!o)\n@@ -669,10 +672,14 @@ static void receive_needs(void)\n \t\t\t\tstruct object *object = shallows.objects[i].item;\n \t\t\t\tobject->flags |= NOT_SHALLOW;\n \t\t\t}\n-\t\telse\n+\t\telse {\n+\t\t\t/* Emulate off-by-one bug in older versions */\n+\t\t\tif (!fixed_depth)\n+\t\t\t\tdepth++;\n \t\t\tbackup = result =\n \t\t\t\tget_shallow_commits(&want_obj, depth,\n \t\t\t\t\t\t SHALLOW, NOT_SHALLOW);\n+\t\t}\n \t\twhile (result) {\n \t\t\tstruct object *object = &result->item->object;\n \t\t\tif (!(object->flags & (CLIENT_SHALLOW|NOT_SHALLOW))) {\n@@ -738,7 +745,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo\n {\n \tstatic const char *capabilities = \"multi_ack thin-pack side-band\"\n \t\t\" side-band-64k ofs-delta shallow no-progress\"\n-\t\t\" include-tag multi_ack_detailed\";\n+\t\t\" include-tag multi_ack_detailed fixed-off-by-one-depth\";\n \tconst char *refname_nons = strip_namespace(refname);\n \tunsigned char peeled[20];\n \n-- \n1.8.3.rc1\n"},{"id":"223053","messageId":"1373541954-16493-3-git-send-email-matthijs@stdin.nl","threadId":"32549","inReplyTo":"1373541954-16493-1-git-send-email-matthijs@stdin.nl","subject":"[PATCH 3/3] fetch-pack: Request fixed-off-by-one-depth when available","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2013-07-11T11:25:54Z","receivedAt":"2013-07-11T11:25:54Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"This server feature changes the meaning of the fetch depth, allowing\nfetching only a single revision instead of at least two as before. To\nmake sure the behaviour only depends on the client version, the depth\nvalue sent over the wire is corrected depending on wether the server has\nthe fix.\n\nThere is one corner case: A server without the fix cannot send less than\n2 commmits, so when --depth=1 is specified a warning is shown and 2\ncommits are fetched instead of 1.\n\nSigned-off-by: Matthijs Kooijman \n---\n fetch-pack.c | 26 ++++++++++++++++++++++++--\n 1 file changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex abe5ffb..799b2c1 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -39,6 +39,7 @@ static int marked;\n \n static struct commit_list *rev_list;\n static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;\n+static int fixed_depth;\n \n static void rev_list_push(struct commit *commit, int mark)\n {\n@@ -327,6 +328,7 @@ static int find_common(struct fetch_pack_args *args,\n \t\t\tif (prefer_ofs_delta) strbuf_addstr(&c, \" ofs-delta\");\n \t\t\tif (agent_supported) strbuf_addf(&c, \" agent=%s\",\n \t\t\t\t\t\t\t git_user_agent_sanitized());\n+\t\t\tif (fixed_depth) strbuf_addstr(&c, \" fixed-off-by-one-depth\");\n \t\t\tpacket_buf_write(&req_buf, \"want %s%s\\n\", remote_hex, c.buf);\n \t\t\tstrbuf_release(&c);\n \t\t} else\n@@ -342,8 +344,23 @@ static int find_common(struct fetch_pack_args *args,\n \n \tif (is_repository_shallow())\n \t\twrite_shallow_commits(&req_buf, 1);\n-\tif (args->depth > 0)\n-\t\tpacket_buf_write(&req_buf, \"deepen %d\", args->depth);\n+\tif (args->depth > 0) {\n+\t\tif (!fixed_depth && args->depth == 1)\n+\t\t\twarning(\"Server does not support depth=1, using depth=2 instead\");\n+\t\tif (!fixed_depth && args->depth > 1) {\n+\t\t\t/* Old server that interprets \"deepen 1\" as\n+\t\t\t \"give me tip + 1 extra commit\" */\n+\t\t\tpacket_buf_write(&req_buf, \"deepen %d\", args->depth - 1);\n+\t\t} else if (!fixed_depth && args->depth == 1) {\n+\t\t\t/* Old servers cannot handle depth=1 (deepen=0\n+\t\t\t means don't change depth / full depth). */\n+\t\t\tpacket_buf_write(&req_buf, \"deepen 1\");\n+\t\t} else {\n+\t\t\t/* New server, send depth as-is */\n+\t\t\tpacket_buf_write(&req_buf, \"deepen %d\", args->depth);\n+\t\t}\n+\t}\n+\n \tpacket_buf_flush(&req_buf);\n \tstate_len = req_buf.len;\n \n@@ -874,6 +891,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args *args,\n \t\t\tfprintf(stderr, \"Server supports ofs-delta\\n\");\n \t} else\n \t\tprefer_ofs_delta = 0;\n+\tif (server_supports(\"fixed-off-by-one-depth\")) {\n+\t\tif (args->verbose)\n+\t\t\tfprintf(stderr, \"Server has fixed meaning of depth value\\n\");\n+\t\tfixed_depth = 1;\n+\t}\n \n \tif ((agent_feature = server_feature_value(\"agent\", &agent_len))) {\n \t\tagent_supported = 1;\n-- \n1.8.3.rc1\n"},{"id":"223061","messageId":"CACsJy8CazcJau0yTYSndbam_bUhZLS5f02p9WD0jjutHh1J6+A@mail.gmail.com","threadId":"32549","inReplyTo":"1373541954-16493-1-git-send-email-matthijs@stdin.nl","subject":"Re: [PATCH 1/3] upload-pack: Remove a piece of dead code","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-07-11T12:08:14Z","receivedAt":"2013-07-11T12:08:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jul 11, 2013 at 6:25 PM, Matthijs Kooijman wrote:\n> Commit 682c7d2 (upload-pack: fix off-by-one depth calculation in shallow\n> clone) introduced a new check in get_shallow_commits to decide when to\n> stop traversing the history and mark the current commit as a shallow\n> root.\n>\n> With this new check in place, the old check can no longer be true, since\n> the first check always fires first. This commit removes that check,\n> making the code a bit more simple again.\n\nTrue. Ack-by: me.\n\n> Signed-off-by: Matthijs Kooijman \n> ---\n> shallow.c | 17 ++++++-----------\n> 1 file changed, 6 insertions(+), 11 deletions(-)\n>\n> diff --git a/shallow.c b/shallow.c\n> index cbe2526..8a9c96d 100644\n> --- a/shallow.c\n> +++ b/shallow.c\n> @@ -110,17 +110,12 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,\n> continue;\n> *pointer = cur_depth;\n> }\n> - if (cur_depth < depth) {\n> - if (p->next)\n> - add_object_array(&p->item->object,\n> - NULL, &stack);\n> - else {\n> - commit = p->item;\n> - cur_depth = *(int *)commit->util;\n> - }\n> - } else {\n> - commit_list_insert(p->item, &result);\n> - p->item->object.flags |= shallow_flag;\n> + if (p->next)\n> + add_object_array(&p->item->object,\n> + NULL, &stack);\n> + else {\n> + commit = p->item;\n> + cur_depth = *(int *)commit->util;\n> }\n> }\n> }\n> --\n> 1.8.3.rc1\n>\n--\nDuy\n"},{"id":"223075","messageId":"7vy59drta7.fsf@alter.siamese.dyndns.org","threadId":"32549","inReplyTo":"CACsJy8CazcJau0yTYSndbam_bUhZLS5f02p9WD0jjutHh1J6+A@mail.gmail.com","subject":"Re: [PATCH 1/3] upload-pack: Remove a piece of dead code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-07-11T15:49:20Z","receivedAt":"2013-07-11T15:49:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Thu, Jul 11, 2013 at 6:25 PM, Matthijs Kooijman wrote:\n>> Commit 682c7d2 (upload-pack: fix off-by-one depth calculation in shallow\n>> clone) introduced a new check in get_shallow_commits to decide when to\n>> stop traversing the history and mark the current commit as a shallow\n>> root.\n>>\n>> With this new check in place, the old check can no longer be true, since\n>> the first check always fires first. This commit removes that check,\n>> making the code a bit more simple again.\n>\n> True. Ack-by: me.\n>\n>> Signed-off-by: Matthijs Kooijman \n\nYeah, thanks both. I tend to agree that 2 and 3 are the right\nchange that came too late after the ship sailed X-(.\n\n>> ---\n>> shallow.c | 17 ++++++-----------\n>> 1 file changed, 6 insertions(+), 11 deletions(-)\n>>\n>> diff --git a/shallow.c b/shallow.c\n>> index cbe2526..8a9c96d 100644\n>> --- a/shallow.c\n>> +++ b/shallow.c\n>> @@ -110,17 +110,12 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,\n>> continue;\n>> *pointer = cur_depth;\n>> }\n>> - if (cur_depth < depth) {\n>> - if (p->next)\n>> - add_object_array(&p->item->object,\n>> - NULL, &stack);\n>> - else {\n>> - commit = p->item;\n>> - cur_depth = *(int *)commit->util;\n>> - }\n>> - } else {\n>> - commit_list_insert(p->item, &result);\n>> - p->item->object.flags |= shallow_flag;\n>> + if (p->next)\n>> + add_object_array(&p->item->object,\n>> + NULL, &stack);\n>> + else {\n>> + commit = p->item;\n>> + cur_depth = *(int *)commit->util;\n>> }\n>> }\n>> }\n>> --\n>> 1.8.3.rc1\n>>\n> --\n> Duy\n"}]}