{"thread":{"id":"10244","subject":"[PATCH] git-gc: by default use safer \"-A\" option to repack when not --prune'ing","startedAt":"2007-10-10T23:49:29Z","lastAt":"2007-10-11T01:00:27Z","messageCount":6,"participants":["Brandon Casey","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"55439","messageId":"470D6509.5020607@nrlssc.navy.mil","threadId":"10244","inReplyTo":null,"subject":"[PATCH] git-gc: by default use safer \"-A\" option to repack when not --prune'ing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2007-10-10T23:49:29Z","receivedAt":"2007-10-10T23:49:29Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"This makes use of repack's new \"-A\" option which does not drop packed\nunreachable objects. This makes git-gc safe to call at any time,\nparticularly when a repository is referenced as an alternate by\nanother repository.\n\ngit-gc --prune will use the \"-a\" option to repack instead of \"-A\", so\nthat packed unreachable objects will be removed.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n builtin-gc.c |   20 ++++++++++++--------\n 1 files changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 23ad2b6..779245b 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -26,7 +26,7 @@ static int gc_auto_pack_limit = 20;\n #define MAX_ADD 10\n static const char *argv_pack_refs[] = {\"pack-refs\", \"--all\", \"--prune\", NULL};\n static const char *argv_reflog[] = {\"reflog\", \"expire\", \"--all\", NULL};\n-static const char *argv_repack[MAX_ADD] = {\"repack\", \"-a\", \"-d\", \"-l\", NULL};\n+static const char *argv_repack[MAX_ADD] = {\"repack\", \"-d\", \"-l\", NULL};\n static const char *argv_prune[] = {\"prune\", NULL};\n static const char *argv_rerere[] = {\"rerere\", \"gc\", NULL};\n \n@@ -143,8 +143,6 @@ static int too_many_packs(void)\n \n static int need_to_gc(void)\n {\n-\tint ac = 0;\n-\n \t/*\n \t * Setting gc.auto and gc.autopacklimit to 0 or negative can\n \t * disable the automatic gc.\n@@ -158,14 +156,10 @@ static int need_to_gc(void)\n \t * we run \"repack -A -d -l\".  Otherwise we tell the caller\n \t * there is no need.\n \t */\n-\targv_repack[ac++] = \"repack\";\n \tif (too_many_packs())\n-\t\targv_repack[ac++] = \"-A\";\n+\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n \telse if (!too_many_loose_objects())\n \t\treturn 0;\n-\targv_repack[ac++] = \"-d\";\n-\targv_repack[ac++] = \"-l\";\n-\targv_repack[ac++] = NULL;\n \treturn 1;\n }\n \n@@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\tprune = 0;\n \t\tif (!need_to_gc())\n \t\t\treturn 0;\n+\t} else {\n+\t\t/*\n+\t\t * Use safer (for shared repos) \"-A\" option to\n+\t\t * repack when not pruning. Auto-gc makes its\n+                 * own decision.\n+\t\t */\n+\t\tif (prune)\n+\t\t\tappend_option(argv_repack, \"-a\", MAX_ADD);\n+\t\telse\n+\t\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n \t}\n \n \tif (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))\n-- \n1.5.3.4.207.g00000\n"},{"id":"55441","messageId":"Pine.LNX.4.64.0710110106160.4174@racer.site","threadId":"10244","inReplyTo":"470D6509.5020607@nrlssc.navy.mil","subject":"Re: [PATCH] git-gc: by default use safer \"-A\" option to repack when not --prune'ing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-11T00:12:44Z","receivedAt":"2007-10-11T00:12:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 10 Oct 2007, Brandon Casey wrote:\n\n>  static int need_to_gc(void)\n>  {\n> -\tint ac = 0;\n> -\n>  \t/*\n>  \t * Setting gc.auto and gc.autopacklimit to 0 or negative can\n>  \t * disable the automatic gc.\n> @@ -158,14 +156,10 @@ static int need_to_gc(void)\n>  \t * we run \"repack -A -d -l\".  Otherwise we tell the caller\n>  \t * there is no need.\n>  \t */\n> -\targv_repack[ac++] = \"repack\";\n>  \tif (too_many_packs())\n> -\t\targv_repack[ac++] = \"-A\";\n> +\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n>  \telse if (!too_many_loose_objects())\n>  \t\treturn 0;\n> -\targv_repack[ac++] = \"-d\";\n> -\targv_repack[ac++] = \"-l\";\n> -\targv_repack[ac++] = NULL;\n>  \treturn 1;\n>  }\n>  \n\nThis is a nice cleanup, but it does not belong in this patch.\n\n> @@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>  \t\tprune = 0;\n>  \t\tif (!need_to_gc())\n>  \t\t\treturn 0;\n> +\t} else {\n> +\t\t/*\n> +\t\t * Use safer (for shared repos) \"-A\" option to\n> +\t\t * repack when not pruning. Auto-gc makes its\n> +                 * own decision.\n> +\t\t */\n> +\t\tif (prune)\n> +\t\t\tappend_option(argv_repack, \"-a\", MAX_ADD);\n> +\t\telse\n> +\t\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n>  \t}\n>  \n>  \tif (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))\n\nTo use the append_option() function, you need to make sure that the argv \nhas enough space, so I suggest adding another NULL to argv_repack[].  This \nis enough, since you only ever add one option.  But you should mention it \nsomewhere, probably above the definition of argv_repack[].\n\nOther than that: I think your patch makes tons of sense; much better than \nwhat I tried to cobble together with the cyclic alternates detection.\n\nCiao,\nDscho\n"},{"id":"55442","messageId":"Pine.LNX.4.64.0710101918060.30020@torch.nrlssc.navy.mil","threadId":"10244","inReplyTo":"Pine.LNX.4.64.0710110106160.4174@racer.site","subject":"Re: [PATCH] git-gc: by default use safer \"-A\" option to repack when not --prune'ing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2007-10-11T00:26:19Z","receivedAt":"2007-10-11T00:26:19Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"On Thu, 11 Oct 2007, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Wed, 10 Oct 2007, Brandon Casey wrote:\n\n<snip>\n\n> This is a nice cleanup, but it does not belong in this patch.\n\nI was thinking the same thing right after I sent it off.\n\n>\n>> @@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>>  \t\tprune = 0;\n>>  \t\tif (!need_to_gc())\n>>  \t\t\treturn 0;\n>> +\t} else {\n>> +\t\t/*\n>> +\t\t * Use safer (for shared repos) \"-A\" option to\n>> +\t\t * repack when not pruning. Auto-gc makes its\n>> +                 * own decision.\n>> +\t\t */\n>> +\t\tif (prune)\n>> +\t\t\tappend_option(argv_repack, \"-a\", MAX_ADD);\n>> +\t\telse\n>> +\t\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n>>  \t}\n>>\n>>  \tif (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))\n>\n> To use the append_option() function, you need to make sure that the argv\n> has enough space, so I suggest adding another NULL to argv_repack[].  This\n> is enough, since you only ever add one option.  But you should mention it\n> somewhere, probably above the definition of argv_repack[].\n\nShouldn't argv_repack have space for MAX_ADD (10) items?\n\nappend_option() is already used with argv_repack when --aggressive is\nused.\n\n> Other than that: I think your patch makes tons of sense; much better than\n> what I tried to cobble together with the cyclic alternates detection.\n\nThanks.\n\n-brandon\n"},{"id":"55444","messageId":"Pine.LNX.4.64.0710110149100.4174@racer.site","threadId":"10244","inReplyTo":"Pine.LNX.4.64.0710101918060.30020@torch.nrlssc.navy.mil","subject":"Re: [PATCH] git-gc: by default use safer \"-A\" option to repack when not --prune'ing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-11T00:49:44Z","receivedAt":"2007-10-11T00:49:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 10 Oct 2007, Brandon Casey wrote:\n\n> On Thu, 11 Oct 2007, Johannes Schindelin wrote:\n> \n> > > @@ -211,6 +205,16 @@ int cmd_gc(int argc, const char **argv, const char\n> > > *prefix)\n> > >  \t\tprune = 0;\n> > >  \t\tif (!need_to_gc())\n> > >  \t\t\treturn 0;\n> > > +\t} else {\n> > > +\t\t/*\n> > > +\t\t * Use safer (for shared repos) \"-A\" option to\n> > > +\t\t * repack when not pruning. Auto-gc makes its\n> > > +                 * own decision.\n> > > +\t\t */\n> > > +\t\tif (prune)\n> > > +\t\t\tappend_option(argv_repack, \"-a\", MAX_ADD);\n> > > +\t\telse\n> > > +\t\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n> > >  \t}\n> > > \n> > >  \tif (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))\n> > \n> > To use the append_option() function, you need to make sure that the \n> > argv has enough space, so I suggest adding another NULL to \n> > argv_repack[].  This is enough, since you only ever add one option.  \n> > But you should mention it somewhere, probably above the definition of \n> > argv_repack[].\n> \n> Shouldn't argv_repack have space for MAX_ADD (10) items?\n\nOuch... I overlooked that.  So please strike this objection.\n\nThanks,\nDscho\n"},{"id":"55445","messageId":"470D7567.6010208@nrlssc.navy.mil","threadId":"10244","inReplyTo":"Pine.LNX.4.64.0710110149100.4174@racer.site","subject":"Re: [PATCH] git-gc: by default use safer \"-A\" option to repack when not --prune'ing","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2007-10-11T00:59:19Z","receivedAt":"2007-10-11T00:59:19Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"This makes use of repack's new \"-A\" option which does not drop packed\nunreachable objects. This makes git-gc safe to call at any time,\nparticularly when a repository is referenced as an alternate by\nanother repository.\n\ngit-gc --prune will use the \"-a\" option to repack instead of \"-A\", so\nthat packed unreachable objects will be removed.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n builtin-gc.c |   12 +++++++++++-\n 1 files changed, 11 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 23ad2b6..6323e0d 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -26,7 +26,7 @@ static int gc_auto_pack_limit = 20;\n #define MAX_ADD 10\n static const char *argv_pack_refs[] = {\"pack-refs\", \"--all\", \"--prune\", NULL};\n static const char *argv_reflog[] = {\"reflog\", \"expire\", \"--all\", NULL};\n-static const char *argv_repack[MAX_ADD] = {\"repack\", \"-a\", \"-d\", \"-l\", NULL};\n+static const char *argv_repack[MAX_ADD] = {\"repack\", \"-d\", \"-l\", NULL};\n static const char *argv_prune[] = {\"prune\", NULL};\n static const char *argv_rerere[] = {\"rerere\", \"gc\", NULL};\n \n@@ -211,6 +211,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\tprune = 0;\n \t\tif (!need_to_gc())\n \t\t\treturn 0;\n+\t} else {\n+\t\t/*\n+\t\t * Use safer (for shared repos) \"-A\" option to\n+\t\t * repack when not pruning. Auto-gc makes its\n+\t\t * own decision.\n+\t\t */\n+\t\tif (prune)\n+\t\t\tappend_option(argv_repack, \"-a\", MAX_ADD);\n+\t\telse\n+\t\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n \t}\n \n \tif (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))\n-- \n1.5.3.4.208.g729c0\n"},{"id":"55446","messageId":"470D75AB.2050404@nrlssc.navy.mil","threadId":"10244","inReplyTo":"Pine.LNX.4.64.0710110149100.4174@racer.site","subject":"[PATCH] git-gc --auto: simplify \"repack\" command line building","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2007-10-11T01:00:27Z","receivedAt":"2007-10-11T01:00:27Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Since \"-a\" is removed from the base repack command line,\nwe can simplify how we add additional options to this\ncommand line when using --auto.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n builtin-gc.c |    8 +-------\n 1 files changed, 1 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 6323e0d..956c32d 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -143,8 +143,6 @@ static int too_many_packs(void)\n \n static int need_to_gc(void)\n {\n-\tint ac = 0;\n-\n \t/*\n \t * Setting gc.auto and gc.autopacklimit to 0 or negative can\n \t * disable the automatic gc.\n@@ -158,14 +156,10 @@ static int need_to_gc(void)\n \t * we run \"repack -A -d -l\".  Otherwise we tell the caller\n \t * there is no need.\n \t */\n-\targv_repack[ac++] = \"repack\";\n \tif (too_many_packs())\n-\t\targv_repack[ac++] = \"-A\";\n+\t\tappend_option(argv_repack, \"-A\", MAX_ADD);\n \telse if (!too_many_loose_objects())\n \t\treturn 0;\n-\targv_repack[ac++] = \"-d\";\n-\targv_repack[ac++] = \"-l\";\n-\targv_repack[ac++] = NULL;\n \treturn 1;\n }\n \n-- \n1.5.3.4.208.g729c0\n"}]}