Merge branch 'ew/gc-auto-pack-limit-fix'
authorJunio C Hamano <gitster@pobox.com>
Wed, 13 Jul 2016 18:24:12 +0000 (11:24 -0700)
committerJunio C Hamano <gitster@pobox.com>
Wed, 13 Jul 2016 18:24:12 +0000 (11:24 -0700)
"gc.autoPackLimit" when set to 1 should not trigger a repacking
when there is only one pack, but the code counted poorly and did
so.

* ew/gc-auto-pack-limit-fix:
gc: fix off-by-one error with gc.autoPackLimit

1  2 
builtin/gc.c
diff --combined builtin/gc.c
index c583aad6ec2896c8a6ad3b35671e92a3c0478bcd,15e27809296d98e687b292ee5fa333de02335dc7..332bcf7e7a0ae1900199349870103c388c9ffb35
@@@ -11,7 -11,6 +11,7 @@@
   */
  
  #include "builtin.h"
 +#include "tempfile.h"
  #include "lockfile.h"
  #include "parse-options.h"
  #include "run-command.h"
@@@ -34,63 -33,24 +34,63 @@@ static int gc_auto_threshold = 6700
  static int gc_auto_pack_limit = 50;
  static int detach_auto = 1;
  static const char *prune_expire = "2.weeks.ago";
 +static const char *prune_worktrees_expire = "3.months.ago";
  
  static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
  static struct argv_array reflog = ARGV_ARRAY_INIT;
  static struct argv_array repack = ARGV_ARRAY_INIT;
  static struct argv_array prune = ARGV_ARRAY_INIT;
 +static struct argv_array prune_worktrees = ARGV_ARRAY_INIT;
  static struct argv_array rerere = ARGV_ARRAY_INIT;
  
 -static char *pidfile;
 +static struct tempfile pidfile;
 +static struct lock_file log_lock;
  
 -static void remove_pidfile(void)
 +static struct string_list pack_garbage = STRING_LIST_INIT_DUP;
 +
 +static void clean_pack_garbage(void)
  {
 -      if (pidfile)
 -              unlink(pidfile);
 +      int i;
 +      for (i = 0; i < pack_garbage.nr; i++)
 +              unlink_or_warn(pack_garbage.items[i].string);
 +      string_list_clear(&pack_garbage, 0);
  }
  
 -static void remove_pidfile_on_signal(int signo)
 +static void report_pack_garbage(unsigned seen_bits, const char *path)
  {
 -      remove_pidfile();
 +      if (seen_bits == PACKDIR_FILE_IDX)
 +              string_list_append(&pack_garbage, path);
 +}
 +
 +static void git_config_date_string(const char *key, const char **output)
 +{
 +      if (git_config_get_string_const(key, output))
 +              return;
 +      if (strcmp(*output, "now")) {
 +              unsigned long now = approxidate("now");
 +              if (approxidate(*output) >= now)
 +                      git_die_config(key, _("Invalid %s: '%s'"), key, *output);
 +      }
 +}
 +
 +static void process_log_file(void)
 +{
 +      struct stat st;
 +      if (!fstat(get_lock_file_fd(&log_lock), &st) && st.st_size)
 +              commit_lock_file(&log_lock);
 +      else
 +              rollback_lock_file(&log_lock);
 +}
 +
 +static void process_log_file_at_exit(void)
 +{
 +      fflush(stderr);
 +      process_log_file();
 +}
 +
 +static void process_log_file_on_signal(int signo)
 +{
 +      process_log_file();
        sigchain_pop(signo);
        raise(signo);
  }
@@@ -111,8 -71,16 +111,8 @@@ static void gc_config(void
        git_config_get_int("gc.auto", &gc_auto_threshold);
        git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
        git_config_get_bool("gc.autodetach", &detach_auto);
 -
 -      if (!git_config_get_string_const("gc.pruneexpire", &prune_expire)) {
 -              if (strcmp(prune_expire, "now")) {
 -                      unsigned long now = approxidate("now");
 -                      if (approxidate(prune_expire) >= now) {
 -                              git_die_config("gc.pruneexpire", _("Invalid gc.pruneexpire: '%s'"),
 -                                              prune_expire);
 -                      }
 -              }
 -      }
 +      git_config_date_string("gc.pruneexpire", &prune_expire);
 +      git_config_date_string("gc.worktreepruneexpire", &prune_worktrees_expire);
        git_config(git_default_config, NULL);
  }
  
@@@ -177,7 -145,7 +177,7 @@@ static int too_many_packs(void
                 */
                cnt++;
        }
-       return gc_auto_pack_limit <= cnt;
+       return gc_auto_pack_limit < cnt;
  }
  
  static void add_repack_all_option(void)
