{"thread":{"id":"7920","subject":"[PATCH 8/8] git-repack --max-pack-size: add option parsing to enable feature","startedAt":"2007-04-30T23:25:16Z","lastAt":"2007-05-01T06:27:14Z","messageCount":4,"participants":["Dana How","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"40776","messageId":"46367ADC.2090704@gmail.com","threadId":"7920","inReplyTo":null,"subject":"[PATCH 8/8] git-repack --max-pack-size: add option parsing to enable feature","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-04-30T23:25:16Z","receivedAt":"2007-04-30T23:25:16Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"\nAdd --max-pack-size parsing and usage messages.\nUpgrade git-repack.sh to handle multiple packfile names.\n\nSigned-off-by: Dana L. How \n---\n builtin-pack-objects.c | 10 +++++++++-\n git-repack.sh | 12 +++++++-----\n 2 files changed, 16 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 328b3cb..ed958d6 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -15,7 +15,7 @@\n #include \"progress.h\"\n \n static const char pack_usage[] = \"\\\n-git-pack-objects [{ -q | --progress | --all-progress }] \\n\\\n+git-pack-objects [{ -q | --progress | --all-progress }] [--max-pack-size=N] \\n\\\n \t[--local] [--incremental] [--window=N] [--depth=N] \\n\\\n \t[--no-reuse-delta] [--delta-base-offset] [--non-empty] \\n\\\n \t[--revs [--unpacked | --all]*] [--reflog] [--stdout | base-name] \\n\\\n@@ -1766,6 +1766,14 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t\tincremental = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!prefixcmp(arg, \"--max-pack-size=\")) {\n+\t\t\tchar *end;\n+\t\t\tpack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;\n+\t\t\tif (!arg[16] || *end)\n+\t\t\t\tusage(pack_usage);\n+\t\t\tno_reuse_delta = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tif (!prefixcmp(arg, \"--window=\")) {\n \t\t\tchar *end;\n \t\t\twindow = strtoul(arg+9, &end, 0);\ndiff --git a/git-repack.sh b/git-repack.sh\nindex ddfa8b4..ebce1b3 100755\n--- a/git-repack.sh\n+++ b/git-repack.sh\n@@ -3,7 +3,7 @@\n # Copyright (c) 2005 Linus Torvalds\n #\n \n-USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--window=N] [--depth=N]'\n+USAGE='[-a] [-d] [-f] [-l] [-n] [-q] [--max-pack-size=N] [--window=N] [--depth=N]'\n SUBDIRECTORY_OK='Yes'\n . git-sh-setup\n \n@@ -18,6 +18,7 @@ do\n \t-q)\tquiet=-q ;;\n \t-f)\tno_reuse_delta=--no-reuse-delta ;;\n \t-l)\tlocal=--local ;;\n+\t--max-pack-size=*) extra=\"$extra $1\" ;;\n \t--window=*) extra=\"$extra $1\" ;;\n \t--depth=*) extra=\"$extra $1\" ;;\n \t*)\tusage ;;\n@@ -62,11 +63,12 @@ case \",$all_into_one,\" in\n esac\n \n args=\"$args $local $quiet $no_reuse_delta$extra\"\n-name=$(git-pack-objects --non-empty --all --reflog $args wrote:\n> Add --max-pack-size parsing and usage messages.\n> Upgrade git-repack.sh to handle multiple packfile names.\n...\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n...\n> +\t\tif (!prefixcmp(arg, \"--max-pack-size=\")) {\n> +\t\t\tchar *end;\n> +\t\t\tpack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;\n> +\t\t\tif (!arg[16] || *end)\n> +\t\t\t\tusage(pack_usage);\n> +\t\t\tno_reuse_delta = 1;\n\nWow, are you serious? This entire change is about making repack\nautomatically split large projects into multiple packfiles. If\nthat happens are you intending that the caller will mark all of\nthose packfiles with .keep files immediately after repacking them?\nIf you want users to create .keep files, can git-repack.sh do that\nfor them when more than one packfile was output?\n\nBecause otherwise a \"quick\" git-gc will not be quick because the\nreuse delta feature (which is a massive performance improvement for\nrepack/gc) will always be disabled. But odds are a future repack\nof the same project will generally keep things that are in the\nsame packfile already in the same packfile again, so delta reuse is\nactually possible for most objects. I think you should find a way\nto make this change work without needing to force no_reuse_delta\njust because this limit was added.\n\n> diff --git a/git-repack.sh b/git-repack.sh\n...\n> +names=$(git-pack-objects --non-empty --all --reflog $args \texit 1\n> -if [ -z \"$name\" ]; then\n> +if [ -z \"$names\" ]; then\n> \techo Nothing new to pack.\n> -else\n> +fi\n> +for name in $names ; do\n\nI think this particular change needs to either preceed the prior\ncommit, or be part of it. If someone tries to bisect this patch\nseries with a large enough project that multiple packfiles are being\nproduced then you run into some ugly problems in this repack script.\n\n-- \nShawn.\n"},{"id":"40799","messageId":"56b7f5510704302315v3d985dcfodb48fcd0bf49853f@mail.gmail.com","threadId":"7920","inReplyTo":"20070501054528.GC5942@spearce.org","subject":"Re: [PATCH 8/8] git-repack --max-pack-size: add option parsing to enable feature","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-01T06:15:09Z","receivedAt":"2007-05-01T06:15:09Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 4/30/07, Shawn O. Pearce wrote:\n> Dana How wrote:\n> > diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> ...\n> > + if (!prefixcmp(arg, \"--max-pack-size=\")) {\n> > + char *end;\n> > + pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;\n> > + if (!arg[16] || *end)\n> > + usage(pack_usage);\n> > + no_reuse_delta = 1;\n>\n> Wow, are you serious? This entire change is about making repack\n> automatically split large projects into multiple packfiles. If\n> that happens are you intending that the caller will mark all of\n> those packfiles with .keep files immediately after repacking them?\n> If you want users to create .keep files, can git-repack.sh do that\n> for them when more than one packfile was output?\n>\n> Because otherwise a \"quick\" git-gc will not be quick because the\n> reuse delta feature (which is a massive performance improvement for\n> repack/gc) will always be disabled. But odds are a future repack\n> of the same project will generally keep things that are in the\n> same packfile already in the same packfile again, so delta reuse is\n> actually possible for most objects. I think you should find a way\n> to make this change work without needing to force no_reuse_delta\n> just because this limit was added.\nIn a previous version of this patchset I explained that I had turned\non no_reuse_delta because I hadn't yet verified all the combinations\nin the reuse codepath(s). I guess I'll be reinspecting them sooner\nrather than later.\n\n> > diff --git a/git-repack.sh b/git-repack.sh\n> ...\n> > +names=$(git-pack-objects --non-empty --all --reflog $args > exit 1\n> > -if [ -z \"$name\" ]; then\n> > +if [ -z \"$names\" ]; then\n> > echo Nothing new to pack.\n> > -else\n> > +fi\n> > +for name in $names ; do\n>\n> I think this particular change needs to either preceed the prior\n> commit, or be part of it. If someone tries to bisect this patch\n> series with a large enough project that multiple packfiles are being\n> produced then you run into some ugly problems in this repack script.\nSorry, you lost me. git is being bisected, or a project managed by git?\nMy intent was that the pack splitting wouldn't be available until\n_all_ patches were applied (active). Bisecting git _within_ this patchset\nwould still be useful -- it could be used to isolate where some of\nthis new code broke some pre-existing feature.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"40800","messageId":"20070501062714.GE5942@spearce.org","threadId":"7920","inReplyTo":"56b7f5510704302315v3d985dcfodb48fcd0bf49853f@mail.gmail.com","subject":"Re: [PATCH 8/8] git-repack --max-pack-size: add option parsing to enable feature","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-01T06:27:14Z","receivedAt":"2007-05-01T06:27:14Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Dana How wrote:\n> >I think this particular change needs to either preceed the prior\n> >commit, or be part of it. If someone tries to bisect this patch\n> >series with a large enough project that multiple packfiles are being\n> >produced then you run into some ugly problems in this repack script.\n...\n> Sorry, you lost me. git is being bisected, or a project managed by git?\n> My intent was that the pack splitting wouldn't be available until\n> _all_ patches were applied (active). Bisecting git _within_ this patchset\n> would still be useful -- it could be used to isolate where some of\n> this new code broke some pre-existing feature.\n\ngit bisecting itself, to search for bugs in itself... In that\ncase a bisect could stop at any random point in the middle of this\nseries, even if this series isn't the one that is at fault for the\ngiven breakage. In such a case we try to hope that git.git is\nalways in a working state at any given commit.\n\nOn second thought looking at your series I see how you were able to\nassure that here. You didn't activate the option until the shell\nscript could also handle more than one name, so since the option\nis not available in any prior commit there's no way to turn on the\nmultiple-name-output that git-repack.sh would have broken on.\n\nBut lets just say the 7/8 patch actually did break git, and we are\nbisecting to look for it. We wouldn't actually see the breakage\nactivate until 8/8, which is misleading, as the broken code is\nactually in 7/8. But if you are looking at 8/8 as broken and see\nthe lines broken, you can easily run git-blame to really figure\nout what changed, and why. So its not really a big deal that the\nseries is organized like this. I blew it out of proportion. :-)\n\n-- \nShawn.\n"}]}