Subject: [PATCH v2 0/8] repack: introduce `--write-midx`

From: Taylor Blau <[email-masked]>
Date: Wed, 15 Sep 2021 14:24:33 -0400

Here is a small reroll of my series to implement the final component of
multi-pack reachability bitamps, which is to be able to write them from `git
repack`.

Nothing substantial has changed since last time. Mostly re-wrapping lines and
removing braces in macro'd for_each_xyz() loops. A larger (but still small)
change is how we read input to `--refs-snapshot`. This version contains a more
idiomatic implementation, but see the sub-thread beginning at [1] for a more
complete discussion.

Otherwise, this series is unchagned. It depends on tb/multi-pack-bitmaps, which
has graduated to master. Range-diff below:

[1]: https://lore.kernel.org/git/[email-masked]/

Taylor Blau (8):
  midx: expose `write_midx_file_only()` publicly
  builtin/multi-pack-index.c: support `--stdin-packs` mode
  midx: preliminary support for `--refs-snapshot`
  builtin/repack.c: keep track of existing packs unconditionally
  builtin/repack.c: extract showing progress to a variable
  builtin/repack.c: support writing a MIDX while repacking
  builtin/repack.c: make largest pack preferred
  builtin/repack.c: pass `--refs-snapshot` when writing bitmaps

 Documentation/git-multi-pack-index.txt |  19 ++
 Documentation/git-repack.txt           |  18 +-
 builtin/multi-pack-index.c             |  36 +++-
 builtin/repack.c                       | 255 ++++++++++++++++++++++---
 midx.c                                 |  96 ++++++++--
 midx.h                                 |  11 +-
 pack-bitmap.c                          |   2 +-
 pack-bitmap.h                          |   1 +
 t/helper/test-read-midx.c              |  25 ++-
 t/lib-midx.sh                          |   8 +
 t/t5319-multi-pack-index.sh            |  15 ++
 t/t5326-multi-pack-bitmaps.sh          |  82 ++++++++
 t/t7700-repack.sh                      |  96 ++++++++++
 t/t7703-repack-geometric.sh            |  22 +++
 14 files changed, 637 insertions(+), 49 deletions(-)
 create mode 100644 t/lib-midx.sh

Range-diff against v1:
1:  6d50f46080 ! 1:  03c1b2c4d3 midx: expose 'write_midx_file_only()' publicly
    @@ Metadata
     Author: Taylor Blau <[email-masked]>
     
      ## Commit message ##
    -    midx: expose 'write_midx_file_only()' publicly
    +    midx: expose `write_midx_file_only()` publicly
     
         Expose a variant of the write_midx_file() function which ignores packs
         that aren't included in an explicit "allow" list.
    @@ midx.c: struct write_midx_context {
      
      static void add_pack_to_midx(const char *full_path, size_t full_path_len,
     @@ midx.c: static void add_pack_to_midx(const char *full_path, size_t full_path_len,
    - 
    - 	if (ends_with(file_name, ".idx")) {
      		display_progress(ctx->progress, ++ctx->pack_paths_checked);
    --		if (ctx->m && midx_contains_pack(ctx->m, file_name))
    --			return;
    -+		if (ctx->m) {
    -+			if (midx_contains_pack(ctx->m, file_name))
    -+				return;
    -+		} else if (ctx->to_include) {
    -+			if (!string_list_has_string(ctx->to_include, file_name))
    -+				return;
    -+		}
    + 		if (ctx->m && midx_contains_pack(ctx->m, file_name))
    + 			return;
    ++		else if (ctx->to_include &&
    ++			 !string_list_has_string(ctx->to_include, file_name))
    ++			return;
      
      		ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc);
      
    @@ midx.h: int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pa
      int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);
      
      int write_midx_file(const char *object_dir, const char *preferred_pack_name, unsigned flags);
    -+int write_midx_file_only(const char *object_dir, struct string_list *packs_to_include, const char *preferred_pack_name, unsigned flags);
    ++int write_midx_file_only(const char *object_dir,
    ++			 struct string_list *packs_to_include,
    ++			 const char *preferred_pack_name,
    ++			 unsigned flags);
      void clear_midx_file(struct repository *r);
      int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);
      int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags);
