{"thread":{"id":"27558","subject":"[PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","startedAt":"2011-06-06T17:26:44Z","lastAt":"2011-06-07T23:01:22Z","messageCount":8,"participants":["Alexander Neronskiy","Junio C Hamano","Alex Neronskiy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"169379","messageId":"BANLkTi=SVZPebW2YXRnaLvkxEDGy_rrtJ3jayt8Oco6Sn8hciQ@mail.gmail.com","threadId":"27558","inReplyTo":null,"subject":"[PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Alexander Neronskiy","fromEmail":"zakmagnus@google.com","sentAt":"2011-06-06T17:26:44Z","receivedAt":"2011-06-06T17:26:44Z","isPatch":true,"sender":{"key":"zakmagnus@google.com","avatar":null},"body":"Explain the exchange that occurs between a client and server when\nthe client is requesting shallow history and/or is already using\na shallow repository.\n\nSigned-off-by: Alex Neronskiy <zakmagnus@google.com>\n---\n Documentation/technical/pack-protocol.txt |   87 ++++++++++++++++++++++-------\n 1 files changed, 66 insertions(+), 21 deletions(-)\n\ndiff --git a/Documentation/technical/pack-protocol.txt\nb/Documentation/technical/pack-protocol.txt\nindex 369f91d..f576386 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -187,26 +187,28 @@ server determine what the minimal packfile\nnecessary for transport is.\n\n Once the client has the initial list of references that the server\n has, as well as the list of capabilities, it will begin telling the\n-server what objects it wants and what objects it has, so the server\n-can make a packfile that only contains the objects that the client needs.\n-The client will also send a list of the capabilities it wants to be in\n-effect, out of what the server said it could do with the first 'want' line.\n+server what objects it wants, its shallow objects (if any), and the\n+maximum commit depth it wants (if any).  The client will also send a\n+list of the capabilities it wants to be in effect, out of what the\n+server said it could do with the first 'want' line.\n\n ----\n   upload-request    =  want-list\n-                      have-list\n-                      compute-end\n+                      *shallow-line\n+                      *1depth-request\n+                      flush-pkt\n\n   want-list         =  first-want\n                       *additional-want\n-                      flush-pkt\n+\n+  shallow-line      =  PKT_LINE(\"shallow\" SP obj-id)\n+\n+  depth-request     =  PKT_LINE(\"deepen\" SP depth)\n\n   first-want        =  PKT-LINE(\"want\" SP obj-id SP capability-list LF)\n   additional-want   =  PKT-LINE(\"want\" SP obj-id LF)\n\n-  have-list         =  *have-line\n-  have-line         =  PKT-LINE(\"have\" SP obj-id LF)\n-  compute-end       =  flush-pkt / PKT-LINE(\"done\")\n+  depth             =  1*DIGIT\n ----\n\n Clients MUST send all the obj-ids it wants from the reference\n@@ -215,21 +217,64 @@ discovery phase as 'want' lines. Clients MUST\nsend at least one\n obj-id in a 'want' command which did not appear in the response\n obtained through ref discovery.\n\n-If client is requesting a shallow clone, it will now send a 'deepen'\n-line with the depth it is requesting.\n+The client MUST write all obj-ids which it only has shallow copies\n+of (meaning that it does not have the parents of a commit) as\n+'shallow' lines so that the server is aware of the limitations of\n+the client's history. Clients MUST NOT mention an obj-id which\n+it does not know exists on the server.\n+\n+The client now sends the maximum commit history depth it wants for\n+this transaction, which is the number of commits it wants from the\n+tip of the history, if any, as a 'deepen' line.  A depth of 0 is the\n+same as not making a depth request. The client does not want to receive\n+any commits beyond this depth, nor objects needed only to complete\n+those commits. Commits whose parents are not received as a result are\n+marked as shallow.\n+\n+Once all the 'want's and 'shallow's (and optional 'deepen') are\n+transferred, clients MUST send a flush-pkt. If the client has all\n+the references on the server, and as much of their commit history\n+as it is interested in, client flushes and disconnects.\n+\n+Otherwise, if the client sent a positive depth request, the server\n+will determine which commits will and will not be shallow and\n+send this information to the client. If the client did not request\n+a positive depth, this step is skipped.\n\n-Once all the \"want\"s (and optional 'deepen') are transferred,\n-clients MUST send a flush-pkt. If the client has all the references\n-on the server, client flushes and disconnects.\n+----\n+  shallow-update    =  *shallow-line\n+                      *unshallow-line\n+                      flush-pkt\n\n-TODO: shallow/unshallow response and document the deepen command in the ABNF.\n+  shallow-line     =  PKT-LINE(\"shallow\" SP obj-id)\n+\n+  unshallow-line   =  PKT-LINE(\"unshallow\" SP obj-id)\n+----\n+\n+If the client has requested a positive depth, the server will compute\n+the set of commits which are no deeper than the desired depth, starting\n+at the client's wants. The server writes 'shallow' lines for each\n+commit whose parents will not be sent as a result. The server writes\n+an 'unshallow' line for each commit which the client has indicated is\n+shallow, but is no longer shallow at the currently requested depth\n+(that is, its parents will now be sent). The server MUST NOT mark\n+as unshallow anything which the client has not indicated was shallow.\n\n Now the client will send a list of the obj-ids it has using 'have'\n-lines.  In multi_ack mode, the canonical implementation will send up\n-to 32 of these at a time, then will send a flush-pkt.  The canonical\n-implementation will skip ahead and send the next 32 immediately,\n-so that there is always a block of 32 \"in-flight on the wire\" at a\n-time.\n+lines, so the server can make a packfile that only contains the objects\n+that the client needs. In multi_ack mode, the canonical implementation\n+will send up to 32 of these at a time, then will send a flush-pkt. The\n+canonical implementation will skip ahead and send the next 32 immediately,\n+so that there is always a block of 32 \"in-flight on the wire\" at a time.\n+\n+----\n+  upload-haves      =  have-list\n+                      compute-end\n+\n+  have-list         =  *have-line\n+  have-line         =  PKT-LINE(\"have\" SP obj-id LF)\n+  compute-end       =  flush-pkt / PKT-LINE(\"done\")\n+----\n\n If the server reads 'have' lines, it then will respond by ACKing any\n of the obj-ids the client said it had that the server also has. The\n--\n1.7.3.1\n"},{"id":"169388","messageId":"7vvcwi95yi.fsf@alter.siamese.dyndns.org","threadId":"27558","inReplyTo":"BANLkTi=SVZPebW2YXRnaLvkxEDGy_rrtJ3jayt8Oco6Sn8hciQ@mail.gmail.com","subject":"Re: [PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-06T18:21:09Z","receivedAt":"2011-06-06T18:21:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Neronskiy <zakmagnus@google.com> writes:\n\n> Explain the exchange that occurs between a client and server when\n> the client is requesting shallow history and/or is already using\n> a shallow repository.\n>\n> Signed-off-by: Alex Neronskiy <zakmagnus@google.com>\n> ---\n>  Documentation/technical/pack-protocol.txt |   87 ++++++++++++++++++++++-------\n\nHmmmm, why is this patch riddled with these &nbsp;s?\n\n> diff --git a/Documentation/technical/pack-protocol.txt\n> b/Documentation/technical/pack-protocol.txt\n> index 369f91d..f576386 100644\n> --- a/Documentation/technical/pack-protocol.txt\n> +++ b/Documentation/technical/pack-protocol.txt\n> @@ -187,26 +187,28 @@ server determine what the minimal packfile\n> necessary for transport is.\n\nLinewrapped, perhaps by your MUA.\n\n>  Once the client has the initial list of references that the server\n>  has, as well as the list of capabilities, it will begin telling the\n> -server what objects it wants and what objects it has, so the server\n> -can make a packfile that only contains the objects that the client needs.\n> -The client will also send a list of the capabilities it wants to be in\n> -effect, out of what the server said it could do with the first 'want' line.\n> +server what objects it wants, its shallow objects (if any), and the\n> +maximum commit depth it wants (if any).  The client will also send a\n> +list of the capabilities it wants to be in effect, out of what the\n> +server said it could do with the first 'want' line.\n>\n>  ----\n>    upload-request    =  want-list\n> -                      have-list\n> -                      compute-end\n> +                      *shallow-line\n> +                      *1depth-request\n> +                      flush-pkt\n>\n>    want-list         =  first-want\n>                        *additional-want\n> -                      flush-pkt\n> +\n> +  shallow-line      =  PKT_LINE(\"shallow\" SP obj-id)\n> +\n> +  depth-request     =  PKT_LINE(\"deepen\" SP depth)\n>\n>    first-want        =  PKT-LINE(\"want\" SP obj-id SP capability-list LF)\n>    additional-want   =  PKT-LINE(\"want\" SP obj-id LF)\n>\n> -  have-list         =  *have-line\n> -  have-line         =  PKT-LINE(\"have\" SP obj-id LF)\n> -  compute-end       =  flush-pkt / PKT-LINE(\"done\")\n> +  depth             =  1*DIGIT\n>  ----\n\nThis change splits the earlier \"upload-request\" that consisted want-list,\nhave-list and compute-end into two phases.  The first phase described\nabove is where the client tells the server what it wants, and what it\ndoesn't have (by giving the \"shallow\" boundaries), and possibly limits the\n\"wants\" by depth. The second phase is described much later and consists of\nthe \"have\" and \"done\", which comes after the \"shallow-update\" phase (whose\ndescription is new).\n\nI think the separation makes sense, as there is a lot to talk about what\nhappens during this phase.\n\n>  Clients MUST send all the obj-ids it wants from the reference\n> @@ -215,21 +217,64 @@ discovery phase as 'want' lines. Clients MUST\n> send at least one\n>  obj-id in a 'want' command which did not appear in the response\n>  obtained through ref discovery.\n>\n> -If client is requesting a shallow clone, it will now send a 'deepen'\n> -line with the depth it is requesting.\n> +The client MUST write all obj-ids which it only has shallow copies\n> +of (meaning that it does not have the parents of a commit) as\n> +'shallow' lines so that the server is aware of the limitations of\n> +the client's history. Clients MUST NOT mention an obj-id which\n> +it does not know exists on the server.\n> +\n> +The client now sends the maximum commit history depth it wants for\n> +this transaction, which is the number of commits it wants from the\n> +tip of the history, if any, as a 'deepen' line.  A depth of 0 is the\n> +same as not making a depth request. The client does not want to receive\n> +any commits beyond this depth, nor objects needed only to complete\n> +those commits. Commits whose parents are not received as a result are\n> +marked as shallow.\n\n... on the server end and will be sent back in the shallow-update phase\nbelow.\n\n> +Once all the 'want's and 'shallow's (and optional 'deepen') are\n> +transferred, clients MUST send a flush-pkt. If the client has all\n> +the references on the server, and as much of their commit history\n> +as it is interested in, client flushes and disconnects.\n\nHmmmmm, are you describing \"everything-local then flush and all-done\" in\ndo_fetch_pack() with the second sentence? If so, placing the description\nhere is misleading. In that case, I do not think any of the find-common\nexchange starting from the \"upload-request\" phase happens.\n\n> +Otherwise, if the client sent a positive depth request, the server\n> +will determine which commits will and will not be shallow and\n> +send this information to the client. If the client did not request\n> +a positive depth, this step is skipped.\n> -Once all the \"want\"s (and optional 'deepen') are transferred,\n> -clients MUST send a flush-pkt. If the client has all the references\n> -on the server, client flushes and disconnects.\n> +----\n> +  shallow-update    =  *shallow-line\n> +                      *unshallow-line\n> +                      flush-pkt\n> ...\n\nThis is not a complaint to this patch, but I had to read the above twice\nto realize that the paragraph \"Otherwise...\" is not a continuation of the\ndetailed discussion of the \"upload-request\" phase, but is a preamble to\nthe next \"shallow-update\" phase. It might make sense to give a subsection\nheading to each of the phases, like...\n\n    Packfile Negotiation\n\n    1. upload-request phase\n\n       After reference and capabilities... (preamble for this phase)\n\n       ----\n         upload-request = want-list ... ABNF\n       ----\n\n       The client MUST send all the ... (detailed description of this\n       phase)\n\n    2. shallow-update phase\n\n       When the client sent a positive depth request, the server will\n       determine ... (preamble for this phase)\n\n       ----\n         shallow-update = *shallow-line ... ABNF\n       ----\n\n       ... detailed description of this phase ...\n\n    3. common ancestor discovery phase\n\n       Now the client will send a list of ...\n\n       ... likewise ...\n\nThanks.\n"},{"id":"169399","messageId":"loom.20110606T213817-376@post.gmane.org","threadId":"27558","inReplyTo":"7vvcwi95yi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Alex Neronskiy","fromEmail":"zakmagnus@google.com","sentAt":"2011-06-06T19:56:00Z","receivedAt":"2011-06-06T19:56:00Z","isPatch":true,"sender":{"key":"zakmagnus@google.com","avatar":null},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n> > +Once all the 'want's and 'shallow's (and optional 'deepen') are\n> > +transferred, clients MUST send a flush-pkt. If the client has all\n> > +the references on the server, and as much of their commit history\n> > +as it is interested in, client flushes and disconnects.\n> \n> Hmmmmm, are you describing \"everything-local then flush and all-done\" in\n> do_fetch_pack() with the second sentence? If so, placing the description\n> here is misleading. In that case, I do not think any of the find-common\n> exchange starting from the \"upload-request\" phase happens.\n\nNo, this refers to the same event which was already described in that document,\nwhich I believe happens from inside find_common. It may just be some confusion\non the meaning of \"having a reference\" on my part, but the idea was to point out\nthat the client could flush at this stage even if it doesn't have every commit.\n\nI tried to amend the existing wording but I suppose it was just misleading, so\nit's better to write something else entirely.\n"},{"id":"169502","messageId":"7v1uz55r24.fsf@alter.siamese.dyndns.org","threadId":"27558","inReplyTo":"loom.20110606T213817-376@post.gmane.org","subject":"Re: [PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-07T20:23:31Z","receivedAt":"2011-06-07T20:23:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Neronskiy <zakmagnus@google.com> writes:\n\n> Junio C Hamano <gitster <at> pobox.com> writes:\n>\n>> > +Once all the 'want's and 'shallow's (and optional 'deepen') are\n>> > +transferred, clients MUST send a flush-pkt. If the client has all\n>> > +the references on the server, and as much of their commit history\n>> > +as it is interested in, client flushes and disconnects.\n>> \n>> Hmmmmm, are you describing \"everything-local then flush and all-done\" in\n>> do_fetch_pack() with the second sentence? If so, placing the description\n>> here is misleading. In that case, I do not think any of the find-common\n>> exchange starting from the \"upload-request\" phase happens.\n>\n> No, this refers to the same event which was already described in that document,\n> which I believe happens from inside find_common.\n\n\"The same event which was already described in that document\" meaning at\nthe beginning of \"Packfile Negotiation\" section?  That is primarily about\nthe \"ls-remote\" that probed the server for the list of current refs, which\nis received in connect.c::get_remote_heads(), but it also covers another\ncase. When fetching, after connect.c::get_remote_heads() finds the list of\ncurrent refs, do_fetch_pack() is called, and then everything_local() in it\nchecks if we have all the objects we are going to ask. If so, we flush and\njump to all_done to terminate the connection, skipping find_common(),\nwithout doing any of the want/shallow/depth/etc.\n\nI don't seem to be able to find where in find_common() and its callee we\ncould quit without telling the server anything (unless we crash ;-). Even\nif get_rev() loop finds nothing, we would at least say \"done\".\n"},{"id":"169506","messageId":"loom.20110607T224355-216@post.gmane.org","threadId":"27558","inReplyTo":"7v1uz55r24.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Alex Neronskiy","fromEmail":"zakmagnus@google.com","sentAt":"2011-06-07T20:47:34Z","receivedAt":"2011-06-07T20:47:34Z","isPatch":true,"sender":{"key":"zakmagnus@google.com","avatar":null},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n> \"The same event which was already described in that document\" meaning at\n> the beginning of \"Packfile Negotiation\" section?  That is primarily about\n> the \"ls-remote\" that probed the server for the list of current refs, which\n> is received in connect.c::get_remote_heads(), but it also covers another\n> case. When fetching, after connect.c::get_remote_heads() finds the list of\n> current refs, do_fetch_pack() is called, and then everything_local() in it\n> checks if we have all the objects we are going to ask. If so, we flush and\n> jump to all_done to terminate the connection, skipping find_common(),\n> without doing any of the want/shallow/depth/etc.\n> \n> I don't seem to be able to find where in find_common() and its callee we\n> could quit without telling the server anything (unless we crash . Even\n> if get_rev() loop finds nothing, we would at least say \"done\".\n> \n\nThe part of the document I'm referring to starts at line 221 and reads: \n\n Once all the \"want\"s (and optional 'deepen') are transferred,\n clients MUST send a flush-pkt. If the client has all the references\n on the server, client flushes and disconnects.\n\nAnd I believe this refers to the code path beginning at line 308 of fetch-pack.c:\n\n        if (!fetching) {\n                strbuf_release(&req_buf);\n                packet_flush(fd[1]);\n                return 1;\n        }\n\nAm I wrong? \n"},{"id":"169510","messageId":"7vr57547sj.fsf@alter.siamese.dyndns.org","threadId":"27558","inReplyTo":"loom.20110607T224355-216@post.gmane.org","subject":"Re: [PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-07T22:05:00Z","receivedAt":"2011-06-07T22:05:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Neronskiy <zakmagnus@google.com> writes:\n\n> The part of the document I'm referring to starts at line 221 and reads: \n>\n>  Once all the \"want\"s (and optional 'deepen') are transferred,\n>  clients MUST send a flush-pkt. If the client has all the references\n>  on the server, client flushes and disconnects.\n>\n> And I believe this refers to the code path beginning at line 308 of fetch-pack.c:\n>\n>         if (!fetching) {\n>                 strbuf_release(&req_buf);\n>                 packet_flush(fd[1]);\n>                 return 1;\n>         }\n>\n> Am I wrong? \n\nAh, I overlooked that codepath, but if that if statement triggered, that\nwould mean fetching is still 0, which in turn means that you never sent\nany \"want\", so \"Once all the 'want's' (and optional 'deepen') are\ntransferred\" is not even true, is it?\n"},{"id":"169514","messageId":"loom.20110608T001220-765@post.gmane.org","threadId":"27558","inReplyTo":"7vr57547sj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Alex Neronskiy","fromEmail":"zakmagnus@google.com","sentAt":"2011-06-07T22:31:12Z","receivedAt":"2011-06-07T22:31:12Z","isPatch":true,"sender":{"key":"zakmagnus@google.com","avatar":null},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n> Ah, I overlooked that codepath, but if that if statement triggered, that\n> would mean fetching is still 0, which in turn means that you never sent\n> any \"want\", so \"Once all the 'want's' (and optional 'deepen') are\n> transferred\" is not even true, is it?\n\nIf you want to get pedantic, it IS true that \"all\" the 'want's are sent; the\ncorrect set of wants to send just happens to be empty. What DOES seem incorrect\nis the part about the 'deepen' (as well as the 'shallow's I'm proposing to add);\nthat part of the code isn't even reached if this termination happens. So, either\nI'm mistaken and that is NOT the right codepath, or this is a mistake already\npresent in the documentation. \n\nAlternatively, the problem there is that it's just deceptively worded. It\nimplies that a 'deepen' can be sent and that termination can still happen\nafterwards; but I don't believe this is possible. If a depth argument is\npresent, everything_local is not called and COMPLETEness is not set, so it's\nimpossible to skip any refs except in the corner case where there aren't any to\nbegin with. The second version of this patch addresses this better.\n"},{"id":"169516","messageId":"7vlixd456l.fsf@alter.siamese.dyndns.org","threadId":"27558","inReplyTo":"7vr57547sj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Document the underlying protocol used by shallow repositories and --depth commands.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-07T23:01:22Z","receivedAt":"2011-06-07T23:01:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Alex Neronskiy <zakmagnus@google.com> writes:\n>\n>> The part of the document I'm referring to starts at line 221 and reads: \n>>\n>>  Once all the \"want\"s (and optional 'deepen') are transferred,\n>>  clients MUST send a flush-pkt. If the client has all the references\n>>  on the server, client flushes and disconnects.\n>>\n>> And I believe this refers to the code path beginning at line 308 of fetch-pack.c:\n>>\n>>         if (!fetching) {\n>>                 strbuf_release(&req_buf);\n>>                 packet_flush(fd[1]);\n>>                 return 1;\n>>         }\n>>\n>> Am I wrong? \n>\n> Ah, I overlooked that codepath, but if that if statement triggered, that\n> would mean fetching is still 0, which in turn means that you never sent\n> any \"want\", so \"Once all the 'want's' (and optional 'deepen') are\n> transferred\" is not even true, is it?\n\nAlso I suspect that this might not even trigger. fetching will stay 0 only\nwhen all the refs->old_sha1 refer to commits already marked as COMPLETE, but\neverything_local() would already have covered that case and returned 1.\n\nThe code dates back to 2759cbc (git-fetch-pack: avoid unnecessary zero\npacking, 2005-10-18) and I tend to trust what Linus wrote, but in that\nmuch simpler version of the code, I think the check is redundant.\n\nIn either case, I think what the server sees (in other words, what goes\nover the wire at the protocol level) is the same. The client receives\nreferences and capabilities, and sends the flush without giving any \"want\"\nto the server.\n\nAnd that is exactly what is described in the first paragraph of \"Packfile\nNegotiation\" section.\n\nI am inclined to conclude that the original documentation \"If the client\nhas all the references on the server, client flushes and disconnects\",\nwhile technically not incorrect, is redundant (because that particular\nbehaviour on-the-wire is already spelled out in the first paragraph), and\nmisleading (because that condition is determined by the client without\nsending \"want\" etc., but the second paragraph \"Once the client has ..., it\nwill begin telling the server...\" that comes way before this \"client can\nquit without requesting anything\" gives a false impression that the client\nwill spend all these effort of sending \"want\" and can flush and disconnect.\n\nIn fact the original does not say the client should not ask for objects it\nalready has (as COMPLETE) with \"want\", and as you noticed, the expression\n\"has all the references\" is probably the source of confusion. It should\nhave said \"If the client did not send any \"want\", it may flush and\ndisconnect\".  Both the original and your update will allow an incorrectly\nimplemented client to send \"want\"s, realize that it does not want any, and\nthen flush to disconnect, but then the server will have to try to produce\na pack to send to the client, which is certainly not what we wanted to\nspecify.\n\nSo I would suggest removing the later part of the paragraph, and update\nthe first paragraph of the section instead.\n\nPerhaps like this...\n\n Documentation/technical/pack-protocol.txt |   21 +++++++++++----------\n 1 files changed, 11 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt\nindex 369f91d..f860f2a 100644\n--- a/Documentation/technical/pack-protocol.txt\n+++ b/Documentation/technical/pack-protocol.txt\n@@ -179,14 +179,15 @@ and descriptions.\n \n Packfile Negotiation\n --------------------\n-After reference and capabilities discovery, the client can decide\n-to terminate the connection by sending a flush-pkt, telling the\n-server it can now gracefully terminate (as happens with the ls-remote\n-command) or it can enter the negotiation phase, where the client and\n-server determine what the minimal packfile necessary for transport is.\n-\n-Once the client has the initial list of references that the server\n-has, as well as the list of capabilities, it will begin telling the\n+After reference and capabilities discovery, the client can decide to\n+terminate the connection by sending a flush-pkt, telling the server it can\n+now gracefully terminate, and disconnect, when it does not need any pack\n+data. This can happen with the ls-remote command, and also can happen when\n+the client already is up-to-date.\n+\n+Otherwise, it enters the negotiation phase, where the client and\n+server determine what the minimal packfile necessary for transport is,\n+by telling the\n server what objects it wants and what objects it has, so the server\n can make a packfile that only contains the objects that the client needs.\n The client will also send a list of the capabilities it wants to be in\n@@ -219,8 +220,8 @@ If client is requesting a shallow clone, it will now send a 'deepen'\n line with the depth it is requesting.\n \n Once all the \"want\"s (and optional 'deepen') are transferred,\n-clients MUST send a flush-pkt. If the client has all the references\n-on the server, client flushes and disconnects.\n+clients MUST send a flush-pkt, to tell the server side that it is\n+done sending the list.\n \n TODO: shallow/unshallow response and document the deepen command in the ABNF.\n \n"}]}