@@@ -226,22 -194,20 +226,22 @@@ static const char *lock_repo_for_gc(in
        uintmax_t pid;
        FILE *fp;
        int fd;
 +      char *pidfile_path;
  
 -      if (pidfile)
 +      if (is_tempfile_active(&pidfile))
                /* already locked */
                return NULL;
  
        if (gethostname(my_host, sizeof(my_host)))
 -              strcpy(my_host, "unknown");
 +              xsnprintf(my_host, sizeof(my_host), "unknown");
  
 -      fd = hold_lock_file_for_update(&lock, git_path("gc.pid"),
 +      pidfile_path = git_pathdup("gc.pid");
 +      fd = hold_lock_file_for_update(&lock, pidfile_path,
                                       LOCK_DIE_ON_ERROR);
        if (!force) {
                static char locking_host[128];
                int should_exit;
 -              fp = fopen(git_path("gc.pid"), "r");
 +              fp = fopen(pidfile_path, "r");
                memset(locking_host, 0, sizeof(locking_host));
                should_exit =
                        fp != NULL &&
                         * running.
                         */
                        time(NULL) - st.st_mtime <= 12 * 3600 &&
 -                      fscanf(fp, "%"PRIuMAX" %127c", &pid, locking_host) == 2 &&
 +                      fscanf(fp, "%"SCNuMAX" %127c", &pid, locking_host) == 2 &&
                        /* be gentle to concurrent "gc" on remote hosts */
                        (strcmp(locking_host, my_host) || !kill(pid, 0) || errno == EPERM);
                if (fp != NULL)
                        if (fd >= 0)
                                rollback_lock_file(&lock);
                        *ret_pid = pid;
 +                      free(pidfile_path);
                        return locking_host;
                }
        }
        write_in_full(fd, sb.buf, sb.len);
        strbuf_release(&sb);
        commit_lock_file(&lock);
 -
 -      pidfile = git_pathdup("gc.pid");
 -      sigchain_push_common(remove_pidfile_on_signal);
 -      atexit(remove_pidfile);
 -
 +      register_tempfile(&pidfile, pidfile_path);
 +      free(pidfile_path);
        return NULL;
  }
  
 +static int report_last_gc_error(void)
 +{
 +      struct strbuf sb = STRBUF_INIT;
 +      int ret;
 +
 +      ret = strbuf_read_file(&sb, git_path("gc.log"), 0);
 +      if (ret > 0)
 +              return error(_("The last gc run reported the following. "
 +                             "Please correct the root cause\n"
 +                             "and remove %s.\n"
 +                             "Automatic cleanup will not be performed "
 +                             "until the file is removed.\n\n"
 +                             "%s"),
 +                           git_path("gc.log"), sb.buf);
 +      strbuf_release(&sb);
 +      return 0;
 +}
 +
  static int gc_before_repack(void)
  {
        if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
@@@ -319,7 -269,6 +319,7 @@@ int cmd_gc(int argc, const char **argv
        int force = 0;
        const char *name;
        pid_t pid;
 +      int daemonized = 0;
  
        struct option builtin_gc_options[] = {
                OPT__QUIET(&quiet, N_("suppress progress reporting")),
        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);
 -      argv_array_pushl(&prune, "prune", "--expire", NULL );
 +      argv_array_pushl(&prune, "prune", "--expire", NULL);
 +      argv_array_pushl(&prune_worktrees, "worktree", "prune", "--expire", NULL);
        argv_array_pushl(&rerere, "rerere", "gc", NULL);
  
        gc_config();
                        fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
                }
                if (detach_auto) {
 +                      if (report_last_gc_error())
 +                              return -1;
 +
                        if (gc_before_repack())
                                return -1;
                        /*
                         * failure to daemonize is ok, we'll continue
                         * in foreground
                         */
 -                      daemonize();
 +                      daemonized = !daemonize();
                }
        } else
                add_repack_all_option();
                    name, (uintmax_t)pid);
        }
  
 +      if (daemonized) {
 +              hold_lock_file_for_update(&log_lock,
 +                                        git_path("gc.log"),
 +                                        LOCK_DIE_ON_ERROR);
 +              dup2(get_lock_file_fd(&log_lock), 2);
 +              sigchain_push_common(process_log_file_on_signal);
 +              atexit(process_log_file_at_exit);
 +      }
 +
        if (gc_before_repack())
                return -1;
  
 -      if (run_command_v_opt(repack.argv, RUN_GIT_CMD))
 -              return error(FAILED_RUN, repack.argv[0]);
 +      if (!repository_format_precious_objects) {
 +              if (run_command_v_opt(repack.argv, RUN_GIT_CMD))
 +                      return error(FAILED_RUN, repack.argv[0]);
  
 -      if (prune_expire) {
 -              argv_array_push(&prune, prune_expire);
 -              if (quiet)
 -                      argv_array_push(&prune, "--no-progress");
 -              if (run_command_v_opt(prune.argv, RUN_GIT_CMD))
 -                      return error(FAILED_RUN, prune.argv[0]);
 +              if (prune_expire) {
 +                      argv_array_push(&prune, prune_expire);
 +                      if (quiet)
 +                              argv_array_push(&prune, "--no-progress");
 +                      if (run_command_v_opt(prune.argv, RUN_GIT_CMD))
 +                              return error(FAILED_RUN, prune.argv[0]);
 +              }
 +      }
 +
 +      if (prune_worktrees_expire) {
 +              argv_array_push(&prune_worktrees, prune_worktrees_expire);
 +              if (run_command_v_opt(prune_worktrees.argv, RUN_GIT_CMD))
 +                      return error(FAILED_RUN, prune_worktrees.argv[0]);
        }
  
        if (run_command_v_opt(rerere.argv, RUN_GIT_CMD))
                return error(FAILED_RUN, rerere.argv[0]);
  
 +      report_garbage = report_pack_garbage;
 +      reprepare_packed_git();
 +      if (pack_garbage.nr > 0)
 +              clean_pack_garbage();
 +
        if (auto_gc && too_many_loose_objects())
                warning(_("There are too many unreachable loose objects; "
                        "run 'git prune' to remove them."));