Merge branch 'js/gc-repack-close-before-remove'
authorJunio C Hamano <gitster@pobox.com>
Fri, 18 Jan 2019 21:49:57 +0000 (13:49 -0800)
committerJunio C Hamano <gitster@pobox.com>
Fri, 18 Jan 2019 21:49:57 +0000 (13:49 -0800)
"git gc" and "git repack" did not close the open packfiles that
they found unneeded before removing them, which didn't work on a
platform incapable of removing an open file. This has been
corrected.

* js/gc-repack-close-before-remove:
gc/repack: release packs when needed

1  2 
builtin/gc.c
builtin/repack.c
diff --combined builtin/gc.c
index 7696017cd4152eb191972d2d662c592030ac811e,8a99ed2af2ba341af037224cd529dbfd031ac222..020f725acc40f413c49f812ea0e6aac0153d097c
@@@ -183,7 -183,7 +183,7 @@@ static struct packed_git *find_base_pac
  {
        struct packed_git *p, *base = NULL;
  
 -      for (p = get_packed_git(the_repository); p; p = p->next) {
 +      for (p = get_all_packs(the_repository); p; p = p->next) {
                if (!p->pack_local)
                        continue;
                if (limit) {
@@@ -208,7 -208,7 +208,7 @@@ static int too_many_packs(void
        if (gc_auto_pack_limit <= 0)
                return 0;
  
 -      for (cnt = 0, p = get_packed_git(the_repository); p; p = p->next) {
 +      for (cnt = 0, p = get_all_packs(the_repository); p; p = p->next) {
                if (!p->pack_local)
                        continue;
                if (p->pack_keep)
@@@ -317,7 -317,7 +317,7 @@@ static void add_repack_all_option(struc
  
  static void add_repack_incremental_option(void)
  {
 -       argv_array_push(&repack, "--no-write-bitmap-index");
 +      argv_array_push(&repack, "--no-write-bitmap-index");
  }
  
  static int need_to_gc(void)
@@@ -441,16 -441,10 +441,16 @@@ static const char *lock_repo_for_gc(in
        return NULL;
  }
  
 +/*
 + * Returns 0 if there was no previous error and gc can proceed, 1 if
 + * gc should not proceed due to an error in the last run. Prints a
 + * message and returns -1 if an error occured while reading gc.log
 + */
  static int report_last_gc_error(void)
  {
        struct strbuf sb = STRBUF_INIT;
        int ret = 0;
 +      ssize_t len;
        struct stat st;
        char *gc_log_path = git_pathdup("gc.log");
  
                if (errno == ENOENT)
                        goto done;
  
 -              ret = error_errno(_("Can't stat %s"), gc_log_path);
 +              ret = error_errno(_("cannot 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)
 -              ret = error(_("The last gc run reported the following. "
 +      len = strbuf_read_file(&sb, gc_log_path, 0);
 +      if (len < 0)
 +              ret = error_errno(_("cannot read '%s'"), gc_log_path);
 +      else if (len > 0) {
 +              /*
 +               * A previous gc failed.  Report the error, and don't
 +               * bother with an automatic gc run since it is likely
 +               * to fail in the same way.
 +               */
 +              warning(_("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"),
                            gc_log_path, sb.buf);
 +              ret = 1;
 +      }
        strbuf_release(&sb);
  done:
        free(gc_log_path);
        return ret;
  }
  
 -static int gc_before_repack(void)
 +static void gc_before_repack(void)
  {
        if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
 -              return error(FAILED_RUN, pack_refs_cmd.argv[0]);
 +              die(FAILED_RUN, pack_refs_cmd.argv[0]);
  
        if (prune_reflogs && run_command_v_opt(reflog.argv, RUN_GIT_CMD))
 -              return error(FAILED_RUN, reflog.argv[0]);
 +              die(FAILED_RUN, reflog.argv[0]);
  
        pack_refs = 0;
        prune_reflogs = 0;
 -      return 0;
  }
  
  int cmd_gc(int argc, const char **argv, const char *prefix)
                        fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n"));
                }
                if (detach_auto) {
 -                      if (report_last_gc_error())
 -                              return -1;
 +                      int ret = report_last_gc_error();
 +                      if (ret < 0)
 +                              /* an I/O error occured, already reported */
 +                              exit(128);
 +                      if (ret == 1)
 +                              /* Last gc --auto failed. Skip this one. */
 +                              return 0;
  
                        if (lock_repo_for_gc(force, &pid))
                                return 0;
 -                      if (gc_before_repack())
 -                              return -1;
 +                      gc_before_repack(); /* dies on failure */
                        delete_tempfile(&pidfile);
  
                        /*
                atexit(process_log_file_at_exit);
        }
  
 -      if (gc_before_repack())
 -              return -1;
 +      gc_before_repack();
  
        if (!repository_format_precious_objects) {
                close_all_packs(the_repository->objects);
                if (run_command_v_opt(repack.argv, RUN_GIT_CMD))
 -                      return error(FAILED_RUN, repack.argv[0]);
 +                      die(FAILED_RUN, repack.argv[0]);
  
                if (prune_expire) {
                        argv_array_push(&prune, prune_expire);
                                argv_array_push(&prune,
                                                "--exclude-promisor-objects");
                        if (run_command_v_opt(prune.argv, RUN_GIT_CMD))
 -                              return error(FAILED_RUN, prune.argv[0]);
 +                              die(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]);
 +                      die(FAILED_RUN, prune_worktrees.argv[0]);
        }
  
        if (run_command_v_opt(rerere.argv, RUN_GIT_CMD))
 -              return error(FAILED_RUN, rerere.argv[0]);
 +              die(FAILED_RUN, rerere.argv[0]);
  
        report_garbage = report_pack_garbage;
        reprepare_packed_git(the_repository);
-       if (pack_garbage.nr > 0)
+       if (pack_garbage.nr > 0) {
+               close_all_packs(the_repository->objects);
                clean_pack_garbage();
+       }
  
        if (gc_write_commit_graph)
 -              write_commit_graph_reachable(get_object_directory(), 0);
 +              write_commit_graph_reachable(get_object_directory(), 0,
 +                                           !quiet && !daemonized);
  
        if (auto_gc && too_many_loose_objects())
                warning(_("There are too many unreachable loose objects; "
diff --combined builtin/repack.c
index 2a1c7b21c5d9cb52c328105b470366d1c7ef7c76,2317608c6b850e3bcf13b153bce583b00af3a28c..67f8978043a43988d653092f83f21bd5baad9751
@@@ -8,14 -8,12 +8,14 @@@
  #include "strbuf.h"
  #include "string-list.h"
  #include "argv-array.h"
 +#include "midx.h"
  #include "packfile.h"
  #include "object-store.h"
  
  static int delta_base_offset = 1;
  static int pack_kept_objects = -1;
  static int write_bitmaps;
 +static int use_delta_islands;
  static char *packdir, *packtmp;
  
  static const char *const git_repack_usage[] = {
@@@ -44,10 -42,6 +44,10 @@@ static int repack_config(const char *va
                write_bitmaps = git_config_bool(var, value);
                return 0;
        }
 +      if (!strcmp(var, "repack.usedeltaislands")) {
 +              use_delta_islands = git_config_bool(var, value);
 +              return 0;
 +      }
        return git_default_config(var, value, cb);
  }
  
@@@ -197,7 -191,7 +197,7 @@@ static int write_oid(const struct objec
  
        if (cmd->in == -1) {
                if (start_command(cmd))
 -                      die("Could not start pack-objects to repack promisor objects");
 +                      die(_("could not start pack-objects to repack promisor objects"));
        }
  
        xwrite(cmd->in, oid_to_hex(oid), GIT_SHA1_HEXSZ);
@@@ -235,8 -229,8 +235,8 @@@ static void repack_promisor_objects(con
        while (strbuf_getline_lf(&line, out) != EOF) {
                char *promisor_name;
                int fd;
 -              if (line.len != 40)
 -                      die("repack: Expecting 40 character sha1 lines only from pack-objects.");
 +              if (line.len != the_hash_algo->hexsz)
 +                      die(_("repack: Expecting full hex object ID lines only from pack-objects."));
                string_list_append(names, line.buf);
  
                /*
                                          line.buf);
                fd = open(promisor_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
                if (fd < 0)
 -                      die_errno("unable to create '%s'", promisor_name);
 +                      die_errno(_("unable to create '%s'"), promisor_name);
                close(fd);
                free(promisor_name);
        }
        fclose(out);
        if (finish_command(&cmd))
 -              die("Could not finish pack-objects to repack promisor objects");
 +              die(_("could not finish pack-objects to repack promisor objects"));
  }
  
  #define ALL_INTO_ONE 1
@@@ -286,7 -280,6 +286,7 @@@ int cmd_repack(int argc, const char **a
        int keep_unreachable = 0;
        struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
        int no_update_server_info = 0;
 +      int midx_cleared = 0;
        struct pack_objects_args po_args = {NULL};
  
        struct option builtin_repack_options[] = {
                                N_("pass --local to git-pack-objects")),
                OPT_BOOL('b', "write-bitmap-index", &write_bitmaps,
                                N_("write bitmap index")),
 +              OPT_BOOL('i', "delta-islands", &use_delta_islands,
 +                              N_("pass --delta-islands to git-pack-objects")),
                OPT_STRING(0, "unpack-unreachable", &unpack_unreachable, N_("approxidate"),
                                N_("with -A, do not loosen objects older than this")),
                OPT_BOOL('k', "keep-unreachable", &keep_unreachable,
                argv_array_push(&cmd.args, "--exclude-promisor-objects");
        if (write_bitmaps)
                argv_array_push(&cmd.args, "--write-bitmap-index");
 +      if (use_delta_islands)
 +              argv_array_push(&cmd.args, "--delta-islands");
  
        if (pack_everything & ALL_INTO_ONE) {
                get_non_kept_pack_filenames(&existing_packs, &keep_pack_list);
  
        out = xfdopen(cmd.out, "r");
        while (strbuf_getline_lf(&line, out) != EOF) {
 -              if (line.len != 40)
 -                      die("repack: Expecting 40 character sha1 lines only from pack-objects.");
 +              if (line.len != the_hash_algo->hexsz)
 +                      die(_("repack: Expecting full hex object ID lines only from pack-objects."));
                string_list_append(&names, line.buf);
        }
        fclose(out);
                return ret;
  
        if (!names.nr && !po_args.quiet)
 -              printf("Nothing new to pack.\n");
 +              printf_ln(_("Nothing new to pack."));
  
+       close_all_packs(the_repository->objects);
        /*
         * Ok we have prepared all new packfiles.
         * First see if there are packs of the same name and if so
        for_each_string_list_item(item, &names) {
                for (ext = 0; ext < ARRAY_SIZE(exts); ext++) {
                        char *fname, *fname_old;
 +
 +                      if (!midx_cleared) {
 +                              clear_midx_file(the_repository);
 +                              midx_cleared = 1;
 +                      }
 +
                        fname = mkpathdup("%s/pack-%s%s", packdir,
                                                item->string, exts[ext].name);
                        if (!file_exists(fname)) {
                if (rollback_failure.nr) {
                        int i;
                        fprintf(stderr,
 -                              "WARNING: Some packs in use have been renamed by\n"
 -                              "WARNING: prefixing old- to their name, in order to\n"
 -                              "WARNING: replace them with the new version of the\n"
 -                              "WARNING: file.  But the operation failed, and the\n"
 -                              "WARNING: attempt to rename them back to their\n"
 -                              "WARNING: original names also failed.\n"
 -                              "WARNING: Please rename them in %s manually:\n", packdir);
 +                              _("WARNING: Some packs in use have been renamed by\n"
 +                                "WARNING: prefixing old- to their name, in order to\n"
 +                                "WARNING: replace them with the new version of the\n"
 +                                "WARNING: file.  But the operation failed, and the\n"
 +                                "WARNING: attempt to rename them back to their\n"
 +                                "WARNING: original names also failed.\n"
 +                                "WARNING: Please rename them in %s manually:\n"), packdir);
                        for (i = 0; i < rollback_failure.nr; i++)
                                fprintf(stderr, "WARNING:   old-%s -> %s\n",
                                        rollback_failure.items[i].string,
        reprepare_packed_git(the_repository);
  
        if (delete_redundant) {
 +              const int hexsz = the_hash_algo->hexsz;
                int opts = 0;
                string_list_sort(&names);
                for_each_string_list_item(item, &existing_packs) {
                        char *sha1;
                        size_t len = strlen(item->string);
 -                      if (len < 40)
 +                      if (len < hexsz)
                                continue;
 -                      sha1 = item->string + len - 40;
 +                      sha1 = item->string + len - hexsz;
                        if (!string_list_has_string(&names, sha1))
                                remove_redundant_pack(packdir, item->string);
                }
        if (!no_update_server_info)
                update_server_info(0);
        remove_temporary_files();
 +
 +      if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0))
 +              write_midx_file(get_object_directory());
 +
        string_list_clear(&names, 0);
        string_list_clear(&rollback, 0);
        string_list_clear(&existing_packs, 0);