Merge branch 'tb/incremental-midx-part-3.2' into seen

Further work on incremental repacking using MIDX/bitmap

* tb/incremental-midx-part-3.2:
  midx: enable reachability bitmaps during MIDX compaction
  midx: implement MIDX compaction
  t/helper/test-read-midx.c: plug memory leak when selecting layer
  midx-write.c: factor fanout layering from `compute_sorted_entries()`
  midx-write.c: enumerate `pack_int_id` values directly
  midx-write.c: extract `fill_pack_from_midx()`
  midx-write.c: introduce `midx_pack_perm()` helper
  git-compat-util.h: introduce `u32_add()`
  midx: do not require packs to be sorted in lexicographic order
  midx-write.c: introduce `struct write_midx_opts`
  midx-write.c: don't use `pack_perm` when assigning `bitmap_pos`
  t/t5319-multi-pack-index.sh: fix copy-and-paste error in t5319.39
  git-multi-pack-index(1): align SYNOPSIS with 'git multi-pack-index -h'
  git-multi-pack-index(1): remove non-existent incompatibility
  builtin/multi-pack-index.c: make '--progress' a common option
  midx: split `get_midx_checksum()` by adding `get_midx_hash()`
  midx: mark `get_midx_checksum()` arguments as const

diff --cc git-compat-util.h
Simple merge

diff --cc midx-write.c
index ce459b02c319cfd557075671e2caeeb553c8dc42,f2dbacef4cdcae2701a72de9dfad67b32def28d8..66c125ccb0a189b286a7cbf7ae6e2b309044832e
--- 1/midx-write.c
--- 2/midx-write.c
+++ b/midx-write.c
@@@ -1014,73 -1131,30 +1131,89 @@@ static void clear_midx_files(struct odb
        strbuf_release(&buf);
  }
  
-       if (ctx->incremental)
 +static bool midx_needs_update(struct multi_pack_index *midx, struct write_midx_context *ctx)
 +{
 +      struct strset packs = STRSET_INIT;
 +      struct strbuf buf = STRBUF_INIT;
 +      bool needed = true;
 +
 +      /*
 +       * Ignore incremental updates for now. The assumption is that any
 +       * incremental update would be either empty (in which case we will bail
 +       * out later) or it would actually cover at least one new pack.
 +       */
- static int write_midx_internal(struct odb_source *source,
-                              struct string_list *packs_to_include,
-                              struct string_list *packs_to_drop,
-                              const char *preferred_pack_name,
-                              const char *refs_snapshot,
-                              unsigned flags)
++      if (ctx->incremental || ctx->compact)
 +              goto out;
 +
 +      /*
 +       * Otherwise, we need to verify that the packs covered by the existing
 +       * MIDX match the packs that we already have. The logic to do so is way
 +       * more complicated than it has any right to be. This is because:
 +       *
 +       *   - We cannot assume any ordering.
 +       *
 +       *   - The MIDX packs may not be loaded at all, and loading them would
 +       *     be wasteful. So we need to use the pack names tracked by the
 +       *     MIDX itself.
 +       *
 +       *   - The MIDX pack names are tracking the ".idx" files, whereas the
 +       *     packs themselves are tracking the ".pack" files. So we need to
 +       *     strip suffixes.
 +       */
 +      if (ctx->nr != midx->num_packs + midx->num_packs_in_base)
 +              goto out;
 +
 +      for (uint32_t i = 0; i < ctx->nr; i++) {
 +              strbuf_reset(&buf);
 +              strbuf_addstr(&buf, pack_basename(ctx->info[i].p));
 +              strbuf_strip_suffix(&buf, ".pack");
 +
 +              if (!strset_add(&packs, buf.buf))
 +                      BUG("same pack added twice?");
 +      }
 +
 +      for (uint32_t i = 0; i < ctx->nr; i++) {
 +              strbuf_reset(&buf);
 +              strbuf_addstr(&buf, midx->pack_names[i]);
 +              strbuf_strip_suffix(&buf, ".idx");
 +
 +              if (!strset_contains(&packs, buf.buf))
 +                      goto out;
 +              strset_remove(&packs, buf.buf);
 +      }
 +
 +      needed = false;
 +
 +out:
 +      strbuf_release(&buf);
 +      strset_clear(&packs);
 +      return needed;
 +}
 +
