Merge branch 'ds/midx-write-fixes'

Fixes multiple crashes around midx write-out codepaths.

* ds/midx-write-fixes:
  midx-write: simplify error cases
  midx-write: reenable signed comparison errors
  midx-write: use uint32_t for preferred_pack_idx
  midx-write: use cleanup when incremental midx fails
  midx-write: put failing response value back
  midx-write: only load initialized packs

diff --cc midx-write.c
index 1e99f6a26e9e9103ceac2b9ff5aa04ab8428a236,b7824d21fc1854d220538acb34de47f052250941..c73010df6d3a4fe5c241597500f35bf5a71188dd
--- 1/midx-write.c
--- 2/midx-write.c
+++ b/midx-write.c
@@@ -24,11 -22,12 +22,12 @@@
  #define BITMAP_POS_UNKNOWN (~((uint32_t)0))
  #define MIDX_CHUNK_FANOUT_SIZE (sizeof(uint32_t) * 256)
  #define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t))
+ #define NO_PREFERRED_PACK (~((uint32_t)0))
  
  extern int midx_checksum_valid(struct multi_pack_index *m);
 -extern void clear_midx_files_ext(const char *object_dir, const char *ext,
 +extern void clear_midx_files_ext(struct odb_source *source, const char *ext,
                                 const char *keep_hash);
 -extern void clear_incremental_midx_files_ext(const char *object_dir,
 +extern void clear_incremental_midx_files_ext(struct odb_source *source,
                                             const char *ext,
                                             const char **keep_hashes,
                                             uint32_t hashes_nr);
@@@ -910,8 -913,32 +910,7 @@@ cleanup
        return ret;
  }
  
- static int fill_packs_from_midx(struct write_midx_context *ctx,
-                               const char *preferred_pack_name, uint32_t flags)
 -static struct multi_pack_index *lookup_multi_pack_index(struct repository *r,
 -                                                      const char *object_dir)
 -{
 -      struct multi_pack_index *result = NULL;
 -      struct multi_pack_index *cur;
 -      char *obj_dir_real = real_pathdup(object_dir, 1);
 -      struct strbuf cur_path_real = STRBUF_INIT;
 -
 -      /* Ensure the given object_dir is local, or a known alternate. */
 -      find_odb(r, obj_dir_real);
 -
 -      for (cur = get_multi_pack_index(r); cur; cur = cur->next) {
 -              strbuf_realpath(&cur_path_real, cur->object_dir, 1);
 -              if (!strcmp(obj_dir_real, cur_path_real.buf)) {
 -                      result = cur;
 -                      goto cleanup;
 -              }
 -      }
 -
 -cleanup:
 -      free(obj_dir_real);
 -      strbuf_release(&cur_path_real);
 -      return result;
 -}
 -
+ static int fill_packs_from_midx(struct write_midx_context *ctx)
  {
        struct multi_pack_index *m;
  
@@@ -919,29 -946,11 +918,10 @@@
                uint32_t i;
  
                for (i = 0; i < m->num_packs; i++) {
-                       ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc);
- 
-                       /*
-                        * If generating a reverse index, need to have
-                        * packed_git's loaded to compare their
-                        * mtimes and object count.
-                        *
-                        * If a preferred pack is specified, need to
-                        * have packed_git's loaded to ensure the chosen
-                        * preferred pack has a non-zero object count.
-                        */
-                       if (flags & MIDX_WRITE_REV_INDEX ||
-                           preferred_pack_name) {
-                               if (prepare_midx_pack(m, m->num_packs_in_base + i)) {
-                                       error(_("could not load pack"));
-                                       return 1;
-                               }
- 
-                               if (open_pack_index(m->packs[i]))
-                                       die(_("could not open index for %s"),
-                                           m->packs[i]->pack_name);
-                       }
 -                      if (prepare_midx_pack(ctx->repo, m,
 -                                            m->num_packs_in_base + i))
++                      if (prepare_midx_pack(m, m->num_packs_in_base + i))
+                               return error(_("could not load pack"));
  
+                       ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc);
                        fill_pack_info(&ctx->info[ctx->nr++], m->packs[i],
                                       m->pack_names[i],
                                       m->num_packs_in_base + i);
@@@ -1042,10 -1052,9 +1022,10 @@@ static int write_midx_internal(struct o
                               const char *refs_snapshot,
                               unsigned flags)
  {
 +      struct repository *r = source->odb->repo;
        struct strbuf midx_name = STRBUF_INIT;
        unsigned char midx_hash[GIT_MAX_RAWSZ];
-       uint32_t i, start_pack;
+       uint32_t start_pack;
        struct hashfile *f = NULL;
        struct lock_file lk;
        struct tempfile *incr;
@@@ -1106,8 -1116,7 +1088,7 @@@
                        if (flags & MIDX_WRITE_BITMAP && load_midx_revindex(m)) {
                                error(_("could not load reverse index for MIDX %s"),
                                      hash_to_hex_algop(get_midx_checksum(m),
 -                                                      m->repo->hash_algo));
 +                                                      m->source->odb->repo->hash_algo));
-                               result = 1;
                                goto cleanup;
                        }
                        ctx.num_multi_pack_indexes_before++;
@@@ -1149,7 -1157,9 +1129,8 @@@
                         * corresponding bitmap (or one wasn't requested).
                         */
                        if (!want_bitmap)
 -                              clear_midx_files_ext(object_dir, "bitmap", NULL);
 -
 +                              clear_midx_files_ext(source, "bitmap", NULL);
+                       result = 0;
                        goto cleanup;
                }
        }
@@@ -1434,10 -1457,10 +1428,10 @@@
                }
  
                if (link_midx_to_chain(ctx.base_midx) < 0)
-                       return -1;
+                       goto cleanup;
  
 -              get_split_midx_filename_ext(r->hash_algo, &final_midx_name,
 -                                          object_dir, midx_hash, MIDX_EXT_MIDX);
 +              get_split_midx_filename_ext(source, &final_midx_name,
 +                                          midx_hash, MIDX_EXT_MIDX);
  
                if (rename_tempfile(&incr, final_midx_name.buf) < 0) {
                        error_errno(_("unable to rename new multi-pack-index layer"));
@@@ -1470,12 -1493,13 +1464,13 @@@
        if (commit_lock_file(&lk) < 0)
                die_errno(_("could not write multi-pack-index"));
  
 -      clear_midx_files(r, object_dir, keep_hashes,
 +      clear_midx_files(source, keep_hashes,
                         ctx.num_multi_pack_indexes_before + 1,
                         ctx.incremental);
+       result = 0;
  
  cleanup:
-       for (i = 0; i < ctx.nr; i++) {
+       for (size_t i = 0; i < ctx.nr; i++) {
                if (ctx.info[i].p) {
                        close_pack(ctx.info[i].p);
                        free(ctx.info[i].p);

diff --cc t/t5319-multi-pack-index.sh
Simple merge