{"thread":{"id":"30516","subject":"[PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","startedAt":"2012-05-12T08:08:54Z","lastAt":"2012-05-19T04:56:40Z","messageCount":17,"participants":["Nguyễn Thái Ngọc Duy","Nguyen Thai Ngoc Duy","Nicolas Pitre","Sverre Rabbelier","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"191437","messageId":"1336810134-3103-1-git-send-email-pclouds@gmail.com","threadId":"30516","inReplyTo":null,"subject":"[PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-12T08:08:54Z","receivedAt":"2012-05-12T08:08:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"A few attempts have been made in the past to change 'gc --auto' [1]\n[2]. This is another such attempt.\n\nCommit d4bb43e (Invoke \"git gc --auto\" from commit, merge, am and\nrebase. - 2007-09-05) used the rule to put \"gc --auto\" is \"where\nupdate-ref occurs\". I would argue that this is not a good condition to\nrun gc, because (at least current) gc is slow. We encourage commit\noften and rebase to make all patches in good shape and this workflow\nshould not be interrupted/slowed down by random \"gc --auto\".\n\nInstead, we could just inform users that \"gc\" should be run soon in\ncommonly used commands (this patch also reinstates \"gc\" check in\ncommit, which was lost at the sh->C conversion). [1] and [2] can annoy\nusers constantly with warnings. This patch shows the warning at most\nonce a day.\n\nThere are commands that are not expected to return immediately. These\nare more suitable for \"gc --auto\". One of them is receive-pack, which\nalready calls \"gc --auto\". The other one is index-pack, which \"gc\n--auto\" is added in by this patch.\n\nIn short, after this patch:\n\n - receive-pack and index-pack (or push, pull/fetch at high level) can\n   do \"gc --auto\".\n - commmit, merge, am, rebase warns once a day, no actual gc.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/184848\n   adds an config option to turn --auto to warning only\n\n[2] http://thread.gmane.org/gmane.comp.version-control.git/187711\n   make it warn early before actual do the house keeping\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/commit.c           |    2 ++\n builtin/gc.c               |   21 ++++++++++++++++++++-\n builtin/index-pack.c       |   16 ++++++++++++++--\n builtin/merge.c            |    2 +-\n git-am.sh                  |    2 +-\n git-rebase--interactive.sh |    2 +-\n 6 files changed, 39 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a2ec73d..4ba5677 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1418,6 +1418,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \n \t\tOPT_END()\n \t};\n+\tconst char *argv_gc_auto[] = { \"gc\", \"--check\", NULL };\n \n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf author_ident = STRBUF_INIT;\n@@ -1589,6 +1590,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t     \"new_index file. Check that disk is not full or quota is\\n\"\n \t\t     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\n \n+\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n \trerere(0);\n \trun_hook(get_index_file(), \"post-commit\", NULL);\n \tif (amend && !no_post_rewrite) {\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 9b4232c..a85f71c 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -171,6 +171,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n {\n \tint aggressive = 0;\n \tint auto_gc = 0;\n+\tint check_gc = 0;\n \tint quiet = 0;\n \n \tstruct option builtin_gc_options[] = {\n@@ -180,12 +181,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },\n \t\tOPT_BOOLEAN(0, \"aggressive\", &aggressive, \"be more thorough (increased runtime)\"),\n \t\tOPT_BOOLEAN(0, \"auto\", &auto_gc, \"enable auto-gc mode\"),\n+\t\tOPT_BOOLEAN(0, \"check\", &check_gc, \"enable auto-gc mode\"),\n \t\tOPT_END()\n \t};\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_gc_usage, builtin_gc_options);\n \n+\tif (auto_gc && check_gc)\n+\t\tdie(_(\"--auto and --check are incompatible\"));\n+\n \targv_array_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n \targv_array_pushl(&reflog, \"reflog\", \"expire\", \"--all\", NULL);\n \targv_array_pushl(&repack, \"repack\", \"-d\", \"-l\", NULL);\n@@ -211,12 +216,26 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \tif (quiet)\n \t\targv_array_push(&repack, \"-q\");\n \n-\tif (auto_gc) {\n+\tif (auto_gc || check_gc) {\n \t\t/*\n \t\t * Auto-gc should be least intrusive as possible.\n \t\t */\n \t\tif (!need_to_gc())\n \t\t\treturn 0;\n+\t\tif (check_gc) {\n+\t\t\tstruct stat st;\n+\t\t\tif (stat(git_path(\"gc_needed\"), &st))\n+\t\t\t\topen(git_path(\"gc_needed\"), O_CREAT | O_RDWR);\n+\t\t\telse {\n+\t\t\t\t/* do not bother users more than once a day */\n+\t\t\t\tif (time(NULL) - st.st_mtime < 86400)\n+\t\t\t\t\treturn 0;\n+\t\t\t\tutime(git_path(\"gc_needed\"), NULL);\n+\t\t\t}\n+\t\t\twarning(_(\"This repository needs maintenance. \"\n+\t\t\t\t  \"Please run \\\"git gc\\\" as soon as possible.\"));\n+\t\t\treturn 0;\n+\t\t}\n \t\tif (quiet)\n \t\t\tfprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n \t\telse\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 83555e5..438245c 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -9,6 +9,7 @@\n #include \"progress.h\"\n #include \"fsck.h\"\n #include \"exec_cmd.h\"\n+#include \"run-command.h\"\n \n static const char index_pack_usage[] =\n \"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])\";\n@@ -63,6 +64,7 @@ static int nr_resolved_deltas;\n static int from_stdin;\n static int strict;\n static int verbose;\n+static int auto_gc;\n \n static struct progress *progress;\n \n@@ -968,6 +970,10 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n \t\t\tdie(\"bad pack.indexversion=%\"PRIu32, opts->version);\n \t\treturn 0;\n \t}\n+\tif (strcmp(k, \"receive.autogc\") == 0) {\n+\t\tauto_gc = git_config_bool(k, v);\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(k, v, cb);\n }\n \n@@ -1254,12 +1260,18 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tcurr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_sha1);\n \tfree(idx_objects);\n \n-\tif (!verify)\n+\tif (!verify) {\n \t\tfinal(pack_name, curr_pack,\n \t\t      index_name, curr_index,\n \t\t      keep_name, keep_msg,\n \t\t      pack_sha1);\n-\telse\n+\t\tif (auto_gc) {\n+\t\t\tconst char *argv_gc_auto[] = {\n+\t\t\t\t\"gc\", \"--auto\", \"--quiet\", NULL,\n+\t\t\t};\n+\t\t\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n+\t\t}\n+\t} else\n \t\tclose(input_fd);\n \tfree(objects);\n \tfree(index_name_buf);\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 470fc57..8b8716e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -385,7 +385,7 @@ static void finish(struct commit *head_commit,\n \t\tif (verbosity >= 0 && !merge_msg.len)\n \t\t\tprintf(_(\"No merge message -- not updating HEAD\\n\"));\n \t\telse {\n-\t\t\tconst char *argv_gc_auto[] = { \"gc\", \"--auto\", NULL };\n+\t\t\tconst char *argv_gc_auto[] = { \"gc\", \"--check\", NULL };\n \t\t\tupdate_ref(reflog_message.buf, \"HEAD\",\n \t\t\t\tnew_head, head, 0,\n \t\t\t\tDIE_ON_ERR);\ndiff --git a/git-am.sh b/git-am.sh\nindex f8b7a0c..0f2dbc4 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -902,4 +902,4 @@ if test -s \"$dotest\"/rewritten; then\n fi\n \n rm -fr \"$dotest\"\n-git gc --auto\n+git gc --check\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 0c19b7c..89996a3 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -573,7 +573,7 @@ do_next () {\n \t\ttrue # we don't care if this hook failed\n \tfi &&\n \trm -rf \"$state_dir\" &&\n-\tgit gc --auto &&\n+\tgit gc --check &&\n \twarn \"Successfully rebased and updated $head_name.\"\n \n \texit\n-- \n1.7.8.36.g69ee2\n"},{"id":"191438","messageId":"CACsJy8BXXrze-0n++YenDERfi=z26L1P_06XjZLFeeMuuLgg8w@mail.gmail.com","threadId":"30516","inReplyTo":"1336810134-3103-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-12T08:18:24Z","receivedAt":"2012-05-12T08:18:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"(CCing Fernnando and complaining about send-email)\n\nI accidentally put buccia@@repnz.net instead of one @. There were\nwarnings in the jungle that send-email spit out:\n\nUse of uninitialized value $cc in string eq at\n/home/pclouds/opt/git/libexec/git-core/git-send-email line 983.\nUse of uninitialized value $cc in quotemeta at\n/home/pclouds/opt/git/libexec/git-core/git-send-email line 983.\nW: unable to extract a valid address from: Fernando Vezzosi <buccia@@repnz.net>\nW: unable to extract a valid address from: Fernando Vezzosi <buccia@@repnz.net>\n\nbut it's really hard to see that. Maybe send-email should not allow me\nto send in this case (unless I give --force), or make the warnings\nmore outstanding?\n\nOn Sat, May 12, 2012 at 3:08 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> A few attempts have been made in the past to change 'gc --auto' [1]\n> [2]. This is another such attempt.\n>\n> Commit d4bb43e (Invoke \"git gc --auto\" from commit, merge, am and\n> rebase. - 2007-09-05) used the rule to put \"gc --auto\" is \"where\n> update-ref occurs\". I would argue that this is not a good condition to\n> run gc, because (at least current) gc is slow. We encourage commit\n> often and rebase to make all patches in good shape and this workflow\n> should not be interrupted/slowed down by random \"gc --auto\".\n>\n> Instead, we could just inform users that \"gc\" should be run soon in\n> commonly used commands (this patch also reinstates \"gc\" check in\n> commit, which was lost at the sh->C conversion). [1] and [2] can annoy\n> users constantly with warnings. This patch shows the warning at most\n> once a day.\n>\n> There are commands that are not expected to return immediately. These\n> are more suitable for \"gc --auto\". One of them is receive-pack, which\n> already calls \"gc --auto\". The other one is index-pack, which \"gc\n> --auto\" is added in by this patch.\n>\n> In short, after this patch:\n>\n>  - receive-pack and index-pack (or push, pull/fetch at high level) can\n>   do \"gc --auto\".\n>  - commmit, merge, am, rebase warns once a day, no actual gc.\n>\n> [1] http://thread.gmane.org/gmane.comp.version-control.git/184848\n>   adds an config option to turn --auto to warning only\n>\n> [2] http://thread.gmane.org/gmane.comp.version-control.git/187711\n>   make it warn early before actual do the house keeping\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  builtin/commit.c           |    2 ++\n>  builtin/gc.c               |   21 ++++++++++++++++++++-\n>  builtin/index-pack.c       |   16 ++++++++++++++--\n>  builtin/merge.c            |    2 +-\n>  git-am.sh                  |    2 +-\n>  git-rebase--interactive.sh |    2 +-\n>  6 files changed, 39 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index a2ec73d..4ba5677 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1418,6 +1418,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>\n>                OPT_END()\n>        };\n> +       const char *argv_gc_auto[] = { \"gc\", \"--check\", NULL };\n>\n>        struct strbuf sb = STRBUF_INIT;\n>        struct strbuf author_ident = STRBUF_INIT;\n> @@ -1589,6 +1590,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n>                     \"new_index file. Check that disk is not full or quota is\\n\"\n>                     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\n>\n> +       run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n>        rerere(0);\n>        run_hook(get_index_file(), \"post-commit\", NULL);\n>        if (amend && !no_post_rewrite) {\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 9b4232c..a85f71c 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -171,6 +171,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>  {\n>        int aggressive = 0;\n>        int auto_gc = 0;\n> +       int check_gc = 0;\n>        int quiet = 0;\n>\n>        struct option builtin_gc_options[] = {\n> @@ -180,12 +181,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>                        PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },\n>                OPT_BOOLEAN(0, \"aggressive\", &aggressive, \"be more thorough (increased runtime)\"),\n>                OPT_BOOLEAN(0, \"auto\", &auto_gc, \"enable auto-gc mode\"),\n> +               OPT_BOOLEAN(0, \"check\", &check_gc, \"enable auto-gc mode\"),\n>                OPT_END()\n>        };\n>\n>        if (argc == 2 && !strcmp(argv[1], \"-h\"))\n>                usage_with_options(builtin_gc_usage, builtin_gc_options);\n>\n> +       if (auto_gc && check_gc)\n> +               die(_(\"--auto and --check are incompatible\"));\n> +\n>        argv_array_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n>        argv_array_pushl(&reflog, \"reflog\", \"expire\", \"--all\", NULL);\n>        argv_array_pushl(&repack, \"repack\", \"-d\", \"-l\", NULL);\n> @@ -211,12 +216,26 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>        if (quiet)\n>                argv_array_push(&repack, \"-q\");\n>\n> -       if (auto_gc) {\n> +       if (auto_gc || check_gc) {\n>                /*\n>                 * Auto-gc should be least intrusive as possible.\n>                 */\n>                if (!need_to_gc())\n>                        return 0;\n> +               if (check_gc) {\n> +                       struct stat st;\n> +                       if (stat(git_path(\"gc_needed\"), &st))\n> +                               open(git_path(\"gc_needed\"), O_CREAT | O_RDWR);\n> +                       else {\n> +                               /* do not bother users more than once a day */\n> +                               if (time(NULL) - st.st_mtime < 86400)\n> +                                       return 0;\n> +                               utime(git_path(\"gc_needed\"), NULL);\n> +                       }\n> +                       warning(_(\"This repository needs maintenance. \"\n> +                                 \"Please run \\\"git gc\\\" as soon as possible.\"));\n> +                       return 0;\n> +               }\n>                if (quiet)\n>                        fprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n>                else\n> diff --git a/builtin/index-pack.c b/builtin/index-pack.c\n> index 83555e5..438245c 100644\n> --- a/builtin/index-pack.c\n> +++ b/builtin/index-pack.c\n> @@ -9,6 +9,7 @@\n>  #include \"progress.h\"\n>  #include \"fsck.h\"\n>  #include \"exec_cmd.h\"\n> +#include \"run-command.h\"\n>\n>  static const char index_pack_usage[] =\n>  \"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])\";\n> @@ -63,6 +64,7 @@ static int nr_resolved_deltas;\n>  static int from_stdin;\n>  static int strict;\n>  static int verbose;\n> +static int auto_gc;\n>\n>  static struct progress *progress;\n>\n> @@ -968,6 +970,10 @@ static int git_index_pack_config(const char *k, const char *v, void *cb)\n>                        die(\"bad pack.indexversion=%\"PRIu32, opts->version);\n>                return 0;\n>        }\n> +       if (strcmp(k, \"receive.autogc\") == 0) {\n> +               auto_gc = git_config_bool(k, v);\n> +               return 0;\n> +       }\n>        return git_default_config(k, v, cb);\n>  }\n>\n> @@ -1254,12 +1260,18 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n>        curr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_sha1);\n>        free(idx_objects);\n>\n> -       if (!verify)\n> +       if (!verify) {\n>                final(pack_name, curr_pack,\n>                      index_name, curr_index,\n>                      keep_name, keep_msg,\n>                      pack_sha1);\n> -       else\n> +               if (auto_gc) {\n> +                       const char *argv_gc_auto[] = {\n> +                               \"gc\", \"--auto\", \"--quiet\", NULL,\n> +                       };\n> +                       run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n> +               }\n> +       } else\n>                close(input_fd);\n>        free(objects);\n>        free(index_name_buf);\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 470fc57..8b8716e 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -385,7 +385,7 @@ static void finish(struct commit *head_commit,\n>                if (verbosity >= 0 && !merge_msg.len)\n>                        printf(_(\"No merge message -- not updating HEAD\\n\"));\n>                else {\n> -                       const char *argv_gc_auto[] = { \"gc\", \"--auto\", NULL };\n> +                       const char *argv_gc_auto[] = { \"gc\", \"--check\", NULL };\n>                        update_ref(reflog_message.buf, \"HEAD\",\n>                                new_head, head, 0,\n>                                DIE_ON_ERR);\n> diff --git a/git-am.sh b/git-am.sh\n> index f8b7a0c..0f2dbc4 100755\n> --- a/git-am.sh\n> +++ b/git-am.sh\n> @@ -902,4 +902,4 @@ if test -s \"$dotest\"/rewritten; then\n>  fi\n>\n>  rm -fr \"$dotest\"\n> -git gc --auto\n> +git gc --check\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 0c19b7c..89996a3 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -573,7 +573,7 @@ do_next () {\n>                true # we don't care if this hook failed\n>        fi &&\n>        rm -rf \"$state_dir\" &&\n> -       git gc --auto &&\n> +       git gc --check &&\n>        warn \"Successfully rebased and updated $head_name.\"\n>\n>        exit\n> --\n> 1.7.8.36.g69ee2\n>\n\n\n\n-- \nDuy\n"},{"id":"191460","messageId":"alpine.LFD.2.02.1205121326380.21030@xanadu.home","threadId":"30516","inReplyTo":"1336810134-3103-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2012-05-12T17:36:28Z","receivedAt":"2012-05-12T17:36:28Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sat, 12 May 2012, Nguyễn Thái Ngọc Duy wrote:\n\n> A few attempts have been made in the past to change 'gc --auto' [1]\n> [2]. This is another such attempt.\n> \n> Commit d4bb43e (Invoke \"git gc --auto\" from commit, merge, am and\n> rebase. - 2007-09-05) used the rule to put \"gc --auto\" is \"where\n> update-ref occurs\". I would argue that this is not a good condition to\n> run gc, because (at least current) gc is slow. We encourage commit\n> often and rebase to make all patches in good shape and this workflow\n> should not be interrupted/slowed down by random \"gc --auto\".\n> \n> Instead, we could just inform users that \"gc\" should be run soon in\n> commonly used commands (this patch also reinstates \"gc\" check in\n> commit, which was lost at the sh->C conversion). [1] and [2] can annoy\n> users constantly with warnings. This patch shows the warning at most\n> once a day.\n\nI agree with this.  However don't bother making this once a day.  There \nis no harm in warning every time.  Referring to the man page as git \n--auto does when it actually meets its treshold wouldn't be a bad thing \neither as incidentally that would contain the info to get rid of the \nwarning.\n\n\nNicolas\n"},{"id":"191484","messageId":"CACsJy8DxoRHYnBvJn7P9dS1A0Y88Bm0ur1meWCUBp09UEfuOgg@mail.gmail.com","threadId":"30516","inReplyTo":"alpine.LFD.2.02.1205121326380.21030@xanadu.home","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-14T09:09:17Z","receivedAt":"2012-05-14T09:09:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, May 13, 2012 at 12:36 AM, Nicolas Pitre <nico@fluxnic.net> wrote:\n> On Sat, 12 May 2012, Nguyễn Thái Ngọc Duy wrote:\n>\n>> A few attempts have been made in the past to change 'gc --auto' [1]\n>> [2]. This is another such attempt.\n>>\n>> Commit d4bb43e (Invoke \"git gc --auto\" from commit, merge, am and\n>> rebase. - 2007-09-05) used the rule to put \"gc --auto\" is \"where\n>> update-ref occurs\". I would argue that this is not a good condition to\n>> run gc, because (at least current) gc is slow. We encourage commit\n>> often and rebase to make all patches in good shape and this workflow\n>> should not be interrupted/slowed down by random \"gc --auto\".\n>>\n>> Instead, we could just inform users that \"gc\" should be run soon in\n>> commonly used commands (this patch also reinstates \"gc\" check in\n>> commit, which was lost at the sh->C conversion). [1] and [2] can annoy\n>> users constantly with warnings. This patch shows the warning at most\n>> once a day.\n>\n> I agree with this.  However don't bother making this once a day.  There\n> is no harm in warning every time.  Referring to the man page as git\n> --auto does when it actually meets its treshold wouldn't be a bad thing\n> either as incidentally that would contain the info to get rid of the\n> warning.\n\nI'm good with always warning too. The moment it shows up, I can just\nopen another terminal for \"gc\". Though (I guess) a new user in the\nmiddle of the work may not want to do that as they fear \"gc\" may\ninterfere their work. Then the warning becomes a constant annoyance\n(and maybe forgotten after they're used to seeing the warning).\n\nIf everybody's good with always warning, I'll make a new patch.\n-- \nDuy\n"},{"id":"191495","messageId":"CAGdFq_hneGBtjKHk=jqmgrVgYFKEFYLTnyS+-OemwL4z7JKmKw@mail.gmail.com","threadId":"30516","inReplyTo":"CACsJy8DxoRHYnBvJn7P9dS1A0Y88Bm0ur1meWCUBp09UEfuOgg@mail.gmail.com","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2012-05-14T15:18:38Z","receivedAt":"2012-05-14T15:18:38Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"On Mon, May 14, 2012 at 4:09 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> I'm good with always warning too. The moment it shows up, I can just\n> open another terminal for \"gc\". Though (I guess) a new user in the\n> middle of the work may not want to do that as they fear \"gc\" may\n> interfere their work.\n\nIf we know that it won't interfere with their work, can we just spawn\ngit gc as a background process?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"191525","messageId":"20120514201100.GA3740@sigill.intra.peff.net","threadId":"30516","inReplyTo":"alpine.LFD.2.02.1205121326380.21030@xanadu.home","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-14T20:11:01Z","receivedAt":"2012-05-14T20:11:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 12, 2012 at 01:36:28PM -0400, Nicolas Pitre wrote:\n\n> > Instead, we could just inform users that \"gc\" should be run soon in\n> > commonly used commands (this patch also reinstates \"gc\" check in\n> > commit, which was lost at the sh->C conversion). [1] and [2] can annoy\n> > users constantly with warnings. This patch shows the warning at most\n> > once a day.\n> \n> I agree with this.  However don't bother making this once a day.  There \n> is no harm in warning every time.\n\nI don't mind seeing a warning every time I run \"git commit\" myself, but\naren't there things which run \"merge\" and \"commit\" in a loop? For\nexample, \"git rebase -m\"? Warning 20 times when 20 commits are rebased\nseems excessive.\n\n-Peff\n"},{"id":"191528","messageId":"20120514205039.GB3740@sigill.intra.peff.net","threadId":"30516","inReplyTo":"1336810134-3103-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-14T20:50:40Z","receivedAt":"2012-05-14T20:50:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 12, 2012 at 03:08:54PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> Instead, we could just inform users that \"gc\" should be run soon in\n> commonly used commands (this patch also reinstates \"gc\" check in\n> commit, which was lost at the sh->C conversion). [1] and [2] can annoy\n> users constantly with warnings. This patch shows the warning at most\n> once a day.\n\nHmm. The missing \"gc\" from git-commit has been noticed before, but we\ndecided not to reinstate it[1]:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/78524/focus=78693\n\nHowever, I'm not sure Junio's argument in that thread is valid:\n\n  I had an impression that we accepted the hook which made \"gc --auto\"\n  more expensive by forcing it to check the hook (and possibly execute\n  it every time) after vetting am, svn and friends to make sure nobody\n  triggered \"gc --auto\" once per every commit, and during that vetting\n  process we noticed that \"git commit\" lost the \"gc --auto\" at the end.\n\nThe pre-auto-gc hook runs only when we need to gc (i.e., when we would\nbe doing something expensive, anyway). So it is still cheap to check\nwhether we need to gc (although if you have an option like \"--check\"\nwhich does not _fix_ the situation after checking, then you may end up\nrunning the hook repeatedly).\n\n-Peff\n\n[1] It also came up other times:\n\n      http://thread.gmane.org/gmane.comp.version-control.git/101667\n\n      http://thread.gmane.org/gmane.comp.version-control.git/114089\n\n    but each time it seems to have been rejected on the basis of the\n    first argument.\n"},{"id":"191534","messageId":"7vy5oufi9k.fsf@alter.siamese.dyndns.org","threadId":"30516","inReplyTo":"20120514205039.GB3740@sigill.intra.peff.net","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-14T23:35:35Z","receivedAt":"2012-05-14T23:35:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> The pre-auto-gc hook runs only when we need to gc (i.e., when we would\n> be doing something expensive, anyway). So it is still cheap to check\n> whether we need to gc (although if you have an option like \"--check\"\n> which does not _fix_ the situation after checking, then you may end up\n> running the hook repeatedly).\n\nWhether we need to gc might be cheap to check, but I think the worry about\nthe new hook added after commit lost auto-gc was the hook to see if the\nuser declines our offer to run \"gc --auto\" may be expensive with an extra\nfork and exec.  But I am personally fine with \"commit\" running \"gc --auto\"\nevery time; pre-auto-gc hook is opt-in after all.\n\nAs you mentioned in your own follow-up message, however, it will make\nthings worse without any real benefit if it is merely \"check and warn but\nnever gc\".  Also when \"gc --auto\" does not make the repository better (I\nvaguely recall some corner cases mentioned here in the past), repeated\ninvocation of it might make such a change annoying.\n"},{"id":"191544","messageId":"CACsJy8AjtE_K53toP6WHh6W9aDSxEur+xOt3wQQ=cDuhBcLuhA@mail.gmail.com","threadId":"30516","inReplyTo":"CAGdFq_hneGBtjKHk=jqmgrVgYFKEFYLTnyS+-OemwL4z7JKmKw@mail.gmail.com","subject":"Re: [PATCH] Update \"gc\" behavior in commit, merge, am, rebase and index-pack","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-15T11:25:39Z","receivedAt":"2012-05-15T11:25:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, May 14, 2012 at 10:18 PM, Sverre Rabbelier <srabbelier@gmail.com> wrote:\n> On Mon, May 14, 2012 at 4:09 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n>> I'm good with always warning too. The moment it shows up, I can just\n>> open another terminal for \"gc\". Though (I guess) a new user in the\n>> middle of the work may not want to do that as they fear \"gc\" may\n>> interfere their work.\n>\n> If we know that it won't interfere with their work, can we just spawn\n> git gc as a background process?\n\nI thought of that for index-pack, but we would need to deal with error\nmessages from gc messing up with foreground command's. I don't like a\ncommand leaving background processes (without asking me) after it\nexits, so index-pack should hang on until gc finishes. Which is ok for\nlong time commands like index-pack, but not ok for commit, rebase...\n-- \nDuy\n"},{"id":"191582","messageId":"1337171377-26960-1-git-send-email-pclouds@gmail.com","threadId":"30516","inReplyTo":"1336810134-3103-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 0/5] \"git gc --auto\" update","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:29:32Z","receivedAt":"2012-05-16T12:29:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Changes since the first patch:\n\n - wait time goes down to 1 hour, not too long, but also avoid flooding\n - avoid 1 fork for calling \"gc --auto\" if there's no actual housekeeping\n - commit learns commit.autogc config key. Scripts shoud make use of this\n\nNguyễn Thái Ngọc Duy (5):\n  Add convenient function to do automatic garbage collection\n  index-pack: perform automatic gc, share receive.gc config with\n    receive-pack\n  commit: reinstate \"gc --auto\"\n  gc: add --dry-run\n  Update \"gc\" behavior in commit, merge, am and rebase\n\n Documentation/config.txt   |   11 +++++++--\n Documentation/git-gc.txt   |    7 +++++-\n builtin/commit.c           |    2 +\n builtin/gc.c               |   46 +++++++++++++++++++++++++++++++++++++++++++-\n builtin/gc.h               |    9 ++++++++\n builtin/index-pack.c       |    6 +++-\n builtin/merge.c            |    8 +-----\n builtin/receive-pack.c     |   14 +-----------\n git-am.sh                  |    2 +-\n git-rebase--interactive.sh |    2 +-\n 10 files changed, 80 insertions(+), 27 deletions(-)\n create mode 100644 builtin/gc.h\n\n-- \n1.7.8.36.g69ee2\n"},{"id":"191583","messageId":"1337171377-26960-2-git-send-email-pclouds@gmail.com","threadId":"30516","inReplyTo":"1337171377-26960-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 1/5] Add convenient function to do automatic garbage collection","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:29:33Z","receivedAt":"2012-05-16T12:29:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This function also avoids forking most of the time by performing some\ncheck in process.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/gc.c           |   27 +++++++++++++++++++++++++--\n builtin/gc.h           |    8 ++++++++\n builtin/merge.c        |    8 ++------\n builtin/receive-pack.c |   14 ++------------\n 4 files changed, 37 insertions(+), 20 deletions(-)\n create mode 100644 builtin/gc.h\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 9b4232c..ce60225 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -15,6 +15,7 @@\n #include \"parse-options.h\"\n #include \"run-command.h\"\n #include \"argv-array.h\"\n+#include \"gc.h\"\n \n #define FAILED_RUN \"failed to run %s\"\n \n@@ -28,6 +29,7 @@ static int aggressive_window = 250;\n static int gc_auto_threshold = 6700;\n static int gc_auto_pack_limit = 50;\n static const char *prune_expire = \"2.weeks.ago\";\n+static int auto_gc = 1;\n \n static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;\n static struct argv_array reflog = ARGV_ARRAY_INIT;\n@@ -64,6 +66,10 @@ static int gc_config(const char *var, const char *value, void *cb)\n \t\t}\n \t\treturn git_config_string(&prune_expire, var, value);\n \t}\n+\tif (cb && !strcmp(var, cb)) {\n+\t\tauto_gc = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -162,11 +168,24 @@ static int need_to_gc(void)\n \telse if (!too_many_loose_objects())\n \t\treturn 0;\n \n-\tif (run_hook(NULL, \"pre-auto-gc\", NULL))\n-\t\treturn 0;\n \treturn 1;\n }\n \n+int gc(const char *cmd, int flags)\n+{\n+\tconst char *av[] = { \"gc\", \"--auto\", NULL, NULL };\n+\tint ac = 2;\n+\n+\tgit_config(gc_config, (void*)cmd);\n+\n+\tif (!auto_gc || !need_to_gc())\n+\t\treturn 0;\n+\n+\tif (flags & GC_QUIET)\n+\t\tav[ac++] = \"--quiet\";\n+\treturn run_command_v_opt(av, RUN_GIT_CMD);\n+}\n+\n int cmd_gc(int argc, const char **argv, const char *prefix)\n {\n \tint aggressive = 0;\n@@ -217,6 +236,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\tif (!need_to_gc())\n \t\t\treturn 0;\n+\n+\t\tif (run_hook(NULL, \"pre-auto-gc\", NULL))\n+\t\t\treturn 0;\n+\n \t\tif (quiet)\n \t\t\tfprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n \t\telse\ndiff --git a/builtin/gc.h b/builtin/gc.h\nnew file mode 100644\nindex 0000000..3482e92\n--- /dev/null\n+++ b/builtin/gc.h\n@@ -0,0 +1,8 @@\n+#ifndef GC_H\n+#define GC_H\n+\n+#define GC_QUIET  1\n+\n+extern int gc(const char *cmd, int flags);\n+\n+#endif\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 470fc57..940259d 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -28,6 +28,7 @@\n #include \"remote.h\"\n #include \"fmt-merge-msg.h\"\n #include \"gpg-interface.h\"\n+#include \"gc.h\"\n \n #define DEFAULT_TWOHEAD (1<<0)\n #define DEFAULT_OCTOPUS (1<<1)\n@@ -385,15 +386,10 @@ static void finish(struct commit *head_commit,\n \t\tif (verbosity >= 0 && !merge_msg.len)\n \t\t\tprintf(_(\"No merge message -- not updating HEAD\\n\"));\n \t\telse {\n-\t\t\tconst char *argv_gc_auto[] = { \"gc\", \"--auto\", NULL };\n \t\t\tupdate_ref(reflog_message.buf, \"HEAD\",\n \t\t\t\tnew_head, head, 0,\n \t\t\t\tDIE_ON_ERR);\n-\t\t\t/*\n-\t\t\t * We ignore errors in 'gc --auto', since the\n-\t\t\t * user should see them.\n-\t\t\t */\n-\t\t\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n+\t\t\tgc(NULL, 0);\n \t\t}\n \t}\n \tif (new_head && show_diffstat) {\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0afb8b2..0c1fe25 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -12,6 +12,7 @@\n #include \"string-list.h\"\n #include \"sha1-array.h\"\n #include \"connected.h\"\n+#include \"gc.h\"\n \n static const char receive_pack_usage[] = \"git receive-pack <git-dir>\";\n \n@@ -36,7 +37,6 @@ static int use_sideband;\n static int quiet;\n static int prefer_ofs_delta = 1;\n static int auto_update_server_info;\n-static int auto_gc = 1;\n static const char *head_name;\n static void *head_name_to_free;\n static int sent_capabilities;\n@@ -108,11 +108,6 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n-\tif (strcmp(var, \"receive.autogc\") == 0) {\n-\t\tauto_gc = git_config_bool(var, value);\n-\t\treturn 0;\n-\t}\n-\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -973,12 +968,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)\n \t\t\treport(commands, unpack_status);\n \t\trun_receive_hook(commands, post_receive_hook, 1);\n \t\trun_update_post_hook(commands);\n-\t\tif (auto_gc) {\n-\t\t\tconst char *argv_gc_auto[] = {\n-\t\t\t\t\"gc\", \"--auto\", \"--quiet\", NULL,\n-\t\t\t};\n-\t\t\trun_command_v_opt(argv_gc_auto, RUN_GIT_CMD);\n-\t\t}\n+\t\tgc(\"receive.autogc\", GC_QUIET);\n \t\tif (auto_update_server_info)\n \t\t\tupdate_server_info(0);\n \t}\n-- \n1.7.8.36.g69ee2\n"},{"id":"191584","messageId":"1337171377-26960-3-git-send-email-pclouds@gmail.com","threadId":"30516","inReplyTo":"1337171377-26960-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 2/5] index-pack: perform automatic gc, share receive.gc config with receive-pack","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:29:34Z","receivedAt":"2012-05-16T12:29:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/config.txt |    7 ++++---\n builtin/index-pack.c     |    6 ++++--\n 2 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 915cb5a..3b6d773 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1754,9 +1754,10 @@ rebase.autosquash::\n \tIf set to true enable '--autosquash' option by default.\n \n receive.autogc::\n-\tBy default, git-receive-pack will run \"git-gc --auto\" after\n-\treceiving data from git-push and updating refs.  You can stop\n-\tit by setting this variable to false.\n+\tBy default, git-receive-pack and git-index-pack will run\n+\t\"git-gc --auto\" after receiving data from git-push and\n+\tupdating refs.  You can stop it by setting this variable to\n+\tfalse.\n \n receive.fsckObjects::\n \tIf it is set to true, git-receive-pack will check all received\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex dc2cfe6..4e888ea 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -10,6 +10,7 @@\n #include \"fsck.h\"\n #include \"exec_cmd.h\"\n #include \"thread-utils.h\"\n+#include \"gc.h\"\n \n static const char index_pack_usage[] =\n \"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--verify] [--strict] (<pack-file> | --stdin [--fix-thin] [<pack-file>])\";\n@@ -1463,12 +1464,13 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tcurr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_sha1);\n \tfree(idx_objects);\n \n-\tif (!verify)\n+\tif (!verify) {\n \t\tfinal(pack_name, curr_pack,\n \t\t      index_name, curr_index,\n \t\t      keep_name, keep_msg,\n \t\t      pack_sha1);\n-\telse\n+\t\tgc(\"receive.autogc\", 0);\n+\t} else\n \t\tclose(input_fd);\n \tfree(objects);\n \tfree(index_name_buf);\n-- \n1.7.8.36.g69ee2\n"},{"id":"191585","messageId":"1337171377-26960-4-git-send-email-pclouds@gmail.com","threadId":"30516","inReplyTo":"1337171377-26960-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 3/5] commit: reinstate \"gc --auto\"","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:29:35Z","receivedAt":"2012-05-16T12:29:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This check was lost after sh->C conversion\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/config.txt |    4 ++++\n builtin/commit.c         |    2 ++\n 2 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 3b6d773..8f5dd48 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -904,6 +904,10 @@ commit.template::\n \t\"`~/`\" is expanded to the value of `$HOME` and \"`~user/`\" to the\n \tspecified user's home directory.\n \n+commit.autogc::\n+\tBy default, git-commit will run \"git gc --auto\" after each\n+\tcommit. You can stop it by setting this variable to false.\n+\n credential.helper::\n \tSpecify an external helper to be called when a username or\n \tpassword credential is needed; the helper may consult external\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex a2ec73d..bf7d0aa 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -28,6 +28,7 @@\n #include \"submodule.h\"\n #include \"gpg-interface.h\"\n #include \"column.h\"\n+#include \"gc.h\"\n \n static const char * const builtin_commit_usage[] = {\n \t\"git commit [options] [--] <filepattern>...\",\n@@ -1589,6 +1590,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t     \"new_index file. Check that disk is not full or quota is\\n\"\n \t\t     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\n \n+\tgc(\"commit.autogc\", 0);\n \trerere(0);\n \trun_hook(get_index_file(), \"post-commit\", NULL);\n \tif (amend && !no_post_rewrite) {\n-- \n1.7.8.36.g69ee2\n"},{"id":"191586","messageId":"1337171377-26960-5-git-send-email-pclouds@gmail.com","threadId":"30516","inReplyTo":"1337171377-26960-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 4/5] gc: add --dry-run","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:29:36Z","receivedAt":"2012-05-16T12:29:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\"git gc --auto --dry-run\" will print instead of doing\nhousekeeping. This option is to be called by other commands rather\nthan by user and will not print more than one warning every hour. gc()\nfunction also behaves this way.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/git-gc.txt |    7 ++++++-\n builtin/gc.c             |   19 +++++++++++++++++++\n 2 files changed, 25 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex b370b02..f69f5e1 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -9,7 +9,7 @@ git-gc - Cleanup unnecessary files and optimize the local repository\n SYNOPSIS\n --------\n [verse]\n-'git gc' [--aggressive] [--auto] [--quiet] [--prune=<date> | --no-prune]\n+'git gc' [--aggressive] [--auto [--dry-run]] [--quiet] [--prune=<date> | --no-prune]\n \n DESCRIPTION\n -----------\n@@ -60,6 +60,11 @@ are consolidated into a single pack by using the `-A` option of\n 'git repack'. Setting `gc.autopacklimit` to 0 disables\n automatic consolidation of packs.\n \n+--dry-run::\n+\tOnly show warning if housekeeping is required. The warning\n+\tis only shown at most once every hour even if\n+\t\"gc --auto --dry-run\" is run continuously.\n+\n --prune=<date>::\n \tPrune loose objects older than date (default is 2 weeks ago,\n \toverridable by the config variable `gc.pruneExpire`).  This\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex ce60225..f82b9ef 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -150,6 +150,8 @@ static void add_repack_all_option(void)\n \n static int need_to_gc(void)\n {\n+\tstruct stat st;\n+\n \t/*\n \t * Setting gc.auto to 0 or negative can disable the\n \t * automatic gc.\n@@ -168,6 +170,13 @@ static int need_to_gc(void)\n \telse if (!too_many_loose_objects())\n \t\treturn 0;\n \n+\tif (stat(git_path(\"gc_needed\"), &st))\n+\t\topen(git_path(\"gc_needed\"), O_CREAT | O_RDWR, 0644);\n+\telse if (time(NULL) - st.st_mtime < 3600)\n+\t\treturn 0;\n+\telse\n+\t\tutime(git_path(\"gc_needed\"), NULL);\n+\n \treturn 1;\n }\n \n@@ -190,6 +199,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n {\n \tint aggressive = 0;\n \tint auto_gc = 0;\n+\tint check_gc = 0;\n \tint quiet = 0;\n \n \tstruct option builtin_gc_options[] = {\n@@ -199,12 +209,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },\n \t\tOPT_BOOLEAN(0, \"aggressive\", &aggressive, \"be more thorough (increased runtime)\"),\n \t\tOPT_BOOLEAN(0, \"auto\", &auto_gc, \"enable auto-gc mode\"),\n+\t\tOPT_BOOLEAN(0, \"dry-run\", &check_gc, \"warn if auto gc is needed\"),\n \t\tOPT_END()\n \t};\n \n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage_with_options(builtin_gc_usage, builtin_gc_options);\n \n+\tif (!auto_gc && check_gc)\n+\t\tdie(_(\"--dry-run is useless without --auto\"));\n+\n \targv_array_pushl(&pack_refs_cmd, \"pack-refs\", \"--all\", \"--prune\", NULL);\n \targv_array_pushl(&reflog, \"reflog\", \"expire\", \"--all\", NULL);\n \targv_array_pushl(&repack, \"repack\", \"-d\", \"-l\", NULL);\n@@ -240,6 +254,11 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\tif (run_hook(NULL, \"pre-auto-gc\", NULL))\n \t\t\treturn 0;\n \n+\t\tif (check_gc) {\n+\t\t\twarning(_(\"This repository needs maintenance. \"\n+\t\t\t\t  \"Please run \\\"git gc\\\" as soon as possible.\"));\n+\t\t\treturn 0;\n+\t\t}\n \t\tif (quiet)\n \t\t\tfprintf(stderr, _(\"Auto packing the repository for optimum performance.\\n\"));\n \t\telse\n-- \n1.7.8.36.g69ee2\n"},{"id":"191587","messageId":"1337171377-26960-6-git-send-email-pclouds@gmail.com","threadId":"30516","inReplyTo":"1337171377-26960-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v2 5/5] Update \"gc\" behavior in commit, merge, am and rebase","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-16T12:29:37Z","receivedAt":"2012-05-16T12:29:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Commit d4bb43e (Invoke \"git gc --auto\" from commit, merge, am and\nrebase. - 2007-09-05) used the rule to put \"gc --auto\" is \"where\nupdate-ref occurs\". I would argue that this is not a good condition to\nrun gc, because (at least current) gc is slow. We encourage commit\noften and rebase to make all patches in good shape and this workflow\nshould not be slowed down by random \"gc\".\n\nInstead, we could just inform users that \"gc\" should be run soon in\ncommonly used commands (commit, merge, am and rebase). The warning is\nshown once every hour to avoid constantly annoy users, or flooding the\noutput with warnings if a command is run repeatedly (e.g. in scripts).\n\nCommands that are not expected to return immediately, like\nreceive-pack or index-pack, are more suitable for \"gc --auto\". These\nkeep \"gc --auto\" as before.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/commit.c           |    2 +-\n builtin/gc.c               |    4 +++-\n builtin/gc.h               |    1 +\n builtin/merge.c            |    2 +-\n git-am.sh                  |    2 +-\n git-rebase--interactive.sh |    2 +-\n 6 files changed, 8 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex bf7d0aa..bfc4b4e 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1590,7 +1590,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\t     \"new_index file. Check that disk is not full or quota is\\n\"\n \t\t     \"not exceeded, and then \\\"git reset HEAD\\\" to recover.\"));\n \n-\tgc(\"commit.autogc\", 0);\n+\tgc(\"commit.autogc\", GC_DRYRUN);\n \trerere(0);\n \trun_hook(get_index_file(), \"post-commit\", NULL);\n \tif (amend && !no_post_rewrite) {\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex f82b9ef..bff3d9c 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -182,7 +182,7 @@ static int need_to_gc(void)\n \n int gc(const char *cmd, int flags)\n {\n-\tconst char *av[] = { \"gc\", \"--auto\", NULL, NULL };\n+\tconst char *av[] = { \"gc\", \"--auto\", NULL, NULL, NULL };\n \tint ac = 2;\n \n \tgit_config(gc_config, (void*)cmd);\n@@ -190,6 +190,8 @@ int gc(const char *cmd, int flags)\n \tif (!auto_gc || !need_to_gc())\n \t\treturn 0;\n \n+\tif (flags & GC_DRYRUN)\n+\t\tav[ac++] = \"--dry-run\";\n \tif (flags & GC_QUIET)\n \t\tav[ac++] = \"--quiet\";\n \treturn run_command_v_opt(av, RUN_GIT_CMD);\ndiff --git a/builtin/gc.h b/builtin/gc.h\nindex 3482e92..d022944 100644\n--- a/builtin/gc.h\n+++ b/builtin/gc.h\n@@ -2,6 +2,7 @@\n #define GC_H\n \n #define GC_QUIET  1\n+#define GC_DRYRUN 2\n \n extern int gc(const char *cmd, int flags);\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 940259d..a0680b0 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -389,7 +389,7 @@ static void finish(struct commit *head_commit,\n \t\t\tupdate_ref(reflog_message.buf, \"HEAD\",\n \t\t\t\tnew_head, head, 0,\n \t\t\t\tDIE_ON_ERR);\n-\t\t\tgc(NULL, 0);\n+\t\t\tgc(NULL, GC_DRYRUN);\n \t\t}\n \t}\n \tif (new_head && show_diffstat) {\ndiff --git a/git-am.sh b/git-am.sh\nindex f8b7a0c..71cd2fd 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -902,4 +902,4 @@ if test -s \"$dotest\"/rewritten; then\n fi\n \n rm -fr \"$dotest\"\n-git gc --auto\n+git gc --auto --dry-run\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 0c19b7c..6c1c054 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -573,7 +573,7 @@ do_next () {\n \t\ttrue # we don't care if this hook failed\n \tfi &&\n \trm -rf \"$state_dir\" &&\n-\tgit gc --auto &&\n+\tgit gc --auto --dry-run &&\n \twarn \"Successfully rebased and updated $head_name.\"\n \n \texit\n-- \n1.7.8.36.g69ee2\n"},{"id":"191686","messageId":"xmqqvcjt15g5.fsf@junio.mtv.corp.google.com","threadId":"30516","inReplyTo":"1337171377-26960-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v2 1/5] Add convenient function to do automatic garbage collection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-18T22:37:30Z","receivedAt":"2012-05-18T22:37:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> This function also avoids forking most of the time by performing some\n> check in process.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ...\n> @@ -64,6 +66,10 @@ static int gc_config(const char *var, const char *value, void *cb)\n>  \t\t}\n>  \t\treturn git_config_string(&prune_expire, var, value);\n>  \t}\n> +\tif (cb && !strcmp(var, cb)) {\n> +\t\tauto_gc = git_config_bool(var, value);\n> +\t\treturn 0;\n> +\t}\n\nThis does not look like \"add convenient function for auto-gc\" nor \"avoid\nforking\"; it is something else that is not explained.\n\n> +int gc(const char *cmd, int flags)\n\nDon't call this \"gc\".\n\nIt is hardcoded to perform a cheaper version \"gc --auto\" and nothing\nelse, and it may not even do that when configured not to.\n\nI do not think the first parameter to this function is \"cmd\"\n(abbreviation of \"command\") either.  Isn't it a configuration key to\ntell if auto-gc is allowed, or something?\n\nOther than that, it seems to be a nice change.\n"},{"id":"191720","messageId":"CACsJy8A-gaQFP_X+f4t_WaLG9Gn0b8e1YmH3gF4jmPhqW8m5xw@mail.gmail.com","threadId":"30516","inReplyTo":"xmqqvcjt15g5.fsf@junio.mtv.corp.google.com","subject":"Re: [PATCH v2 1/5] Add convenient function to do automatic garbage collection","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-19T04:56:40Z","receivedAt":"2012-05-19T04:56:40Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, May 19, 2012 at 5:37 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> This function also avoids forking most of the time by performing some\n>> check in process.\n>>\n>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n>> ...\n>> @@ -64,6 +66,10 @@ static int gc_config(const char *var, const char *value, void *cb)\n>>               }\n>>               return git_config_string(&prune_expire, var, value);\n>>       }\n>> +     if (cb && !strcmp(var, cb)) {\n>> +             auto_gc = git_config_bool(var, value);\n>> +             return 0;\n>> +     }\n>\n> This does not look like \"add convenient function for auto-gc\" nor \"avoid\n> forking\"; it is something else that is not explained.\n\nIt takes care of \"foo.autogc\" config key in commands that allows to\nsuppress auto gc. But yes, commit message can use some improvement.\n-- \nDuy\n"}]}