{"thread":{"id":"5869","subject":"[PATCH] repack: allow simultaneous packing and pruning","startedAt":"2006-10-10T10:14:53Z","lastAt":"2006-10-10T23:45:22Z","messageCount":9,"participants":["Sam Vilain","Linus Torvalds","Eran Tromer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"28507","messageId":"20061010102210.568341380D6@magnus.utsl.gen.nz","threadId":"5869","inReplyTo":null,"subject":"[PATCH] repack: allow simultaneous packing and pruning","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2006-10-10T10:14:53Z","receivedAt":"2006-10-10T10:14:53Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"If using git-repack -a, unreferenced objects are kept behind in the\npack.  This might be the best default, but there are no good ways\nto clean up the packfiles if a lot of rebasing is happening, or\nbranches have been deleted.\n---\nsee also http://colabti.de/irclogger/irclogger_log/git?date=2006-10-10,Tue&sel=27#l75\n\n Documentation/git-repack.txt |    7 ++++++-\n git-repack.sh                |   14 +++++++++++++-\n 2 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt\nindex 9516227..63ee7cb 100644\n--- a/Documentation/git-repack.txt\n+++ b/Documentation/git-repack.txt\n@@ -9,7 +9,7 @@ objects into pack files.\n \n SYNOPSIS\n --------\n-'git-repack' [-a] [-d] [-f] [-l] [-n] [-q]\n+'git-repack' [-a] [-d] [-f] [-l] [-n] [-q] [-p]\n \n DESCRIPTION\n -----------\n@@ -40,6 +40,11 @@ OPTIONS\n \texisting packs redundant, remove the redundant packs.\n \tAlso runs gitlink:git-prune-packed[1].\n \n+-p::\n+\tBefore packing, remove any unreferenced objects with\n+\tgitlink:git-prune[1].  When used with '-a', unreferenced\n+\tobjects in the old packs are not taken across.\n+\n -l::\n         Pass the `--local` option to `git pack-objects`, see\n         gitlink:git-pack-objects[1].\ndiff --git a/git-repack.sh b/git-repack.sh\nindex 640ad8d..a2ad955 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -7,13 +7,14 @@ USAGE='[-a] [-d] [-f] [-l] [-n] [-q]'\n . git-sh-setup\n \n no_update_info= all_into_one= remove_redundant=\n-local= quiet= no_reuse_delta= extra=\n+local= quiet= no_reuse_delta= extra= prune=\n while case \"$#\" in 0) break ;; esac\n do\n \tcase \"$1\" in\n \t-n)\tno_update_info=t ;;\n \t-a)\tall_into_one=t ;;\n \t-d)\tremove_redundant=t ;;\n+\t-p)     prune=t ;;\n \t-q)\tquiet=-q ;;\n \t-f)\tno_reuse_delta=--no-reuse-delta ;;\n \t-l)\tlocal=--local ;;\n@@ -32,6 +33,11 @@ case \",$all_into_one,\" in\n ,,)\n \trev_list='--unpacked'\n \tpack_objects='--incremental'\n+\tif [ -n \"$prune\" ]\n+\tthen\n+\t    # prune junk first\n+\t    git-prune\n+\tfi\n \t;;\n ,t,)\n \trev_list=\n@@ -40,8 +46,14 @@ case \",$all_into_one,\" in\n \t# Redundancy check in all-into-one case is trivial.\n \texisting=`cd \"$PACKDIR\" && \\\n \t    find . -type f \\( -name '*.pack' -o -name '*.idx' \\) -print`\n+\n+\tif [ -n \"$prune\" ]\n+\tthen\n+\t    rev_list=`cd \"$GIT_DIR\" && find refs -type f -print`\n+\tfi\n \t;;\n esac\n+\n pack_objects=\"$pack_objects $local $quiet $no_reuse_delta$extra\"\n name=$(git-rev-list --objects --all $rev_list 2>&1 |\n \tgit-pack-objects --non-empty $pack_objects .tmp-pack) ||\n-- \n1.4.2.g0ea2\n"},{"id":"28510","messageId":"452B7E39.8040707@vilain.net","threadId":"5869","inReplyTo":"20061010102210.568341380D6@magnus.utsl.gen.nz","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2006-10-10T11:04:25Z","receivedAt":"2006-10-10T11:04:25Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"er, I guess I assumed that the critique about the behaviour was true\nrather than checking it myself... this patch is mostly a null-op.  *and*\nit's not whitespace clean!\n\nSam.\n\nSam Vilain wrote:\n> If using git-repack -a, unreferenced objects are kept behind in the\n> pack.  This might be the best default, but there are no good ways\n> to clean up the packfiles if a lot of rebasing is happening, or\n> branches have been deleted.\n> ---\n> see also http://colabti.de/irclogger/irclogger_log/git?date=2006-10-10,Tue&sel=27#l75\n> \n>  Documentation/git-repack.txt |    7 ++++++-\n>  git-repack.sh                |   14 +++++++++++++-\n>  2 files changed, 19 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt\n> index 9516227..63ee7cb 100644\n> --- a/Documentation/git-repack.txt\n> +++ b/Documentation/git-repack.txt\n> @@ -9,7 +9,7 @@ objects into pack files.\n>  \n>  SYNOPSIS\n>  --------\n> -'git-repack' [-a] [-d] [-f] [-l] [-n] [-q]\n> +'git-repack' [-a] [-d] [-f] [-l] [-n] [-q] [-p]\n>  \n>  DESCRIPTION\n>  -----------\n> @@ -40,6 +40,11 @@ OPTIONS\n>  \texisting packs redundant, remove the redundant packs.\n>  \tAlso runs gitlink:git-prune-packed[1].\n>  \n> +-p::\n> +\tBefore packing, remove any unreferenced objects with\n> +\tgitlink:git-prune[1].  When used with '-a', unreferenced\n> +\tobjects in the old packs are not taken across.\n> +\n>  -l::\n>          Pass the `--local` option to `git pack-objects`, see\n>          gitlink:git-pack-objects[1].\n> diff --git a/git-repack.sh b/git-repack.sh\n> index 640ad8d..a2ad955 100755\n> --- a/git-repack.sh\n> +++ b/git-repack.sh\n> @@ -7,13 +7,14 @@ USAGE='[-a] [-d] [-f] [-l] [-n] [-q]'\n>  . git-sh-setup\n>  \n>  no_update_info= all_into_one= remove_redundant=\n> -local= quiet= no_reuse_delta= extra=\n> +local= quiet= no_reuse_delta= extra= prune=\n>  while case \"$#\" in 0) break ;; esac\n>  do\n>  \tcase \"$1\" in\n>  \t-n)\tno_update_info=t ;;\n>  \t-a)\tall_into_one=t ;;\n>  \t-d)\tremove_redundant=t ;;\n> +\t-p)     prune=t ;;\n>  \t-q)\tquiet=-q ;;\n>  \t-f)\tno_reuse_delta=--no-reuse-delta ;;\n>  \t-l)\tlocal=--local ;;\n> @@ -32,6 +33,11 @@ case \",$all_into_one,\" in\n>  ,,)\n>  \trev_list='--unpacked'\n>  \tpack_objects='--incremental'\n> +\tif [ -n \"$prune\" ]\n> +\tthen\n> +\t    # prune junk first\n> +\t    git-prune\n> +\tfi\n>  \t;;\n>  ,t,)\n>  \trev_list=\n> @@ -40,8 +46,14 @@ case \",$all_into_one,\" in\n>  \t# Redundancy check in all-into-one case is trivial.\n>  \texisting=`cd \"$PACKDIR\" && \\\n>  \t    find . -type f \\( -name '*.pack' -o -name '*.idx' \\) -print`\n> +\n> +\tif [ -n \"$prune\" ]\n> +\tthen\n> +\t    rev_list=`cd \"$GIT_DIR\" && find refs -type f -print`\n> +\tfi\n>  \t;;\n>  esac\n> +\n>  pack_objects=\"$pack_objects $local $quiet $no_reuse_delta$extra\"\n>  name=$(git-rev-list --objects --all $rev_list 2>&1 |\n>  \tgit-pack-objects --non-empty $pack_objects .tmp-pack) ||\n"},{"id":"28518","messageId":"Pine.LNX.4.64.0610100800490.3952@g5.osdl.org","threadId":"5869","inReplyTo":"20061010102210.568341380D6@magnus.utsl.gen.nz","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-10T15:03:54Z","receivedAt":"2006-10-10T15:03:54Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 10 Oct 2006, Sam Vilain wrote:\n>\n> If using git-repack -a, unreferenced objects are kept behind in the\n> pack.  This might be the best default, but there are no good ways\n> to clean up the packfiles if a lot of rebasing is happening, or\n> branches have been deleted.\n\nDon't do this.\n\nI understand why you want to do it, but the fact is, it's dangerous.\n\nRight now, \"git repack\" is actually safe to run even on a repository which \nis being modified! And that's actually important, if you have something \nlike a shared repo that gets re-packed every once in a while from a \ncron-job!\n\nSo the refs might be up-dated as it runs, and if that happens, your \npruning doesn't really do the right thing - it might consider a new loose \nobject to be unreachable, because it didn't check whether the refs have \nchanged since it read them so that it might actually _be_ reachable after \nall.\n\nSo please don't do this. \n\nIt's important for operations to always think about \"what happens if \nsomebody does a 'commit' or pushes into the tree at the same time?\".\n\nFor example, the \"git prune-packed\" that gets run afterwards is _not_ \nracy, because it will only prune objects that already exist in the pack.\n\n\t\tLinus\n"},{"id":"28543","messageId":"452BF8B3.5090305@tromer.org","threadId":"5869","inReplyTo":"Pine.LNX.4.64.0610100800490.3952@g5.osdl.org","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-10T19:46:59Z","receivedAt":"2006-10-10T19:46:59Z","isPatch":true,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"On 2006-10-10 17:03, Linus Torvalds wrote:\n> On Tue, 10 Oct 2006, Sam Vilain wrote:\n>> If using git-repack -a, unreferenced objects are kept behind in the\n>> pack.  This might be the best default, but there are no good ways\n>> to clean up the packfiles if a lot of rebasing is happening, or\n>> branches have been deleted.\n> \n> Don't do this.\n\nToo late: \"git repack -a -d\" already does it, in contradiction to its\nmanpage. It creates a new pack by following .git/refs, and then deletes\nall old pack files.\n\n> I understand why you want to do it, but the fact is, it's dangerous.\n> \n> Right now, \"git repack\" is actually safe to run even on a repository which \n> is being modified! And that's actually important, if you have something \n> like a shared repo that gets re-packed every once in a while from a \n> cron-job!\n\nDon't run it on a shared repo, then. And grab a coffee while it runs.\nBut why force leaf repositories to accumulate garbage?\n\nThis functionality is just as racy, and just as necessary, as\n\"git-prune\". It merely garbage-collects the packs as well. Git seems to\ncollect unreferenced objects faster than the space between the cushions\nin my sofa, and there ought to be a way to tidy up things.\n\nLinus, I see why you neither need nor want this functionality in your\ntypical workflow, but things look different for a downstream developer\nwho engages in a variety of garbage-generating activities like tracking\nwild trees, rebasing patches and using stgit. I really don't need that\nunreferenced copy of 2.6.15-rc2-mm1 in my packs anymore.\n\n  Eran\n"},{"id":"28546","messageId":"7vk637zzpk.fsf@assigned-by-dhcp.cox.net","threadId":"5869","inReplyTo":"Pine.LNX.4.64.0610100800490.3952@g5.osdl.org","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-10T20:24:23Z","receivedAt":"2006-10-10T20:24:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> On Tue, 10 Oct 2006, Sam Vilain wrote:\n>>\n>> If using git-repack -a, unreferenced objects are kept behind in the\n>> pack.  This might be the best default, but there are no good ways\n>> to clean up the packfiles if a lot of rebasing is happening, or\n>> branches have been deleted.\n>\n> Don't do this.\n>\n> I understand why you want to do it, but the fact is, it's dangerous.\n\nSorry, I understand \"it's dangerous\" part, but I do not\nunderstand \"why you want to do it\" part.\n\n@@ -32,6 +33,11 @@ case \",$all_into_one,\" in\n ,,)\n \trev_list='--unpacked'\n \tpack_objects='--incremental'\n+\tif [ -n \"$prune\" ]\n+\tthen\n+\t    # prune junk first\n+\t    git-prune\n+\tfi\n \t;;\n ,t,)\n \trev_list=\n\nThis shouldn't make any difference if the repository is\nquiescent (and is dangerous if it isn't).  pack-objects will\nnot get fed things that are not reachable.\n\n@@ -40,8 +46,14 @@ case \",$all_into_one,\" in\n \t# Redundancy check in all-into-one case is trivial.\n \texisting=`cd \"$PACKDIR\" && \\\n \t    find . -type f \\( -name '*.pack' -o -name '*.idx' \\) -print`\n+\n+\tif [ -n \"$prune\" ]\n+\tthen\n+\t    rev_list=`cd \"$GIT_DIR\" && find refs -type f -print`\n+\tfi\n \t;;\n esac\n+\n\nWe give --all to rev-list so this should not have any effect\neither; other than that the code introduced by this hunk is\nbroken with packed-refs.\n\nIsn't \"repack -a -d\" what Sam wants?\n"},{"id":"28557","messageId":"Pine.LNX.4.64.0610101423561.3952@g5.osdl.org","threadId":"5869","inReplyTo":"452BF8B3.5090305@tromer.org","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-10T21:25:49Z","receivedAt":"2006-10-10T21:25:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 10 Oct 2006, Eran Tromer wrote:\n> \n> Too late: \"git repack -a -d\" already does it, in contradiction to its \n> manpage. It creates a new pack by following .git/refs, and then deletes \n> all old pack files.\n\nThat's very different.\n\nThat just means that you should not try to do two _concurrent_ repacks. \n\n> Don't run it on a shared repo, then. And grab a coffee while it runs.\n> But why force leaf repositories to accumulate garbage?\n\nNobody forces that.\n\nYou can run \"git prune\" if you want to. But at least we know that \"git \nprune\" is unsafe.\n\n\t\t\tLinus\n"},{"id":"28561","messageId":"452C19FC.7030001@tromer.org","threadId":"5869","inReplyTo":"Pine.LNX.4.64.0610101423561.3952@g5.osdl.org","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-10T22:09:00Z","receivedAt":"2006-10-10T22:09:00Z","isPatch":true,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"On 2006-10-10 23:25, Linus Torvalds wrote:\n> On Tue, 10 Oct 2006, Eran Tromer wrote:\n>> Too late: \"git repack -a -d\" already does it, in contradiction to its \n>> manpage. It creates a new pack by following .git/refs, and then deletes \n>> all old pack files.\n> \n> That's very different.\n> \n> That just means that you should not try to do two _concurrent_ repacks. \n\nHow so? This process loses the unreferenced objects from the old packs,\nwhere \"referenced\" is determined in a racy way. Same problem.\n\n> \n>> Don't run it on a shared repo, then. And grab a coffee while it runs.\n>> But why force leaf repositories to accumulate garbage?\n> \n> Nobody forces that.\n> \n> You can run \"git prune\" if you want to. But at least we know that \"git \n> prune\" is unsafe.\n\nBut \"git prune\" does not GC packs, only loose objects.\n\nAnyway, I think the right thing to do is to make \"git repack -a -d\"\noperate safely (not drop any objects), and add a new --prune option\nso that \"git repack -a -d --prune\" does what \"git repack -a -d\" used to do.\n\n  Eran\n"},{"id":"28565","messageId":"Pine.LNX.4.64.0610101524050.3952@g5.osdl.org","threadId":"5869","inReplyTo":"452C19FC.7030001@tromer.org","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-10-10T22:27:13Z","receivedAt":"2006-10-10T22:27:13Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 11 Oct 2006, Eran Tromer wrote:\n> \n> How so? This process loses the unreferenced objects from the old packs,\n> where \"referenced\" is determined in a racy way. Same problem.\n\nNo.\n\nThose unreferenced objects are old history that won't be part of any new \nhistory.\n\nIf you create new history, they won't be in the pack.\n\nIt's obviously possible that you create new history that has a blob that \nis equal to some old history (and no loose object will be created), but by \nthen we're _really_ reaching. \n\n> But \"git prune\" does not GC packs, only loose objects.\n\nRight. And you'd want to repack _and_ prune, but they should be kept \nseparate, because one is safe, the other is not.\n\nOf course, if the code were to check that no references have changed over \nthe operation, then I wouldn't have any objections.\n\n\t\tLinus\n"},{"id":"28568","messageId":"452C3092.7090003@tromer.org","threadId":"5869","inReplyTo":"Pine.LNX.4.64.0610101524050.3952@g5.osdl.org","subject":"Re: [PATCH] repack: allow simultaneous packing and pruning","fromName":"Eran Tromer","fromEmail":"git2eran@tromer.org","sentAt":"2006-10-10T23:45:22Z","receivedAt":"2006-10-10T23:45:22Z","isPatch":true,"sender":{"key":"git2eran@tromer.org","avatar":null},"body":"On 2006-10-11 00:27, Linus Torvalds wrote:\n> Those unreferenced objects are old history that won't be part of any new \n> history.\n> \n> If you create new history, they won't be in the pack.\n\n... because git-repack moves only already-referenced objects to packs\n(and once they're referenced a subsequent \"git-repack -a -d\" won't lose\nthem). Curiously, this critically depends on Documentation/git-repack\nbeing wrong:\n\n  This script is used to combine all objects that do not currently\n  reside in a \"pack\", into a pack.\n\nHowever, this means there is no safe way to create a new pack without\nadding all its content as loose objects first.\n\nFor example, the following is racy because there's a point where the new\npack is on disk but not yet referenced:\n\n$ git-fetch --keep foo &  git-repack -a -d\n\n\n>> But \"git prune\" does not GC packs, only loose objects.\n> \n> Right. And you'd want to repack _and_ prune, but they should be kept \n> separate, because one is safe, the other is not.\n\nAh, semantics.\n\nThe request was for removing unreferenced objects (\"pruning\") in *packs*\nwhile doing the repacking. This turns out to be already implemented\n(contrary to the docs) and, as you explained, safe.\n\nPruning both packed and loose objects while repacking is neither safe\nnor requested (and is indeed roughly equivalent to just\n\"git-repack -a -d; git-prune\").\n\n\n  Eran\n"}]}