{"thread":{"id":"35289","subject":"[PATCH 1/2] receive-pack: advertise thin-pack","startedAt":"2013-11-06T15:04:21Z","lastAt":"2013-11-23T15:09:22Z","messageCount":10,"participants":["Carlos Martín Nieto","Junio C Hamano","Jeff King","Shawn Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"230140","messageId":"1383750263-32495-1-git-send-email-cmn@elego.de","threadId":"35289","inReplyTo":null,"subject":"[PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2013-11-06T15:04:21Z","receivedAt":"2013-11-06T15:04:21Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Hi all,\n\nThis comes as a result of the discussion starting at [0] about\ngit-push assuming that a server will always support thin packs. Most\nout there in fact do, but this isn't necessarily the case.\n\nSome implementations may not have support for it yet, or the server\nmight be running in an environment where it is not feasible for it to\ntry to fill in the missing objects.\n\nJonathan and Duy mentioned that separate patches for receive-pack and\nsend-pack could let us work around adding this at such a late stage,\nso here they are. The second patch can maybe lie in waiting for a\nwhile.\n\n\n[0] http://thread.gmane.org/gmane.comp.version-control.git/235766/focus=236402\n\nCarlos Martín Nieto (2):\n receive-pack: advertise thin-pack\n send-pack: only send a thin pack if the server supports it\n\n Documentation/technical/protocol-capabilities.txt | 20 +++++++++++++++-----\n builtin/receive-pack.c | 2 +-\n send-pack.c | 2 ++\n 3 files changed, 18 insertions(+), 6 deletions(-)\n\n-- \n1.8.4.652.g0d6e0ce\n"},{"id":"230139","messageId":"1383750263-32495-2-git-send-email-cmn@elego.de","threadId":"35289","inReplyTo":"1383750263-32495-1-git-send-email-cmn@elego.de","subject":"[PATCH 1/2] receive-pack: advertise thin-pack","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2013-11-06T15:04:22Z","receivedAt":"2013-11-06T15:04:22Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"upload-pack has long advertised thin-pack, letting the clients request\nthese smaller packs. The client however unconditionally assumes that a\nserver is able to fix thin packs and there is no way of telling the\nclient that this is in fact not the case.\n\nMake receive-pack advertise 'thin-pack' in anticipation of the client\ntoggling the assumption and document this capability when used by\nreceive-pack.\n\nSigned-off-by: Carlos Martín Nieto \n---\n Documentation/technical/protocol-capabilities.txt | 20 +++++++++++++++-----\n builtin/receive-pack.c | 2 +-\n 2 files changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt\nindex fd8ffa5..4e96d51 100644\n--- a/Documentation/technical/protocol-capabilities.txt\n+++ b/Documentation/technical/protocol-capabilities.txt\n@@ -72,15 +72,25 @@ interleaved with S-R-Q.\n thin-pack\n ---------\n \n-This capability means that the server can send a 'thin' pack, a pack\n-which does not contain base objects; if those base objects are available\n-on client side. Client requests 'thin-pack' capability when it\n-understands how to \"thicken\" it by adding required delta bases making\n-it self-contained.\n+A thin pack is one with deltas which reference base objects not\n+contained within the pack (but are known to exist at the receiving\n+end). This can reduce the network traffic significantly, but it\n+requires the receiving end to know how to \"thicken\" these packs by\n+adding the missing bases to the pack.\n+\n+The upload-pack server advertises 'thin-pack' when it can generate and\n+send a thin pack. The receive-pack server advertises 'thin-pack' when\n+it knows how to \"thicken\" the pack it receives.\n+\n+Likewise, the client requests the 'thin-pack' capability when it\n+understands how to \"thicken\" it.\n \n Client MUST NOT request 'thin-pack' capability if it cannot turn a thin\n pack into a self-contained pack.\n \n+Client MUST NOT send a thin pack if the server does not advertise this\n+capability.\n+\n \n side-band, side-band-64k\n ------------------------\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3eb5fc..0e35c02 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -132,7 +132,7 @@ static void show_ref(const char *path, const unsigned char *sha1)\n \telse\n \t\tpacket_write(1, \"%s %s%c%s%s agent=%s\\n\",\n \t\t\t sha1_to_hex(sha1), path, 0,\n-\t\t\t \" report-status delete-refs side-band-64k quiet\",\n+\t\t\t \" report-status delete-refs side-band-64k quiet thin-pack\",\n \t\t\t prefer_ofs_delta ? \" ofs-delta\" : \"\",\n \t\t\t git_user_agent_sanitized());\n \tsent_capabilities = 1;\n-- \n1.8.4.652.g0d6e0ce\n"},{"id":"230141","messageId":"1383750263-32495-3-git-send-email-cmn@elego.de","threadId":"35289","inReplyTo":"1383750263-32495-1-git-send-email-cmn@elego.de","subject":"[PATCH 2/2] send-pack: only send a thin pack if the server supports it","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2013-11-06T15:04:23Z","receivedAt":"2013-11-06T15:04:23Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"In combination a the previous patch making receive-pack advertise the\nthin-pack capability, this allows git to push to a server in a\nconstrained environment which is not able to fix thin packs while taking\nadvantage of the feature for servers which can.\n\nSigned-off-by: Carlos Martín Nieto \n---\n send-pack.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/send-pack.c b/send-pack.c\nindex 7d172ef..7b88ac8 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args,\n \t\tquiet_supported = 1;\n \tif (server_supports(\"agent\"))\n \t\tagent_supported = 1;\n+\tif (!server_supports(\"thin-pack\"))\n+\t\targs->use_thin_pack = 0;\n \n \tif (!remote_refs) {\n \t\tfprintf(stderr, \"No refs in common and none specified; doing nothing.\\n\"\n-- \n1.8.4.652.g0d6e0ce\n"},{"id":"230154","messageId":"xmqqbo1x8e60.fsf@gitster.dls.corp.google.com","threadId":"35289","inReplyTo":"1383750263-32495-1-git-send-email-cmn@elego.de","subject":"Re: [PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-06T20:32:07Z","receivedAt":"2013-11-06T20:32:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto writes:\n\n> Hi all,\n>\n> This comes as a result of the discussion starting at [0] about\n> git-push assuming that a server will always support thin packs. Most\n> out there in fact do, but this isn't necessarily the case.\n>\n> Some implementations may not have support for it yet, or the server\n> might be running in an environment where it is not feasible for it to\n> try to fill in the missing objects.\n>\n> Jonathan and Duy mentioned that separate patches for receive-pack and\n> send-pack could let us work around adding this at such a late stage,\n> so here they are. The second patch can maybe lie in waiting for a\n> while.\n\nI'll queue these for now, but I doubt the wisdom of this series,\ngiven that the ship has already sailed long time ago.\n\nCurrently, no third-party implementation of a receiving end can\naccept thin push, because \"thin push\" is not a capability that needs\nto be checked by the current clients. People will have to wait\nuntil the clients with 2/2 patch are widely deployed before starting\nto use such a receiving end that is incapable of \"thin push\".\n\nWouldn't the world be a better place if instead they used that time\nwaiting to help such a third-party receiving end to implement \"thin\npush\" support?\n"},{"id":"230158","messageId":"1383774082.2850.10.camel@centaur.cmartin.tk","threadId":"35289","inReplyTo":"xmqqbo1x8e60.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2013-11-06T21:41:22Z","receivedAt":"2013-11-06T21:41:22Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote:\n> I'll queue these for now, but I doubt the wisdom of this series,\n> given that the ship has already sailed long time ago.\n> \n> Currently, no third-party implementation of a receiving end can\n> accept thin push, because \"thin push\" is not a capability that needs\n> to be checked by the current clients. People will have to wait\n> until the clients with 2/2 patch are widely deployed before starting\n> to use such a receiving end that is incapable of \"thin push\".\n> \n> Wouldn't the world be a better place if instead they used that time\n> waiting to help such a third-party receiving end to implement \"thin\n> push\" support?\n> \n\nSupport in the code isn't always enough. The particular case that\nbrought this on is one where the index-pack implementation can deal with\nthin packs just fine.\n\nThis particular service takes the pack which the client sent and does\npost-processing on it to store it elsewhere. During the receive-pack\nequivalent, there is no git object db that it can query for the missing\nbase objects. I realise this is pretty a unusual situation.\n\nCheers,\n cmn\n"},{"id":"230162","messageId":"xmqqvc056uc1.fsf@gitster.dls.corp.google.com","threadId":"35289","inReplyTo":"1383774082.2850.10.camel@centaur.cmartin.tk","subject":"Re: [PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-06T22:25:50Z","receivedAt":"2013-11-06T22:25:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto writes:\n\n> On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote:\n>> I'll queue these for now, but I doubt the wisdom of this series,\n>> given that the ship has already sailed long time ago.\n>> \n>> Currently, no third-party implementation of a receiving end can\n>> accept thin push, because \"thin push\" is not a capability that needs\n>> to be checked by the current clients. People will have to wait\n>> until the clients with 2/2 patch are widely deployed before starting\n>> to use such a receiving end that is incapable of \"thin push\".\n>> \n>> Wouldn't the world be a better place if instead they used that time\n>> waiting to help such a third-party receiving end to implement \"thin\n>> push\" support?\n>> \n>\n> Support in the code isn't always enough. The particular case that\n> brought this on is one where the index-pack implementation can deal with\n> thin packs just fine.\n>\n> This particular service takes the pack which the client sent and does\n> post-processing on it to store it elsewhere. During the receive-pack\n> equivalent, there is no git object db that it can query for the missing\n> base objects. I realise this is pretty a unusual situation.\n\nOK, I agree that it sounds quite niche-y, but it still is sensible.\nIf a receiving end does not want to (this includes \"it is incapable\nof doing so\", but does not have to be limited to) complete a thin\npack, the series will give it such an option in the longer term.\n\nThanks.\n"},{"id":"230165","messageId":"20131106225414.GA15920@sigill.intra.peff.net","threadId":"35289","inReplyTo":"xmqqvc056uc1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-11-06T22:54:14Z","receivedAt":"2013-11-06T22:54:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 06, 2013 at 02:25:50PM -0800, Junio C Hamano wrote:\n\n> > Support in the code isn't always enough. The particular case that\n> > brought this on is one where the index-pack implementation can deal with\n> > thin packs just fine.\n> >\n> > This particular service takes the pack which the client sent and does\n> > post-processing on it to store it elsewhere. During the receive-pack\n> > equivalent, there is no git object db that it can query for the missing\n> > base objects. I realise this is pretty a unusual situation.\n> \n> OK, I agree that it sounds quite niche-y, but it still is sensible.\n> If a receiving end does not want to (this includes \"it is incapable\n> of doing so\", but does not have to be limited to) complete a thin\n> pack, the series will give it such an option in the longer term.\n\nI wonder if we want to make the flag go in the opposite direction, then.\n\nRight now we have no flag, and we assume the other side can handle a\nthin pack. If we add a \"thin\" flag, then the timeline is roughly:\n\n 1. Receive-pack starts advertising \"thin\".\n\n 2. Send-pack cannot assume lack of \"thin\" means the other side cannot\n handle \"thin\" (it might just be an older receive-pack), and keeps\n sending thin packs.\n\n [time passes]\n\n 3. Send-pack can safely assume that every server has learned \"thin\"\n and can assume that lack of \"thin\" means the server does not want a\n thin pack.\n\nIn other words, the benefit happens at step 3, and we do not get any\neffect until some long assumption time passes.\n\nIf we instead introduced \"no-thin\", it is more like:\n\n 1. Receive-pack starts advertising \"no-thin\" (as dictated by\n circumstances, as Carlos describes).\n\n 2. Send-pack which does not understand no-thin will ignore it and send\n a thin pack. This is the same as now, and the same as step 2 above.\n\n 3. An upgraded send-pack will understand no-thin and do as the server\n asks.\n\nSo an upgraded client and server can start cooperating immediately, and\nwe do not have to wait for the long assumption time to pass before\napplying the second half.\n\nIt is tempting to think about a \"thin\" flag because that would be the\nnatural way to have implemented it from the very beginning. But it is\nnot the beginning, and the negative flag is the only way at this point\nto say \"if you understand this, please behave differently than we used\nto\" (because the status quo is \"send a thin pack, whether I said it was\nOK or not\").\n\n-Peff\n"},{"id":"230169","messageId":"CAJo=hJtUMZit8Mtt7NQ=SiAXmnHf3xQqCKMo3F3XksHoq0tCkw@mail.gmail.com","threadId":"35289","inReplyTo":"1383774082.2850.10.camel@centaur.cmartin.tk","subject":"Re: [PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2013-11-06T23:42:30Z","receivedAt":"2013-11-06T23:42:30Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Wed, Nov 6, 2013 at 1:41 PM, Carlos Martín Nieto wrote:\n> On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote:\n>> I'll queue these for now, but I doubt the wisdom of this series,\n>> given that the ship has already sailed long time ago.\n>>\n>> Currently, no third-party implementation of a receiving end can\n>> accept thin push, because \"thin push\" is not a capability that needs\n>> to be checked by the current clients. People will have to wait\n>> until the clients with 2/2 patch are widely deployed before starting\n>> to use such a receiving end that is incapable of \"thin push\".\n>>\n>> Wouldn't the world be a better place if instead they used that time\n>> waiting to help such a third-party receiving end to implement \"thin\n>> push\" support?\n>>\n>\n> Support in the code isn't always enough. The particular case that\n> brought this on is one where the index-pack implementation can deal with\n> thin packs just fine.\n>\n> This particular service takes the pack which the client sent and does\n> post-processing on it to store it elsewhere. During the receive-pack\n> equivalent, there is no git object db that it can query for the missing\n> base objects. I realise this is pretty a unusual situation.\n\nHow... odd?\n\nAt Google we have made effort to ensure servers can accept thin packs,\neven though its clearly easier to accept non-thin, because clients in\nthe wild already send thin packs and changing the deployed clients is\nharder than implementing the existing protocol.\n\nIf the server can't complete the pack, I guess this also means the\nclient cannot immediately fetch from the server it just pushed to?\n"},{"id":"230171","messageId":"CAJo=hJvX5wDcr0OxaQEjRtm1-mVNbAMLcLQxgEJWT=J8NPwEKg@mail.gmail.com","threadId":"35289","inReplyTo":"20131106225414.GA15920@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2013-11-06T23:47:13Z","receivedAt":"2013-11-06T23:47:13Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Wed, Nov 6, 2013 at 2:54 PM, Jeff King wrote:\n> If we instead introduced \"no-thin\", it is more like:\n>\n> 1. Receive-pack starts advertising \"no-thin\" (as dictated by\n> circumstances, as Carlos describes).\n>\n> 2. Send-pack which does not understand no-thin will ignore it and send\n> a thin pack. This is the same as now, and the same as step 2 above.\n>\n> 3. An upgraded send-pack will understand no-thin and do as the server\n> asks.\n>\n> So an upgraded client and server can start cooperating immediately, and\n> we do not have to wait for the long assumption time to pass before\n> applying the second half.\n>\n> It is tempting to think about a \"thin\" flag because that would be the\n> natural way to have implemented it from the very beginning. But it is\n> not the beginning, and the negative flag is the only way at this point\n> to say \"if you understand this, please behave differently than we used\n> to\" (because the status quo is \"send a thin pack, whether I said it was\n> OK or not\").\n\nI think the only sane option at this point is a \"no-thin\" flag, or\njust require servers that want to be wire compatible to accept thin\npacks.\n"},{"id":"230993","messageId":"1385219362.2665.18.camel@centaur.cmartin.tk","threadId":"35289","inReplyTo":"CAJo=hJtUMZit8Mtt7NQ=SiAXmnHf3xQqCKMo3F3XksHoq0tCkw@mail.gmail.com","subject":"Re: [PATCH 0/2] thin-pack capability for send-pack/receive-pack","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2013-11-23T15:09:22Z","receivedAt":"2013-11-23T15:09:22Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On Wed, 2013-11-06 at 15:42 -0800, Shawn Pearce wrote:\n> On Wed, Nov 6, 2013 at 1:41 PM, Carlos Martín Nieto wrote:\n> > On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote:\n> >> I'll queue these for now, but I doubt the wisdom of this series,\n> >> given that the ship has already sailed long time ago.\n> >>\n> >> Currently, no third-party implementation of a receiving end can\n> >> accept thin push, because \"thin push\" is not a capability that needs\n> >> to be checked by the current clients. People will have to wait\n> >> until the clients with 2/2 patch are widely deployed before starting\n> >> to use such a receiving end that is incapable of \"thin push\".\n> >>\n> >> Wouldn't the world be a better place if instead they used that time\n> >> waiting to help such a third-party receiving end to implement \"thin\n> >> push\" support?\n> >>\n> >\n> > Support in the code isn't always enough. The particular case that\n> > brought this on is one where the index-pack implementation can deal with\n> > thin packs just fine.\n> >\n> > This particular service takes the pack which the client sent and does\n> > post-processing on it to store it elsewhere. During the receive-pack\n> > equivalent, there is no git object db that it can query for the missing\n> > base objects. I realise this is pretty a unusual situation.\n> \n> How... odd?\n> \n> At Google we have made effort to ensure servers can accept thin packs,\n> even though its clearly easier to accept non-thin, because clients in\n> the wild already send thin packs and changing the deployed clients is\n> harder than implementing the existing protocol.\n\nIt is harder, but IMO also more correct, as thin packs are an\noptimisation that was added somewhat later. Not to say it shouldn't be\nsomething you should attempt to do, but it's a trade-off between the\ncomplexity of the communication between the pieces and the potential\namount of extra data you're willing to put up with.\n\nThe Google (Code) servers don't just support thin packs, but for\nupload-pack, they force it upon the client, which is quite frustrating\nas it won't even tell you why it closes the connection but sends a 500\ninstead, but that's a different story.\n\n> \n> If the server can't complete the pack, I guess this also means the\n> client cannot immediately fetch from the server it just pushed to?\n\nNot all the details have been worked out yet, but the new history should\nbe converted into the target format before reporting success and closing\nthe connection. The Git frontend/protocol is one way of putting data\ninto the system, but that's not its native data storage format. The\ndatabase where this is getting stored only has very limited knowledge of\ngit.\n\nI'll reroll the series with \"no-thin\" as mentioned elsewhere in this\nthread.\n\n cmn\n"}]}