From: Nguyen Thai Ngoc Duy Date: Sat, 12 May 2012 08:18:24 GMT Subject: Re: [PATCH] Update "gc" behavior in commit, merge, am, rebase and index-pack Message-ID: In-Reply-To: <1336810134-3103-1-git-send-email-pclouds@gmail.com> (CCing Fernnando and complaining about send-email) I accidentally put buccia@@repnz.net instead of one @. There were warnings in the jungle that send-email spit out: Use of uninitialized value $cc in string eq at /home/pclouds/opt/git/libexec/git-core/git-send-email line 983. Use of uninitialized value $cc in quotemeta at /home/pclouds/opt/git/libexec/git-core/git-send-email line 983. W: unable to extract a valid address from: Fernando Vezzosi W: unable to extract a valid address from: Fernando Vezzosi but it's really hard to see that. Maybe send-email should not allow me to send in this case (unless I give --force), or make the warnings more outstanding? On Sat, May 12, 2012 at 3:08 PM, Nguyễn Thái Ngọc Duy wrote: > A few attempts have been made in the past to change 'gc --auto' [1] > [2]. This is another such attempt. > > Commit d4bb43e (Invoke "git gc --auto" from commit, merge, am and > rebase. - 2007-09-05) used the rule to put "gc --auto" is "where > update-ref occurs". I would argue that this is not a good condition to > run gc, because (at least current) gc is slow. We encourage commit > often and rebase to make all patches in good shape and this workflow > should not be interrupted/slowed down by random "gc --auto". > > Instead, we could just inform users that "gc" should be run soon in > commonly used commands (this patch also reinstates "gc" check in > commit, which was lost at the sh->C conversion). [1] and [2] can annoy > users constantly with warnings. This patch shows the warning at most > once a day. > > There are commands that are not expected to return immediately. These > are more suitable for "gc --auto". One of them is receive-pack, which > already calls "gc --auto". The other one is index-pack, which "gc > --auto" is added in by this patch. > > In short, after this patch: > >  - receive-pack and index-pack (or push, pull/fetch at high level) can >   do "gc --auto". >  - commmit, merge, am, rebase warns once a day, no actual gc. > > [1] http://thread.gmane.org/gmane.comp.version-control.git/184848 >   adds an config option to turn --auto to warning only > > [2] http://thread.gmane.org/gmane.comp.version-control.git/187711 >   make it warn early before actual do the house keeping > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- >  builtin/commit.c           |    2 ++ >  builtin/gc.c               |   21 ++++++++++++++++++++- >  builtin/index-pack.c       |   16 ++++++++++++++-- >  builtin/merge.c            |    2 +- >  git-am.sh                  |    2 +- >  git-rebase--interactive.sh |    2 +- >  6 files changed, 39 insertions(+), 6 deletions(-) > > diff --git a/builtin/commit.c b/builtin/commit.c > index a2ec73d..4ba5677 100644 > --- a/builtin/commit.c > +++ b/builtin/commit.c > @@ -1418,6 +1418,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) > >                OPT_END() >        }; > +       const char *argv_gc_auto[] = { "gc", "--check", NULL }; > >        struct strbuf sb = STRBUF_INIT; >        struct strbuf author_ident = STRBUF_INIT; > @@ -1589,6 +1590,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) >                     "new_index file. Check that disk is not full or quota is\n" >                     "not exceeded, and then \"git reset HEAD\" to recover.")); > > +       run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); >        rerere(0); >        run_hook(get_index_file(), "post-commit", NULL); >        if (amend && !no_post_rewrite) { > diff --git a/builtin/gc.c b/builtin/gc.c > index 9b4232c..a85f71c 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -171,6 +171,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) >  { >        int aggressive = 0; >        int auto_gc = 0; > +       int check_gc = 0; >        int quiet = 0; > >        struct option builtin_gc_options[] = { > @@ -180,12 +181,16 @@ int cmd_gc(int argc, const char **argv, const char *prefix) >                        PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire }, >                OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"), >                OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"), > +               OPT_BOOLEAN(0, "check", &check_gc, "enable auto-gc mode"), >                OPT_END() >        }; > >        if (argc == 2 && !strcmp(argv[1], "-h")) >                usage_with_options(builtin_gc_usage, builtin_gc_options); > > +       if (auto_gc && check_gc) > +               die(_("--auto and --check are incompatible")); > + >        argv_array_pushl(&pack_refs_cmd, "pack-refs", "--all", "--prune", NULL); >        argv_array_pushl(&reflog, "reflog", "expire", "--all", NULL); >        argv_array_pushl(&repack, "repack", "-d", "-l", NULL); > @@ -211,12 +216,26 @@ int cmd_gc(int argc, const char **argv, const char *prefix) >        if (quiet) >                argv_array_push(&repack, "-q"); > > -       if (auto_gc) { > +       if (auto_gc || check_gc) { >                /* >                 * Auto-gc should be least intrusive as possible. >                 */ >                if (!need_to_gc()) >                        return 0; > +               if (check_gc) { > +                       struct stat st; > +                       if (stat(git_path("gc_needed"), &st)) > +                               open(git_path("gc_needed"), O_CREAT | O_RDWR); > +                       else { > +                               /* do not bother users more than once a day */ > +                               if (time(NULL) - st.st_mtime < 86400) > +                                       return 0; > +                               utime(git_path("gc_needed"), NULL); > +                       } > +                       warning(_("This repository needs maintenance. " > +                                 "Please run \"git gc\" as soon as possible.")); > +                       return 0; > +               } >                if (quiet) >                        fprintf(stderr, _("Auto packing the repository for optimum performance.\n")); >                else > diff --git a/builtin/index-pack.c b/builtin/index-pack.c > index 83555e5..438245c 100644 > --- a/builtin/index-pack.c > +++ b/builtin/index-pack.c > @@ -9,6 +9,7 @@ >  #include "progress.h" >  #include "fsck.h" >  #include "exec_cmd.h" > +#include "run-command.h" > >  static const char index_pack_usage[] = >  "git index-pack [-v] [-o ] [--keep | --keep=] [--verify] [--strict] ( | --stdin [--fix-thin] [])"; > @@ -63,6 +64,7 @@ static int nr_resolved_deltas; >  static int from_stdin; >  static int strict; >  static int verbose; > +static int auto_gc; > >  static struct progress *progress; > > @@ -968,6 +970,10 @@ static int git_index_pack_config(const char *k, const char *v, void *cb) >                        die("bad pack.indexversion=%"PRIu32, opts->version); >                return 0; >        } > +       if (strcmp(k, "receive.autogc") == 0) { > +               auto_gc = git_config_bool(k, v); > +               return 0; > +       } >        return git_default_config(k, v, cb); >  } > > @@ -1254,12 +1260,18 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) >        curr_index = write_idx_file(index_name, idx_objects, nr_objects, &opts, pack_sha1); >        free(idx_objects); > > -       if (!verify) > +       if (!verify) { >                final(pack_name, curr_pack, >                      index_name, curr_index, >                      keep_name, keep_msg, >                      pack_sha1); > -       else > +               if (auto_gc) { > +                       const char *argv_gc_auto[] = { > +                               "gc", "--auto", "--quiet", NULL, > +                       }; > +                       run_command_v_opt(argv_gc_auto, RUN_GIT_CMD); > +               } > +       } else >                close(input_fd); >        free(objects); >        free(index_name_buf); > diff --git a/builtin/merge.c b/builtin/merge.c > index 470fc57..8b8716e 100644 > --- a/builtin/merge.c > +++ b/builtin/merge.c > @@ -385,7 +385,7 @@ static void finish(struct commit *head_commit, >                if (verbosity >= 0 && !merge_msg.len) >                        printf(_("No merge message -- not updating HEAD\n")); >                else { > -                       const char *argv_gc_auto[] = { "gc", "--auto", NULL }; > +                       const char *argv_gc_auto[] = { "gc", "--check", NULL }; >                        update_ref(reflog_message.buf, "HEAD", >                                new_head, head, 0, >                                DIE_ON_ERR); > diff --git a/git-am.sh b/git-am.sh > index f8b7a0c..0f2dbc4 100755 > --- a/git-am.sh > +++ b/git-am.sh > @@ -902,4 +902,4 @@ if test -s "$dotest"/rewritten; then >  fi > >  rm -fr "$dotest" > -git gc --auto > +git gc --check > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 0c19b7c..89996a3 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -573,7 +573,7 @@ do_next () { >                true # we don't care if this hook failed >        fi && >        rm -rf "$state_dir" && > -       git gc --auto && > +       git gc --check && >        warn "Successfully rebased and updated $head_name." > >        exit > -- > 1.7.8.36.g69ee2 > -- Duy