From: Jeff King <peff@peff.net>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Elijah Newren <newren@gmail.com>, Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH 06/16] midx: support custom `--base` for incremental MIDX writes
Date: Tue, 7 Apr 2026 01:57:53 -0400 [thread overview]
Message-ID: <20260407055753.GA714878@coredump.intra.peff.net> (raw)
In-Reply-To: <6b42d284b79a25e97d506e22c709e4dbe6fddd58.1774820449.git.me@ttaylorr.com>
On Sun, Mar 29, 2026 at 05:41:06PM -0400, Taylor Blau wrote:
> Introduce a new `--base` option for both subcommands to specify the
> checksum of the MIDX layer to use as the base. The given checksum must
> refer to a valid layer in the MIDX chain that is an ancestor of the
> topmost layer being written or compacted.
OK, that makes sense.
> The special value "none" is accepted to produce a root layer with no
> parent. This will be needed when the incremental repacking machinery
> determines that the bottommost layers of the chain should be replaced.
My gut feeling on reading is that a chain based on "none" is really just
a non-incremental midx, so we could just write that. But:
1. I don't recall off-hand how expensive it is to convert a
non-incremental to an incremental one. I.e., can we just rename the
file and write out a chain link and it becomes incremental?
2. Even if it is cheap to convert, I can believe it makes geometric
repacking easier to think about if we just always think about it as
an incremental midx.
And perhaps (3), I am just wrong that they are the same thing. ;) I
haven't thought it through all that carefully.
> If no `--base` is given, behavior is unchanged: `compact` uses "from's"
> immediate parent in the chain, and `write` appends to the existing tip.
>
> For the `write` subcommand, `--base` requires `--checksum-only`. A plain
> `write --incremental` appends a new layer to the live chain tip with no
> mechanism to atomically replace it; overriding the base would produce a
> layer that does not extend the tip, breaking chain invariants. With
> `--checksum-only` the chain is left unmodified and the caller is
> responsible for assembling a valid chain.
OK, makes sense. If we are using --base we are doing something clever
and weird, and the chain tip should be left untouched. I guess "midx
write" would never delete an incremental (it is always just adding one
more to the chain), so we don't have to worry about that. Presumably
we'll eventually write out the new chain tip from the caller, which will
then have to remember to delete the now-orphaned chain elements. But
that's kind of the point of the new geometric approach, so it seems like
things are falling into place.
> For `compact`, no such restriction applies. The compaction operation
> atomically replaces the compacted range in the chain file, so writing
> the result on top of any valid ancestor preserves chain invariants.
OK, so there's an implicit base..tip compaction here (as opposed to
without --base, an implicit root..trip compaction?). Makes sense.
> @@ -83,6 +85,13 @@ marker).
> and packs not present in an existing MIDX layer.
> Migrates non-incremental MIDXs to incremental ones when
> necessary.
> +
> + --base=<checksum>::
> + Specify the checksum of an existing MIDX layer to use
> + as the base when writing a new incremental layer.
> + The special value `none` indicates that the new layer
> + should have no base (i.e., it becomes a root layer).
> + Requires `--checksum-only`.
Not really a critique of this patch, but I'd guess that a lot of these
options are getting esoteric enough that it is hard to understand
> [...]
The patch looked as advertised, and I didn't find anything in it to
comment on.
-Peff
next prev parent reply other threads:[~2026-04-07 5:57 UTC|newest]
Thread overview: 92+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-29 21:40 [PATCH 00/16] repack: incremental MIDX/bitmap-based repacking Taylor Blau
2026-03-29 21:40 ` [PATCH 01/16] midx-write: handle noop writes when converting incremental chains Taylor Blau
2026-03-30 22:33 ` Jeff King
2026-03-31 21:43 ` Taylor Blau
2026-03-29 21:40 ` [PATCH 02/16] midx: use `string_list` for retained MIDX files Taylor Blau
2026-03-30 22:38 ` Jeff King
2026-03-31 21:49 ` Taylor Blau
2026-03-29 21:40 ` [PATCH 03/16] strvec: introduce `strvec_init_alloc()` Taylor Blau
2026-03-30 22:46 ` Jeff King
2026-03-29 21:41 ` [PATCH 04/16] midx: use `strvec` for `keep_hashes` Taylor Blau
2026-03-30 23:01 ` Jeff King
2026-03-31 22:26 ` Taylor Blau
2026-03-31 22:50 ` Taylor Blau
2026-03-31 23:17 ` Jeff King
2026-04-01 15:41 ` Taylor Blau
2026-04-01 19:25 ` Jeff King
2026-03-29 21:41 ` [PATCH 05/16] midx: introduce `--checksum-only` for incremental MIDX writes Taylor Blau
2026-03-30 23:15 ` Jeff King
2026-04-02 22:51 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 06/16] midx: support custom `--base` " Taylor Blau
2026-04-07 5:57 ` Jeff King [this message]
2026-04-14 22:09 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 07/16] repack: track the ODB source via existing_packs Taylor Blau
2026-04-07 6:04 ` Jeff King
2026-04-14 22:24 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 08/16] midx: expose `midx_layer_contains_pack()` Taylor Blau
2026-04-07 6:05 ` Jeff King
2026-03-29 21:41 ` [PATCH 09/16] repack-midx: factor out `repack_prepare_midx_command()` Taylor Blau
2026-03-29 21:41 ` [PATCH 10/16] repack-midx: extract `repack_fill_midx_stdin_packs()` Taylor Blau
2026-04-07 6:08 ` Jeff King
2026-03-29 21:41 ` [PATCH 11/16] repack-geometry: prepare for incremental MIDX repacking Taylor Blau
2026-04-07 6:10 ` Jeff King
2026-04-16 22:51 ` Elijah Newren
2026-04-21 19:34 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 12/16] builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK` Taylor Blau
2026-04-07 6:18 ` Jeff King
2026-03-29 21:41 ` [PATCH 13/16] packfile: ensure `close_pack_revindex()` frees in-memory revindex Taylor Blau
2026-04-07 6:29 ` Jeff King
2026-03-29 21:41 ` [PATCH 14/16] repack: implement incremental MIDX repacking Taylor Blau
2026-04-16 22:53 ` Elijah Newren
2026-04-21 19:40 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 15/16] repack: introduce `--write-midx=incremental` Taylor Blau
2026-04-16 22:53 ` Elijah Newren
2026-04-21 19:52 ` Taylor Blau
2026-03-29 21:41 ` [PATCH 16/16] repack: allow `--write-midx=incremental` without `--geometric` Taylor Blau
2026-04-14 22:38 ` [PATCH 00/16] repack: incremental MIDX/bitmap-based repacking Taylor Blau
2026-04-21 20:37 ` [PATCH v2 " Taylor Blau
2026-04-21 20:37 ` [PATCH v2 01/16] midx-write: handle noop writes when converting incremental chains Taylor Blau
2026-04-21 20:37 ` [PATCH v2 02/16] midx: use `strset` for retained MIDX files Taylor Blau
2026-04-21 20:37 ` [PATCH v2 03/16] midx: build `keep_hashes` array in order Taylor Blau
2026-04-21 20:37 ` [PATCH v2 04/16] midx: use `strvec` for `keep_hashes` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 05/16] midx: introduce `--no-write-chain-file` for incremental MIDX writes Taylor Blau
2026-04-21 20:37 ` [PATCH v2 06/16] midx: support custom `--base` " Taylor Blau
2026-04-21 20:37 ` [PATCH v2 07/16] repack: track the ODB source via existing_packs Taylor Blau
2026-04-21 20:37 ` [PATCH v2 08/16] midx: expose `midx_layer_contains_pack()` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 09/16] repack-midx: factor out `repack_prepare_midx_command()` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 10/16] repack-midx: extract `repack_fill_midx_stdin_packs()` Taylor Blau
2026-04-29 8:08 ` Jeff King
2026-04-29 22:40 ` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 11/16] repack-geometry: prepare for incremental MIDX repacking Taylor Blau
2026-04-21 20:37 ` [PATCH v2 12/16] builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 13/16] packfile: ensure `close_pack_revindex()` frees in-memory revindex Taylor Blau
2026-04-21 20:37 ` [PATCH v2 14/16] repack: implement incremental MIDX repacking Taylor Blau
2026-04-29 7:51 ` Jeff King
2026-04-29 23:36 ` Taylor Blau
2026-04-29 8:10 ` Jeff King
2026-04-29 23:39 ` Taylor Blau
2026-04-21 20:37 ` [PATCH v2 15/16] repack: introduce `--write-midx=incremental` Taylor Blau
2026-04-21 21:02 ` Taylor Blau
2026-04-21 20:38 ` [PATCH v2 16/16] repack: allow `--write-midx=incremental` without `--geometric` Taylor Blau
2026-04-22 14:45 ` [PATCH v2 00/16] repack: incremental MIDX/bitmap-based repacking Elijah Newren
2026-04-29 8:10 ` Jeff King
2026-04-30 0:13 ` [PATCH v3 " Taylor Blau
2026-04-30 0:13 ` [PATCH v3 01/16] midx-write: handle noop writes when converting incremental chains Taylor Blau
2026-04-30 0:13 ` [PATCH v3 02/16] midx: use `strset` for retained MIDX files Taylor Blau
2026-04-30 0:13 ` [PATCH v3 03/16] midx: build `keep_hashes` array in order Taylor Blau
2026-04-30 0:13 ` [PATCH v3 04/16] midx: use `strvec` for `keep_hashes` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 05/16] midx: introduce `--no-write-chain-file` for incremental MIDX writes Taylor Blau
2026-04-30 0:13 ` [PATCH v3 06/16] midx: support custom `--base` " Taylor Blau
2026-04-30 0:13 ` [PATCH v3 07/16] repack: track the ODB source via existing_packs Taylor Blau
2026-04-30 0:13 ` [PATCH v3 08/16] midx: expose `midx_layer_contains_pack()` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 09/16] repack-midx: factor out `repack_prepare_midx_command()` Taylor Blau
2026-05-13 21:45 ` SZEDER Gábor
2026-04-30 0:13 ` [PATCH v3 10/16] repack-midx: extract `repack_fill_midx_stdin_packs()` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 11/16] repack-geometry: prepare for incremental MIDX repacking Taylor Blau
2026-04-30 0:13 ` [PATCH v3 12/16] builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK` Taylor Blau
2026-04-30 0:13 ` [PATCH v3 13/16] packfile: ensure `close_pack_revindex()` frees in-memory revindex Taylor Blau
2026-04-30 0:13 ` [PATCH v3 14/16] repack: implement incremental MIDX repacking Taylor Blau
2026-04-30 0:13 ` [PATCH v3 15/16] repack: introduce `--write-midx=incremental` Taylor Blau
2026-05-13 23:08 ` Jeff King
2026-04-30 0:13 ` [PATCH v3 16/16] repack: allow `--write-midx=incremental` without `--geometric` Taylor Blau
2026-05-01 6:46 ` [PATCH v3 00/16] repack: incremental MIDX/bitmap-based repacking Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260407055753.GA714878@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=newren@gmail.com \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox