{"thread":{"id":"43217","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","startedAt":"2006-10-30T22:36:15Z","lastAt":"2006-10-31T06:56:35Z","messageCount":9,"participants":["Nicolas Pitre","Junio C Hamano","Shawn Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"295951","messageId":"20061030223615.GH5775@spearce.org","threadId":"43217","inReplyTo":null,"subject":"[PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-30T22:36:15Z","receivedAt":"2006-10-30T22:36:15Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Since keeping a pushed pack or exploding it into loose objects should\nbe a local repository decision this teaches receive-pack to decide\nif it should call unpack-objects or index-pack --stdin --fix-thin\nbased on the setting of receive.unpackLooseObjects.\n\nIf receive.unpackLooseObjects is true (which it is by default for\nnow) then we unpack-objects as we have in the past.\n\nIf it is false then we call index-pack and ask it to include our\npid and hostname in the .keep file to make it easier to identify\nwhy a given pack has been kept in the repository.\n\nCurrently this leaves every received pack as a kept pack.  We really\ndon't want that as received packs will tend to be small.  Instead we\nwant to delete the .keep file automatically after all refs have\nbeen updated.  That is being left as room for future improvement.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n Documentation/config.txt |    9 ++++++++-\n cache.h                  |    1 +\n receive-pack.c           |   35 ++++++++++++++++++++++++++++++++---\n sha1_file.c              |    2 +-\n 4 files changed, 42 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex d9e73da..4eab9e8 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -301,7 +301,14 @@ imap::\n \tThe configuration variables in the 'imap' section are described\n \tin gitlink:git-imap-send[1].\n \n-receive.denyNonFastforwads::\n+receive.unpackLooseObjects::\n+\tIf set to true, git-receive-pack will unpack each received\n+\tobject into a loose object in the repository.  If set to\n+\tfalse then the received pack file will be kept as is (but\n+\tmay have delta bases appended onto the end).  Large pushes\n+\tinto a repository will generally go faster if this is false.\n+\n+receive.denyNonFastForwards::\n \tIf set to true, git-receive-pack will deny a ref update which is\n \tnot a fast forward. Use this to prevent such an update via a push,\n \teven if that push is forced. This configuration variable is\ndiff --git a/cache.h b/cache.h\nindex e997a85..6cb7e1d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -376,6 +376,7 @@ extern struct packed_git *parse_pack_ind\n \t\t\t\t\t\tchar *idx_path);\n \n extern void prepare_packed_git(void);\n+extern void reprepare_packed_git(void);\n extern void install_packed_git(struct packed_git *pack);\n \n extern struct packed_git *find_sha1_pack(const unsigned char *sha1, \ndiff --git a/receive-pack.c b/receive-pack.c\nindex 7e154c5..6e377f0 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -7,9 +7,8 @@\n \n static const char receive_pack_usage[] = \"git-receive-pack <git-dir>\";\n \n-static const char *unpacker[] = { \"unpack-objects\", NULL };\n-\n static int deny_non_fast_forwards = 0;\n+static int unpack_loose_objects = 1;\n static int report_status;\n \n static char capabilities[] = \"report-status\";\n@@ -25,6 +24,12 @@ static int receive_pack_config(const cha\n \t\treturn 0;\n \t}\n \n+\tif (strcmp(var, \"receive.unpacklooseobjects\") == 0)\n+\t{\n+\t\tunpack_loose_objects = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn 0;\n }\n \n@@ -229,7 +234,31 @@ static void read_head_info(void)\n \n static const char *unpack()\n {\n-\tint code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);\n+\tstatic const char *unpacker[] = { \"unpack-objects\", NULL };\n+\tint code;\n+\t\n+\tif (unpack_loose_objects)\n+\t\tcode = run_command_v_opt(1, unpacker, RUN_GIT_CMD);\n+\telse {\n+\t\tconst char *keeper[5], *why_kept;\n+\t\tchar my_host[255], keep_arg[128 + 255];\n+\n+\t\tif (gethostname(my_host, sizeof(my_host)))\n+\t\t\tstrcpy(my_host, \"localhost\");\n+\t\tsnprintf(keep_arg, sizeof(keep_why),\n+\t\t\t\"--keep=receive-pack %i on %s\",\n+\t\t\tgetpid(), my_host);\n+\t\twhy_kept = keep_arg + 7;\n+\n+\t\tkeeper[0] = \"index-pack\";\n+\t\tkeeper[1] = \"--stdin\";\n+\t\tkeeper[2] = \"--fix-thin\";\n+\t\tkeeper[3] = keep_arg;\n+\t\tkeeper[4] = NULL;\n+\t\tcode = run_command_v_opt(1, keeper, RUN_GIT_CMD);\n+\t\tif (!code)\n+\t\t\treprepare_packed_git();\n+\t}\n \n \tswitch (code) {\n \tcase 0:\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 5e6c8b8..7bda2d4 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -663,7 +663,7 @@ void prepare_packed_git(void)\n \tprepare_packed_git_run_once = 1;\n }\n \n-static void reprepare_packed_git(void)\n+void reprepare_packed_git(void)\n {\n \tprepare_packed_git_run_once = 0;\n \tprepare_packed_git();\n-- \n1.4.3.3.g7d63\n"},{"id":"294127","messageId":"7vu01ltnfs.fsf@assigned-by-dhcp.cox.net","threadId":"43217","inReplyTo":"20061030223615.GH5775@spearce.org","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-30T23:04:55Z","receivedAt":"2006-10-30T23:04:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n> Currently this leaves every received pack as a kept pack.  We really\n> don't want that as received packs will tend to be small.\n\nUntil this is resolved, I feel there should be some way to\ncontrol the behaviour, so while I agree with the general\ndirection, I think the patch to revert the sender's wish should\ncome at the end.\n"},{"id":"298657","messageId":"7vlkmxtmln.fsf@assigned-by-dhcp.cox.net","threadId":"43217","inReplyTo":"20061030223615.GH5775@spearce.org","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-30T23:23:00Z","receivedAt":"2006-10-30T23:23:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n> Since keeping a pushed pack or exploding it into loose objects should\n> be a local repository decision this teaches receive-pack to decide\n> if it should call unpack-objects or index-pack --stdin --fix-thin\n> based on the setting of receive.unpackLooseObjects.\n\nOne thing you can cheaply do is to tell the number of new\ncommits that is coming to receive-pack from send-pack when it\nsends the old..new pairs before it sends the packfile payload.\nIt would be just a single internal rev-list call inside\nsend-pack, which is reasonably cheap.\n\nIf the receiving end knows how to process that new information\n(invent a \"send-count\" protocol extension and send it just like\nwe already send \"report-status\" request), send one extra packet\nafter flushing the list of old..new from send-pack to\nreceive-pack, to tell what the number of commits are, and make a\nmatching change in receive-pack.\n\nThen, instead of receive.unpackLooseObjects being a boolean, you\ncan have it as a ceiling to decide if you have more than 100\ncommits you would keep it packed and otherwise you would\nexplode.  That would be very specific to the projects' size\n(width of the tree) and style (huge commits vs lots of small\nchanges).\n\n"},{"id":"294054","messageId":"Pine.LNX.4.64.0610302258400.11384@xanadu.home","threadId":"43217","inReplyTo":"7vlkmxtmln.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-31T04:08:56Z","receivedAt":"2006-10-31T04:08:56Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 30 Oct 2006, Junio C Hamano wrote:\n\n> One thing you can cheaply do is to tell the number of new\n> commits that is coming to receive-pack from send-pack when it\n> sends the old..new pairs before it sends the packfile payload.\n> It would be just a single internal rev-list call inside\n> send-pack, which is reasonably cheap.\n\nWell, it could even be avoided.\n\n> If the receiving end knows how to process that new information\n> (invent a \"send-count\" protocol extension and send it just like\n> we already send \"report-status\" request), send one extra packet\n> after flushing the list of old..new from send-pack to\n> receive-pack, to tell what the number of commits are, and make a\n> matching change in receive-pack.\n\nI don't like this much since a commit can carry along very little or \nvery large amount of objects.  You really want to decide on the number \nof objects.\n\nWhy not just parse the pack header in receive-pack / fetch-pack, and \ndecide on the first-hand information?  Sure the pack header is then \ngone, but then the only thing that is needed is an extra flag to both \nunpack-objects and index-pack to tell them that we've already parsed the \npack header and that the pack version is x and the number of objects is \ny.  Simply something like --pack_header=x,y.  No protocol extension \nneeded, no extra rev-list, no reliance on the remote server providing \nthe needed info.\n\n\n"},{"id":"294239","messageId":"7v7iyhrsoi.fsf@assigned-by-dhcp.cox.net","threadId":"43217","inReplyTo":"Pine.LNX.4.64.0610302258400.11384@xanadu.home","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-31T04:54:37Z","receivedAt":"2006-10-31T04:54:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n> Why not just parse the pack header in receive-pack / fetch-pack, and \n> decide on the first-hand information?  Sure the pack header is then \n> gone, but then the only thing that is needed is an extra flag to both \n> unpack-objects and index-pack to tell them that we've already parsed the \n> pack header and that the pack version is x and the number of objects is \n> y.  Simply something like --pack_header=x,y.  No protocol extension \n> needed, no extra rev-list, no reliance on the remote server providing \n> the needed info.\n\nI like it.\n\nBecause that approach assumes recieve-pack and unpack-objects\nand index-pack are from the same vintage (otherwise your\nreceive-pack would need to have a way to see if unpack-objects\nand index-pack would grok --pack_header argument), we could even\nget away without passing the pack version if we wanted to.\n\n\n"},{"id":"296482","messageId":"20061031063312.GA7375@spearce.org","threadId":"43217","inReplyTo":"7vu01ltnfs.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-31T06:33:12Z","receivedAt":"2006-10-31T06:33:12Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> Until this is resolved, I feel there should be some way to\n> control the behaviour, so while I agree with the general\n> direction, I think the patch to revert the sender's wish should\n> come at the end.\n \nI disagree, but I'm not the maintainer.  :-)\n\nI reverted it before making my changes as I was editing the same\nlines as c7740a modified.  Therefore reverting c7740a after my\nchanges would no longer be a clean revert, making it slightly\nharder to actuallly do the revert as you would need to manually\nfix up receive-pack.c.\n\n-- \n"},{"id":"295773","messageId":"20061031063941.GB7375@spearce.org","threadId":"43217","inReplyTo":"7v7iyhrsoi.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-31T06:39:41Z","receivedAt":"2006-10-31T06:39:41Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> Nicolas Pitre <nico@cam.org> writes:\n> \n> > Why not just parse the pack header in receive-pack / fetch-pack, and \n> > decide on the first-hand information?  Sure the pack header is then \n> > gone, but then the only thing that is needed is an extra flag to both \n> > unpack-objects and index-pack to tell them that we've already parsed the \n> > pack header and that the pack version is x and the number of objects is \n> > y.  Simply something like --pack_header=x,y.  No protocol extension \n> > needed, no extra rev-list, no reliance on the remote server providing \n> > the needed info.\n> \n> I like it.\n> \n> Because that approach assumes recieve-pack and unpack-objects\n> and index-pack are from the same vintage (otherwise your\n> receive-pack would need to have a way to see if unpack-objects\n> and index-pack would grok --pack_header argument), we could even\n> get away without passing the pack version if we wanted to.\n\nHeh.  On Saturday I almost did exactly what Nico proposes above.\nBut I thought both of you would find the --pack_header=x,y option\ntoo brittle and would reject the change.\n\nBut since all three of us liked the same idea I'll code it up\nand resend my receive-pack patch using Nico's suggestion instead.\nHopefully I'll get it out tomorrow.\n\nBTW I think we do need to pass the pack version in the option.\nIf we ever do increment the pack version its going to be after this\noption is introduced so supporting the option does not imply that\nthe callee is able to parse the pack file without knowing what\nversion the file is, especially if the callee supports both the\ncurrent version and the new version.\n\n-- \n"},{"id":"298498","messageId":"7vwt6hq8nu.fsf@assigned-by-dhcp.cox.net","threadId":"43217","inReplyTo":"20061031063941.GB7375@spearce.org","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-31T06:52:21Z","receivedAt":"2006-10-31T06:52:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n>> Because that approach assumes recieve-pack and unpack-objects\n>> and index-pack are from the same vintage (otherwise your\n>> receive-pack would need to have a way to see if unpack-objects\n>> and index-pack would grok --pack_header argument), we could even\n>> get away without passing the pack version if we wanted to.\n>...\n> BTW I think we do need to pass the pack version in the option.\n\nWe are in agreement; I mentioned \"we could ... if we wanted to\"\nbecause I wanted to see if you are paying attention ;-).\n\n"},{"id":"298128","messageId":"20061031065635.GC7375@spearce.org","threadId":"43217","inReplyTo":"7vwt6hq8nu.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Teach receive-pack how to keep pack files when unpacklooseobjects = 0.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-31T06:56:35Z","receivedAt":"2006-10-31T06:56:35Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <junkio@cox.net> wrote:\n> Shawn Pearce <spearce@spearce.org> writes:\n> \n> >> Because that approach assumes recieve-pack and unpack-objects\n> >> and index-pack are from the same vintage (otherwise your\n> >> receive-pack would need to have a way to see if unpack-objects\n> >> and index-pack would grok --pack_header argument), we could even\n> >> get away without passing the pack version if we wanted to.\n> >...\n> > BTW I think we do need to pass the pack version in the option.\n> \n> We are in agreement; I mentioned \"we could ... if we wanted to\"\n> because I wanted to see if you are paying attention ;-).\n\nHah.  Trying to keep me on my toes aren't you.  :-)\n\nI almost have the change finished. I'll send it before I go to bed.\n\n-- \n"}]}