2:  66aa7f4b48 ! 2:  59556e5545 builtin/multi-pack-index.c: support --stdin-packs mode
    @@ Metadata
     Author: Taylor Blau <[email-masked]>
     
      ## Commit message ##
    -    builtin/multi-pack-index.c: support --stdin-packs mode
    +    builtin/multi-pack-index.c: support `--stdin-packs` mode
     
         To power a new `--write-midx` mode, `git repack` will want to write a
         multi-pack index containing a certain set of packs in the repository.
    @@ builtin/multi-pack-index.c: static struct option *add_common_options(struct opti
     +static void read_packs_from_stdin(struct string_list *to)
     +{
     +	struct strbuf buf = STRBUF_INIT;
    -+	while (strbuf_getline(&buf, stdin) != EOF) {
    -+		string_list_append(to, strbuf_detach(&buf, NULL));
    -+	}
    ++	while (strbuf_getline(&buf, stdin) != EOF)
    ++		string_list_append(to, buf.buf);
     +	string_list_sort(to);
    ++
    ++	strbuf_release(&buf);
     +}
     +
      static int cmd_multi_pack_index_write(int argc, const char **argv)
    @@ builtin/multi-pack-index.c: static int cmd_multi_pack_index_write(int argc, cons
      	FREE_AND_NULL(options);
      
     +	if (opts.stdin_packs) {
    -+		struct string_list packs = STRING_LIST_INIT_NODUP;
    ++		struct string_list packs = STRING_LIST_INIT_DUP;
     +		int ret;
     +
     +		read_packs_from_stdin(&packs);
3:  f74a967be3 ! 3:  42f1ae9ede midx: preliminary support for `--refs-snapshot`
    @@ midx.h: int fill_midx_entry(struct repository *r, const struct object_id *oid, s
      int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);
      
     -int write_midx_file(const char *object_dir, const char *preferred_pack_name, unsigned flags);
    --int write_midx_file_only(const char *object_dir, struct string_list *packs_to_include, const char *preferred_pack_name, unsigned flags);
    -+int write_midx_file(const char *object_dir, const char *preferred_pack_name, const char *refs_snapshot, unsigned flags);
    -+int write_midx_file_only(const char *object_dir, struct string_list *packs_to_include, const char *preferred_pack_name, const char *refs_snapshot, unsigned flags);
    ++int write_midx_file(const char *object_dir,
    ++		    const char *preferred_pack_name,
    ++		    const char *refs_snapshot,
    ++		    unsigned flags);
    + int write_midx_file_only(const char *object_dir,
    + 			 struct string_list *packs_to_include,
    + 			 const char *preferred_pack_name,
    ++			 const char *refs_snapshot,
    + 			 unsigned flags);
      void clear_midx_file(struct repository *r);
      int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);
    - int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags);
     
      ## t/t5326-multi-pack-bitmaps.sh ##
     @@ t/t5326-multi-pack-bitmaps.sh: test_expect_success 'pack.preferBitmapTips' '
4:  5067c2cc65 = 4:  c0d045a9de builtin/repack.c: keep track of existing packs unconditionally
5:  d5990e3b69 = 5:  09de03de47 builtin/repack.c: extract showing progress to a variable
6:  d32d800954 ! 6:  83dfdb8b12 builtin/repack.c: support writing a MIDX while repacking
    @@ builtin/repack.c: static void clear_pack_geometry(struct pack_geometry *geometry
     +{
     +	struct string_list_item *item;
     +
    -+	for_each_string_list_item(item, existing_kept_packs) {
    ++	for_each_string_list_item(item, existing_kept_packs)
     +		string_list_insert(include, xstrfmt("%s.idx", item->string));
    -+	}
    -+	for_each_string_list_item(item, names) {
    ++	for_each_string_list_item(item, names)
     +		string_list_insert(include, xstrfmt("pack-%s.idx", item->string));
    -+	}
     +	if (geometry) {
     +		struct strbuf buf = STRBUF_INIT;
     +		uint32_t i;
    @@ builtin/repack.c: static void clear_pack_geometry(struct pack_geometry *geometry
     +		return ret;
     +
     +	in = xfdopen(cmd.in, "w");
    -+	for_each_string_list_item(item, include) {
    ++	for_each_string_list_item(item, include)
     +		fprintf(in, "%s\n", item->string);
    -+	}
     +	fclose(in);
     +
     +	return finish_command(&cmd);
7:  515e1d95a6 ! 7:  68bc49d8ae builtin/repack.c: make largest pack preferred
    @@ builtin/repack.c: static void split_pack_geometry(struct pack_geometry *geometry
      
     +static struct packed_git *get_largest_active_pack(struct pack_geometry *geometry)
     +{
    ++	if (!geometry) {
    ++		/*
    ++		 * No geometry means either an all-into-one repack (in which
    ++		 * case there is only one pack left and it is the largest) or an
    ++		 * incremental one.
    ++		 *
    ++		 * If repacking incrementally, then we could check the size of
    ++		 * all packs to determine which should be preferred, but leave
    ++		 * this for later.
    ++		 */
    ++		return NULL;
    ++	}
     +	if (geometry->split == geometry->pack_nr)
     +		return NULL;
     +	return geometry->pack[geometry->pack_nr - 1];
    @@ builtin/repack.c: static void midx_included_packs(struct string_list *include,
      				     int show_progress, int write_bitmaps)
      {
      	struct child_process cmd = CHILD_PROCESS_INIT;
    + 	struct string_list_item *item;
    ++	struct packed_git *largest = get_largest_active_pack(geometry);
    + 	FILE *in;
    + 	int ret;
    + 
     @@ builtin/repack.c: static int write_midx_included_packs(struct string_list *include,
      	if (write_bitmaps)
      		strvec_push(&cmd.args, "--bitmap");
      
    -+	if (geometry) {
    -+		struct packed_git *largest = get_largest_active_pack(geometry);
    -+		if (largest)
    -+			strvec_pushf(&cmd.args, "--preferred-pack=%s",
    -+				     pack_basename(largest));
    -+		else
    -+			/*
    -+			 * The largest pack was repacked, meaning only one pack
    -+			 * exists (and tautologically, it is the largest).
    -+			 */
    -+			;
    -+	}
    ++	if (largest)
    ++		strvec_pushf(&cmd.args, "--preferred-pack=%s",
    ++			     pack_basename(largest));
     +
      	ret = start_command(&cmd);
      	if (ret)
    @@ t/t7703-repack-geometric.sh: test_expect_success '--geometric ignores kept packs
     +	test_when_finished "rm -fr geometric" &&
     +	(
     +		cd geometric &&
    -+		git config core.multiPackIndex true &&
     +
     +		# These packs already form a geometric progression.
     +		test_commit_bulk --start=1 1 && # 3 objects
8:  74a9da0ef0 ! 8:  eb24b308ec builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
    @@ builtin/repack.c: static void clear_pack_geometry(struct pack_geometry *geometry
     +	return 0;
     +}
     +
    -+static int midx_snapshot_refs(struct tempfile *f)
    ++static void midx_snapshot_refs(struct tempfile *f)
     +{
     +	struct midx_snapshot_ref_data data;
     +	const struct string_list *preferred = bitmap_preferred_tips(the_repository);
    @@ builtin/repack.c: static void clear_pack_geometry(struct pack_geometry *geometry
     +		struct string_list_item *item;
     +
     +		data.preferred = 1;
    -+		for_each_string_list_item(item, preferred) {
    ++		for_each_string_list_item(item, preferred)
     +			for_each_ref_in(item->string, midx_snapshot_ref_one, &data);
    -+		}
     +		data.preferred = 0;
     +	}
     +
     +	for_each_ref(midx_snapshot_ref_one, &data);
     +
    -+	return close_tempfile_gently(f);
    ++	if (close_tempfile_gently(f)) {
    ++		int save_errno = errno;
    ++		delete_tempfile(&f);
    ++		errno = save_errno;
    ++		die_errno(_("could not close refs snapshot tempfile"));
    ++	}
     +}
     +
      static void midx_included_packs(struct string_list *include,
    @@ builtin/repack.c: static void midx_included_packs(struct string_list *include,
      {
      	struct child_process cmd = CHILD_PROCESS_INIT;
     @@ builtin/repack.c: static int write_midx_included_packs(struct string_list *include,
    - 			;
    - 	}
    + 		strvec_pushf(&cmd.args, "--preferred-pack=%s",
    + 			     pack_basename(largest));
      
     +	if (refs_snapshot)
     +		strvec_pushf(&cmd.args, "--refs-snapshot=%s", refs_snapshot);
    @@ builtin/repack.c: int cmd_repack(int argc, const char **argv, const char *prefix
     +			    "bitmap-ref-tips");
     +
     +		refs_snapshot = xmks_tempfile(path.buf);
    -+		if (midx_snapshot_refs(refs_snapshot) < 0)
    -+			die(_("could not take a snapshot of references"));
    ++		midx_snapshot_refs(refs_snapshot);
     +
     +		strbuf_release(&path);
     +	}
-- 
2.33.0.96.g73915697e6