Merge branch 'aw/gc-lockfile-fscanf-fix'
authorJunio C Hamano <gitster@pobox.com>
Mon, 25 Sep 2017 06:24:08 +0000 (15:24 +0900)
committerJunio C Hamano <gitster@pobox.com>
Mon, 25 Sep 2017 06:24:09 +0000 (15:24 +0900)
"git gc" tries to avoid running two instances at the same time by
reading and writing pid/host from and to a lock file; it used to
use an incorrect fscanf() format when reading, which has been
corrected.

* aw/gc-lockfile-fscanf-fix:
gc: call fscanf() with %<len>s, not %<len>c, when reading hostname

1  2 
builtin/gc.c
diff --combined builtin/gc.c
index c22787ac723c421b17032ecf0b718ff434203c86,22b35a627e60be8bf15f28624834ac1f913efcd8..3c5eae0edf12e461e84c71c0e2988fcfa58d3d44
@@@ -11,7 -11,6 +11,7 @@@
   */
  
  #include "builtin.h"
 +#include "config.h"
  #include "tempfile.h"
  #include "lockfile.h"
  #include "parse-options.h"
@@@ -19,7 -18,6 +19,7 @@@
  #include "sigchain.h"
  #include "argv-array.h"
  #include "commit.h"
 +#include "packfile.h"
  
  #define FAILED_RUN "failed to run %s"
  
@@@ -35,8 -33,6 +35,8 @@@ static int aggressive_window = 250
  static int gc_auto_threshold = 6700;
  static int gc_auto_pack_limit = 50;
  static int detach_auto = 1;
 +static timestamp_t gc_log_expire_time;
 +static const char *gc_log_expire = "1.day.ago";
  static const char *prune_expire = "2.weeks.ago";
  static const char *prune_worktrees_expire = "3.months.ago";
  
@@@ -47,7 -43,7 +47,7 @@@ static struct argv_array prune = ARGV_A
  static struct argv_array prune_worktrees = ARGV_ARRAY_INIT;
  static struct argv_array rerere = ARGV_ARRAY_INIT;
  
 -static struct tempfile pidfile;
 +static struct tempfile *pidfile;
  static struct lock_file log_lock;
  
  static struct string_list pack_garbage = STRING_LIST_INIT_DUP;
@@@ -66,31 -62,24 +66,31 @@@ static void report_pack_garbage(unsigne
                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)
 +      if (fstat(get_lock_file_fd(&log_lock), &st)) {
 +              /*
 +               * Perhaps there was an i/o error or another
 +               * unlikely situation.  Try to make a note of
 +               * this in gc.log along with any existing
 +               * messages.
 +               */
 +              int saved_errno = errno;
 +              fprintf(stderr, _("Failed to fstat %s: %s"),
 +                      get_tempfile_path(log_lock.tempfile),
 +                      strerror(saved_errno));
 +              fflush(stderr);
                commit_lock_file(&log_lock);
 -      else
 +              errno = saved_errno;
 +      } else if (st.st_size) {
 +              /* There was some error recorded in the lock file */
 +              commit_lock_file(&log_lock);
 +      } else {
 +              /* No error, clean up any old gc.log */
 +              unlink(git_path("gc.log"));
                rollback_lock_file(&log_lock);
 +      }
  }
  
  static void process_log_file_at_exit(void)
@@@ -122,10 -111,8 +122,10 @@@ 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);
 -      git_config_date_string("gc.pruneexpire", &prune_expire);
 -      git_config_date_string("gc.worktreepruneexpire", &prune_worktrees_expire);
 +      git_config_get_expiry("gc.pruneexpire", &prune_expire);
 +      git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
 +      git_config_get_expiry("gc.logexpiry", &gc_log_expire);
 +
        git_config(git_default_config, NULL);
  }
  
