{"thread":{"id":"11869","subject":"[RFC,PATCH] Make git prune remove temporary packs that look like write failures","startedAt":"2008-02-04T14:10:32Z","lastAt":"2008-02-04T17:54:00Z","messageCount":11,"participants":["David Tweed","Nicolas Pitre","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"67362","messageId":"e1dab3980802040610s27a54a9due3b42db5f59c0cd5@mail.gmail.com","threadId":"11869","inReplyTo":null,"subject":"[RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"David Tweed","fromEmail":"david.tweed@gmail.com","sentAt":"2008-02-04T14:10:32Z","receivedAt":"2008-02-04T14:10:32Z","isPatch":true,"sender":{"key":"david.tweed@gmail.com","avatar":null},"body":"Make git prune remove temporary packs that look like write failures\n\nWrite errors when repacking (eg, due to out-of-space conditions)\ncan leave temporary packs lying around which no existing\ncodepath removes and which aren't obvious to the casual user.\nUnfortunately the only way to tell in builtin-prune that a tmp_pack file is\nof this sort is that it hasn't been modified recently. We assume a pack\nwhich hasn't been modified within 10 minutes is of this sort and\ndelete it, printing a notification to help debugging. (Nicolas Pitre\nsuggested this functionality should be activated only by --prune.)\n\nSigned-off-by: David Tweed (david.tweed@gmail.com)\n---\n\nI KNOW this initial RFC is mailer whitespace damaged. Finally version won't be.\n\nIn principle this is a really trivial patch, but I'm being cautious because I\ndon't like the fact that I don't know (and AFAICS can't reliably check) that\nthe files being deleting are definitely dead. An alternative would\nbe to make prune just print out that the suspicious packs are\nthere and let the user delete them manually. (My itch is that once\na write-failure pack gets created, nothing in git operations tells the\nuser that a generally multimegabyte file hidden in .git occupying space.)\n\n builtin-prune.c | 34 ++++++++++++++++++++++++++++++++++\n 1 files changed, 34 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-prune.c b/builtin-prune.c\nindex b5e7684..90111ab 100644\n--- a/builtin-prune.c\n+++ b/builtin-prune.c\n@@ -83,6 +83,39 @@ static void prune_object_dir(const char *path)\n }\n }\n\n+/*\n+ * Write errors (particularly out of space) can result in\n+ * failed temporary packs accumulating in the object directory.\n+ * This removes anything in the object directory beginning\n+ * with tmp_ using the heuristic that anything\n+ * that was last modified more than 10 minutes\n+ * ago is the abandoned result of a write failure.\n+ */\n+static void remove_temporary_files(void)\n+{\n+ DIR *dir;\n+ struct stat status;\n+ time_t now;\n+ struct dirent *de;\n+ char* dirname=get_object_directory();\n+\n+ now = time(NULL);\n+ dir = opendir(dirname);\n+ while ((de = readdir(dir)) != NULL) {\n+ if(strncmp(de->d_name, \"tmp_\", 4) == 0){\n+ char name[4096];\n+ int c=snprintf(name, 4095, \"%s/%s\", dirname,\nde->d_name);\n+ if(c>0 && c<4096 && stat(name, &status) == 0\n+ && status.st_mtime < now - 600){\n+ printf(\"Removing apparently abandoned\n%s\\n\",name);\n+ unlink(name);\n+ }\n+ }\n+ }\n+ closedir(dir);\n+}\n+\n+\n int cmd_prune(int argc, const char **argv, const char *prefix)\n {\n int i;\n@@ -115,5 +148,6 @@ int cmd_prune(int argc, const char **argv, const\nchar *prefix)\n\n sync();\n prune_packed_objects(show_only);\n+ remove_temporary_files();\n return 0;\n }\n"},{"id":"67367","messageId":"alpine.LFD.1.00.0802040945370.2732@xanadu.home","threadId":"11869","inReplyTo":"e1dab3980802040610s27a54a9due3b42db5f59c0cd5@mail.gmail.com","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-04T14:52:38Z","receivedAt":"2008-02-04T14:52:38Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 4 Feb 2008, David Tweed wrote:\n\n> In principle this is a really trivial patch, but I'm being cautious because I\n> don't like the fact that I don't know (and AFAICS can't reliably check) that\n> the files being deleting are definitely dead. An alternative would\n> be to make prune just print out that the suspicious packs are\n> there and let the user delete them manually. (My itch is that once\n> a write-failure pack gets created, nothing in git operations tells the\n> user that a generally multimegabyte file hidden in .git occupying space.)\n\nThe lifelessness of a temporary pack is the same as for loose objects, \nhence the same rule should apply in both cases. Just asking the user to \ndelete them manually isn't too nice either. A prune operation is \nalready said to be dangerous and should be performed only when no other \nactivities are occurring in the same repository. That should cover the \ncase of dead temporary pack files just as well.\n\n\nNicolas\n"},{"id":"67372","messageId":"alpine.LSU.1.00.0802041512140.7372@racer.site","threadId":"11869","inReplyTo":"e1dab3980802040610s27a54a9due3b42db5f59c0cd5@mail.gmail.com","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-04T15:16:12Z","receivedAt":"2008-02-04T15:16:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 4 Feb 2008, David Tweed wrote:\n\n> + if(c>0 && c<4096 && stat(name, &status) == 0\n> + && status.st_mtime < now - 600){\n\nPlease have spaces after the \"if\" and before the \"{\" (just imitate the \nstyle of the rest of the file).\n\nAlso, 10 minutes grace period for any ongoing fetch or repack seems a bit \narbitrary. Maybe default to 10 minutes, and introduce \nprune.packGracePeriod?\n\n(Which reminds me that it might be useful to add a \nprune.looseObjectsGracePeriod to avoid having to type --expire= all the \ntime?)\n\nCiao,\nDscho\n"},{"id":"67373","messageId":"e1dab3980802040724l5ef12528y69f1d572b7ac8d54@mail.gmail.com","threadId":"11869","inReplyTo":"alpine.LSU.1.00.0802041512140.7372@racer.site","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"David Tweed","fromEmail":"david.tweed@gmail.com","sentAt":"2008-02-04T15:24:21Z","receivedAt":"2008-02-04T15:24:21Z","isPatch":true,"sender":{"key":"david.tweed@gmail.com","avatar":null},"body":"On Feb 4, 2008 3:16 PM, Johannes Schindelin wrote:\n> Hi,\n>\n> On Mon, 4 Feb 2008, David Tweed wrote:\n>\n> > + if(c>0 && c<4096 && stat(name, &status) == 0\n> > + && status.st_mtime < now - 600){\n>\n> Please have spaces after the \"if\" and before the \"{\" (just imitate the\n> style of the rest of the file).\n>\n> Also, 10 minutes grace period for any ongoing fetch or repack seems a bit\n> arbitrary. Maybe default to 10 minutes, and introduce\n> prune.packGracePeriod?\n\nIn response to this and to Nico's earlier mail, I _think_ the usage\nwith repack is completely safe. What I'm not sure about is that other\nthings like git-svn create temporary packs with usage/semantics I'm\nnot sure about. I'm happy to delete immediately if those who\nunderstand the interactions in the whole of git say that's acceptable\nwhen the user specifically calls git-prune.\n\n-- \ncheers, dave tweed__________________________\ndavid.tweed@gmail.com\nRm 124, School of Systems Engineering, University of Reading.\n\"while having code so boring anyone can maintain it, use Python.\" --\nattempted insult seen on slashdot\n"},{"id":"67376","messageId":"alpine.LFD.1.00.0802041105170.2732@xanadu.home","threadId":"11869","inReplyTo":"alpine.LSU.1.00.0802041512140.7372@racer.site","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-04T16:06:39Z","receivedAt":"2008-02-04T16:06:39Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 4 Feb 2008, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Mon, 4 Feb 2008, David Tweed wrote:\n> \n> > + if(c>0 && c<4096 && stat(name, &status) == 0\n> > + && status.st_mtime < now - 600){\n> \n> Please have spaces after the \"if\" and before the \"{\" (just imitate the \n> style of the rest of the file).\n> \n> Also, 10 minutes grace period for any ongoing fetch or repack seems a bit \n> arbitrary. Maybe default to 10 minutes, and introduce \n> prune.packGracePeriod?\n> \n> (Which reminds me that it might be useful to add a \n> prune.looseObjectsGracePeriod to avoid having to type --expire= all the \n> time?)\n\nPlease use the same parameter for both. There is no need to have \nseparate settings.\n\n\nNicolas\n"},{"id":"67378","messageId":"alpine.LSU.1.00.0802041633190.7372@racer.site","threadId":"11869","inReplyTo":"alpine.LFD.1.00.0802041105170.2732@xanadu.home","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-04T16:33:29Z","receivedAt":"2008-02-04T16:33:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 4 Feb 2008, Nicolas Pitre wrote:\n\n> On Mon, 4 Feb 2008, Johannes Schindelin wrote:\n> \n> > On Mon, 4 Feb 2008, David Tweed wrote:\n> > \n> > > + if(c>0 && c<4096 && stat(name, &status) == 0\n> > > + && status.st_mtime < now - 600){\n> > \n> > Please have spaces after the \"if\" and before the \"{\" (just imitate the \n> > style of the rest of the file).\n> > \n> > Also, 10 minutes grace period for any ongoing fetch or repack seems a \n> > bit arbitrary. Maybe default to 10 minutes, and introduce \n> > prune.packGracePeriod?\n> > \n> > (Which reminds me that it might be useful to add a \n> > prune.looseObjectsGracePeriod to avoid having to type --expire= all \n> > the time?)\n> \n> Please use the same parameter for both. There is no need to have \n> separate settings.\n\nRight.\n\nCiao,\nDscho\n"},{"id":"67385","messageId":"alpine.LSU.1.00.0802041714560.7372@racer.site","threadId":"11869","inReplyTo":"e1dab3980802040724l5ef12528y69f1d572b7ac8d54@mail.gmail.com","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-02-04T17:21:23Z","receivedAt":"2008-02-04T17:21:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 4 Feb 2008, David Tweed wrote:\n\n> In response to this and to Nico's earlier mail, I _think_ the usage with \n> repack is completely safe.\n\nIt would have been nicer of you to defend that, instead of sending me off \nto look for myself. Having looked for myself, I am not convinced at all.\n\nAnd it would have been surprising: if your patch would play nicely with a \nrepack in progress, then it would fail to remove the temporary packs left \nby a crashed repack.\n\nCiao,\nDscho\n"},{"id":"67391","messageId":"e1dab3980802040939u1329ab6xa730f5ecc52c809a@mail.gmail.com","threadId":"11869","inReplyTo":"alpine.LSU.1.00.0802041714560.7372@racer.site","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"David Tweed","fromEmail":"david.tweed@gmail.com","sentAt":"2008-02-04T17:39:43Z","receivedAt":"2008-02-04T17:39:43Z","isPatch":true,"sender":{"key":"david.tweed@gmail.com","avatar":null},"body":"On Feb 4, 2008 5:21 PM, Johannes Schindelin wrote:\n> On Mon, 4 Feb 2008, David Tweed wrote:\n\n> > In response to this and to Nico's earlier mail, I _think_ the usage with\n> > repack is completely safe.\n>\n> It would have been nicer of you to defend that, instead of sending me off\n> to look for myself. Having looked for myself, I am not convinced at all.\n\nI probably ought to have put the underlines around the \"I\". I'm\nconvinced, but since this is deleting things I'm more cautious than I\nwould be, say, parsing options.\n\n> And it would have been surprising: if your patch would play nicely with a\n> repack in progress, then it would fail to remove the temporary packs left\n> by a crashed repack.\n\nI should been more careful what I said: I only use repack via \"git gc\"\nwhich calls the repack as a subcommand. If the repack fails then the\nwhole process dies and you've got a dead tmp pack. The _next_ time you\ncall \"git gc\" it will do the repack, finish and then call \"git prune\"\n(assuming --prune) and delete the temporary pack. Used in this way, I\nhave tried and I cannot see an execution path where this can go wrong.\nYou're right (and I didn't intend to suggest otherwise) that it would\nbe safe when running a \"git prune\" concurrently with a separate \"git\nrepack\".\n\nHowever, I'm not familiar with what things like git-svn, cvs, etc, do.\nGiven that I've seen patches adding \"git gc\" periodically during\nvarious imports, I wanted to someone who knows that area to confirm\nthe patch isn't violating any assumptions.\n\n-- \ncheers, dave tweed__________________________\ndavid.tweed@gmail.com\nRm 124, School of Systems Engineering, University of Reading.\n\"while having code so boring anyone can maintain it, use Python.\" --\nattempted insult seen on slashdot\n"},{"id":"67390","messageId":"e1dab3980802040942k493e97efr438f75a553400331@mail.gmail.com","threadId":"11869","inReplyTo":"e1dab3980802040939u1329ab6xa730f5ecc52c809a@mail.gmail.com","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"David Tweed","fromEmail":"david.tweed@gmail.com","sentAt":"2008-02-04T17:42:05Z","receivedAt":"2008-02-04T17:42:05Z","isPatch":true,"sender":{"key":"david.tweed@gmail.com","avatar":null},"body":"Ugh:\nOn Feb 4, 2008 5:39 PM, David Tweed wrote:\n> You're right (and I didn't intend to suggest otherwise) that it would\n> be safe when running a \"git prune\" concurrently with a separate \"git\ns/safe/unsafe/\n> repack\".\n\n-- \ncheers, dave tweed__________________________\ndavid.tweed@gmail.com\nRm 124, School of Systems Engineering, University of Reading.\n\"while having code so boring anyone can maintain it, use Python.\" --\nattempted insult seen on slashdot\n"},{"id":"67392","messageId":"alpine.LFD.1.00.0802041245170.2732@xanadu.home","threadId":"11869","inReplyTo":"e1dab3980802040939u1329ab6xa730f5ecc52c809a@mail.gmail.com","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2008-02-04T17:47:23Z","receivedAt":"2008-02-04T17:47:23Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 4 Feb 2008, David Tweed wrote:\n\n> However, I'm not familiar with what things like git-svn, cvs, etc, do.\n> Given that I've seen patches adding \"git gc\" periodically during\n> various imports, I wanted to someone who knows that area to confirm\n> the patch isn't violating any assumptions.\n\nIf they're prunning old objects already, they can prune old temporary \npack files assuming the same level of (non) risk.\n\n\nNicolas\n"},{"id":"67393","messageId":"e1dab3980802040954y41b0c7c7o5307101bddd1cc1b@mail.gmail.com","threadId":"11869","inReplyTo":"alpine.LFD.1.00.0802041245170.2732@xanadu.home","subject":"Re: [RFC,PATCH] Make git prune remove temporary packs that look like write failures","fromName":"David Tweed","fromEmail":"david.tweed@gmail.com","sentAt":"2008-02-04T17:54:00Z","receivedAt":"2008-02-04T17:54:00Z","isPatch":true,"sender":{"key":"david.tweed@gmail.com","avatar":null},"body":"On Feb 4, 2008 5:47 PM, Nicolas Pitre wrote:\n> On Mon, 4 Feb 2008, David Tweed wrote:\n> If they're prunning old objects already, they can prune old temporary\n> pack files assuming the same level of (non) risk.\n\nThanks. I'll leave it a day or so, then post a final patch without the\nmodification time check.\n\n-- \ncheers, dave tweed__________________________\ndavid.tweed@gmail.com\nRm 124, School of Systems Engineering, University of Reading.\n\"while having code so boring anyone can maintain it, use Python.\" --\nattempted insult seen on slashdot\n"}]}