{"thread":{"id":"6535","subject":"Seriously broken \"git pack-refs\"","startedAt":"2007-01-26T00:51:21Z","lastAt":"2007-01-26T06:50:18Z","messageCount":6,"participants":["Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"32670","messageId":"Pine.LNX.4.64.0701251636060.25027@woody.linux-foundation.org","threadId":"6535","inReplyTo":null,"subject":"Seriously broken \"git pack-refs\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-01-26T00:51:21Z","receivedAt":"2007-01-26T00:51:21Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nDo *NOT* try this on a repository you care about:\n\n\tgit pack-refs --all --prune\n\tgit pack-refs\n\nbecause while the first \"pack-refs\" does the right thing, the second \npack-refs will totally screw you over.\n\nWhy? \n\nThe default for \"git pack-refs\" is to pack just tags. Which is a HORRIBLE \nMISTAKE. Becuase it means that if you actually had any packed non-tags, \nthey now get removed entirely.\n\nI'd call whoever made that decision a complete crack-addict and total \nidiot, but it might be me, so I'll just tread carefully and call the \nchoice \"interesting\".\n\nThis should fix it.\n\n\t\tLinus\n----\ndiff --git a/builtin-pack-refs.c b/builtin-pack-refs.c\nindex 6de7128..3de9b3e 100644\n--- a/builtin-pack-refs.c\n+++ b/builtin-pack-refs.c\n@@ -37,7 +37,9 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,\n \tif ((flags & REF_ISSYMREF))\n \t\treturn 0;\n \tis_tag_ref = !strncmp(path, \"refs/tags/\", 10);\n-\tif (!cb->all && !is_tag_ref)\n+\n+\t/* ALWAYS pack refs that were already packed or are tags */\n+\tif (!cb->all && !is_tag_ref && !(flags & REF_ISPACKED))\n \t\treturn 0;\n \n \tfprintf(cb->refs_file, \"%s %s\\n\", sha1_to_hex(sha1), path);\n"},{"id":"32674","messageId":"7v7iva4glo.fsf@assigned-by-dhcp.cox.net","threadId":"6535","inReplyTo":"Pine.LNX.4.64.0701251636060.25027@woody.linux-foundation.org","subject":"Re: Seriously broken \"git pack-refs\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-26T03:22:27Z","receivedAt":"2007-01-26T03:22:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> The default for \"git pack-refs\" is to pack just tags. Which is a HORRIBLE \n> MISTAKE. Becuase it means that if you actually had any packed non-tags, \n> they now get removed entirely.\n>\n> I'd call whoever made that decision a complete crack-addict and total \n> idiot, but it might be me, so I'll just tread carefully and call the \n> choice \"interesting\".\n\nGuilty as charged, I guess (I haven't asked 'blame' about it but\nI am reasonably sure it was me). And thanks for catching it.\n\nBut I think \"only packing tags\" is a reasonable default, as\nlocal branch heads are meant to be advanced, and remote tracking\nbranches follow whatever happens on the remote end upon every\nfetch. Tags on the other hand are meant to be stationary.\n\nWhat was a HORRIBLE MISTAKE was not repacking what came out of\nthe pack if it was pruned.\n"},{"id":"32683","messageId":"Pine.LNX.4.64.0701252202130.25027@woody.linux-foundation.org","threadId":"6535","inReplyTo":"7v7iva4glo.fsf@assigned-by-dhcp.cox.net","subject":"Re: Seriously broken \"git pack-refs\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-01-26T06:05:18Z","receivedAt":"2007-01-26T06:05:18Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 25 Jan 2007, Junio C Hamano wrote:\n> \n> But I think \"only packing tags\" is a reasonable default\n\nSure, I agree. I just think that it had the unintended side effect when \nthe code then didn't repack stuff that had already been packed earlier.\n\nWhich is why my fix doesn't actually change the fact that we only pack \ntags by default, but just fixes the thing that caused us to then totally \ndrop previously packed non-tags.\n\nSo now the default isn't really \"pack only tags\", but \"pack only tags *or* \nthings that were already packed\".\n\nThe reason, of course, is that if we don't pack stuff that used to be \npacked, it will just be dropped entirely. Oops.\n\n\t\tLinus\n"},{"id":"32684","messageId":"7vireu2uc6.fsf@assigned-by-dhcp.cox.net","threadId":"6535","inReplyTo":"Pine.LNX.4.64.0701252202130.25027@woody.linux-foundation.org","subject":"Re: Seriously broken \"git pack-refs\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-26T06:08:41Z","receivedAt":"2007-01-26T06:08:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> So now the default isn't really \"pack only tags\", but \"pack only tags *or* \n> things that were already packed\".\n>\n> The reason, of course, is that if we don't pack stuff that used to be \n> packed, it will just be dropped entirely. Oops.\n\nSorry again.\n\nOn the other hand, I do not think it is worth unpacking non-tags\nthat are packed when --all is not given, so...\n"},{"id":"32687","messageId":"Pine.LNX.4.64.0701252218540.25027@woody.linux-foundation.org","threadId":"6535","inReplyTo":"7vireu2uc6.fsf@assigned-by-dhcp.cox.net","subject":"Re: Seriously broken \"git pack-refs\"","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-01-26T06:24:35Z","receivedAt":"2007-01-26T06:24:35Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 25 Jan 2007, Junio C Hamano wrote:\n> \n> On the other hand, I do not think it is worth unpacking non-tags\n> that are packed when --all is not given, so...\n\nExactly. \n\nI'd much rather have\n\n\tgit pack-refs --all --prune\n\njust pack everything once and for all. Afterwards, if you do just\n\n\tgit pack-refs\n\nit will keep everything that was packed packed, and any loose refs (which \nmight be loose because they are new, but perhaps because they simply got \noverridden by being changed) will not be added to the pack unless they are \ntags.\n\nWhich should be the semantics with my patch applied. \n\nThe above actually makes tons of sense: operations like big imports (or \nclones) might want to start out with *everything* packed, but then as we \nupdate, commit and modify, what we actually want to do is to basically let \nthe branches that are actively developed \"become unpacked\" as they are \nupdated.\n\nSo the \"repack refs that were already packed, or refs that are tags\" is \nactually a very sane default. It's not just that we don't want to lose the \nrefs entirely, it's also that what we actually want to do by default is to \npack the stuff we have reason to believe won't be actively changing. \n\nTags automatically fall under that heading (which is why it makes sense to \npack them by default in the first place!) but so does any ref that was \nalready packed, and hadn't become unpacked since the last repack. By \ndefinition, such a ref isn't \"actively changing\" even if it isn't a tag.\n\nNo?\n\n\t\tLinus\n"},{"id":"32688","messageId":"7vac062set.fsf@assigned-by-dhcp.cox.net","threadId":"6535","inReplyTo":"Pine.LNX.4.64.0701252218540.25027@woody.linux-foundation.org","subject":"Re: Seriously broken \"git pack-refs\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-26T06:50:18Z","receivedAt":"2007-01-26T06:50:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Tags automatically fall under that heading (which is why it makes sense to \n> pack them by default in the first place!) but so does any ref that was \n> already packed, and hadn't become unpacked since the last repack. By \n> definition, such a ref isn't \"actively changing\" even if it isn't a tag.\n>\n> No?\n\nYes.\n\ndiff --git a/Documentation/git-pack-refs.txt b/Documentation/git-pack-refs.txt\nindex 464269f..b79b79d 100644\n--- a/Documentation/git-pack-refs.txt\n+++ b/Documentation/git-pack-refs.txt\n@@ -34,11 +34,16 @@ OPTIONS\n \n \\--all::\n \n-The command by default packs all tags and leaves branch tips\n+The command by default packs all tags and refs that are already\n+packed, and leaves other refs\n alone. This is because branches are expected to be actively\n developed and packing their tips does not help performance.\n This option causes branch tips to be packed as well. Useful for\n a repository with many branches of historical interests.\n+Once packing with `\\--all` is done, further repacking with the\n+default will keep the already packed refs (including branch\n+heads) packed, while still actively developed branch heads will\n+not be packed (because they become unpacked once modified).\n \n \\--no-prune::\n \n"}]}