@@@ -137,6 -124,8 +137,6 @@@ static int too_many_loose_objects(void
         * distributed, we can check only one and get a reasonable
         * estimate.
         */
 -      char path[PATH_MAX];
 -      const char *objdir = get_object_directory();
        DIR *dir;
        struct dirent *ent;
        int auto_threshold;
        if (gc_auto_threshold <= 0)
                return 0;
  
 -      if (sizeof(path) <= snprintf(path, sizeof(path), "%s/17", objdir)) {
 -              warning(_("insanely long object directory %.*s"), 50, objdir);
 -              return 0;
 -      }
 -      dir = opendir(path);
 +      dir = opendir(git_path("objects/17"));
        if (!dir)
                return 0;
  
 -      auto_threshold = (gc_auto_threshold + 255) / 256;
 +      auto_threshold = DIV_ROUND_UP(gc_auto_threshold, 256);
        while ((ent = readdir(dir)) != NULL) {
                if (strspn(ent->d_name, "0123456789abcdef") != 38 ||
                    ent->d_name[38] != '\0')
@@@ -198,11 -191,6 +198,11 @@@ static void add_repack_all_option(void
        }
  }
  
 +static void add_repack_incremental_option(void)
 +{
 +       argv_array_push(&repack, "--no-write-bitmap-index");
 +}
 +
  static int need_to_gc(void)
  {
        /*
         */
        if (too_many_packs())
                add_repack_all_option();
 -      else if (!too_many_loose_objects())
 +      else if (too_many_loose_objects())
 +              add_repack_incremental_option();
 +      else
                return 0;
  
        if (run_hook_le(NULL, "pre-auto-gc", NULL))
@@@ -242,11 -228,11 +242,11 @@@ static const char *lock_repo_for_gc(in
        int fd;
        char *pidfile_path;
  
 -      if (is_tempfile_active(&pidfile))
 +      if (is_tempfile_active(pidfile))
                /* already locked */
                return NULL;
  
 -      if (gethostname(my_host, sizeof(my_host)))
 +      if (xgethostname(my_host, sizeof(my_host)))
                xsnprintf(my_host, sizeof(my_host), "unknown");
  
        pidfile_path = git_pathdup("gc.pid");
                int should_exit;
  
                if (!scan_fmt)
-                       scan_fmt = xstrfmt("%s %%%dc", "%"SCNuMAX, HOST_NAME_MAX);
+                       scan_fmt = xstrfmt("%s %%%ds", "%"SCNuMAX, HOST_NAME_MAX);
                fp = fopen(pidfile_path, "r");
                memset(locking_host, 0, sizeof(locking_host));
                should_exit =
        write_in_full(fd, sb.buf, sb.len);
        strbuf_release(&sb);
        commit_lock_file(&lock);
 -      register_tempfile(&pidfile, pidfile_path);
 +      pidfile = register_tempfile(pidfile_path);
        free(pidfile_path);
        return NULL;
  }
  static int report_last_gc_error(void)
  {
        struct strbuf sb = STRBUF_INIT;
 -      int ret;
 +      int ret = 0;
 +      struct stat st;
 +      char *gc_log_path = git_pathdup("gc.log");
 +
 +      if (stat(gc_log_path, &st)) {
 +              if (errno == ENOENT)
 +                      goto done;
  
 -      ret = strbuf_read_file(&sb, git_path("gc.log"), 0);
 +              ret = error_errno(_("Can't stat %s"), gc_log_path);
 +              goto done;
 +      }
 +
 +      if (st.st_mtime < gc_log_expire_time)
 +              goto done;
 +
 +      ret = strbuf_read_file(&sb, gc_log_path, 0);
        if (ret > 0)
 -              return error(_("The last gc run reported the following. "
 +              ret = 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);
 +                          gc_log_path, sb.buf);
        strbuf_release(&sb);
 -      return 0;
 +done:
 +      free(gc_log_path);
 +      return ret;
  }
  
  static int gc_before_repack(void)
@@@ -375,10 -346,7 +375,10 @@@ int cmd_gc(int argc, const char **argv
        argv_array_pushl(&prune_worktrees, "worktree", "prune", "--expire", NULL);
        argv_array_pushl(&rerere, "rerere", "gc", NULL);
  
 +      /* default expiry time, overwritten in gc_config */
        gc_config();
 +      if (parse_expiry_date(gc_log_expire, &gc_log_expire_time))
 +              die(_("Failed to parse gc.logexpiry value %s"), gc_log_expire);
  
        if (pack_refs < 0)
                pack_refs = !is_bare_repository();
                        if (report_last_gc_error())
                                return -1;
  
 +                      if (lock_repo_for_gc(force, &pid))
 +                              return 0;
                        if (gc_before_repack())
                                return -1;
 +                      delete_tempfile(&pidfile);
 +
                        /*
                         * failure to daemonize is ok, we'll continue
                         * in foreground
                warning(_("There are too many unreachable loose objects; "
                        "run 'git prune' to remove them."));
  
 +      if (!daemonized)
 +              unlink(git_path("gc.log"));
 +
        return 0;
  }