+ static int midx_hashcmp(const struct multi_pack_index *a,
+                       const struct multi_pack_index *b,
+                       const struct git_hash_algo *algop)
  {
-       struct repository *r = source->odb->repo;
+       return hashcmp(get_midx_hash(a), get_midx_hash(b), algop);
+ }
+ 
+ struct write_midx_opts {
+       struct odb_source *source;
+ 
+       struct string_list *packs_to_include;
+       struct string_list *packs_to_drop;
+ 
+       struct multi_pack_index *compact_from;
+       struct multi_pack_index *compact_to;
+ 
+       const char *preferred_pack_name;
+       const char *refs_snapshot;
+       unsigned flags;
+ };
+ 
+ static int write_midx_internal(struct write_midx_opts *opts)
+ {
+       struct repository *r = opts->source->odb->repo;
        struct strbuf midx_name = STRBUF_INIT;
        unsigned char midx_hash[GIT_MAX_RAWSZ];
        uint32_t start_pack;
@@@ -1166,44 -1257,43 +1317,54 @@@
        else
                ctx.progress = NULL;
  
-       ctx.to_include = packs_to_include;
+       if (ctx.compact) {
+               int bitmap_order = 0;
+               if (opts->preferred_pack_name)
+                       bitmap_order |= 1;
+               else if (opts->flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP))
+                       bitmap_order |= 1;
  
-       for_each_file_in_pack_dir(source->path, add_pack_to_midx, &ctx);
+               fill_packs_from_midx_range(&ctx, bitmap_order);
+       } else {
+               ctx.to_include = opts->packs_to_include;
+               for_each_file_in_pack_dir(opts->source->path, add_pack_to_midx, &ctx);
+       }
        stop_progress(&ctx.progress);
  
-       if (!packs_to_drop) {
 -      if ((ctx.m && ctx.nr == ctx.m->num_packs + ctx.m->num_packs_in_base) &&
 -          !ctx.incremental &&
 -          !ctx.compact &&
 -          !(opts->packs_to_include || opts->packs_to_drop)) {
 -              struct bitmap_index *bitmap_git;
 -              int bitmap_exists;
 -              int want_bitmap = opts->flags & MIDX_WRITE_BITMAP;
 -
 -              bitmap_git = prepare_midx_bitmap_git(ctx.m);
 -              bitmap_exists = bitmap_git && bitmap_is_midx(bitmap_git);
 -              free_bitmap_index(bitmap_git);
 -
 -              if (bitmap_exists || !want_bitmap) {
 -                      /*
 -                       * The correct MIDX already exists, and so does a
 -                       * corresponding bitmap (or one wasn't requested).
 -                       */
 -                      if (!want_bitmap)
 -                              clear_midx_files_ext(opts->source, "bitmap",
 -                                                   NULL);
 -                      result = 0;
 -                      goto cleanup;
++      if (!opts->packs_to_drop) {
 +              /*
 +               * If there is no MIDX then either it doesn't exist, or we're
 +               * doing a geometric repack. Try to load it from the source to
 +               * tell these two cases apart.
 +               */
 +              struct multi_pack_index *midx = ctx.m;
 +              if (!midx)
 +                      midx = midx_to_free = load_multi_pack_index(ctx.source);
 +
 +              if (midx && !midx_needs_update(midx, &ctx)) {
 +                      struct bitmap_index *bitmap_git;
 +                      int bitmap_exists;
-                       int want_bitmap = flags & MIDX_WRITE_BITMAP;
++                      int want_bitmap = opts->flags & MIDX_WRITE_BITMAP;
 +
 +                      bitmap_git = prepare_midx_bitmap_git(midx);
 +                      bitmap_exists = bitmap_git && bitmap_is_midx(bitmap_git);
 +                      free_bitmap_index(bitmap_git);
 +
 +                      if (bitmap_exists || !want_bitmap) {
 +                              /*
 +                               * The correct MIDX already exists, and so does a
 +                               * corresponding bitmap (or one wasn't requested).
 +                               */
 +                              if (!want_bitmap)
-                                       clear_midx_files_ext(source, "bitmap", NULL);
++                                      clear_midx_files_ext(opts->source,
++                                                           "bitmap", NULL);
 +                              result = 0;
 +                              goto cleanup;
 +                      }
                }
 +
 +              close_midx(midx_to_free);
 +              midx_to_free = NULL;
        }
  
        if (ctx.incremental && !ctx.nr) {

diff --cc midx.c
Simple merge

diff --cc pack-revindex.c
Simple merge

diff --cc t/meson.build
Simple merge

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