{"thread":{"id":"43082","subject":"Re: [PATCH] send-pack --keep: do not explode into loose objects on the receiving end.","startedAt":"2006-10-26T03:44:12Z","lastAt":"2006-10-30T01:44:02Z","messageCount":51,"participants":["Shawn Pearce","Junio C Hamano","Eran Tromer","Nicolas Pitre","Sean","Petr Baudis","Linus Torvalds","J. Bruce Fields","Alex Riesen","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"294284","messageId":"Pine.LNX.4.64.0610252333540.12418@xanadu.home","threadId":"43082","inReplyTo":null,"subject":"fetching packs and storing them as packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-26T03:44:12Z","receivedAt":"2006-10-26T03:44:12Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"With the last few patches I just posted it is now possible to receive \n(fetch) packs, validate them on the fly, complete them if they are thin \npacks, and store them directly without exploding them into loose \nobjects.\n\nThere are advantages and inconvenients to both methods, so I think this \nshould become a configuration option and/or even a command line argument \nto git-fetch. I think there are many more advantages to keeping packs \npacked hence I think using index-pack should become the default.\n\nBut I'm a bit tired to play with it and the final integration is for \nsomeone else to do. I've tested it lightly using the extremely crude \npatch below to hook it in the fetch process.\n\nHave fun!\n\ndiff --git a/fetch-clone.c b/fetch-clone.c\nindex 76b99af..28796c3 100644\n--- a/fetch-clone.c\n+++ b/fetch-clone.c\n@@ -142,7 +142,8 @@ int receive_unpack_pack(int xd[2], const\n \t\tdup2(fd[0], 0);\n \t\tclose(fd[0]);\n \t\tclose(fd[1]);\n-\t\texecl_git_cmd(\"unpack-objects\", quiet ? \"-q\" : NULL, NULL);\n+\t\texecl_git_cmd(\"index-pack\", \"--stdin\", \"--fix-thin\",\n+\t\t\t quiet ? NULL : \"-v\", NULL);\n \t\tdie(\"git-unpack-objects exec failed\");\n \t}\n \tclose(fd[0]);\ndiff --git a/receive-pack.c b/receive-pack.c\nindex 1fcf3a9..7f6dc49 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -7,7 +7,7 @@\n \n static const char receive_pack_usage[] = \"git-receive-pack \";\n \n-static const char *unpacker[] = { \"unpack-objects\", NULL };\n+static const char *unpacker[] = { \"index-pack\", \"-v\", \"--stdin\", \"--fix-thin\", NULL };\n \n static int report_status;\n"},{"id":"297557","messageId":"4540CA0C.6030300@tromer.org","threadId":"43082","inReplyTo":"Pine.LNX.4.64.0610252333540.12418@xanadu.home","subject":"Re: fetching packs and storing them as packs","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-26T14:45:32Z","receivedAt":"2006-10-26T14:45:32Z","isPatch":false,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"On 2006-10-26 05:44, Nicolas Pitre wrote:\n> diff --git a/receive-pack.c b/receive-pack.c\n> index 1fcf3a9..7f6dc49 100644\n> --- a/receive-pack.c\n> +++ b/receive-pack.c\n> @@ -7,7 +7,7 @@\n> \n> static const char receive_pack_usage[] = \"git-receive-pack \";\n> \n> -static const char *unpacker[] = { \"unpack-objects\", NULL };\n> +static const char *unpacker[] = { \"index-pack\", \"-v\", \"--stdin\", \"--fix-thin\", NULL };\n> \n> static int report_status;\n\nThis creates a race condition w.r.t. \"git repack -a -d\", similar to the\nexisting race condition between \"git fetch --keep\" and\n\"git repack -a -d\". There's a point in time where the new pack is stored\nbut not yet referenced, and if \"git repack -a -d\" runs at that point it\nwill eradicate the pack. When the heads are finally updated, you get a\ncorrupted repository.\n\n(That's for the shell implementation of git-repack, at least. I assume\nthe new builtin preserves the old semantics.)\n\nSince people run the supposedly safe \"git repack -a -d\" on regular\nbasis, this is going to bite.\n\n Eran\n"},{"id":"293969","messageId":"45413209.2000905@tromer.org","threadId":"43082","inReplyTo":"Pine.LNX.4.64.0610261105200.12418@xanadu.home","subject":"Re: fetching packs and storing them as packs","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-26T22:09:13Z","receivedAt":"2006-10-26T22:09:13Z","isPatch":false,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"Hi,\n\nOn 2006-10-26 17:08, Nicolas Pitre wrote:\n> On Thu, 26 Oct 2006, Eran Tromer wrote:\n>> This creates a race condition w.r.t. \"git repack -a -d\", similar to the\n>> existing race condition between \"git fetch --keep\" and\n>> \"git repack -a -d\". There's a point in time where the new pack is stored\n>> but not yet referenced, and if \"git repack -a -d\" runs at that point it\n>> will eradicate the pack. When the heads are finally updated, you get a\n>> corrupted repository.\n> \n> And how is it different from receiving a pack through git-unpack-objects \n> where lots of loose objects are created, and git-repack -a -d removing \n> those unconnected loose objects before the heads are updated?\n\ngit-repack -a -d does not touch unconnected loose objects.\nIt removes only unconnected packed objects.\n\nOnly git-prune removes unconnected loose objects, and that's documented\nas unsafe.\n\n"},{"id":"296316","messageId":"Pine.LNX.4.64.0610262038320.11384@xanadu.home","threadId":"43082","inReplyTo":"45413209.2000905@tromer.org","subject":"Re: fetching packs and storing them as packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-27T00:50:12Z","receivedAt":"2006-10-27T00:50:12Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 27 Oct 2006, Eran Tromer wrote:\n\n> Hi,\n> \n> On 2006-10-26 17:08, Nicolas Pitre wrote:\n> > On Thu, 26 Oct 2006, Eran Tromer wrote:\n> >> This creates a race condition w.r.t. \"git repack -a -d\", similar to the\n> >> existing race condition between \"git fetch --keep\" and\n> >> \"git repack -a -d\". There's a point in time where the new pack is stored\n> >> but not yet referenced, and if \"git repack -a -d\" runs at that point it\n> >> will eradicate the pack. When the heads are finally updated, you get a\n> >> corrupted repository.\n> > \n> > And how is it different from receiving a pack through git-unpack-objects \n> > where lots of loose objects are created, and git-repack -a -d removing \n> > those unconnected loose objects before the heads are updated?\n> \n> git-repack -a -d does not touch unconnected loose objects.\n> It removes only unconnected packed objects.\n\nRight.\n\n> Only git-prune removes unconnected loose objects, and that's documented\n> as unsafe.\n\nWell, the race does exist. Don't do repack -a -d at the same time then.\n\nThis race should be adressed somehow if it is really a problem. Now \nthat I've used index-pack in place of unpack-objects for a while, I \ndon't think I'll want to go back. It is simply faster to fetch, faster \nto checkout, faster to repack, wastes less disk space, etc.\n\n\n"},{"id":"296376","messageId":"20061027014229.GA28407@spearce.org","threadId":"43082","inReplyTo":"Pine.LNX.4.64.0610262038320.11384@xanadu.home","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-27T01:42:29Z","receivedAt":"2006-10-27T01:42:29Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nicolas Pitre wrote:\n> On Fri, 27 Oct 2006, Eran Tromer wrote:\n> \n> > Hi,\n> > \n> > On 2006-10-26 17:08, Nicolas Pitre wrote:\n> > > On Thu, 26 Oct 2006, Eran Tromer wrote:\n> > >> This creates a race condition w.r.t. \"git repack -a -d\", similar to the\n> > >> existing race condition between \"git fetch --keep\" and\n> > >> \"git repack -a -d\". There's a point in time where the new pack is stored\n> > >> but not yet referenced, and if \"git repack -a -d\" runs at that point it\n> > >> will eradicate the pack. When the heads are finally updated, you get a\n> > >> corrupted repository.\n> > > \n> > > And how is it different from receiving a pack through git-unpack-objects \n> > > where lots of loose objects are created, and git-repack -a -d removing \n> > > those unconnected loose objects before the heads are updated?\n> > \n> > git-repack -a -d does not touch unconnected loose objects.\n> > It removes only unconnected packed objects.\n> \n> Right.\n> \n> > Only git-prune removes unconnected loose objects, and that's documented\n> > as unsafe.\n> \n> Well, the race does exist. Don't do repack -a -d at the same time then.\n\nThis is an issue for \"central\" repositories that people push into\nand which might be getting repacked according to a cronjob.\n\nUnfortunately I don't have a solution. I tried to come up with\none but didn't. :-)\n\n-- \n"},{"id":"294426","messageId":"BAYC1-PASMTP10C050A5FAA4C70AD57679AE040@CEZ.ICE","threadId":"43082","inReplyTo":"20061027014229.GA28407@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2006-10-27T02:38:04Z","receivedAt":"2006-10-27T02:38:04Z","isPatch":false,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Thu, 26 Oct 2006 21:42:29 -0400\nShawn Pearce wrote:\n\n> This is an issue for \"central\" repositories that people push into\n> and which might be getting repacked according to a cronjob.\n> \n> Unfortunately I don't have a solution. I tried to come up with\n> one but didn't. :-)\n\nWhat about creating a temporary ref before pushing, and then removing\nit only after the HEAD has been updated?\n\n"},{"id":"295497","messageId":"Pine.LNX.4.64.0610262230080.11384@xanadu.home","threadId":"43082","inReplyTo":"20061027014229.GA28407@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-27T02:41:49Z","receivedAt":"2006-10-27T02:41:49Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 26 Oct 2006, Shawn Pearce wrote:\n\n> Nicolas Pitre wrote:\n> > On Fri, 27 Oct 2006, Eran Tromer wrote:\n> > \n> > > Hi,\n> > > \n> > > On 2006-10-26 17:08, Nicolas Pitre wrote:\n> > > > On Thu, 26 Oct 2006, Eran Tromer wrote:\n> > > >> This creates a race condition w.r.t. \"git repack -a -d\", similar to the\n> > > >> existing race condition between \"git fetch --keep\" and\n> > > >> \"git repack -a -d\". There's a point in time where the new pack is stored\n> > > >> but not yet referenced, and if \"git repack -a -d\" runs at that point it\n> > > >> will eradicate the pack. When the heads are finally updated, you get a\n> > > >> corrupted repository.\n> > > > \n> > > > And how is it different from receiving a pack through git-unpack-objects \n> > > > where lots of loose objects are created, and git-repack -a -d removing \n> > > > those unconnected loose objects before the heads are updated?\n> > > \n> > > git-repack -a -d does not touch unconnected loose objects.\n> > > It removes only unconnected packed objects.\n> > \n> > Right.\n> > \n> > > Only git-prune removes unconnected loose objects, and that's documented\n> > > as unsafe.\n> > \n> > Well, the race does exist. Don't do repack -a -d at the same time then.\n> \n> This is an issue for \"central\" repositories that people push into\n> and which might be getting repacked according to a cronjob.\n> \n> Unfortunately I don't have a solution. I tried to come up with\n> one but didn't. :-)\n\nJust continue to explode received packs into loose objects then. It is \nthat simple. I said there were advantages and inconvenients to both \nmethods. This one is a nice example.\n\nI won't repack from a cron job, so I don't expect to run a repack and a \nfetch at the same time on my private repositories. I therefore don't \ncare about that race and so is the case for the vast majority of users.\n\n\n"},{"id":"298036","messageId":"45417205.6020805@tromer.org","threadId":"43082","inReplyTo":"20061027014229.GA28407@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-27T02:42:13Z","receivedAt":"2006-10-27T02:42:13Z","isPatch":false,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"On 2006-10-27 03:42, Shawn Pearce wrote:\n> Nicolas Pitre wrote:\n>> On Fri, 27 Oct 2006, Eran Tromer wrote:\n>> Well, the race does exist. Don't do repack -a -d at the same time then.\n> \n> This is an issue for \"central\" repositories that people push into\n> and which might be getting repacked according to a cronjob.\n\nAFAICT, the bottom line of the \"Re: auto-packing on kernel.org? please?\"\nthread last October was \"sure, go ahead\".\n\n\n> Unfortunately I don't have a solution. I tried to come up with\n> one but didn't. :-)\n\nHere's one way to do it.\nChange git-repack to follow references under $GIT_DIR/tmp/refs/ too.\nTo receive or fetch a pack:\n1. Add references to the new heads in\n `mktemp $GIT_DIR/tmp/refs/XXXXXX`.\n2. Put the new .pack under $GIT_DIR/objects/pack/.\n3. Put the new .idx under $GIT_DIR/objects/pack/.\n4. Update the relevant heads under $GIT_DIR/refs/.\n5. Delete the references from step 1.\n\nThis is repack-safe and never corrupts the repo. The worst-case failure\nmode is if you die before cleaning the refs from $GIT_DIR/tmp/refs. That\nmay mean some packed objects will never be removed by \"repack -a -d\"\neven if they lose all references from $GIT_DIR/refs, so do \"tmpwatch -m\n240 $GIT_DIR/tmp/refs\" to take care of that.\n\n Eran\n"},{"id":"294343","messageId":"20061027030054.GB28407@spearce.org","threadId":"43082","inReplyTo":"45417205.6020805@tromer.org","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-27T03:00:54Z","receivedAt":"2006-10-27T03:00:54Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Eran Tromer wrote:\n> > Unfortunately I don't have a solution. I tried to come up with\n> > one but didn't. :-)\n> \n> Here's one way to do it.\n> Change git-repack to follow references under $GIT_DIR/tmp/refs/ too.\n> To receive or fetch a pack:\n> 1. Add references to the new heads in\n> `mktemp $GIT_DIR/tmp/refs/XXXXXX`.\n> 2. Put the new .pack under $GIT_DIR/objects/pack/.\n> 3. Put the new .idx under $GIT_DIR/objects/pack/.\n> 4. Update the relevant heads under $GIT_DIR/refs/.\n> 5. Delete the references from step 1.\n> \n> This is repack-safe and never corrupts the repo. The worst-case failure\n> mode is if you die before cleaning the refs from $GIT_DIR/tmp/refs. That\n> may mean some packed objects will never be removed by \"repack -a -d\"\n> even if they lose all references from $GIT_DIR/refs, so do \"tmpwatch -m\n> 240 $GIT_DIR/tmp/refs\" to take care of that.\n\nThat was actually my (and also Sean's) solution. Except I would\nput the temporary refs as \"$GIT_DIR/refs/ref_XXXXXX\" as this is\nless code to change and its consistent with how temporary loose\nobjects are created.\n\nUnfortunately it does not completely work.\n\nWhat happens when the incoming pack (steps #2 and #3) takes 15\nminutes to upload (slow ADSL modem, lots of objects) and the\nbackground repack process sees those temporary refs and starts\ntrying to include those objects? It can't walk the DAG that those\nrefs point at because the objects aren't in the current repository.\n\nFrom what I know of that code the pack-objects process will fail to\nfind the object pointed at by the ref, rescan the packs directory,\nfind no new packs, look for the object again, and abort over the\n\"corruption\".\n\nOK so the repository won't get corrupted but the repack would be\nforced to abort.\n\n\nAnother issue I just thought about tonight is we may need a\ncount-packs utility that like count-objects lists the number\nof active packs and their total size. If we start hanging onto\nevery pack we receive over the wire the pack directory is going to\ngrow pretty fast and we'll need a way to tell us when its time to\n`repack -a -d`.\n\n-- \n"},{"id":"295304","messageId":"BAYC1-PASMTP03992F75428088AF83AE39AE040@CEZ.ICE","threadId":"43082","inReplyTo":"20061027030054.GB28407@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2006-10-27T03:13:54Z","receivedAt":"2006-10-27T03:13:54Z","isPatch":false,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Thu, 26 Oct 2006 23:00:54 -0400\nShawn Pearce wrote:\n\n> What happens when the incoming pack (steps #2 and #3) takes 15\n> minutes to upload (slow ADSL modem, lots of objects) and the\n> background repack process sees those temporary refs and starts\n> trying to include those objects? It can't walk the DAG that those\n> refs point at because the objects aren't in the current repository.\n\nAs long as there was standard naming for such temporary refs,\nthey could be completely ignored by the repack process, no?\n\n"},{"id":"296788","messageId":"ehrtt8$rm3$1@sea.gmane.org","threadId":"43082","inReplyTo":"BAYC1-PASMTP03992F75428088AF83AE39AE040@CEZ.ICE","subject":"Re: fetching packs and storing them as packs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-27T03:20:53Z","receivedAt":"2006-10-27T03:20:53Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Sean wrote:\n\n> On Thu, 26 Oct 2006 23:00:54 -0400\n> Shawn Pearce wrote:\n> \n>> What happens when the incoming pack (steps #2 and #3) takes 15\n>> minutes to upload (slow ADSL modem, lots of objects) and the\n>> background repack process sees those temporary refs and starts\n>> trying to include those objects? It can't walk the DAG that those\n>> refs point at because the objects aren't in the current repository.\n> \n> As long as there was standard naming for such temporary refs,\n> they could be completely ignored by the repack process, no?\n\nYou meant I think: half ignored. Taken into account when finding\nwhich parts are referenced to delete (-d part), but not complain\nif they don't point to anything (validation).\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n"},{"id":"298698","messageId":"BAYC1-PASMTP03EEA2550392E1AF2851AEAE040@CEZ.ICE","threadId":"43082","inReplyTo":"ehrtt8$rm3$1@sea.gmane.org","subject":"Re: fetching packs and storing them as packs","fromName":"Sean","fromEmail":"seanlkml@sympatico.ca","sentAt":"2006-10-27T03:27:40Z","receivedAt":"2006-10-27T03:27:40Z","isPatch":false,"sender":{"key":"seanlkml@sympatico.ca","avatar":"https://gravatar.com/avatar/f92923f54fc08c401fc59b71829d4b89e9b8087fbba45ff87c82e6a83aee02ae?d=mp&s=160"},"body":"On Fri, 27 Oct 2006 05:20:53 +0200\nJakub Narebski wrote:\n\n> You meant I think: half ignored. Taken into account when finding\n> which parts are referenced to delete (-d part), but not complain\n> if they don't point to anything (validation).\n\nYes, ignored by repack, not ignored by prune.\n\n"},{"id":"298113","messageId":"4541850B.8060608@tromer.org","threadId":"43082","inReplyTo":"20061027030054.GB28407@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-27T04:03:23Z","receivedAt":"2006-10-27T04:03:23Z","isPatch":false,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"Hi,\n\nOn 2006-10-27 05:00, Shawn Pearce wrote:\n>> Change git-repack to follow references under $GIT_DIR/tmp/refs/ too.\n>> To receive or fetch a pack:\n>> 1. Add references to the new heads in\n>> `mktemp $GIT_DIR/tmp/refs/XXXXXX`.\n>> 2. Put the new .pack under $GIT_DIR/objects/pack/.\n>> 3. Put the new .idx under $GIT_DIR/objects/pack/.\n>> 4. Update the relevant heads under $GIT_DIR/refs/.\n>> 5. Delete the references from step 1.\n\n> That was actually my (and also Sean's) solution. Except I would\n> put the temporary refs as \"$GIT_DIR/refs/ref_XXXXXX\" as this is\n> less code to change and its consistent with how temporary loose\n> objects are created.\n\nIf you do that, other programs (e.g., anyone who uses rev-list --all)\nmay try to walk those heads or consider them available before the pack\nis really there. The point about $GIT_DIR/tmp/refs is that only programs\nmeddling with physical packs (git-fetch, git-receive-pack, git-repack)\nwill know about it.\n\n\n> What happens when the incoming pack (steps #2 and #3) takes 15\n> minutes to upload (slow ADSL modem, lots of objects) and the\n> background repack process sees those temporary refs and starts\n> trying to include those objects? It can't walk the DAG that those\n> refs point at because the objects aren't in the current repository.\n> \n>>From what I know of that code the pack-objects process will fail to\n> find the object pointed at by the ref, rescan the packs directory,\n> find no new packs, look for the object again, and abort over the\n> \"corruption\".\n\nGood point. Then I guess we'll need to change git-repack to ignore\nmissing objects if they're referenced from $GIT_DIR/tmp/refs but not\nfrom $GIT_DIR/refs. Ugly, but shouldn't be too hard.\n\n\n"},{"id":"296334","messageId":"20061027044233.GA29057@spearce.org","threadId":"43082","inReplyTo":"4541850B.8060608@tromer.org","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-27T04:42:34Z","receivedAt":"2006-10-27T04:42:34Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Eran Tromer wrote:\n> On 2006-10-27 05:00, Shawn Pearce wrote:\n> >> Change git-repack to follow references under $GIT_DIR/tmp/refs/ too.\n> >> To receive or fetch a pack:\n> >> 1. Add references to the new heads in\n> >> `mktemp $GIT_DIR/tmp/refs/XXXXXX`.\n> >> 2. Put the new .pack under $GIT_DIR/objects/pack/.\n> >> 3. Put the new .idx under $GIT_DIR/objects/pack/.\n> >> 4. Update the relevant heads under $GIT_DIR/refs/.\n> >> 5. Delete the references from step 1.\n> \n> > That was actually my (and also Sean's) solution. Except I would\n> > put the temporary refs as \"$GIT_DIR/refs/ref_XXXXXX\" as this is\n> > less code to change and its consistent with how temporary loose\n> > objects are created.\n> \n> If you do that, other programs (e.g., anyone who uses rev-list --all)\n> may try to walk those heads or consider them available before the pack\n> is really there. The point about $GIT_DIR/tmp/refs is that only programs\n> meddling with physical packs (git-fetch, git-receive-pack, git-repack)\n> will know about it.\n \nDoh. Yes, of course, that makes much sense.\n\nHmm... Looking at git-repack we have two things currently pending\nto rework in there:\n\n - Historical vs. active packs.\n - Don't delete a possibly still incoming pack during -d.\n\nThese have a lot of the same implementation issues. We need to\nbe able to identify a set of packs which should be allowed for\nrepack with -a, and allowed for removal with -d if -a was also used.\nA newly uploaded pack cannot be in that list unless its contents are\nreferenced by one or more refs (which implies that the receive-pack\nprocess has completed).\n\nI'm thinking that the ref thing might be unnecessary. We just\nneed to fix repack so it builds a list of \"active packs\" whose\nobjects should be copied into the new pack, and then only packs\nloose objects and those objects contained by an active packs.\n\nSo the receive-pack process becomes:\n\n a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n c. Write pack and index.\n d. Move pack to $GIT_DIR/objects/pack/...\n e. Move index to $GIT_DIR/objects/pack...\n f. Update refs.\n g. Arrange for new pack and index to be considered active.\n\nAnd the repack -a -d process becomes:\n\n 1. List all active packs and store in memory.\n 2. Repack only loose objects and objects contained in active packs.\n 3. Move new pack and idx into $GIT_DIR/objects/pack/...\n 4. Arrange for new pack and idx to be considered active.\n 5. Delete active packs found by step #1.\n\nJunio was originally considering making historical packs\nhistorical by placing their names into an information file (such as\n`$GIT_DIR/objects/info/historical-packs`) and then consider all other\npacks as active. Thus step #1 is list all packs and removes those\nwhose names appear in historical-packs, while step #4 is unnecessary.\n\nI was thinking about just changing the \"pack-\" prefix to \"hist-\" for\nthe historical packs and assuming all \"pack-*.pack\" to be active.\nThus step #1 is a simple glob on the pack directory and step #4\nis unnecessary.\n\nIn the latter case its easy to mark an existing pack as historical\n(just hardlink hist- names for pack, then idx, then unlink previous\nnames) and its also easy to mark new incoming packs as non active\nby using a different prefix (e.g. \"incm-\") during step #d/#e and\nthen relinking them as \"pack-\" during step #g. Its also very safe\non systems that support hardlinks.\n\nWe shouldn't ever need to worry about race conditions with repacking\nhistorical packs. For starters historical packs will tend to be\nseveral years' worth of object accumulation and will be so large\nthat repacking them might take 45 minutes or more. Thus they\nprobably will never get repacked. An active pack will simply move\ninto historical status after it gets so large that its no longer\nworthwhile to keep repacking it. They also will tend to have objects\nthat are so old that at least one ref in the repository will point\nat their entire DAG and thus everything would carry over on a repack.\n\nSo this would be cleaner then messing around with temporary refs and\ngets us the historical pack feature we've been looking to implement.\n\n-- \n"},{"id":"298566","messageId":"7viri6i6uu.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"BAYC1-PASMTP10C050A5FAA4C70AD57679AE040@CEZ.ICE","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-27T06:57:13Z","receivedAt":"2006-10-27T06:57:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sean writes:\n\n> On Thu, 26 Oct 2006 21:42:29 -0400\n> Shawn Pearce wrote:\n>\n>> This is an issue for \"central\" repositories that people push into\n>> and which might be getting repacked according to a cronjob.\n>> \n>> Unfortunately I don't have a solution. I tried to come up with\n>> one but didn't. :-)\n>\n> What about creating a temporary ref before pushing, and then removing\n> it only after the HEAD has been updated?\n\nThat won't work. If repack is faster than index-pack, repack\nwould fail to find necessary objects, barf, and would not remove\nthe existing or new pack, and then index-pack would eventually\nsucceed and when it does at least your repository is complete\neven though it may still have redundant objects in packs.\n\nSo in that sense, it is not a disaster, so it might be a good\nenough solution.\n\nI'd almost say \"heavy repository-wide operations like 'repack -a\n-d' and 'prune' should operate under a single repository lock\",\nbut historically we've avoided locks and instead tried to do\nthings optimistically and used compare-and-swap to detect\nconflicts, so maybe that avenue might be worth pursuing.\n\nHow about (I'm thinking aloud and I'm sure there will be\nholes -- I won't think about prune for now)...\n\n* \"repack -a -d\":\n\n (1) initially run show-ref (or \"ls-remote .\") and store the\n result in .git/$ref_pack_lock_file;\n\n (2) enumerate existing packs;\n\n (3) do the usual \"rev-list --all | pack-objects\" thing; this\n may end up including more objects than what are reachable\n from the result of (1) if somebody else updates refs in the\n meantime;\n\n (4) enumerate existing packs; if there is difference from (2)\n other than what (3) created, that means somebody else added\n a pack in the meantime; stop and do not do the \"-d\" part;\n\n (5) run \"ls-remote .\" again and compare it with what it got in\n (1); if different, somebody else updated a ref in the\n meantime; stop and do not do the \"-d\" part;\n\n (6) do the \"-d\" part as usual by removing packs we saw in (2)\n but do not remove the pack we created in (3);\n\n (7) remove .git/$ref_pack_lock_file.\n\n* \"fetch --thin\" and \"index-pack --stdin\":\n\n (1) check the .git/$ref_pack_lock_file, and refuse to operate\n if there is such (this is not strictly needed for\n correctness but only to give an early exit);\n\n (2) create a new pack under a temporary name, and when\n complete, make the pack/index pair .pack and .idx;\n\n (3) update the refs.\n\n"},{"id":"295463","messageId":"81b0412b0610270042w29279b90t7c94d8590d701519@mail.gmail.com","threadId":"43082","inReplyTo":"20061027044233.GA29057@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-10-27T07:42:47Z","receivedAt":"2006-10-27T07:42:47Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"> So the receive-pack process becomes:\n>\n> a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n> b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n\nWhy not $GIT_DIR/objects/tmp/pack... and ignore it everywhere?\n\nOn 10/27/06, Shawn Pearce wrote:\n> Eran Tromer wrote:\n> > On 2006-10-27 05:00, Shawn Pearce wrote:\n> > >> Change git-repack to follow references under $GIT_DIR/tmp/refs/ too.\n> > >> To receive or fetch a pack:\n> > >> 1. Add references to the new heads in\n> > >> `mktemp $GIT_DIR/tmp/refs/XXXXXX`.\n> > >> 2. Put the new .pack under $GIT_DIR/objects/pack/.\n> > >> 3. Put the new .idx under $GIT_DIR/objects/pack/.\n> > >> 4. Update the relevant heads under $GIT_DIR/refs/.\n> > >> 5. Delete the references from step 1.\n> >\n> > > That was actually my (and also Sean's) solution. Except I would\n> > > put the temporary refs as \"$GIT_DIR/refs/ref_XXXXXX\" as this is\n> > > less code to change and its consistent with how temporary loose\n> > > objects are created.\n> >\n> > If you do that, other programs (e.g., anyone who uses rev-list --all)\n> > may try to walk those heads or consider them available before the pack\n> > is really there. The point about $GIT_DIR/tmp/refs is that only programs\n> > meddling with physical packs (git-fetch, git-receive-pack, git-repack)\n> > will know about it.\n>\n> Doh. Yes, of course, that makes much sense.\n>\n> Hmm... Looking at git-repack we have two things currently pending\n> to rework in there:\n>\n> - Historical vs. active packs.\n> - Don't delete a possibly still incoming pack during -d.\n>\n> These have a lot of the same implementation issues. We need to\n> be able to identify a set of packs which should be allowed for\n> repack with -a, and allowed for removal with -d if -a was also used.\n> A newly uploaded pack cannot be in that list unless its contents are\n> referenced by one or more refs (which implies that the receive-pack\n> process has completed).\n>\n> I'm thinking that the ref thing might be unnecessary. We just\n> need to fix repack so it builds a list of \"active packs\" whose\n> objects should be copied into the new pack, and then only packs\n> loose objects and those objects contained by an active packs.\n>\n> So the receive-pack process becomes:\n>\n> a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n> b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n> c. Write pack and index.\n> d. Move pack to $GIT_DIR/objects/pack/...\n> e. Move index to $GIT_DIR/objects/pack...\n> f. Update refs.\n> g. Arrange for new pack and index to be considered active.\n>\n> And the repack -a -d process becomes:\n>\n> 1. List all active packs and store in memory.\n> 2. Repack only loose objects and objects contained in active packs.\n> 3. Move new pack and idx into $GIT_DIR/objects/pack/...\n> 4. Arrange for new pack and idx to be considered active.\n> 5. Delete active packs found by step #1.\n>\n> Junio was originally considering making historical packs\n> historical by placing their names into an information file (such as\n> `$GIT_DIR/objects/info/historical-packs`) and then consider all other\n> packs as active. Thus step #1 is list all packs and removes those\n> whose names appear in historical-packs, while step #4 is unnecessary.\n>\n> I was thinking about just changing the \"pack-\" prefix to \"hist-\" for\n> the historical packs and assuming all \"pack-*.pack\" to be active.\n> Thus step #1 is a simple glob on the pack directory and step #4\n> is unnecessary.\n>\n> In the latter case its easy to mark an existing pack as historical\n> (just hardlink hist- names for pack, then idx, then unlink previous\n> names) and its also easy to mark new incoming packs as non active\n> by using a different prefix (e.g. \"incm-\") during step #d/#e and\n> then relinking them as \"pack-\" during step #g. Its also very safe\n> on systems that support hardlinks.\n>\n> We shouldn't ever need to worry about race conditions with repacking\n> historical packs. For starters historical packs will tend to be\n> several years' worth of object accumulation and will be so large\n> that repacking them might take 45 minutes or more. Thus they\n> probably will never get repacked. An active pack will simply move\n> into historical status after it gets so large that its no longer\n> worthwhile to keep repacking it. They also will tend to have objects\n> that are so old that at least one ref in the repository will point\n> at their entire DAG and thus everything would carry over on a repack.\n>\n> So this would be cleaner then messing around with temporary refs and\n> gets us the historical pack feature we've been looking to implement.\n>\n> --\n> Shawn.\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"297711","messageId":"20061027075229.GD29057@spearce.org","threadId":"43082","inReplyTo":"81b0412b0610270042w29279b90t7c94d8590d701519@mail.gmail.com","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-27T07:52:29Z","receivedAt":"2006-10-27T07:52:29Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Alex Riesen wrote:\n> >So the receive-pack process becomes:\n> >\n> > a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n> >b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n> \n> Why not $GIT_DIR/objects/tmp/pack... and ignore it everywhere?\n\nBecause there is a race condition.\n\nThe contents of the new pack must be accessable as a normal pack\nbefore we update and unlock the refs that are being changed. This\nmeans it must be a normal pack in $GIT_DIR/objects/pack.\n\nCurrently all packs under $GIT_DIR/objects/pack are deleted during\n`repack -a -d`. Those packs may have been added to that directory\nafter the repack started resulting in them getting deleted when\nthe repack completes, but with none of their contained objects in\nthe newly created pack. Thus the repository is suddenly missing\neverything that was just pushed (or fetched).\n\n-- \n"},{"id":"296015","messageId":"81b0412b0610270108t7b93d04y2c99be20b7f41387@mail.gmail.com","threadId":"43082","inReplyTo":"20061027075229.GD29057@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2006-10-27T08:08:17Z","receivedAt":"2006-10-27T08:08:17Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":">> >So the receive-pack process becomes:\n>> >\n>> > a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n>> >b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n>>\n>> Why not $GIT_DIR/objects/tmp/pack... and ignore it everywhere?\n>\n> Because there is a race condition.\n\nOh, right. Incidentally, is there a lockfile for packs?\n\nOn 10/27/06, Shawn Pearce wrote:\n> Alex Riesen wrote:\n> > >So the receive-pack process becomes:\n> > >\n> > > a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n> > >b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n> >\n> > Why not $GIT_DIR/objects/tmp/pack... and ignore it everywhere?\n>\n> Because there is a race condition.\n>\n> The contents of the new pack must be accessable as a normal pack\n> before we update and unlock the refs that are being changed. This\n> means it must be a normal pack in $GIT_DIR/objects/pack.\n>\n> Currently all packs under $GIT_DIR/objects/pack are deleted during\n> `repack -a -d`. Those packs may have been added to that directory\n> after the repack started resulting in them getting deleted when\n> the repack completes, but with none of their contained objects in\n> the newly created pack. Thus the repository is suddenly missing\n> everything that was just pushed (or fetched).\n>\n> --\n> Shawn.\n"},{"id":"294102","messageId":"20061027081343.GE29057@spearce.org","threadId":"43082","inReplyTo":"81b0412b0610270108t7b93d04y2c99be20b7f41387@mail.gmail.com","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-27T08:13:43Z","receivedAt":"2006-10-27T08:13:43Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Alex Riesen wrote:\n> >>>So the receive-pack process becomes:\n> >>>\n> >>> a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n> >>>b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n> >>\n> >>Why not $GIT_DIR/objects/tmp/pack... and ignore it everywhere?\n> >\n> >Because there is a race condition.\n> \n> Oh, right. Incidentally, is there a lockfile for packs?\n\nNo, we have never needed them before. And I think we'd like to\navoid adding them now.\n\nWe do have the rule that a pack can only be accessed if its .idx\nfile exists; therefore we generate a pack and its index, then place\nthe pack into the object directory, then the index. That way the\npack is there before the index but another process won't attempt\nto read the pack until the index exists. It also means we should\nnever put an index into the pack directory before its associated\npack. :-)\n\nIf we are pruneing packs or loose objects after the repack we delete\nonly after the pack and index are in place. Running processes\nrescan the existing packs once if they look for an object and\ncannot find it in the loose objects directory or in the packs it\nalready knows about; this lets most running processes automatically\nrecover should a parallel repack delete the loose objects it needs\n(as they were just packed).\n\n-- \n"},{"id":"294608","messageId":"Pine.LNX.4.64.0610271022240.11384@xanadu.home","threadId":"43082","inReplyTo":"20061027030054.GB28407@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-27T14:27:05Z","receivedAt":"2006-10-27T14:27:05Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 26 Oct 2006, Shawn Pearce wrote:\n\n> Unfortunately it does not completely work.\n> \n> What happens when the incoming pack (steps #2 and #3) takes 15\n> minutes to upload (slow ADSL modem, lots of objects) and the\n> background repack process sees those temporary refs and starts\n> trying to include those objects? It can't walk the DAG that those\n> refs point at because the objects aren't in the current repository.\n> \n> From what I know of that code the pack-objects process will fail to\n> find the object pointed at by the ref, rescan the packs directory,\n> find no new packs, look for the object again, and abort over the\n> \"corruption\".\n> \n> OK so the repository won't get corrupted but the repack would be\n> forced to abort.\n\nMaybe this is the best way out? Abort git-repack with \"a fetch is in \nprogress -- retry later\". No one will really suffer if the repack has \nto wait for the next scheduled cron job, especially if the fetch doesn't \nexplode packs into loose objects anymore.\n\n> Another issue I just thought about tonight is we may need a\n> count-packs utility that like count-objects lists the number\n> of active packs and their total size. If we start hanging onto\n> every pack we receive over the wire the pack directory is going to\n> grow pretty fast and we'll need a way to tell us when its time to\n> `repack -a -d`.\n\nSure. Although the pack count is going to grow much less rapidly. \nThink of one pack per fetch instead of many many objects per fetch.\n\n\n"},{"id":"294456","messageId":"20061027143854.GC20017@pasky.or.cz","threadId":"43082","inReplyTo":"Pine.LNX.4.64.0610271022240.11384@xanadu.home","subject":"Re: fetching packs and storing them as packs","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-27T14:38:54Z","receivedAt":"2006-10-27T14:38:54Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Fri, Oct 27, 2006 at 04:27:05PM CEST, I got a letter\nwhere Nicolas Pitre said that...\n> On Thu, 26 Oct 2006, Shawn Pearce wrote:\n> > OK so the repository won't get corrupted but the repack would be\n> > forced to abort.\n> \n> Maybe this is the best way out? Abort git-repack with \"a fetch is in \n> progress -- retry later\". No one will really suffer if the repack has \n> to wait for the next scheduled cron job, especially if the fetch doesn't \n> explode packs into loose objects anymore.\n\nI don't really like this that much. Big projects can have 10 commits per\nhour on average, and they also take potentially long time to repack, so\nyou might get to never really repack them.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i I don't really like this that much. Big projects can have 10 commits per\n> hour on average, and they also take potentially long time to repack, so\n> you might get to never really repack them.\n\nAn average of 10 per minute doesn't mean there aren't frequent long idle\ntimes. That commit traffic is probably extremely bursty, right?\n\n"},{"id":"297640","messageId":"20061027150334.GD20017@pasky.or.cz","threadId":"43082","inReplyTo":"20061027144839.GB32451@fieldses.org","subject":"Re: fetching packs and storing them as packs","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2006-10-27T15:03:34Z","receivedAt":"2006-10-27T15:03:34Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Dear diary, on Fri, Oct 27, 2006 at 04:48:39PM CEST, I got a letter\nwhere \"J. Bruce Fields\" said that...\n> On Fri, Oct 27, 2006 at 04:38:54PM +0200, Petr Baudis wrote:\n> > I don't really like this that much. Big projects can have 10 commits per\n> > hour on average, and they also take potentially long time to repack, so\n> > you might get to never really repack them.\n> \n> An average of 10 per minute doesn't mean there aren't frequent long idle\n> times. That commit traffic is probably extremely bursty, right?\n\n10 per _hour_. :-)\n\nE.g. GNOME is 7 commits per hour average, and it does tend to be pretty\nspread out:\n\n\thttp://cia.navi.cx/stats/project/gnome\n\n(Unfortunately I can't figure out how to squeeze more commits from the\nweb interface. KDE gets even more commits than GNOME and Gentoo tops\nall the CIA-tracked projects.)\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\n#!/bin/perl -sp0777i Dear diary, on Fri, Oct 27, 2006 at 04:48:39PM CEST, I got a letter\n> where \"J. Bruce Fields\" said that...\n\nYa know, it'd be cool if that fit on one line....\n\n> > On Fri, Oct 27, 2006 at 04:38:54PM +0200, Petr Baudis wrote:\n> > > I don't really like this that much. Big projects can have 10 commits per\n> > > hour on average, and they also take potentially long time to repack, so\n> > > you might get to never really repack them.\n> > \n> > An average of 10 per minute doesn't mean there aren't frequent long idle\n> > times. That commit traffic is probably extremely bursty, right?\n> \n> 10 per _hour_. :-)\n\nWhoops, right.\n\n> E.g. GNOME is 7 commits per hour average, and it does tend to be pretty\n> spread out:\n> \n> \thttp://cia.navi.cx/stats/project/gnome\n> \n> (Unfortunately I can't figure out how to squeeze more commits from the\n> web interface. KDE gets even more commits than GNOME and Gentoo tops\n> all the CIA-tracked projects.)\n\nThat's not enough to tell how long on average you'd have to wait for a\ngap of a certain length.\n\nI think if you expect x commits per hour, and need y hours to prune,\nthen you should be able to get a worst-case estimate of hours between\ny-hour gaps from\n\n\toctave -q --eval \"1/poisscdf(0,x/y)\"\n\nbut my statistics isn't great, so maybe that's not quite right.\n\nAnd in any case the commit arrival times are probably very far from\nindependent, which probably makes gaps more likely.\n\n"},{"id":"295586","messageId":"20061027160559.GB3670@fieldses.org","threadId":"43082","inReplyTo":"20061027160450.GA3670@fieldses.org","subject":"Re: fetching packs and storing them as packs","fromName":"J. Bruce Fields","fromEmail":"bfields@fieldses.org","sentAt":"2006-10-27T16:05:59Z","receivedAt":"2006-10-27T16:05:59Z","isPatch":false,"sender":{"key":"bfields@citi.umich.edu","avatar":null},"body":"On Fri, Oct 27, 2006 at 12:04:50PM -0400, bfields wrote:\n> I think if you expect x commits per hour, and need y hours to prune,\n> then you should be able to get a worst-case estimate of hours between\n> y-hour gaps from\n> \n> \toctave -q --eval \"1/poisscdf(0,x/y)\"\n\nUh, sorry, that should be x*y, not x/y....\n\n"},{"id":"295565","messageId":"Pine.LNX.4.64.0610271252390.11384@xanadu.home","threadId":"43082","inReplyTo":"7viri6i6uu.fsf@assigned-by-dhcp.cox.net","subject":"Re: fetching packs and storing them as packs","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-27T17:23:31Z","receivedAt":"2006-10-27T17:23:31Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 26 Oct 2006, Junio C Hamano wrote:\n\n> I'd almost say \"heavy repository-wide operations like 'repack -a\n> -d' and 'prune' should operate under a single repository lock\",\n> but historically we've avoided locks and instead tried to do\n> things optimistically and used compare-and-swap to detect\n> conflicts, so maybe that avenue might be worth pursuing.\n> \n> How about (I'm thinking aloud and I'm sure there will be\n> holes -- I won't think about prune for now)...\n> \n> * \"repack -a -d\":\n> \n> (1) initially run show-ref (or \"ls-remote .\") and store the\n> result in .git/$ref_pack_lock_file;\n> \n> (2) enumerate existing packs;\n> \n> (3) do the usual \"rev-list --all | pack-objects\" thing; this\n> may end up including more objects than what are reachable\n> from the result of (1) if somebody else updates refs in the\n> meantime;\n> \n> (4) enumerate existing packs; if there is difference from (2)\n> other than what (3) created, that means somebody else added\n> a pack in the meantime; stop and do not do the \"-d\" part;\n> \n> (5) run \"ls-remote .\" again and compare it with what it got in\n> (1); if different, somebody else updated a ref in the\n> meantime; stop and do not do the \"-d\" part;\n> \n> (6) do the \"-d\" part as usual by removing packs we saw in (2)\n> but do not remove the pack we created in (3);\n> \n> (7) remove .git/$ref_pack_lock_file.\n> \n> * \"fetch --thin\" and \"index-pack --stdin\":\n> \n> (1) check the .git/$ref_pack_lock_file, and refuse to operate\n> if there is such (this is not strictly needed for\n> correctness but only to give an early exit);\n\nI don't think this is a good idea. A fetch should always work \nirrespective of any repack taking place. The fetch really should have \npriority over a repack since it is directly related to the user \nexperience. The repack can fail or produce suboptimal results if a race \noccurs, but the fetch must not fail for such a reason.\n\n> (2) create a new pack under a temporary name, and when\n> complete, make the pack/index pair .pack and .idx;\n\nActually this is what already happens if you don't specify a name to \ngit-index-pack --stdin.\n\n> (3) update the refs.\n\nSo the actual race is the really small interval between the time the new \npack+index are moved to .git/objects/pack/ and the moment the refs are \nupdated. In practice this is probably less than a second. All that is \nneeded here is to somehow go back to (2) if that interval occurs between \n(2) and (3).\n\n\n"},{"id":"293850","messageId":"7vy7r1egfl.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"20061027143854.GC20017@pasky.or.cz","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-27T18:56:14Z","receivedAt":"2006-10-27T18:56:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> Dear diary, on Fri, Oct 27, 2006 at 04:27:05PM CEST, I got a letter\n> where Nicolas Pitre said that...\n>> On Thu, 26 Oct 2006, Shawn Pearce wrote:\n>> > OK so the repository won't get corrupted but the repack would be\n>> > forced to abort.\n>> \n>> Maybe this is the best way out? Abort git-repack with \"a fetch is in \n>> progress -- retry later\". No one will really suffer if the repack has \n>> to wait for the next scheduled cron job, especially if the fetch doesn't \n>> explode packs into loose objects anymore.\n>\n> I don't really like this that much. Big projects can have 10 commits per\n> hour on average, and they also take potentially long time to repack, so\n> you might get to never really repack them.\n\nOne question about that statistics is if the frequency of 10\ncommits per hour is 10 pushes into the central repository per\nhour or 10 commits distributed all over the world in dozens of\ndevelopers' repositories.\n\nEven if the number is 10 pushes into the central repository per\nhour, I do not see it as a big problem in practice from the\nworkflow point of view. Even people sticking to their CVS\nworkflow to have a central repository model are gaining big time\nfrom being able to keep working disconnected by switching to git\nusing the shared repository mode, and it should not be a big\ndeal if the central repository master shuts down pushes into the\nrepository for N minutes a day for scheduled repacking. So it\ncould be that a more practical way out is to say \"'repack -a -d'\nand 'prune' are to be run when things are quiescent\".\n\nA cron job for the scheduled repack/prune can set a flag\n(repository wide lockfile or something) to ask new push/fetch to\nwait and come back later, and we could set up a pre-* hooks for\npush/fetch to notice it. While push/fetch processes that have\nalready been started can still interfere, as long as they cause\nrepack/prune to fail the \"deletion\" part, eventually outstanding\npush/fetch will die out and the cron job will have that\nquiescent window.\n\n"},{"id":"297174","messageId":"Pine.LNX.4.64.0610271310450.3849@g5.osdl.org","threadId":"43082","inReplyTo":"4540CA0C.6030300@tromer.org","subject":"Re: fetching packs and storing them as packs","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-27T20:22:18Z","receivedAt":"2006-10-27T20:22:18Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 26 Oct 2006, Eran Tromer wrote:\n>\n> This creates a race condition w.r.t. \"git repack -a -d\", similar to the\n> existing race condition between \"git fetch --keep\" and\n> \"git repack -a -d\". There's a point in time where the new pack is stored\n> but not yet referenced, and if \"git repack -a -d\" runs at that point it\n> will eradicate the pack. When the heads are finally updated, you get a\n> corrupted repository.\n\n(I note that there's a whole thread on this, but I was off doing other \nthings, so I probably missed part of it)\n\nWe really should _never_ create a pack in-place with the final name.\n\nThe way to fix the race is to simply not create the patch as\n\n\t.git/objects/packed/pack-xyz.{pack|idx}\n\nin the first place, but simply \"mv\" them into place later. If you do that \nafter you've written the temporary pointer to it, there is no race (the \ntemporary pointer may not be usable, of course, but that's a separate \nissue).\n\nThat said, I think some of the \"git repack -d\" logic is also unnecessarily \nfragile. In particular, it shouldn't just do\n\n\texisting=$(find . -type f \\( -name '*.pack' -o -name '*.idx' \\))\n\nlike it does to generate the \"existing\" list, it should probably only ever \nremove a pack-file and index file _pair_, ie it should do something like\n\n\texisting=$(find . -type f -name '*.pack')\n\nand then do\n\n\tfor pack in $existing\n\tdo\n\t\tindex=\"$(basename $pack).idx\"\n\t\tif [ -f $index ] && [ \"$pack\"!= \"$newpack\" ]\n\t\tthen\n\t\t\trm -f \"$pack\" \"$index\"\n\t\tfi\n\tdone\n\netc, exactly so that it would never remove anything that is getting \nindexed or is otherwise half-way done, regardless of any other issues.\n\nHmm?\n\n"},{"id":"297705","messageId":"7v3b99e87c.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"Pine.LNX.4.64.0610271310450.3849@g5.osdl.org","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-27T21:53:59Z","receivedAt":"2006-10-27T21:53:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> We really should _never_ create a pack in-place with the final name.\n\nThe \"fattening\" index-pack Nico did does not have this problem\nas far as I can see. Under --stdin, it creates a temporary pack\nfile \"$GIT_OBJECT_DIRECTORY/pack_XXXXXX\"; after the received\npack is fattened by adding missing delta-base objects and fixing\nup the pack header, final() moves the {pack,idx} pair to the\nfinal location.\n\nThe race is about this sequence:\n\n\t- git-receive-pack is spawned from remove git-send-pack;\n it lets \"index-pack --stdin --fatten\" to keep the pack.\n\n\t- index-pack does its magic and moves the pack and idx\n to their final location;\n\n\t- \"repack -a -d\" is started by somebody else; it first\n remembers all the existing packs; it does the usual\n repacking-into-one.\n\n\t- git-receive-pack that invoked the index-pack waits for\n index-pack to finish, and then updates the refs;\n\n\t- \"repack -a -d\" is done repacking; removes the packs\n that existed when it checked earlier.\n\nTwo instances of receive-pack running simultaneously is safe (in\nthe sense that it does not corrupt the repository; one instance\ncan fail after noticing the other updated the ref it wanted to\nupdate) and there is no reason to exclude each other. But\n\"repack -a -d\" and receive-pack are not.\n\nCan we perhaps have reader-writer lock on the filesystem to\npretect the repository? \"prune\" can also be made into a writer\nfor that lock and \"fetch-pack --keep\" would be a reader for the\nlock. That reader-writer lock would solve the issue rather\nnicely.\n\n> That said, I think some of the \"git repack -d\" logic is also unnecessarily \n> fragile.\n\nNoted; will fix.\n"},{"id":"295672","messageId":"20061028034206.GA14044@spearce.org","threadId":"43082","inReplyTo":"7v3b99e87c.fsf@assigned-by-dhcp.cox.net","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-28T03:42:06Z","receivedAt":"2006-10-28T03:42:06Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Linus Torvalds writes:\n[snip]\n> Can we perhaps have reader-writer lock on the filesystem to\n> pretect the repository? \"prune\" can also be made into a writer\n> for that lock and \"fetch-pack --keep\" would be a reader for the\n> lock. That reader-writer lock would solve the issue rather\n> nicely.\n> \n> > That said, I think some of the \"git repack -d\" logic is also unnecessarily \n> > fragile.\n> \n> Noted; will fix.\n\nSo a reader-writer lock is preferred over\na non-locking solution such as I posted in\nhttp://article.gmane.org/gmane.comp.version-control.git/30288 ?\n\nNot to mention that such a solution would also fix the -d issue\nLinus points out above.\n\n-- \n"},{"id":"295147","messageId":"7vd58daxoh.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"20061028034206.GA14044@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-28T04:09:34Z","receivedAt":"2006-10-28T04:09:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> So a reader-writer lock is preferred over\n> a non-locking solution such as I posted in\n> http://article.gmane.org/gmane.comp.version-control.git/30288 ?\n\nIf you mean these two in your message to be \"solution\":\n\n So the receive-pack process becomes:\n\n a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n c. Write pack and index.\n d. Move pack to $GIT_DIR/objects/pack/...\n e. Move index to $GIT_DIR/objects/pack...\n f. Update refs.\n g. Arrange for new pack and index to be considered active.\n\n And the repack -a -d process becomes:\n\n 1. List all active packs and store in memory.\n 2. Repack only loose objects and objects contained in active packs.\n 3. Move new pack and idx into $GIT_DIR/objects/pack/...\n 4. Arrange for new pack and idx to be considered active.\n 5. Delete active packs found by step #1.\n\nI am not so sure how it solves anything at all.\n\nThe race is about this sequence:\n\n - git-receive-pack is spawned from remove git-send-pack;\n it lets \"index-pack --stdin --fatten\" to keep the pack.\n\n - index-pack does its magic and moves the pack and idx\n to their final location;\n\n - \"repack -a -d\" is started by somebody else; it first\n remembers all the existing packs; it does the usual\n repacking-into-one.\n\n - git-receive-pack that invoked the index-pack waits for\n index-pack to finish, and then updates the refs;\n\n - \"repack -a -d\" is done repacking; removes the packs\n that existed when it checked earlier.\n\nNow, I am not sure what your plan to \"arrange for new pack and\nidx to be considered active\" is. Care to explain?\n\nThere is a tricky constraints imposed on us by (arguably broken)\ncommit walkers in that it relies on the (arguably broken)\nsha1_file.c:sha1_pack_name() interface, so naming historical\nones $GIT_OBJECT_DIR/pack/hist-X{40}.pack would not work; we\nwould need to fix commit walkers for that first.\n\n\n\n"},{"id":"294680","messageId":"Pine.LNX.4.64.0610272109500.3849@g5.osdl.org","threadId":"43082","inReplyTo":"20061028034206.GA14044@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-28T04:18:07Z","receivedAt":"2006-10-28T04:18:07Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 27 Oct 2006, Shawn Pearce wrote:\n> \n> So a reader-writer lock is preferred over\n> a non-locking solution such as I posted in\n> http://article.gmane.org/gmane.comp.version-control.git/30288 ?\n> \n> Not to mention that such a solution would also fix the -d issue\n> Linus points out above.\n\nBe very careful.\n\nThere's a good reason why git doesn't use locking, and tends to use the \n\"create file exclusively and move over the old version after having tested \nthat the old version is still relevant\" approach.\n\nTwo _major_ issues:\n\n - just about any other locking algorithm simply doesn't work on some \n filesystems. And then you're just royally screwed.\n\n - I want to be able to push out, regardless of whether there is somebody \n (or millions of somebodies) reading the repository at the same time. So \n locking is not acceptable for \"normal operations\" at all - at most this \n would be a \"keep a repack from interfering with another repack\" kind of \n thing.\n\nI would MUCH rather we just rename the index/pack file to something that \ngit can _use_, but that \"git repack -a -d\" won't remove. In other words, \nrather than locking, it would be much better to just use a naming rule: \nwhen we download a new pack, the new pack will be called\n\n\tnew-pack-.pack\n\tnew-pack-.idx\n\nand we just make the rule that \"git repack -a -d\" will only ever touch \npacks that are called just \"pack-*.{pack|idx}\", and never anything else.\n\nIt really is that simple. Allow normal git object opens to open the \n\"temporary file\" naming version too (so that you can install the refs \nbefore the rename, and all the objects will be visible), but don't allow \n\"git repack\" to remove packs that are in the process of being installed.\n\nRace removed, and no locking really needed. At most, we might need to be \nable to match up a \"new-pack-*.idx\" file with a \"pack-*.pack\" file when we \nopen pack-files, simply because we can't rename two files atomically, so \nthe pack-file and index file would potentially exist with \"different\" \nnames for a short window. \n\nThat kind of small semantic changes are _way_ better than introducing \nlocking, which will inevitably have much worse error cases (not working, \nstale locks, inability to push because something is really slow, or any \nnumber of other problems).\n\n"},{"id":"297987","messageId":"7vwt6l9etn.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"Pine.LNX.4.64.0610272109500.3849@g5.osdl.org","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-28T05:42:12Z","receivedAt":"2006-10-28T05:42:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Fri, 27 Oct 2006, Shawn Pearce wrote:\n>> \n>> So a reader-writer lock is preferred over\n>> a non-locking solution such as I posted in\n>> http://article.gmane.org/gmane.comp.version-control.git/30288 ?\n>> \n>> Not to mention that such a solution would also fix the -d issue\n>> Linus points out above.\n>\n> Be very careful.\n>\n> There's a good reason why git doesn't use locking, and tends to use the \n> \"create file exclusively and move over the old version after having tested \n> that the old version is still relevant\" approach.\n>\n> Two _major_ issues:\n>...\n>\n> I would MUCH rather we just rename the index/pack file to something that \n> git can _use_, but that \"git repack -a -d\" won't remove....\n\nTwo points.\n\nThe \"locking\" I mentioned was between receive-pack and repack -a\n-d; upload-pack (what millions people are using to read from the\nrepository you are pushing into) is not affected. So in that\nsense, we can afford to use lock without much contention.\n\nI just thought of a cute hack that does not involve renaming\npacks at all (so no need to match new-pack-X.pack with\npack-X.idx), and Shawn's sequence actually would work, which is:\n\nThe receive-pack side:\n\n a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n c. Write pack and index, in \"inactive\" state.\n d. Move pack to $GIT_DIR/objects/pack/...\n e. Move idx to $GIT_DIR/objects/pack...\n f. Update refs.\n g. Mark new pack and idx as \"active\".\n\nThe \"repack -a -d\" side:\n\n 1. List all active packs and store in memory.\n 2. Repack only loose objects and objects contained in active packs.\n 3. Move new pack and idx into $GIT_DIR/objects/pack/...\n 4. Mark new pack and idx as \"active\".\n 5. Delete active packs found by step #1.\n\nPack-idx pair is marked \"active\" by \"chmod u+s\" the .pack file.\nDuring the normal operation, all .pack/.idx pair in objects/pack/\ndirectories are usable regardless of the setuid bit; we would\nnever make .pack files executable so u+s would not otherwise\nhurt us either. \"active\" probably is better read as \"eligible\nfor repacking\".\n\n\n"},{"id":"297635","messageId":"20061028072146.GB14607@spearce.org","threadId":"43082","inReplyTo":"7vwt6l9etn.fsf@assigned-by-dhcp.cox.net","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-28T07:21:46Z","receivedAt":"2006-10-28T07:21:46Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Linus Torvalds writes:\n> > I would MUCH rather we just rename the index/pack file to something that \n> > git can _use_, but that \"git repack -a -d\" won't remove....\n> \n> Two points.\n> \n> The \"locking\" I mentioned was between receive-pack and repack -a\n> -d; upload-pack (what millions people are using to read from the\n> repository you are pushing into) is not affected. So in that\n> sense, we can afford to use lock without much contention.\n\nAnd giving how difficult locking is to get on most filesystems I'd\njust rather avoid any sort of locking whenever possible. That's one\nreason why reflog is 1 file per ref and not 1 file per repository...\n \n> I just thought of a cute hack that does not involve renaming\n> packs at all (so no need to match new-pack-X.pack with\n> pack-X.idx), and Shawn's sequence actually would work, which is:\n\nI take this above statement to mean that you answered your own\nquestion about how my sequence is able to resolve the race condition?\n \n> The receive-pack side:\n> \n> a. Create temporary pack file in $GIT_DIR/objects/pack_XXXXX.\n> b. Create temporary index file in $GIT_DIR/objects/index_XXXXX.\n> c. Write pack and index, in \"inactive\" state.\n> d. Move pack to $GIT_DIR/objects/pack/...\n> e. Move idx to $GIT_DIR/objects/pack...\n> f. Update refs.\n> g. Mark new pack and idx as \"active\".\n> \n> The \"repack -a -d\" side:\n> \n> 1. List all active packs and store in memory.\n> 2. Repack only loose objects and objects contained in active packs.\n> 3. Move new pack and idx into $GIT_DIR/objects/pack/...\n> 4. Mark new pack and idx as \"active\".\n> 5. Delete active packs found by step #1.\n> \n> Pack-idx pair is marked \"active\" by \"chmod u+s\" the .pack file.\n> During the normal operation, all .pack/.idx pair in objects/pack/\n> directories are usable regardless of the setuid bit; we would\n> never make .pack files executable so u+s would not otherwise\n> hurt us either. \"active\" probably is better read as \"eligible\n> for repacking\".\n\nAs cool as that trick is I'm against using the file mode as a\nway to indicate the status of a pack file. For one thing not\nevery filesystem that Git is used on handles file modes properly.\nWe already have core.filemode thanks to some of those and I use\nGit on at least one of those \"not so friendly\" filesystems...\n\nWhy not just use create a new flag file?\n\nLets say that a pack X is NOT eligible to be repacked if\n\"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n\nThus we want to have the new \".keep\" file for historical packs and\nincoming receive-pack between steps c and g. In the former case\nthe historical pack is already \"very large\" and thus one additional\nempty file to indicate we want to retain that pack as-is is trivial\noverhead (relatively speaking); in the latter case the lifespan of\nthe file is relatively short and thus any overhead associated with it\non the local filesystem is free (it may never even hit the platter).\n\nIn the sequence above we create pack-X.keep between steps b and c\nduring receive-pack ensuring that even before the pack is usable by\na Git reader process that it can't be swept up by a `repack -a -d`\nand we delete the pack-X.keep file in step g to mark it active.\n\nFurther only repack and the receive-pack side code changes: all\nexisting packs are automatically taken to be active while only\npacks coming in from receive-pack or those marked by a human as\n\"historical\" will be kept.\n\nTwo birds, one stone. Thoughts?\n\n-- \n"},{"id":"297809","messageId":"20061028084001.GC14607@spearce.org","threadId":"43082","inReplyTo":"20061028072146.GB14607@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-28T08:40:01Z","receivedAt":"2006-10-28T08:40:01Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Shawn Pearce wrote:\n> Why not just use create a new flag file?\n> \n> Lets say that a pack X is NOT eligible to be repacked if\n> \"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n\nHere's the `git repack -a -d` portion of that.\nThoughts?\n\n-- >8 --\n[PATCH] Only repack active packs by skipping over kept packs.\n\nDuring `git repack -a -d` only repack objects which are loose or\nwhich reside in an active (a non-kept) pack. This allows the user\nto keep large packs as-is without continuous repacking and can be\nvery helpful on large repositories. It should also help us resolve\na race condition between `git repack -a -d` and the new pack store\nfunctionality in `git-receive-pack`.\n\nKept packs are those which have a corresponding .keep file in\n$GIT_OBJECT_DIRECTORY/pack. That is pack-X.pack will be kept\n(not repacked and not deleted) if pack-X.keep exists in the same\ndirectory when `git repack -a -d` starts.\n\nCurrently this feature is not documented and there is no user\ninterface to keep an existing pack.\n\nSigned-off-by: Shawn O. Pearce \n---\n git-repack.sh | 48 +++++++++++++++++++++++++++++++-----------------\n 1 files changed, 31 insertions(+), 17 deletions(-)\n\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 17e2452..fe1e2ef 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -43,13 +43,30 @@ trap 'rm -f \"$PACKTMP\"-*' 0 1 2 3 15\n case \",$all_into_one,\" in\n ,,)\n \targs='--unpacked --incremental'\n+\tactive=\n \t;;\n ,t,)\n-\targs=\n-\n-\t# Redundancy check in all-into-one case is trivial.\n-\texisting=`test -d \"$PACKDIR\" && cd \"$PACKDIR\" && \\\n-\t find . -type f \\( -name '*.pack' -o -name '*.idx' \\) -print`\n+\targs=--unpacked\n+\tactive=\n+\tif test -d \"$PACKDIR\"\n+\tthen\n+\t\tfor p in `find \"$PACKDIR\" -type f -name '*.pack' -print`\n+\t\tdo\n+\t\t\tn=`basename \"$p\" .pack`\n+\t\t\td=`dirname \"$p\"`\n+\t\t\tif test -e \"$d/$n.keep\"\n+\t\t\tthen\n+\t\t\t\t: keep\n+\t\t\telse\n+\t\t\t\targs=\"$args --unpacked=$p\"\n+\t\t\t\tactive=\"$active $n\"\n+\t\t\tfi\n+\t\tdone\n+\tfi\n+\tif test \"X$args\" = X--unpacked\n+\tthen\n+\t\targs='--unpacked --incremental'\n+\tfi\n \t;;\n esac\n \n@@ -86,20 +103,17 @@ fi\n \n if test \"$remove_redundant\" = t\n then\n-\t# We know $existing are all redundant only when\n-\t# all-into-one is used.\n-\tif test \"$all_into_one\" != '' && test \"$existing\" != ''\n+\t# We know $active are all redundant.\n+\tif test \"$active\" != ''\n \tthen\n \t\tsync\n-\t\t( cd \"$PACKDIR\" &&\n-\t\t for e in $existing\n-\t\t do\n-\t\t\tcase \"$e\" in\n-\t\t\t./pack-$name.pack | ./pack-$name.idx) ;;\n-\t\t\t*)\trm -f $e ;;\n-\t\t\tesac\n-\t\t done\n-\t\t)\n+\t\tfor n in $active\n+\t\tdo\n+\t\t\tif test \"$n\" != \"pack-$name\"\n+\t\t\tthen\n+\t\t\t\trm -f \"$PACKDIR/$n.pack\" \"$PACKDIR/$n.idx\"\n+\t\t\tfi\n+\t\tdone\n \tfi\n \tgit-prune-packed\n fi\n-- \n1.4.3.3.g7d63\n"},{"id":"294822","messageId":"Pine.LNX.4.64.0610281058070.3849@g5.osdl.org","threadId":"43082","inReplyTo":"20061028072146.GB14607@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-28T17:59:31Z","receivedAt":"2006-10-28T17:59:31Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 28 Oct 2006, Shawn Pearce wrote:\n> \n> Why not just use create a new flag file?\n> \n> Lets say that a pack X is NOT eligible to be repacked if\n> \"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n\nYeah, me likee. Simple and straightforward, and mixes well with the \n\"--keep\" flag that has been discussed for git repack anyway.\n\n"},{"id":"298387","messageId":"7vk62k9tnk.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"20061028072146.GB14607@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-28T18:34:07Z","receivedAt":"2006-10-28T18:34:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> I take this above statement to mean that you answered your own\n> question about how my sequence is able to resolve the race condition?\n\nYes. I needed more thought after I asked that question.\n\n>...\n> Why not just use create a new flag file?\n>\n> Lets say that a pack X is NOT eligible to be repacked if\n> \"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n\nI like it.\n"},{"id":"297899","messageId":"7vfyd88d6s.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"20061028084001.GC14607@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-28T19:15:07Z","receivedAt":"2006-10-28T19:15:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> Shawn Pearce wrote:\n>> Why not just use create a new flag file?\n>> \n>> Lets say that a pack X is NOT eligible to be repacked if\n>> \"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n>\n> Here's the `git repack -a -d` portion of that.\n> Thoughts?\n\n> +\targs=--unpacked\n> +\tactive=\n> +\tif test -d \"$PACKDIR\"\n> +\tthen\n> +\t\tfor p in `find \"$PACKDIR\" -type f -name '*.pack' -print`\n\nThis change to run 'find \"$PACKDIR\"' is fragile when your\n$GIT_OBJECT_DIRECTORY has $IFS in it; running \"find .\" after\n\"cd\" in a subprocess was done very much on purpose to avoid that\nissue. Please don't break it.\n\n> +\t\tdo\n> +\t\t\tn=`basename \"$p\" .pack`\n> +\t\t\td=`dirname \"$p\"`\n> +\t\t\tif test -e \"$d/$n.keep\"\n> +\t\t\tthen\n> +\t\t\t\t: keep\n> +\t\t\telse\n> +\t\t\t\targs=\"$args --unpacked=$p\"\n> +\t\t\t\tactive=\"$active $n\"\n> +\t\t\tfi\n> +\t\tdone\n> +\tfi\n> +\tif test \"X$args\" = X--unpacked\n> +\tthen\n> +\t\targs='--unpacked --incremental'\n> +\tfi\n> \t;;\n> esac\n\nI do not remember offhand what --incremental meant, but\npresumably this is for the very initial \"repack\" (PACKDIR did\nnot exist or find loop did not find anything to repack) and the\nflag would not make a difference? Care to explain?\n\nOther than that, the overall structure seems quite sane.\n\n"},{"id":"298596","messageId":"4543DA2E.9030300@tromer.org","threadId":"43082","inReplyTo":"20061028072146.GB14607@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-28T22:31:10Z","receivedAt":"2006-10-28T22:31:10Z","isPatch":false,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"Hi Shawn,\n\nOn 2006-10-28 09:21, Shawn Pearce wrote:\n> Lets say that a pack X is NOT eligible to be repacked if\n> \"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n> \n> Thus we want to have the new \".keep\" file for historical packs and\n> incoming receive-pack between steps c and g. In the former case\n> the historical pack is already \"very large\" and thus one additional\n> empty file to indicate we want to retain that pack as-is is trivial\n> overhead (relatively speaking); in the latter case the lifespan of\n> the file is relatively short and thus any overhead associated with it\n> on the local filesystem is free (it may never even hit the platter).\n\nSounds perfect.\n\nIt would be nice to have whoever creates a pack-*.keep file put\nsomething useful as the content of the file, so we'll know what to clean\nup after abnormal termination:\n\n$ grep -l ^git-receive-pack $GIT_DIR/objects/pack/pack-*.keep\n\n\n"},{"id":"294752","messageId":"20061029033829.GA3435@spearce.org","threadId":"43082","inReplyTo":"4543DA2E.9030300@tromer.org","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-29T03:38:29Z","receivedAt":"2006-10-29T03:38:29Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Eran Tromer wrote:\n> Hi Shawn,\n> \n> On 2006-10-28 09:21, Shawn Pearce wrote:\n> > Lets say that a pack X is NOT eligible to be repacked if\n> > \"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n> > \n> > Thus we want to have the new \".keep\" file for historical packs and\n> > incoming receive-pack between steps c and g. In the former case\n> > the historical pack is already \"very large\" and thus one additional\n> > empty file to indicate we want to retain that pack as-is is trivial\n> > overhead (relatively speaking); in the latter case the lifespan of\n> > the file is relatively short and thus any overhead associated with it\n> > on the local filesystem is free (it may never even hit the platter).\n> \n> Sounds perfect.\n> \n> It would be nice to have whoever creates a pack-*.keep file put\n> something useful as the content of the file, so we'll know what to clean\n> up after abnormal termination:\n> \n> $ grep -l ^git-receive-pack $GIT_DIR/objects/pack/pack-*.keep\n\nYes, that's a very good idea. When I do the git-receive-pack\nimplementation tonight I'll try to dump useful information to the\n.keep file such that you can easily grep for the stale .keeps\nand decide which ones should go.\n\n-- \n"},{"id":"297578","messageId":"ei18ak$nv4$1@sea.gmane.org","threadId":"43082","inReplyTo":"20061029033829.GA3435@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2006-10-29T03:48:36Z","receivedAt":"2006-10-29T03:48:36Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Shawn Pearce wrote:\n\n> Eran Tromer wrote:\n>>\n>> It would be nice to have whoever creates a pack-*.keep file put\n>> something useful as the content of the file, so we'll know what to clean\n>> up after abnormal termination:\n>> \n>> $ grep -l ^git-receive-pack $GIT_DIR/objects/pack/pack-*.keep\n> \n> Yes, that's a very good idea. When I do the git-receive-pack\n> implementation tonight I'll try to dump useful information to the\n> .keep file such that you can easily grep for the stale .keeps\n> and decide which ones should go.\n\nPerhaps git-count-packs (or enhanced git-count-objects, or git-count-stuff;\nwhatever it would be named) could also list (with some option) the reasons\nfor packs to be kept...\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n\n"},{"id":"296181","messageId":"20061029035025.GC3435@spearce.org","threadId":"43082","inReplyTo":"7vfyd88d6s.fsf@assigned-by-dhcp.cox.net","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-29T03:50:25Z","receivedAt":"2006-10-29T03:50:25Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Shawn Pearce writes:\n> \n> > Shawn Pearce wrote:\n> >> Why not just use create a new flag file?\n> >> \n> >> Lets say that a pack X is NOT eligible to be repacked if\n> >> \"$GIT_DIR/objects/pack/pack-X.keep\" exists.\n> >\n> > Here's the `git repack -a -d` portion of that.\n> > Thoughts?\n> \n> > +\targs=--unpacked\n> > +\tactive=\n> > +\tif test -d \"$PACKDIR\"\n> > +\tthen\n> > +\t\tfor p in `find \"$PACKDIR\" -type f -name '*.pack' -print`\n> \n> This change to run 'find \"$PACKDIR\"' is fragile when your\n> $GIT_OBJECT_DIRECTORY has $IFS in it; running \"find .\" after\n> \"cd\" in a subprocess was done very much on purpose to avoid that\n> issue. Please don't break it.\n\nI only broke it because you backed me into a corner with --unpacked=\n:-)\n\nThe issue is --unpacked= uses the path of the pack name, which\nincludes $GIT_OBJECT_DIRECTORY, whatever that may be. This makes it\nimpossible for the shell script to hand through a proper --unpacked=\nline for the active packs without including $GIT_OBJECT_DIRECTORY\nas part of the option.\n\nI agree with you about the $IFS issue. I'll redraft this patch\ntonight such that $IFS doesn't get broken here but that's going\nto take a small code patch over in revisions.c, which I'll also\ndo tonight.\n\n> > +\tif test \"X$args\" = X--unpacked\n> > +\tthen\n> > +\t\targs='--unpacked --incremental'\n> > +\tfi\n \n> I do not remember offhand what --incremental meant, but\n> presumably this is for the very initial \"repack\" (PACKDIR did\n> not exist or find loop did not find anything to repack) and the\n> flag would not make a difference? Care to explain?\n \nI think there is a bug in pack-objects but I couldn't find it last\nnight. Using --incremental worked around it. :-)\n\nAccording to the documentation:\n\n --unpacked tells pack-objects to only pack loose objects.\n\n --incremental tells pack-objects to skip any object that is\n already contained in a pack even if it appears in the input list.\n\nWhat I really wanted here was to just use '--unpacked'; if there\nare no active packs and the user asked for '-a' we actually just\nwant to pack the loose objects into a new active pack as we aren't\nallowed to touch any of the existing packs (they are all kept or\nthere simply aren't any packs yet).\n\nHowever on the git.git repository if I ran `git repack -a -d`\nwith every single object in a kept pack and no loose objects I kept\nrepacking the same 102 objects into a new active pack, even though\nthere were no loose objects to repack and no active packs. Uh, yea.\n\nAdding --incremental in this case kept it from repacking those\nsame 102 objects on every invocation. Yea, its a bug. I meant to\nmention it in my email.\n\n-- \n"},{"id":"296640","messageId":"20061029035254.GD3435@spearce.org","threadId":"43082","inReplyTo":"ei18ak$nv4$1@sea.gmane.org","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-29T03:52:54Z","receivedAt":"2006-10-29T03:52:54Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jakub Narebski wrote:\n> Shawn Pearce wrote:\n> \n> > Eran Tromer wrote:\n> >>\n> >> It would be nice to have whoever creates a pack-*.keep file put\n> >> something useful as the content of the file, so we'll know what to clean\n> >> up after abnormal termination:\n> >> \n> >> $ grep -l ^git-receive-pack $GIT_DIR/objects/pack/pack-*.keep\n> > \n> > Yes, that's a very good idea. When I do the git-receive-pack\n> > implementation tonight I'll try to dump useful information to the\n> > .keep file such that you can easily grep for the stale .keeps\n> > and decide which ones should go.\n> \n> Perhaps git-count-packs (or enhanced git-count-objects, or git-count-stuff;\n> whatever it would be named) could also list (with some option) the reasons\n> for packs to be kept...\n\nThat would be more like 'git ls-packs' to me, but as Junio pointed\nout why add a command just to count packs, and as others have said\nrecently \"Git has too many commands!\".\n\n\nI like git-count-stuff. So generic. Maybe we can shove\ngit-pickaxe in as the --count-lines-and-authors-andthings option of\ngit-count-stuff, seeing as how some people didn't like its name. :-)\n\n\n-- \n"},{"id":"298733","messageId":"7vejsr68y9.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"20061029035025.GC3435@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-29T04:29:34Z","receivedAt":"2006-10-29T04:29:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> The issue is --unpacked= uses the path of the pack name, which\n> includes $GIT_OBJECT_DIRECTORY, whatever that may be. This makes it\n> impossible for the shell script to hand through a proper --unpacked=\n> line for the active packs without including $GIT_OBJECT_DIRECTORY\n> as part of the option.\n\nYeah, I realize that; you need to know how to build shell script\nthat is properly shell quoted to be eval'ed, which is not hard\nbut is not usually done and is cumbersome.\n\nI would suspect it is probably easier to just say --unpacked\n(without packname) means \"unpacked objects, and objects in packs\nthat do not have corresponding .keep\". However, that would be a\nchange in semantics for --unpacked (without packname), which is\nnot nice.\n\nSo how about pack-X{40}.volatile that marks an eligible one for\nrepacking?\n\nThen we can make \"pack-objects --unpacked\" to pretend the ones\nwith corresponding .volatile as if the objects in them are\nloose, without breaking backward compatibility.\n\n> However on the git.git repository if I ran `git repack -a -d`\n> with every single object in a kept pack and no loose objects I\n> kept repacking the same 102 objects into a new active pack,\n> even though there were no loose objects to repack and no\n> active packs. Uh, yea.\n\nWill take a look myself if you are otherwise busy.\n"},{"id":"294729","messageId":"20061029043818.GA3650@spearce.org","threadId":"43082","inReplyTo":"7vejsr68y9.fsf@assigned-by-dhcp.cox.net","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-29T04:38:19Z","receivedAt":"2006-10-29T04:38:19Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Shawn Pearce writes:\n> \n> > The issue is --unpacked= uses the path of the pack name, which\n> > includes $GIT_OBJECT_DIRECTORY, whatever that may be. This makes it\n> > impossible for the shell script to hand through a proper --unpacked=\n> > line for the active packs without including $GIT_OBJECT_DIRECTORY\n> > as part of the option.\n> \n> Yeah, I realize that; you need to know how to build shell script\n> that is properly shell quoted to be eval'ed, which is not hard\n> but is not usually done and is cumbersome.\n\nToo much work. :-)\n\n> I would suspect it is probably easier to just say --unpacked\n> (without packname) means \"unpacked objects, and objects in packs\n> that do not have corresponding .keep\". However, that would be a\n> change in semantics for --unpacked (without packname), which is\n> not nice.\n> \n> So how about pack-X{40}.volatile that marks an eligible one for\n> repacking?\n\nThen anyone who has an existing pack would need to create that\nfile first as soon as they got this newer version of Git... not\nvery upgrade friendly if you ask me.\n \n> Then we can make \"pack-objects --unpacked\" to pretend the ones\n> with corresponding .volatile as if the objects in them are\n> loose, without breaking backward compatibility.\n\nCurrently I'm changing --unpacked= to match without needing quoting.\nI'm allowing it to match an exact pack name or if it starts with\n\"pack-\" and matches the last 50 (\"pack-X{40}.pack\") of the pack name.\n\nI figure this should work fine as probably anyone who has a pack\nname that matches 50 characters and starts with \"pack-\" is using a\npack file name which has the SHA1 of the object list contained in\nit and is thus probably unique.\n \n-- \n"},{"id":"295234","messageId":"7v3b9766rc.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"20061029043818.GA3650@spearce.org","subject":"Re: fetching packs and storing them as packs","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-29T05:16:55Z","receivedAt":"2006-10-29T05:16:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> Junio C Hamano wrote:\n>> Shawn Pearce writes:\n>> \n>> So how about pack-X{40}.volatile that marks an eligible one for\n>> repacking?\n>\n> Then anyone who has an existing pack would need to create that\n> file first as soon as they got this newer version of Git... not\n> very upgrade friendly if you ask me.\n\nAh, I mixed things up completely. You're right. Having .keep\nleaves that pack as is (and lack of matching .keep causes it to\nbe repacked -- people do not have .keep so everything should be\nrepacked as before).\n\n>> Then we can make \"pack-objects --unpacked\" to pretend the ones\n>> with corresponding .volatile as if the objects in them are\n>> loose, without breaking backward compatibility.\n>\n> Currently I'm changing --unpacked= to match without needing quoting.\n> I'm allowing it to match an exact pack name or if it starts with\n> \"pack-\" and matches the last 50 (\"pack-X{40}.pack\") of the pack name.\n\nI think is a very sane thing to do (I should have done that from\nthe beginning). I do not like \"the last 50\", but I do not have\nobjection to make it take either full path or just the filename\nunder objects/pack/ (so not \"the last 50\" but \"filename w/o\nslash\").\n"},{"id":"297201","messageId":"20061029052146.GA3847@spearce.org","threadId":"43082","inReplyTo":"7v3b9766rc.fsf@assigned-by-dhcp.cox.net","subject":"Re: fetching packs and storing them as packs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-29T05:21:46Z","receivedAt":"2006-10-29T05:21:46Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> Shawn Pearce writes:\n> >> Then we can make \"pack-objects --unpacked\" to pretend the ones\n> >> with corresponding .volatile as if the objects in them are\n> >> loose, without breaking backward compatibility.\n> >\n> > Currently I'm changing --unpacked= to match without needing quoting.\n> > I'm allowing it to match an exact pack name or if it starts with\n> > \"pack-\" and matches the last 50 (\"pack-X{40}.pack\") of the pack name.\n> \n> I think is a very sane thing to do (I should have done that from\n> the beginning). I do not like \"the last 50\", but I do not have\n> objection to make it take either full path or just the filename\n> under objects/pack/ (so not \"the last 50\" but \"filename w/o\n> slash\").\n\nOK. I coded it with the last 50 but will rewrite that commit\nwithout as the code is slightly shorter that way. :-)\n\n-- \n"},{"id":"297771","messageId":"7vwt6j4l77.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"Pine.LNX.4.64.0610252333540.12418@xanadu.home","subject":"[PATCH] send-pack --keep: do not explode into loose objects on the receiving end.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-29T07:47:56Z","receivedAt":"2006-10-29T07:47:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This adds \"keep-pack\" extension to send-pack vs receive pack protocol,\nand makes the receiver invoke \"index-pack --stdin --fix-thin\".\n\nWith this, you can ask send-pack not to explode the result into\nloose objects on the receiving end.\n\nI've patched has_sha1_file() to re-check for added packs just\nlike is done in read_sha1_file() for now, but I think the static\n\"re-prepare\" interface for packs was a mistake. Creation of a\nnew pack inside a process that needs to read objects in them\nback ought to be a rare event, so we are better off making the\ncallers (such as receive-pack that calls \"index-pack --stdin\n--fix-thin\") explicitly call re-prepare. That way we do not\nhave to penalize ordinary users of read_sha1_file() and\nhas_sha1_file().\n\nWe would need to fix this someday.\n\nSigned-off-by: Junio C Hamano \n---\n receive-pack.c | 19 ++++++++++++++++---\n send-pack.c | 23 ++++++++++++++++++-----\n sha1_file.c | 7 +++++--\n t/t5400-send-pack.sh | 9 +++++++++\n 4 files changed, 48 insertions(+), 10 deletions(-)\n\ndiff --git a/receive-pack.c b/receive-pack.c\nindex ea2dbd4..ef50226 100644\n--- a/receive-pack.c\n+++ b/receive-pack.c\n@@ -8,10 +8,14 @@\n static const char receive_pack_usage[] = \"git-receive-pack \";\n \n static const char *unpacker[] = { \"unpack-objects\", NULL };\n+static const char *keep_packer[] = {\n+\t\"index-pack\", \"--stdin\", \"--fix-thin\", NULL\n+};\n \n static int report_status;\n+static int keep_pack;\n \n-static char capabilities[] = \"report-status\";\n+static char capabilities[] = \"report-status keep-pack\";\n static int capabilities_sent;\n \n static int show_ref(const char *path, const unsigned char *sha1)\n@@ -261,6 +265,8 @@ static void read_head_info(void)\n \t\tif (reflen + 82 < len) {\n \t\t\tif (strstr(refname + reflen + 1, \"report-status\"))\n \t\t\t\treport_status = 1;\n+\t\t\tif (strstr(refname + reflen + 1, \"keep-pack\"))\n+\t\t\t\tkeep_pack = 1;\n \t\t}\n \t\tcmd = xmalloc(sizeof(struct command) + len - 80);\n \t\thashcpy(cmd->old_sha1, old_sha1);\n@@ -275,7 +281,14 @@ static void read_head_info(void)\n \n static const char *unpack(int *error_code)\n {\n-\tint code = run_command_v_opt(1, unpacker, RUN_GIT_CMD);\n+\tint code;\n+\n+\tif (keep_pack)\n+\t\tcode = run_command_v_opt(ARRAY_SIZE(keep_packer) - 1,\n+\t\t\t\t\t keep_packer, RUN_GIT_CMD);\n+\telse\n+\t\tcode = run_command_v_opt(ARRAY_SIZE(unpacker) - 1,\n+\t\t\t\t\t unpacker, RUN_GIT_CMD);\n \n \t*error_code = 0;\n \tswitch (code) {\n@@ -335,7 +348,7 @@ int main(int argc, char **argv)\n \tif (!dir)\n \t\tusage(receive_pack_usage);\n \n-\tif(!enter_repo(dir, 0))\n+\tif (!enter_repo(dir, 0))\n \t\tdie(\"'%s': unable to chdir or not a git archive\", dir);\n \n \twrite_head_info();\ndiff --git a/send-pack.c b/send-pack.c\nindex 5bb123a..54d218c 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -6,13 +6,14 @@\n #include \"exec_cmd.h\"\n \n static const char send_pack_usage[] =\n-\"git-send-pack [--all] [--exec=git-receive-pack] [...]\\n\"\n+\"git-send-pack [--all] [--keep] [--exec=git-receive-pack] [...]\\n\"\n \" --all and explicit specification are mutually exclusive.\";\n static const char *exec = \"git-receive-pack\";\n static int verbose;\n static int send_all;\n static int force_update;\n static int use_thin_pack;\n+static int keep_pack;\n \n static int is_zero_sha1(const unsigned char *sha1)\n {\n@@ -270,6 +271,7 @@ static int send_pack(int in, int out, in\n \tint new_refs;\n \tint ret = 0;\n \tint ask_for_status_report = 0;\n+\tint ask_to_keep_pack = 0;\n \tint expect_status_report = 0;\n \n \t/* No funny business with the matcher */\n@@ -279,6 +281,8 @@ static int send_pack(int in, int out, in\n \t/* Does the other end support the reporting? */\n \tif (server_supports(\"report-status\"))\n \t\task_for_status_report = 1;\n+\tif (server_supports(\"keep-pack\") && keep_pack)\n+\t\task_to_keep_pack = 1;\n \n \t/* match them up */\n \tif (!remote_tail)\n@@ -355,12 +359,17 @@ static int send_pack(int in, int out, in\n \t\tstrcpy(old_hex, sha1_to_hex(ref->old_sha1));\n \t\tnew_hex = sha1_to_hex(ref->new_sha1);\n \n-\t\tif (ask_for_status_report) {\n-\t\t\tpacket_write(out, \"%s %s %s%c%s\",\n+\t\tif (ask_for_status_report || ask_to_keep_pack) {\n+\t\t\tpacket_write(out, \"%s %s %s%c%s%s\",\n \t\t\t\t old_hex, new_hex, ref->name, 0,\n-\t\t\t\t \"report-status\");\n+\t\t\t\t ask_for_status_report\n+\t\t\t\t ? \" report-status\" : \"\",\n+\t\t\t\t ask_to_keep_pack\n+\t\t\t\t ? \" keep-pack\" : \"\");\n+\t\t\tif (ask_for_status_report)\n+\t\t\t\texpect_status_report = 1;\n \t\t\task_for_status_report = 0;\n-\t\t\texpect_status_report = 1;\n+\t\t\task_to_keep_pack = 0;\n \t\t}\n \t\telse\n \t\t\tpacket_write(out, \"%s %s %s\",\n@@ -419,6 +428,10 @@ int main(int argc, char **argv)\n \t\t\t\tverbose = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--keep\")) {\n+\t\t\t\tkeep_pack = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--thin\")) {\n \t\t\t\tuse_thin_pack = 1;\n \t\t\t\tcontinue;\ndiff --git a/sha1_file.c b/sha1_file.c\nindex e89d24c..278ba2f 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -1292,7 +1292,7 @@ static void *read_packed_sha1(const unsi\n \treturn unpack_entry(&e, type, size);\n }\n \n-void * read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size)\n+void *read_sha1_file(const unsigned char *sha1, char *type, unsigned long *size)\n {\n \tunsigned long mapsize;\n \tvoid *map, *buf;\n@@ -1757,7 +1757,10 @@ int has_sha1_file(const unsigned char *s\n \n \tif (find_pack_entry(sha1, &e, NULL))\n \t\treturn 1;\n-\treturn find_sha1_file(sha1, &st) ? 1 : 0;\n+\tif (find_sha1_file(sha1, &st))\n+\t\treturn 1;\n+\treprepare_packed_git();\n+\treturn find_pack_entry(sha1, &e, NULL);\n }\n \n /*\ndiff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh\nindex 8afb899..d831f8d 100755\n--- a/t/t5400-send-pack.sh\n+++ b/t/t5400-send-pack.sh\n@@ -78,4 +78,13 @@ test_expect_success \\\n \t! diff -u .git/refs/heads/master victim/.git/refs/heads/master\n '\n \n+test_expect_success 'push with --keep' '\n+\tt=`cd victim && git-rev-parse --verify refs/heads/master` &&\n+\tgit-update-ref refs/heads/master $t &&\n+\t: > foo &&\n+\tgit add foo &&\n+\tgit commit -m \"one more\" &&\n+\tgit-send-pack --keep ./victim/.git/ master\n+'\n+\n test_done\n-- \n1.4.3.3.g7d63\n\n"},{"id":"293823","messageId":"20061029075638.GB3847@spearce.org","threadId":"43082","inReplyTo":"7vwt6j4l77.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] send-pack --keep: do not explode into loose objects on the receiving end.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-29T07:56:38Z","receivedAt":"2006-10-29T07:56:38Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> This adds \"keep-pack\" extension to send-pack vs receive pack protocol,\n> and makes the receiver invoke \"index-pack --stdin --fix-thin\".\n\nI'm torn on this. I see that keeping a pack vs. exploding to loose\nobjects is a local repository decision and thus should be determined\nby the receiving repository, not the sending one.\n\nI was thinking of just reading the pack header in receive-pack,\nchecking the object count, and if its over a configured threshold\ncall index-pack rather than unpack-objects. Unfortunately I just\nrealized that if we read the pack header to make that decision then\nits gone and the child process won't have it. :-(\n\n-- \n"},{"id":"294233","messageId":"7vslh74kdq.fsf@assigned-by-dhcp.cox.net","threadId":"43082","inReplyTo":"20061029075638.GB3847@spearce.org","subject":"Re: [PATCH] send-pack --keep: do not explode into loose objects on the receiving end.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-29T08:05:37Z","receivedAt":"2006-10-29T08:05:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce writes:\n\n> I was thinking of just reading the pack header in receive-pack,\n> checking the object count, and if its over a configured threshold\n> call index-pack rather than unpack-objects. Unfortunately I just\n> realized that if we read the pack header to make that decision then\n> its gone and the child process won't have it. :-(\n\nIf you want to do that, that is certainly possible.\n\nYou can read the first block in the parent (without discarding),\nmake the decision and then fork()+exec() either unpack-objects\nor index-pack and feed it from the parent. The parent first\nfeeds the initial block it read to make that decision, and then\nbecomes a cat that reads from send-pack and writes to the child\nprocess that is either unpack-objects or index-pack.\n"},{"id":"298286","messageId":"Pine.LNX.4.64.0610292027160.11384@xanadu.home","threadId":"43082","inReplyTo":"20061029075638.GB3847@spearce.org","subject":"Re: [PATCH] send-pack --keep: do not explode into loose objects on the receiving end.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-30T01:44:02Z","receivedAt":"2006-10-30T01:44:02Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 29 Oct 2006, Shawn Pearce wrote:\n\n> Junio C Hamano wrote:\n> > This adds \"keep-pack\" extension to send-pack vs receive pack protocol,\n> > and makes the receiver invoke \"index-pack --stdin --fix-thin\".\n> \n> I'm torn on this. I see that keeping a pack vs. exploding to loose\n> objects is a local repository decision and thus should be determined\n> by the receiving repository, not the sending one.\n\nI second this. I think it is really not the remote end's business to \ndecide what the local storage policy is, and vice versa.\n\n\n"}]}