* [PATCH v3 12/16] builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK`
From: Taylor Blau @ 2026-04-30 0:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1777507303.git.me@ttaylorr.com>
Change the --write-midx (-m) flag from an OPT_BOOL to an OPT_CALLBACK
that accepts an optional mode argument. Introduce an enum with
REPACK_WRITE_MIDX_NONE and REPACK_WRITE_MIDX_DEFAULT to distinguish
between the two states, and update all existing boolean checks
accordingly.
For now, passing no argument (or just `-m`) selects the default mode,
preserving existing behavior. A subsequent commit will add a new mode
for writing incremental MIDXs.
Extract repack_write_midx() as a dispatcher that selects the
appropriate MIDX-writing implementation based on the mode.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 50 ++++++++++++++++++++++++++++++++++++------------
repack-midx.c | 14 +++++++++++++-
repack.h | 8 +++++++-
3 files changed, 58 insertions(+), 14 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 24be147d39a..5d366340c34 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -97,6 +97,24 @@ static int repack_config(const char *var, const char *value,
return git_default_config(var, value, ctx, cb);
}
+static int option_parse_write_midx(const struct option *opt, const char *arg,
+ int unset)
+{
+ enum repack_write_midx_mode *cfg = opt->value;
+
+ if (unset) {
+ *cfg = REPACK_WRITE_MIDX_NONE;
+ return 0;
+ }
+
+ if (!arg || !*arg)
+ *cfg = REPACK_WRITE_MIDX_DEFAULT;
+ else
+ return error(_("unknown value for %s: %s"), opt->long_name, arg);
+
+ return 0;
+}
+
int cmd_repack(int argc,
const char **argv,
const char *prefix,
@@ -119,7 +137,7 @@ int cmd_repack(int argc,
struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
struct pack_objects_args po_args = PACK_OBJECTS_ARGS_INIT;
struct pack_objects_args cruft_po_args = PACK_OBJECTS_ARGS_INIT;
- int write_midx = 0;
+ enum repack_write_midx_mode write_midx = REPACK_WRITE_MIDX_NONE;
const char *cruft_expiration = NULL;
const char *expire_to = NULL;
const char *filter_to = NULL;
@@ -185,8 +203,14 @@ int cmd_repack(int argc,
N_("do not repack this pack")),
OPT_INTEGER('g', "geometric", &geometry.split_factor,
N_("find a geometric progression with factor <N>")),
- OPT_BOOL('m', "write-midx", &write_midx,
- N_("write a multi-pack index of the resulting packs")),
+ OPT_CALLBACK_F(0, "write-midx", &write_midx,
+ N_("mode"),
+ N_("write a multi-pack index of the resulting packs"),
+ PARSE_OPT_OPTARG, option_parse_write_midx),
+ OPT_SET_INT_F('m', NULL, &write_midx,
+ N_("write a multi-pack index of the resulting packs"),
+ REPACK_WRITE_MIDX_DEFAULT,
+ PARSE_OPT_HIDDEN),
OPT_STRING(0, "expire-to", &expire_to, N_("dir"),
N_("pack prefix to store a pack containing pruned objects")),
OPT_STRING(0, "filter-to", &filter_to, N_("dir"),
@@ -221,14 +245,16 @@ int cmd_repack(int argc,
pack_everything |= ALL_INTO_ONE;
if (write_bitmaps < 0) {
- if (!write_midx &&
+ if (write_midx == REPACK_WRITE_MIDX_NONE &&
(!(pack_everything & ALL_INTO_ONE) || !is_bare_repository()))
write_bitmaps = 0;
}
if (po_args.pack_kept_objects < 0)
- po_args.pack_kept_objects = write_bitmaps > 0 && !write_midx;
+ po_args.pack_kept_objects = write_bitmaps > 0 &&
+ write_midx == REPACK_WRITE_MIDX_NONE;
- if (write_bitmaps && !(pack_everything & ALL_INTO_ONE) && !write_midx)
+ if (write_bitmaps && !(pack_everything & ALL_INTO_ONE) &&
+ write_midx == REPACK_WRITE_MIDX_NONE)
die(_(incremental_bitmap_conflict_error));
if (write_bitmaps && po_args.local &&
@@ -244,7 +270,7 @@ int cmd_repack(int argc,
write_bitmaps = 0;
}
- if (write_midx && write_bitmaps) {
+ if (write_midx != REPACK_WRITE_MIDX_NONE && write_bitmaps) {
struct strbuf path = STRBUF_INIT;
strbuf_addf(&path, "%s/%s_XXXXXX",
@@ -297,7 +323,7 @@ int cmd_repack(int argc,
}
if (repo_has_promisor_remote(repo))
strvec_push(&cmd.args, "--exclude-promisor-objects");
- if (!write_midx) {
+ if (write_midx == REPACK_WRITE_MIDX_NONE) {
if (write_bitmaps > 0)
strvec_push(&cmd.args, "--write-bitmap-index");
else if (write_bitmaps < 0)
@@ -519,7 +545,7 @@ int cmd_repack(int argc,
if (delete_redundant && pack_everything & ALL_INTO_ONE)
existing_packs_mark_for_deletion(&existing, &names);
- if (write_midx) {
+ if (write_midx != REPACK_WRITE_MIDX_NONE) {
struct repack_write_midx_opts opts = {
.existing = &existing,
.geometry = &geometry,
@@ -528,11 +554,11 @@ int cmd_repack(int argc,
.packdir = packdir,
.show_progress = show_progress,
.write_bitmaps = write_bitmaps > 0,
- .midx_must_contain_cruft = midx_must_contain_cruft
+ .midx_must_contain_cruft = midx_must_contain_cruft,
+ .mode = write_midx,
};
- ret = write_midx_included_packs(&opts);
-
+ ret = repack_write_midx(&opts);
if (ret)
goto cleanup;
}
diff --git a/repack-midx.c b/repack-midx.c
index 78f069c2151..4a568a2a9b8 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -315,7 +315,7 @@ static int repack_fill_midx_stdin_packs(struct child_process *cmd,
return finish_command(cmd);
}
-int write_midx_included_packs(struct repack_write_midx_opts *opts)
+static int write_midx_included_packs(struct repack_write_midx_opts *opts)
{
struct child_process cmd = CHILD_PROCESS_INIT;
struct string_list include = STRING_LIST_INIT_DUP;
@@ -378,3 +378,15 @@ int write_midx_included_packs(struct repack_write_midx_opts *opts)
return ret;
}
+
+int repack_write_midx(struct repack_write_midx_opts *opts)
+{
+ switch (opts->mode) {
+ case REPACK_WRITE_MIDX_NONE:
+ BUG("write_midx mode is NONE?");
+ case REPACK_WRITE_MIDX_DEFAULT:
+ return write_midx_included_packs(opts);
+ default:
+ BUG("unhandled write_midx mode: %d", opts->mode);
+ }
+}
diff --git a/repack.h b/repack.h
index 77d24ee45fb..81907fcce7f 100644
--- a/repack.h
+++ b/repack.h
@@ -134,6 +134,11 @@ void pack_geometry_release(struct pack_geometry *geometry);
struct tempfile;
+enum repack_write_midx_mode {
+ REPACK_WRITE_MIDX_NONE,
+ REPACK_WRITE_MIDX_DEFAULT,
+};
+
struct repack_write_midx_opts {
struct existing_packs *existing;
struct pack_geometry *geometry;
@@ -143,10 +148,11 @@ struct repack_write_midx_opts {
int show_progress;
int write_bitmaps;
int midx_must_contain_cruft;
+ enum repack_write_midx_mode mode;
};
void midx_snapshot_refs(struct repository *repo, struct tempfile *f);
-int write_midx_included_packs(struct repack_write_midx_opts *opts);
+int repack_write_midx(struct repack_write_midx_opts *opts);
int write_filtered_pack(const struct write_pack_opts *opts,
struct existing_packs *existing,
--
2.54.0.16.g1c05dfce579
^ permalink raw reply related
* [PATCH v3 13/16] packfile: ensure `close_pack_revindex()` frees in-memory revindex
From: Taylor Blau @ 2026-04-30 0:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1777507303.git.me@ttaylorr.com>
The following commit will introduce a case where we write a MIDX bitmap
over packs that do not themselves have on-disk *.rev files.
This case is supported within Git, and we will simply fall back to
generating the revindex in memory. But we don't ever release that
memory, causing a leak that is exposed by a test introduced in the
following commit.
(As far as I could find, we never free()'d memory allocated as a
byproduct of creating an in-memory revindex, likely because that code
predates the leak-checking niceties we have in the test suite now.)
Rectify this by calling `FREE_AND_NULL()` on the `p->revindex` field
when calling `close_pack_revindex()`.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
packfile.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/packfile.c b/packfile.c
index b012d648ada..a1e88fdb223 100644
--- a/packfile.c
+++ b/packfile.c
@@ -420,6 +420,8 @@ void close_pack_index(struct packed_git *p)
static void close_pack_revindex(struct packed_git *p)
{
+ FREE_AND_NULL(p->revindex);
+
if (!p->revindex_map)
return;
--
2.54.0.16.g1c05dfce579
^ permalink raw reply related
* [PATCH v3 14/16] repack: implement incremental MIDX repacking
From: Taylor Blau @ 2026-04-30 0:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1777507303.git.me@ttaylorr.com>
Implement the `write_midx_incremental()` function, which builds and
maintains an incremental MIDX chain as part of the geometric repacking
process.
Unlike the default mode which writes a single flat MIDX, the incremental
mode constructs a compaction plan that determines which MIDX layers to
write, compact, or copy, and then executes each step using `git
multi-pack-index` subcommands with the --no-write-chain-file flag.
The repacking strategy works as follows:
* Acquire the lock guarding the multi-pack-index-chain.
* A new MIDX layer is always written containing the newly created
pack(s). If the tip MIDX layer was rewritten during geometric
repacking, any surviving packs from that layer are also included.
* Starting from the new layer, adjacent MIDX layers are merged together
as long as the accumulated object count exceeds half the object count
of the next deeper layer (controlled by 'repack.midxSplitFactor').
* Remaining layers in the chain are evaluated pairwise and either
compacted or copied as-is, following the same merging condition.
* Write the contents of the new multi-pack-index chain, atomically move
it into place, and then release the lock.
* Delete any now-unused MIDX layers.
After writing the new layer, the strategy is evaluated among the
existing MIDX layers in order from oldest to newest. Each step that
writes a new MIDX layer uses "--no-write-chain-file" to avoid updating
the multi-pack-index-chain file. After all steps are complete, the new
chain file is written and then atomically moved into place.
At present, this functionality is exposed behind a new enum value,
`REPACK_WRITE_MIDX_INCREMENTAL`, but has no external callers. A
subsequent commit will expose this mode via `git repack
--write-midx=incremental`.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 5 +
repack-midx.c | 593 +++++++++++++++++++++++++++++++++++++++++++++--
repack.h | 3 +
3 files changed, 588 insertions(+), 13 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 5d366340c34..75c57736780 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -42,6 +42,9 @@ static const char incremental_bitmap_conflict_error[] = N_(
"--no-write-bitmap-index or disable the pack.writeBitmaps configuration."
);
+#define DEFAULT_MIDX_SPLIT_FACTOR 2
+#define DEFAULT_MIDX_NEW_LAYER_THRESHOLD 8
+
struct repack_config_ctx {
struct pack_objects_args *po_args;
struct pack_objects_args *cruft_po_args;
@@ -555,6 +558,8 @@ int cmd_repack(int argc,
.show_progress = show_progress,
.write_bitmaps = write_bitmaps > 0,
.midx_must_contain_cruft = midx_must_contain_cruft,
+ .midx_split_factor = DEFAULT_MIDX_SPLIT_FACTOR,
+ .midx_new_layer_threshold = DEFAULT_MIDX_NEW_LAYER_THRESHOLD,
.mode = write_midx,
};
diff --git a/repack-midx.c b/repack-midx.c
index 4a568a2a9b8..405f4662174 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -2,12 +2,16 @@
#include "repack.h"
#include "hash.h"
#include "hex.h"
+#include "lockfile.h"
+#include "midx.h"
#include "odb.h"
#include "oidset.h"
#include "pack-bitmap.h"
+#include "path.h"
#include "refs.h"
#include "run-command.h"
#include "tempfile.h"
+#include "trace2.h"
struct midx_snapshot_ref_data {
struct repository *repo;
@@ -293,26 +297,30 @@ static void repack_prepare_midx_command(struct child_process *cmd,
}
static int repack_fill_midx_stdin_packs(struct child_process *cmd,
- struct string_list *include)
+ struct string_list *include,
+ struct string_list *out)
{
+ struct strbuf in_buf = STRBUF_INIT;
+ struct strbuf out_buf = STRBUF_INIT;
struct string_list_item *item;
- FILE *in;
int ret;
- cmd->in = -1;
-
strvec_push(&cmd->args, "--stdin-packs");
- ret = start_command(cmd);
- if (ret)
- return ret;
-
- in = xfdopen(cmd->in, "w");
for_each_string_list_item(item, include)
- fprintf(in, "%s\n", item->string);
- fclose(in);
+ strbuf_addf(&in_buf, "%s\n", item->string);
- return finish_command(cmd);
+ ret = pipe_command(cmd, in_buf.buf, in_buf.len,
+ out ? &out_buf : NULL, 0, NULL, 0);
+
+ if (out)
+ string_list_split_f(out, out_buf.buf, "\n", -1,
+ STRING_LIST_SPLIT_NONEMPTY);
+
+ strbuf_release(&in_buf);
+ strbuf_release(&out_buf);
+
+ return ret;
}
static int write_midx_included_packs(struct repack_write_midx_opts *opts)
@@ -369,7 +377,7 @@ static int write_midx_included_packs(struct repack_write_midx_opts *opts)
strvec_pushf(&cmd.args, "--refs-snapshot=%s",
opts->refs_snapshot);
- ret = repack_fill_midx_stdin_packs(&cmd, &include);
+ ret = repack_fill_midx_stdin_packs(&cmd, &include, NULL);
done:
if (!ret && opts->write_bitmaps)
remove_redundant_bitmaps(&include, opts->packdir);
@@ -379,6 +387,563 @@ static int write_midx_included_packs(struct repack_write_midx_opts *opts)
return ret;
}
+struct midx_compaction_step {
+ union {
+ struct multi_pack_index *copy;
+ struct string_list write;
+ struct {
+ struct multi_pack_index *from;
+ struct multi_pack_index *to;
+ } compact;
+ } u;
+
+ uint32_t objects_nr;
+ char *csum;
+
+ enum {
+ MIDX_COMPACTION_STEP_UNKNOWN,
+ MIDX_COMPACTION_STEP_COPY,
+ MIDX_COMPACTION_STEP_WRITE,
+ MIDX_COMPACTION_STEP_COMPACT,
+ } type;
+};
+
+static const char *midx_compaction_step_base(const struct midx_compaction_step *step)
+{
+ switch (step->type) {
+ case MIDX_COMPACTION_STEP_UNKNOWN:
+ BUG("cannot use UNKNOWN step as a base");
+ case MIDX_COMPACTION_STEP_COPY:
+ return midx_get_checksum_hex(step->u.copy);
+ case MIDX_COMPACTION_STEP_WRITE:
+ BUG("cannot use WRITE step as a base");
+ case MIDX_COMPACTION_STEP_COMPACT:
+ return midx_get_checksum_hex(step->u.compact.to);
+ default:
+ BUG("unhandled midx compaction step type %d", step->type);
+ }
+}
+
+static int midx_compaction_step_exec_copy(struct midx_compaction_step *step)
+{
+ step->csum = xstrdup(midx_get_checksum_hex(step->u.copy));
+ return 0;
+}
+
+static int midx_compaction_step_exec_write(struct midx_compaction_step *step,
+ struct repack_write_midx_opts *opts,
+ const char *base)
+{
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct string_list hash = STRING_LIST_INIT_DUP;
+ struct string_list_item *item;
+ const char *preferred_pack = NULL;
+ int ret = 0;
+
+ if (!step->u.write.nr) {
+ ret = error(_("no packs to write MIDX during compaction"));
+ goto out;
+ }
+
+ for_each_string_list_item(item, &step->u.write) {
+ if (item->util)
+ preferred_pack = item->string;
+ }
+
+ repack_prepare_midx_command(&cmd, opts, "write");
+ strvec_pushl(&cmd.args, "--incremental", "--no-write-chain-file", NULL);
+ strvec_pushf(&cmd.args, "--base=%s", base ? base : "none");
+
+ if (preferred_pack) {
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addstr(&buf, preferred_pack);
+ strbuf_strip_suffix(&buf, ".idx");
+ strbuf_addstr(&buf, ".pack");
+
+ strvec_pushf(&cmd.args, "--preferred-pack=%s", buf.buf);
+
+ strbuf_release(&buf);
+ }
+
+ ret = repack_fill_midx_stdin_packs(&cmd, &step->u.write, &hash);
+ if (hash.nr != 1) {
+ ret = error(_("expected exactly one line during MIDX write, "
+ "got: %"PRIuMAX),
+ (uintmax_t)hash.nr);
+ goto out;
+ }
+
+ step->csum = xstrdup(hash.items[0].string);
+
+out:
+ string_list_clear(&hash, 0);
+
+ return ret;
+}
+
+static int midx_compaction_step_exec_compact(struct midx_compaction_step *step,
+ struct repack_write_midx_opts *opts)
+{
+ struct child_process cmd = CHILD_PROCESS_INIT;
+ struct strbuf buf = STRBUF_INIT;
+ FILE *out = NULL;
+ int ret;
+
+ repack_prepare_midx_command(&cmd, opts, "compact");
+ strvec_pushl(&cmd.args, "--incremental", "--no-write-chain-file",
+ midx_get_checksum_hex(step->u.compact.from),
+ midx_get_checksum_hex(step->u.compact.to), NULL);
+
+ cmd.out = -1;
+
+ ret = start_command(&cmd);
+ if (ret)
+ goto out;
+
+ out = xfdopen(cmd.out, "r");
+ while (strbuf_getline_lf(&buf, out) != EOF) {
+ if (step->csum) {
+ ret = error(_("unexpected MIDX output: '%s'"), buf.buf);
+ fclose(out);
+ out = NULL;
+ finish_command(&cmd);
+ goto out;
+ }
+ step->csum = strbuf_detach(&buf, NULL);
+ }
+
+ ret = finish_command(&cmd);
+
+out:
+ if (out)
+ fclose(out);
+ strbuf_release(&buf);
+
+ return ret;
+}
+
+static int midx_compaction_step_exec(struct midx_compaction_step *step,
+ struct repack_write_midx_opts *opts,
+ const char *base)
+{
+ switch (step->type) {
+ case MIDX_COMPACTION_STEP_UNKNOWN:
+ BUG("cannot execute UNKNOWN midx compaction step");
+ case MIDX_COMPACTION_STEP_COPY:
+ return midx_compaction_step_exec_copy(step);
+ case MIDX_COMPACTION_STEP_WRITE:
+ return midx_compaction_step_exec_write(step, opts, base);
+ case MIDX_COMPACTION_STEP_COMPACT:
+ return midx_compaction_step_exec_compact(step, opts);
+ default:
+ BUG("unhandled midx compaction step type %d", step->type);
+ }
+}
+
+static void midx_compaction_step_release(struct midx_compaction_step *step)
+{
+ if (step->type == MIDX_COMPACTION_STEP_WRITE)
+ string_list_clear(&step->u.write, 0);
+ free(step->csum);
+}
+
+static int repack_make_midx_compaction_plan(struct repack_write_midx_opts *opts,
+ struct midx_compaction_step **steps_p,
+ size_t *steps_nr_p)
+{
+ struct multi_pack_index *m;
+ struct midx_compaction_step *steps = NULL;
+ struct midx_compaction_step step = { 0 };
+ struct strbuf buf = STRBUF_INIT;
+ size_t steps_nr = 0, steps_alloc = 0;
+ uint32_t i;
+ int ret = 0;
+
+ trace2_region_enter("repack", "make_midx_compaction_plan",
+ opts->existing->repo);
+
+ odb_reprepare(opts->existing->repo->objects);
+ m = get_multi_pack_index(opts->existing->source);
+
+ for (i = 0; m && i < m->num_packs + m->num_packs_in_base; i++) {
+ if (prepare_midx_pack(m, i)) {
+ ret = error(_("could not load pack %"PRIu32" from MIDX"),
+ i);
+ goto out;
+ }
+ }
+
+ trace2_region_enter("repack", "steps:write", opts->existing->repo);
+
+ /*
+ * The first MIDX in the resulting chain is always going to be
+ * new.
+ *
+ * At a minimum, it will include all of the newly written packs.
+ * If there is an existing MIDX whose tip layer contains packs
+ * that were repacked, it will also include any of its packs
+ * which were *not* rolled up as part of the geometric repack
+ * (if any), and the previous tip will be replaced.
+ *
+ * It may grow to include the packs from zero or more MIDXs from
+ * the old chain, beginning either at the old tip (if the MIDX
+ * was *not* rewritten) or the old tip's base MIDX layer
+ * (otherwise).
+ */
+ step.type = MIDX_COMPACTION_STEP_WRITE;
+ string_list_init_dup(&step.u.write);
+
+ for (i = 0; i < opts->names->nr; i++) {
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "pack-%s.idx", opts->names->items[i].string);
+ string_list_append(&step.u.write, buf.buf);
+
+ trace2_data_string("repack", opts->existing->repo,
+ "include:fresh",
+ step.u.write.items[step.u.write.nr - 1].string);
+ }
+ for (i = 0; i < opts->geometry->split; i++) {
+ struct packed_git *p = opts->geometry->pack[i];
+ if (unsigned_add_overflows(step.objects_nr, p->num_objects)) {
+ ret = error(_("too many objects in MIDX compaction step"));
+ goto out;
+ }
+
+ step.objects_nr += p->num_objects;
+ }
+ trace2_data_intmax("repack", opts->existing->repo,
+ "include:fresh:objects_nr",
+ (uintmax_t)step.objects_nr);
+
+ /*
+ * Now handle any existing packs which were *not* rewritten.
+ *
+ * The list of packs in opts->geometry only contains MIDX'd
+ * packs from the newest layer when that layer has more than
+ * 'repack.midxNewLayerThreshold' number of packs.
+ *
+ * If the MIDX tip was rewritten (that is, one or more of those
+ * packs appear below the split line), then add all packs above
+ * the split line to the new layer, as the old one is no longer
+ * usable.
+ *
+ * If the MIDX tip was not rewritten (that is, all MIDX'd packs
+ * from the youngest layer appear below the split line, or were
+ * not included in the geometric repack at all because there
+ * were too few of them), ignore them since we'll retain the
+ * existing layer as-is.
+ */
+ for (i = opts->geometry->split; i < opts->geometry->pack_nr; i++) {
+ struct packed_git *p = opts->geometry->pack[i];
+ struct string_list_item *item;
+
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, pack_basename(p));
+ strbuf_strip_suffix(&buf, ".pack");
+ strbuf_addstr(&buf, ".idx");
+
+ if (p->multi_pack_index &&
+ !opts->geometry->midx_tip_rewritten) {
+ trace2_data_string("repack", opts->existing->repo,
+ "exclude:unmodified", buf.buf);
+ continue;
+ }
+
+ trace2_data_string("repack", opts->existing->repo,
+ "include:unmodified", buf.buf);
+ trace2_data_string("repack", opts->existing->repo,
+ "include:unmodified:midx",
+ p->multi_pack_index ? "true" : "false");
+
+ item = string_list_append(&step.u.write, buf.buf);
+ if (p->multi_pack_index || i == opts->geometry->pack_nr - 1)
+ item->util = (void *)1; /* mark as preferred */
+
+ if (unsigned_add_overflows(step.objects_nr, p->num_objects)) {
+ ret = error(_("too many objects in MIDX compaction step"));
+ goto out;
+ }
+
+ step.objects_nr += p->num_objects;
+ }
+ trace2_data_intmax("repack", opts->existing->repo,
+ "include:unmodified:objects_nr",
+ (uintmax_t)step.objects_nr);
+
+ /*
+ * If the MIDX tip was rewritten, then we no longer consider it
+ * a candidate for compaction, since it will not exist in the
+ * MIDX chain being built.
+ */
+ if (opts->geometry->midx_tip_rewritten)
+ m = m->base_midx;
+
+ trace2_data_string("repack", opts->existing->repo, "midx:rewrote-tip",
+ opts->geometry->midx_tip_rewritten ? "true" : "false");
+
+ trace2_region_enter("repack", "compact", opts->existing->repo);
+
+ /*
+ * Compact additional MIDX layers into this proposed one until
+ * the merging condition is violated.
+ */
+ while (m) {
+ uint32_t preferred_pack_idx;
+
+ trace2_data_string("repack", opts->existing->repo,
+ "candidate", midx_get_checksum_hex(m));
+
+ if (step.objects_nr < m->num_objects / opts->midx_split_factor) {
+ /*
+ * Stop compacting MIDX layer as soon as the
+ * merged size is less than half the size of the
+ * next layer in the chain.
+ */
+ trace2_data_string("repack", opts->existing->repo,
+ "compact", "violated");
+ trace2_data_intmax("repack", opts->existing->repo,
+ "objects_nr",
+ (uintmax_t)step.objects_nr);
+ trace2_data_intmax("repack", opts->existing->repo,
+ "next_objects_nr",
+ (uintmax_t)m->num_objects);
+ trace2_data_intmax("repack", opts->existing->repo,
+ "split_factor",
+ (uintmax_t)opts->midx_split_factor);
+
+ break;
+ }
+
+ if (midx_preferred_pack(m, &preferred_pack_idx) < 0) {
+ ret = error(_("could not find preferred pack for MIDX "
+ "%s"), midx_get_checksum_hex(m));
+ goto out;
+ }
+
+ for (i = 0; i < m->num_packs; i++) {
+ struct string_list_item *item;
+ uint32_t pack_int_id = i + m->num_packs_in_base;
+ struct packed_git *p = nth_midxed_pack(m, pack_int_id);
+
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, pack_basename(p));
+ strbuf_strip_suffix(&buf, ".pack");
+ strbuf_addstr(&buf, ".idx");
+
+ trace2_data_string("repack", opts->existing->repo,
+ "midx:pack", buf.buf);
+
+ item = string_list_append(&step.u.write, buf.buf);
+ if (pack_int_id == preferred_pack_idx)
+ item->util = (void *)1; /* mark as preferred */
+ }
+
+ if (unsigned_add_overflows(step.objects_nr, m->num_objects)) {
+ ret = error(_("too many objects in MIDX compaction step"));
+ goto out;
+ }
+ step.objects_nr += m->num_objects;
+
+ m = m->base_midx;
+ }
+
+ if (step.u.write.nr > 0) {
+ /*
+ * As long as there is at least one new pack to write
+ * (and thus the MIDX is non-empty), add it to the plan.
+ */
+ ALLOC_GROW(steps, steps_nr + 1, steps_alloc);
+ steps[steps_nr++] = step;
+ }
+
+ trace2_data_intmax("repack", opts->existing->repo,
+ "step:objects_nr", (uintmax_t)step.objects_nr);
+ trace2_data_intmax("repack", opts->existing->repo,
+ "step:packs_nr", (uintmax_t)step.u.write.nr);
+
+ trace2_region_leave("repack", "compact", opts->existing->repo);
+ trace2_region_leave("repack", "steps:write", opts->existing->repo);
+
+ trace2_region_enter("repack", "steps:rest", opts->existing->repo);
+
+ /*
+ * Then start over, repeat, and either compact or keep as-is
+ * each MIDX layer until we have exhausted the chain.
+ *
+ * Finally, evaluate the remainder of the chain (if any) and
+ * either compact a sequence of adjacent layers, or keep
+ * individual layers as-is according to the same merging
+ * condition as above.
+ */
+ while (m) {
+ struct multi_pack_index *next = m;
+
+ ALLOC_GROW(steps, steps_nr + 1, steps_alloc);
+
+ memset(&step, 0, sizeof(step));
+ step.type = MIDX_COMPACTION_STEP_UNKNOWN;
+
+ trace2_region_enter("repack", "step", opts->existing->repo);
+
+ trace2_data_string("repack", opts->existing->repo,
+ "from", midx_get_checksum_hex(m));
+
+ while (next) {
+ uint32_t proposed_objects_nr;
+ if (unsigned_add_overflows(step.objects_nr, next->num_objects)) {
+ ret = error(_("too many objects in MIDX compaction step"));
+ trace2_region_leave("repack", "step", opts->existing->repo);
+ goto out;
+ }
+
+ proposed_objects_nr = step.objects_nr + next->num_objects;
+
+ trace2_data_string("repack", opts->existing->repo,
+ "proposed",
+ midx_get_checksum_hex(next));
+ trace2_data_intmax("repack", opts->existing->repo,
+ "proposed:objects_nr",
+ (uintmax_t)next->num_objects);
+
+ if (!next->base_midx) {
+ /*
+ * If we are at the end of the MIDX
+ * chain, there is nothing to compact,
+ * so mark it and stop.
+ */
+ step.objects_nr = proposed_objects_nr;
+ break;
+ }
+
+ if (proposed_objects_nr < next->base_midx->num_objects / opts->midx_split_factor) {
+ /*
+ * If there is a MIDX following this
+ * one, but our accumulated size is less
+ * than half of its size, compacting
+ * them would violate the merging
+ * condition, so stop here.
+ */
+
+ trace2_data_string("repack", opts->existing->repo,
+ "compact:violated:at",
+ midx_get_checksum_hex(next->base_midx));
+ trace2_data_intmax("repack", opts->existing->repo,
+ "compact:violated:at:objects_nr",
+ (uintmax_t)next->base_midx->num_objects);
+ break;
+ }
+
+ /*
+ * Otherwise, it is OK to compact the next layer
+ * into this one. Do so, and then continue
+ * through the remainder of the chain.
+ */
+ step.objects_nr = proposed_objects_nr;
+ trace2_data_intmax("repack", opts->existing->repo,
+ "step:objects_nr",
+ (uintmax_t)step.objects_nr);
+ next = next->base_midx;
+ }
+
+ if (m == next) {
+ step.type = MIDX_COMPACTION_STEP_COPY;
+ step.u.copy = m;
+
+ trace2_data_string("repack", opts->existing->repo,
+ "type", "copy");
+ } else {
+ step.type = MIDX_COMPACTION_STEP_COMPACT;
+ step.u.compact.from = next;
+ step.u.compact.to = m;
+
+ trace2_data_string("repack", opts->existing->repo,
+ "to", midx_get_checksum_hex(m));
+ trace2_data_string("repack", opts->existing->repo,
+ "type", "compact");
+ }
+
+ m = next->base_midx;
+ steps[steps_nr++] = step;
+ trace2_region_leave("repack", "step", opts->existing->repo);
+ }
+
+ trace2_region_leave("repack", "steps:rest", opts->existing->repo);
+
+out:
+ *steps_p = steps;
+ *steps_nr_p = steps_nr;
+
+ strbuf_release(&buf);
+
+ trace2_region_leave("repack", "make_midx_compaction_plan",
+ opts->existing->repo);
+
+ return ret;
+}
+
+static int write_midx_incremental(struct repack_write_midx_opts *opts)
+{
+ struct midx_compaction_step *steps = NULL;
+ struct strbuf lock_name = STRBUF_INIT;
+ struct lock_file lf;
+ size_t steps_nr = 0;
+ size_t i;
+ int ret = 0;
+
+ get_midx_chain_filename(opts->existing->source, &lock_name);
+ if (safe_create_leading_directories(opts->existing->repo,
+ lock_name.buf))
+ die_errno(_("unable to create leading directories of %s"),
+ lock_name.buf);
+ hold_lock_file_for_update(&lf, lock_name.buf, LOCK_DIE_ON_ERROR);
+
+ if (!fdopen_lock_file(&lf, "w")) {
+ ret = error_errno(_("unable to open multi-pack-index chain file"));
+ goto done;
+ }
+
+ if (repack_make_midx_compaction_plan(opts, &steps, &steps_nr) < 0) {
+ ret = error(_("unable to generate compaction plan"));
+ goto done;
+ }
+
+ for (i = 0; i < steps_nr; i++) {
+ struct midx_compaction_step *step = &steps[i];
+ char *base = NULL;
+
+ if (i + 1 < steps_nr)
+ base = xstrdup(midx_compaction_step_base(&steps[i + 1]));
+
+ if (midx_compaction_step_exec(step, opts, base) < 0) {
+ ret = error(_("unable to execute compaction step %"PRIuMAX),
+ (uintmax_t)i);
+ free(base);
+ goto done;
+ }
+
+ free(base);
+ }
+
+ i = steps_nr;
+ while (i--) {
+ struct midx_compaction_step *step = &steps[i];
+ if (!step->csum)
+ BUG("missing result for compaction step %"PRIuMAX,
+ (uintmax_t)i);
+ fprintf(get_lock_file_fp(&lf), "%s\n", step->csum);
+ }
+
+ commit_lock_file(&lf);
+
+done:
+ strbuf_release(&lock_name);
+ for (i = 0; i < steps_nr; i++)
+ midx_compaction_step_release(&steps[i]);
+ free(steps);
+ return ret;
+}
+
int repack_write_midx(struct repack_write_midx_opts *opts)
{
switch (opts->mode) {
@@ -386,6 +951,8 @@ int repack_write_midx(struct repack_write_midx_opts *opts)
BUG("write_midx mode is NONE?");
case REPACK_WRITE_MIDX_DEFAULT:
return write_midx_included_packs(opts);
+ case REPACK_WRITE_MIDX_INCREMENTAL:
+ return write_midx_incremental(opts);
default:
BUG("unhandled write_midx mode: %d", opts->mode);
}
diff --git a/repack.h b/repack.h
index 81907fcce7f..831ccfb1c6c 100644
--- a/repack.h
+++ b/repack.h
@@ -137,6 +137,7 @@ struct tempfile;
enum repack_write_midx_mode {
REPACK_WRITE_MIDX_NONE,
REPACK_WRITE_MIDX_DEFAULT,
+ REPACK_WRITE_MIDX_INCREMENTAL,
};
struct repack_write_midx_opts {
@@ -148,6 +149,8 @@ struct repack_write_midx_opts {
int show_progress;
int write_bitmaps;
int midx_must_contain_cruft;
+ int midx_split_factor;
+ int midx_new_layer_threshold;
enum repack_write_midx_mode mode;
};
--
2.54.0.16.g1c05dfce579
^ permalink raw reply related
* [PATCH v3 15/16] repack: introduce `--write-midx=incremental`
From: Taylor Blau @ 2026-04-30 0:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1777507303.git.me@ttaylorr.com>
Expose the incremental MIDX repacking mode (implemented in an earlier
commit) via a new --write-midx=incremental option for `git repack`.
Add "incremental" as a recognized argument to the --write-midx
OPT_CALLBACK, mapping it to REPACK_WRITE_MIDX_INCREMENTAL. When this
mode is active and --geometric is in use, set the midx_layer_threshold
on the pack geometry so that only packs in sufficiently large tip layers
are considered for repacking.
Two new configuration options control the compaction behavior:
- repack.midxSplitFactor (default: 2): the factor used in the
geometric merging condition for MIDX layers.
- repack.midxNewLayerThreshold (default: 8): the minimum number of
packs in the tip MIDX layer before its packs are considered as
candidates for geometric repacking.
Add tests exercising the new mode across a variety of scenarios
including basic geometric violations, multi-round chain integrity,
branching and merging histories, cross-layer object uniqueness, and
threshold-based compaction.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/config/repack.adoc | 18 ++
Documentation/git-repack.adoc | 39 ++-
builtin/repack.c | 49 ++-
midx.c | 31 ++
midx.h | 3 +
repack-geometry.c | 13 +-
repack-midx.c | 5 +
repack.c | 56 +++-
repack.h | 10 +-
t/meson.build | 1 +
t/t7705-repack-incremental-midx.sh | 470 +++++++++++++++++++++++++++++
11 files changed, 671 insertions(+), 24 deletions(-)
create mode 100755 t/t7705-repack-incremental-midx.sh
diff --git a/Documentation/config/repack.adoc b/Documentation/config/repack.adoc
index e9e78dcb198..4c22a499f62 100644
--- a/Documentation/config/repack.adoc
+++ b/Documentation/config/repack.adoc
@@ -46,3 +46,21 @@ repack.midxMustContainCruft::
`--write-midx`. When false, cruft packs are only included in the MIDX
when necessary (e.g., because they might be required to form a
reachability closure with MIDX bitmaps). Defaults to true.
+
+repack.midxSplitFactor::
+ The factor used in the geometric merging condition when
+ compacting incremental MIDX layers during `git repack` when
+ invoked with the `--write-midx=incremental` option.
++
+Adjacent layers are merged when the accumulated object count of the
+newer layer exceeds `1/<N>` of the object count of the next deeper
+layer. Must be at least 2. Defaults to 2.
+
+repack.midxNewLayerThreshold::
+ The minimum number of packs in the tip MIDX layer before those
+ packs are considered as candidates for geometric repacking
+ during `git repack --write-midx=incremental`.
++
+When the tip layer has fewer packs than this threshold, those packs are
+excluded from the geometric repack entirely, and are thus left
+unmodified. Must be at least 1. Defaults to 8.
diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc
index 673ce910837..27a99cc46f4 100644
--- a/Documentation/git-repack.adoc
+++ b/Documentation/git-repack.adoc
@@ -11,7 +11,7 @@ SYNOPSIS
[verse]
'git repack' [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m]
[--window=<n>] [--depth=<n>] [--threads=<n>] [--keep-pack=<pack-name>]
- [--write-midx] [--name-hash-version=<n>] [--path-walk]
+ [--write-midx[=<mode>]] [--name-hash-version=<n>] [--path-walk]
DESCRIPTION
-----------
@@ -250,9 +250,42 @@ pack as the preferred pack for object selection by the MIDX (see
linkgit:git-multi-pack-index[1]).
-m::
---write-midx::
+--write-midx[=<mode>]::
Write a multi-pack index (see linkgit:git-multi-pack-index[1])
- containing the non-redundant packs.
+ containing the non-redundant packs. The following modes are
+ available:
++
+--
+ `default`;;
+ Write a single MIDX covering all packs. This is the
+ default when `--write-midx` is given without an
+ explicit mode.
+
+ `incremental`;;
+ Write an incremental MIDX chain instead of a single
+ flat MIDX. This mode requires `--geometric`.
++
+The incremental mode maintains a chain of MIDX layers that is compacted
+over time using a geometric merging strategy. Each repack creates a new
+tip layer containing the newly written pack(s). Adjacent layers are then
+merged whenever the newer layer's object count exceeds
+`1/repack.midxSplitFactor` of the next deeper layer's count. Layers
+that do not meet this condition are retained as-is.
++
+The result is that newer (tip) layers tend to contain many small packs
+with relatively few objects, while older (deeper) layers contain fewer,
+larger packs covering more objects. Because compaction is driven by the
+tip of the chain, newer layers are also rewritten more frequently than
+older ones, which are only touched when enough objects have accumulated
+to justify merging into them. This keeps the total number of layers
+logarithmic relative to the total number of objects.
++
+Only packs in the tip MIDX layer are considered as candidates for the
+geometric repack; packs in deeper layers are left untouched. If the tip
+layer contains fewer packs than `repack.midxNewLayerThreshold`, those
+packs are excluded from the geometry entirely, and a new layer is
+created for any new pack(s) without disturbing the existing chain.
+--
--name-hash-version=<n>::
Provide this argument to the underlying `git pack-objects` process.
diff --git a/builtin/repack.c b/builtin/repack.c
index 75c57736780..5ffa18e085e 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -33,7 +33,7 @@ static int midx_must_contain_cruft = 1;
static const char *const git_repack_usage[] = {
N_("git repack [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m]\n"
"[--window=<n>] [--depth=<n>] [--threads=<n>] [--keep-pack=<pack-name>]\n"
- "[--write-midx] [--name-hash-version=<n>] [--path-walk]"),
+ "[--write-midx[=<mode>]] [--name-hash-version=<n>] [--path-walk]"),
NULL
};
@@ -48,6 +48,8 @@ static const char incremental_bitmap_conflict_error[] = N_(
struct repack_config_ctx {
struct pack_objects_args *po_args;
struct pack_objects_args *cruft_po_args;
+ int midx_split_factor;
+ int midx_new_layer_threshold;
};
static int repack_config(const char *var, const char *value,
@@ -97,6 +99,16 @@ static int repack_config(const char *var, const char *value,
midx_must_contain_cruft = git_config_bool(var, value);
return 0;
}
+ if (!strcmp(var, "repack.midxsplitfactor")) {
+ repack_ctx->midx_split_factor = git_config_int(var, value,
+ ctx->kvi);
+ return 0;
+ }
+ if (!strcmp(var, "repack.midxnewlayerthreshold")) {
+ repack_ctx->midx_new_layer_threshold = git_config_int(var, value,
+ ctx->kvi);
+ return 0;
+ }
return git_default_config(var, value, ctx, cb);
}
@@ -112,6 +124,8 @@ static int option_parse_write_midx(const struct option *opt, const char *arg,
if (!arg || !*arg)
*cfg = REPACK_WRITE_MIDX_DEFAULT;
+ else if (!strcmp(arg, "incremental"))
+ *cfg = REPACK_WRITE_MIDX_INCREMENTAL;
else
return error(_("unknown value for %s: %s"), opt->long_name, arg);
@@ -226,6 +240,8 @@ int cmd_repack(int argc,
memset(&config_ctx, 0, sizeof(config_ctx));
config_ctx.po_args = &po_args;
config_ctx.cruft_po_args = &cruft_po_args;
+ config_ctx.midx_split_factor = DEFAULT_MIDX_SPLIT_FACTOR;
+ config_ctx.midx_new_layer_threshold = DEFAULT_MIDX_NEW_LAYER_THRESHOLD;
repo_config(repo, repack_config, &config_ctx);
@@ -247,6 +263,9 @@ int cmd_repack(int argc,
if (pack_everything & PACK_CRUFT)
pack_everything |= ALL_INTO_ONE;
+ if (write_midx == REPACK_WRITE_MIDX_INCREMENTAL && !geometry.split_factor)
+ die(_("--write-midx=incremental requires --geometric"));
+
if (write_bitmaps < 0) {
if (write_midx == REPACK_WRITE_MIDX_NONE &&
(!(pack_everything & ALL_INTO_ONE) || !is_bare_repository()))
@@ -273,6 +292,13 @@ int cmd_repack(int argc,
write_bitmaps = 0;
}
+ if (config_ctx.midx_split_factor < 2)
+ die(_("invalid value for %s: %d"), "--midx-split-factor",
+ config_ctx.midx_split_factor);
+ if (config_ctx.midx_new_layer_threshold < 1)
+ die(_("invalid value for %s: %d"), "--midx-new-layer-threshold",
+ config_ctx.midx_new_layer_threshold);
+
if (write_midx != REPACK_WRITE_MIDX_NONE && write_bitmaps) {
struct strbuf path = STRBUF_INIT;
@@ -296,6 +322,10 @@ int cmd_repack(int argc,
if (geometry.split_factor) {
if (pack_everything)
die(_("options '%s' and '%s' cannot be used together"), "--geometric", "-A/-a");
+ if (write_midx == REPACK_WRITE_MIDX_INCREMENTAL) {
+ geometry.midx_layer_threshold = config_ctx.midx_new_layer_threshold;
+ geometry.midx_layer_threshold_set = true;
+ }
pack_geometry_init(&geometry, &existing, &po_args);
pack_geometry_split(&geometry);
}
@@ -545,8 +575,11 @@ int cmd_repack(int argc,
packtmp);
/* End of pack replacement. */
- if (delete_redundant && pack_everything & ALL_INTO_ONE)
+ if (delete_redundant && pack_everything & ALL_INTO_ONE) {
+ if (write_midx == REPACK_WRITE_MIDX_INCREMENTAL)
+ existing_packs_retain_midx_packs(&existing);
existing_packs_mark_for_deletion(&existing, &names);
+ }
if (write_midx != REPACK_WRITE_MIDX_NONE) {
struct repack_write_midx_opts opts = {
@@ -558,8 +591,8 @@ int cmd_repack(int argc,
.show_progress = show_progress,
.write_bitmaps = write_bitmaps > 0,
.midx_must_contain_cruft = midx_must_contain_cruft,
- .midx_split_factor = DEFAULT_MIDX_SPLIT_FACTOR,
- .midx_new_layer_threshold = DEFAULT_MIDX_NEW_LAYER_THRESHOLD,
+ .midx_split_factor = config_ctx.midx_split_factor,
+ .midx_new_layer_threshold = config_ctx.midx_new_layer_threshold,
.mode = write_midx,
};
@@ -572,11 +605,15 @@ int cmd_repack(int argc,
if (delete_redundant) {
int opts = 0;
- existing_packs_remove_redundant(&existing, packdir);
+ bool wrote_incremental_midx = write_midx == REPACK_WRITE_MIDX_INCREMENTAL;
+
+ existing_packs_remove_redundant(&existing, packdir,
+ wrote_incremental_midx);
if (geometry.split_factor)
pack_geometry_remove_redundant(&geometry, &names,
- &existing, packdir);
+ &existing, packdir,
+ wrote_incremental_midx);
if (show_progress)
opts |= PRUNE_PACKED_VERBOSE;
prune_packed_objects(opts);
diff --git a/midx.c b/midx.c
index dc86c8e7fee..cd31fa20788 100644
--- a/midx.c
+++ b/midx.c
@@ -850,6 +850,37 @@ void clear_midx_file(struct repository *r)
strbuf_release(&midx);
}
+void clear_incremental_midx_files(struct repository *r,
+ const struct strvec *keep_hashes)
+{
+ struct strbuf chain = STRBUF_INIT;
+
+ get_midx_chain_filename(r->objects->sources, &chain);
+
+ if (r->objects) {
+ struct odb_source *source = r->objects->sources;
+ for (source = r->objects->sources; source; source = source->next) {
+ struct odb_source_files *files = odb_source_files_downcast(source);
+ if (files->packed->midx)
+ close_midx(files->packed->midx);
+ files->packed->midx = NULL;
+ }
+ }
+
+ if (!keep_hashes && remove_path(chain.buf))
+ die(_("failed to clear multi-pack-index chain at %s"),
+ chain.buf);
+
+ clear_incremental_midx_files_ext(r->objects->sources, MIDX_EXT_BITMAP,
+ keep_hashes);
+ clear_incremental_midx_files_ext(r->objects->sources, MIDX_EXT_REV,
+ keep_hashes);
+ clear_incremental_midx_files_ext(r->objects->sources, MIDX_EXT_MIDX,
+ keep_hashes);
+
+ strbuf_release(&chain);
+}
+
static int verify_midx_error;
__attribute__((format (printf, 1, 2)))
diff --git a/midx.h b/midx.h
index 3ee12dd08ec..63853a03a47 100644
--- a/midx.h
+++ b/midx.h
@@ -9,6 +9,7 @@ struct repository;
struct bitmapped_pack;
struct git_hash_algo;
struct odb_source;
+struct strvec;
#define MIDX_SIGNATURE 0x4d494458 /* "MIDX" */
#define MIDX_VERSION_V1 1
@@ -143,6 +144,8 @@ int write_midx_file_compact(struct odb_source *source,
const char *incremental_base,
unsigned flags);
void clear_midx_file(struct repository *r);
+void clear_incremental_midx_files(struct repository *r,
+ const struct strvec *keep_hashes);
int verify_midx_file(struct odb_source *source, unsigned flags);
int expire_midx_packs(struct odb_source *source, unsigned flags);
int midx_repack(struct odb_source *source, size_t batch_size, unsigned flags);
diff --git a/repack-geometry.c b/repack-geometry.c
index 2408b8a3cc2..2064683dcfe 100644
--- a/repack-geometry.c
+++ b/repack-geometry.c
@@ -249,7 +249,8 @@ static void remove_redundant_packs(struct packed_git **pack,
uint32_t pack_nr,
struct string_list *names,
struct existing_packs *existing,
- const char *packdir)
+ const char *packdir,
+ bool wrote_incremental_midx)
{
const struct git_hash_algo *algop = existing->repo->hash_algo;
struct strbuf buf = STRBUF_INIT;
@@ -269,7 +270,8 @@ static void remove_redundant_packs(struct packed_git **pack,
(string_list_has_string(&existing->kept_packs, buf.buf)))
continue;
- repack_remove_redundant_pack(existing->repo, packdir, buf.buf);
+ repack_remove_redundant_pack(existing->repo, packdir, buf.buf,
+ wrote_incremental_midx);
}
strbuf_release(&buf);
@@ -278,12 +280,13 @@ static void remove_redundant_packs(struct packed_git **pack,
void pack_geometry_remove_redundant(struct pack_geometry *geometry,
struct string_list *names,
struct existing_packs *existing,
- const char *packdir)
+ const char *packdir,
+ bool wrote_incremental_midx)
{
remove_redundant_packs(geometry->pack, geometry->split,
- names, existing, packdir);
+ names, existing, packdir, wrote_incremental_midx);
remove_redundant_packs(geometry->promisor_pack, geometry->promisor_split,
- names, existing, packdir);
+ names, existing, packdir, wrote_incremental_midx);
}
void pack_geometry_release(struct pack_geometry *geometry)
diff --git a/repack-midx.c b/repack-midx.c
index 405f4662174..bb3bee03ace 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -887,6 +887,7 @@ static int write_midx_incremental(struct repack_write_midx_opts *opts)
struct midx_compaction_step *steps = NULL;
struct strbuf lock_name = STRBUF_INIT;
struct lock_file lf;
+ struct strvec keep_hashes = STRVEC_INIT;
size_t steps_nr = 0;
size_t i;
int ret = 0;
@@ -932,11 +933,15 @@ static int write_midx_incremental(struct repack_write_midx_opts *opts)
BUG("missing result for compaction step %"PRIuMAX,
(uintmax_t)i);
fprintf(get_lock_file_fp(&lf), "%s\n", step->csum);
+ strvec_push(&keep_hashes, step->csum);
}
commit_lock_file(&lf);
+ clear_incremental_midx_files(opts->existing->repo, &keep_hashes);
+
done:
+ strvec_clear(&keep_hashes);
strbuf_release(&lock_name);
for (i = 0; i < steps_nr; i++)
midx_compaction_step_release(&steps[i]);
diff --git a/repack.c b/repack.c
index 2ee6b51420a..571dabb665e 100644
--- a/repack.c
+++ b/repack.c
@@ -55,14 +55,18 @@ void pack_objects_args_release(struct pack_objects_args *args)
}
void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
- const char *base_name)
+ const char *base_name,
+ bool wrote_incremental_midx)
{
struct strbuf buf = STRBUF_INIT;
struct odb_source *source = repo->objects->sources;
struct multi_pack_index *m = get_multi_pack_index(source);
strbuf_addf(&buf, "%s.pack", base_name);
- if (m && source->local && midx_contains_pack(m, buf.buf))
+ if (m && source->local && midx_contains_pack(m, buf.buf)) {
clear_midx_file(repo);
+ if (!wrote_incremental_midx)
+ clear_incremental_midx_files(repo, NULL);
+ }
strbuf_insertf(&buf, 0, "%s/", dir_name);
unlink_pack_path(buf.buf, 1);
strbuf_release(&buf);
@@ -250,25 +254,63 @@ void existing_packs_mark_for_deletion(struct existing_packs *existing,
&existing->cruft_packs);
}
+/*
+ * Mark every pack that is referenced by the existing MIDX chain as
+ * retained, so that a subsequent call to
+ * existing_packs_mark_for_deletion() will not mark them for deletion.
+ *
+ * This is used when writing an incremental MIDX layer on top of an
+ * existing chain: retained layers continue to reference the same
+ * packs on disk, so those packs must not be unlinked even if the
+ * freshly-written pack supersedes them.
+ */
+void existing_packs_retain_midx_packs(struct existing_packs *existing)
+{
+ struct string_list_item *item;
+ struct strbuf buf = STRBUF_INIT;
+
+ for_each_string_list_item(item, &existing->midx_packs) {
+ struct string_list_item *found;
+
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, item->string);
+ strbuf_strip_suffix(&buf, ".pack");
+ strbuf_strip_suffix(&buf, ".idx");
+
+ found = string_list_lookup(&existing->non_kept_packs, buf.buf);
+ if (found)
+ existing_packs_mark_retained(found);
+
+ found = string_list_lookup(&existing->cruft_packs, buf.buf);
+ if (found)
+ existing_packs_mark_retained(found);
+ }
+
+ strbuf_release(&buf);
+}
+
static void remove_redundant_packs_1(struct repository *repo,
struct string_list *packs,
- const char *packdir)
+ const char *packdir,
+ bool wrote_incremental_midx)
{
struct string_list_item *item;
for_each_string_list_item(item, packs) {
if (!existing_pack_is_marked_for_deletion(item))
continue;
- repack_remove_redundant_pack(repo, packdir, item->string);
+ repack_remove_redundant_pack(repo, packdir, item->string,
+ wrote_incremental_midx);
}
}
void existing_packs_remove_redundant(struct existing_packs *existing,
- const char *packdir)
+ const char *packdir,
+ bool wrote_incremental_midx)
{
remove_redundant_packs_1(existing->repo, &existing->non_kept_packs,
- packdir);
+ packdir, wrote_incremental_midx);
remove_redundant_packs_1(existing->repo, &existing->cruft_packs,
- packdir);
+ packdir, wrote_incremental_midx);
}
void existing_packs_release(struct existing_packs *existing)
diff --git a/repack.h b/repack.h
index 831ccfb1c6c..f9fbc895f02 100644
--- a/repack.h
+++ b/repack.h
@@ -34,7 +34,8 @@ void prepare_pack_objects(struct child_process *cmd,
void pack_objects_args_release(struct pack_objects_args *args);
void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
- const char *base_name);
+ const char *base_name,
+ bool wrote_incremental_midx);
struct write_pack_opts {
struct pack_objects_args *po_args;
@@ -83,8 +84,10 @@ void existing_packs_retain_cruft(struct existing_packs *existing,
struct packed_git *cruft);
void existing_packs_mark_for_deletion(struct existing_packs *existing,
struct string_list *names);
+void existing_packs_retain_midx_packs(struct existing_packs *existing);
void existing_packs_remove_redundant(struct existing_packs *existing,
- const char *packdir);
+ const char *packdir,
+ bool wrote_incremental_midx);
void existing_packs_release(struct existing_packs *existing);
struct generated_pack;
@@ -129,7 +132,8 @@ struct packed_git *pack_geometry_preferred_pack(struct pack_geometry *geometry);
void pack_geometry_remove_redundant(struct pack_geometry *geometry,
struct string_list *names,
struct existing_packs *existing,
- const char *packdir);
+ const char *packdir,
+ bool wrote_incremental_midx);
void pack_geometry_release(struct pack_geometry *geometry);
struct tempfile;
diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5f..25f0d823d8e 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -951,6 +951,7 @@ integration_tests = [
't7702-repack-cyclic-alternate.sh',
't7703-repack-geometric.sh',
't7704-repack-cruft.sh',
+ 't7705-repack-incremental-midx.sh',
't7800-difftool.sh',
't7810-grep.sh',
't7811-grep-open.sh',
diff --git a/t/t7705-repack-incremental-midx.sh b/t/t7705-repack-incremental-midx.sh
new file mode 100755
index 00000000000..9e317ff6e8f
--- /dev/null
+++ b/t/t7705-repack-incremental-midx.sh
@@ -0,0 +1,470 @@
+#!/bin/sh
+
+test_description='git repack --write-midx=incremental'
+
+. ./test-lib.sh
+
+GIT_TEST_MULTI_PACK_INDEX=0
+GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
+GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL=0
+
+objdir=.git/objects
+packdir=$objdir/pack
+midxdir=$packdir/multi-pack-index.d
+midx_chain=$midxdir/multi-pack-index-chain
+
+# incrementally_repack N
+#
+# Make "N" new commits, each stored in their own pack, and then repacked
+# with the --write-midx=incremental strategy.
+incrementally_repack () {
+ for i in $(test_seq 1 "$1")
+ do
+ test_commit "$i" &&
+
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+ git multi-pack-index verify || return 1
+ done
+}
+
+# Create packs with geometrically increasing sizes so that they
+# satisfy the geometric progression and survive a --geometric=2
+# repack without being rolled up. Creates 3 packs containing 1,
+# 2, and 6 commits (3, 6, and 18 objects) respectively.
+create_geometric_packs () {
+ test_commit "small" &&
+ git repack -d &&
+
+ test_commit_bulk --message="medium" 2 &&
+ test_commit_bulk --message="large" 6 &&
+
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index
+}
+
+# create_layer <test_commit_bulk args>
+#
+# Creates a new MIDX layer with the contents of "test_commit_bulk $@".
+create_layer () {
+ test_commit_bulk "$@" &&
+
+ git multi-pack-index write --incremental --bitmap
+}
+
+# create_layers
+#
+# Reads lines of "<message> <nr>" from stdin and creates a new MIDX
+# layer for each line. See create_layer above for more.
+create_layers () {
+ while read msg nr
+ do
+ create_layer --message="$msg" "$nr" || return 1
+ done
+}
+
+test_expect_success '--write-midx=incremental requires --geometric' '
+ test_must_fail git repack --write-midx=incremental 2>err &&
+
+ test_grep -- "--write-midx=incremental requires --geometric" err
+'
+
+test_expect_success 'below layer threshold, tip packs excluded' '
+ git init below-layer-threshold-tip-packs-excluded &&
+ (
+ cd below-layer-threshold-tip-packs-excluded &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 4 &&
+ git config repack.midxsplitfactor 2 &&
+
+ # Create 3 packs forming a geometric progression by
+ # object count such that they are unmodified by the
+ # initial repack. The MIDX chain thusly contains a
+ # single layer with three packs.
+ create_geometric_packs &&
+ ls $packdir/pack-*.idx | sort >packs.before &&
+ test_line_count = 1 $midx_chain &&
+ cp $midx_chain $midx_chain.before &&
+
+ # Repack a new commit. Since the layer threshold is
+ # unmet, a new MIDX layer is added on top of the
+ # existing one.
+ test_commit extra &&
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+ git multi-pack-index verify &&
+
+ ls $packdir/pack-*.idx | sort >packs.after &&
+ comm -13 packs.before packs.after >packs.new &&
+ test_line_count = 1 packs.new &&
+
+ test_line_count = 2 "$midx_chain" &&
+ head -n 1 "$midx_chain.before" >expect &&
+ head -n 1 "$midx_chain" >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success 'above layer threshold, tip packs repacked' '
+ git init above-layer-threshold-tip-packs-repacked &&
+ (
+ cd above-layer-threshold-tip-packs-repacked &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 2 &&
+ git config repack.midxsplitfactor 2 &&
+
+ # Same setup, but with the layer threshold set to 2.
+ # Since the tip MIDX layer meets that threshold, its
+ # packs are considered repack candidates.
+ create_geometric_packs &&
+ cp $midx_chain $midx_chain.before &&
+
+ # Perturb the existing progression such that it is
+ # rolled up into a single new pack, invalidating the
+ # existing MIDX layer and replacing it with a new one.
+ test_commit extra &&
+ git repack -d &&
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ ! test_cmp $midx_chain.before $midx_chain &&
+ test_line_count = 1 $midx_chain &&
+
+ git multi-pack-index verify
+ )
+'
+
+test_expect_success 'above layer threshold, tip layer preserved' '
+ git init above-layer-threshold-tip-layer-preserved &&
+ (
+ cd above-layer-threshold-tip-layer-preserved &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 2 &&
+ git config repack.midxsplitfactor 2 &&
+
+ test_commit_bulk --message="medium" 2 &&
+ test_commit_bulk --message="large" 6 &&
+
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ test_line_count = 1 "$midx_chain" &&
+ ls $packdir/pack-*.idx | sort >packs.before &&
+ cp $midx_chain $midx_chain.before &&
+
+ # Create objects to form a pack satisfying the geometric
+ # progression (thus preserving the tip layer), but not
+ # so large that it meets the layer merging condition.
+ test_commit_bulk --message="small" 1 &&
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ ls $packdir/pack-*.idx | sort >packs.after &&
+ comm -13 packs.before packs.after >packs.new &&
+
+ test_line_count = 1 packs.new &&
+ test_line_count = 3 packs.after &&
+ test_line_count = 2 "$midx_chain" &&
+ head -n 1 "$midx_chain.before" >expect &&
+ head -n 1 "$midx_chain" >actual &&
+ test_cmp expect actual &&
+
+ git multi-pack-index verify
+ )
+'
+
+test_expect_success 'above layer threshold, tip packs preserved' '
+ git init above-layer-threshold-tip-packs-preserved &&
+ (
+ cd above-layer-threshold-tip-packs-preserved &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 2 &&
+ git config repack.midxsplitfactor 2 &&
+
+ create_geometric_packs &&
+ ls $packdir/pack-*.idx | sort >packs.before &&
+ cp $midx_chain $midx_chain.before &&
+
+ # Same setup as above, but this time the new objects do
+ # not satisfy the new layer merging condition, resulting
+ # in a new tip layer.
+ test_commit_bulk --message="huge" 18 &&
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ ls $packdir/pack-*.idx | sort >packs.after &&
+ comm -13 packs.before packs.after >packs.new &&
+
+ ! test_cmp $midx_chain.before $midx_chain &&
+ test_line_count = 1 $midx_chain &&
+ test_line_count = 1 packs.new &&
+
+ git multi-pack-index verify
+ )
+'
+
+test_expect_success 'new tip absorbs multiple layers' '
+ git init new-tip-absorbs-multiple-layers &&
+ (
+ cd new-tip-absorbs-multiple-layers &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 1 &&
+ git config repack.midxsplitfactor 2 &&
+
+ # Build a 4-layer chain where each layer is too small to
+ # absorb the one below it. The sizes must satisfy L(n) <
+ # L(n-1)/2 for each adjacent pair:
+ #
+ # L0 (oldest): 75 obj (25 commits)
+ # L1: 21 obj (7 commits, 21 < 75/2)
+ # L2: 9 obj (3 commits, 9 < 21/2)
+ # L3 (tip): 3 obj (1 commit, 3 < 9/2)
+ create_layers <<-\EOF &&
+ L0 25
+ L1 7
+ L2 3
+ L3 1
+ EOF
+
+ test_line_count = 4 "$midx_chain" &&
+ cp $midx_chain $midx_chain.before &&
+
+ # Now add a new commit. The merging condition is
+ # satisfied between L3-L1, but violated at L0, which is
+ # too large relative to the accumulated size.
+ #
+ # As a result, the chain shrinks from 4 to 2 layers.
+ test_commit new &&
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ ! test_cmp $midx_chain.before $midx_chain &&
+ test_line_count = 2 "$midx_chain" &&
+ git multi-pack-index verify
+ )
+'
+
+test_expect_success 'compaction of older layers' '
+ git init compaction-of-older-layers &&
+ (
+ cd compaction-of-older-layers &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 1 &&
+ git config repack.midxsplitfactor 2 &&
+
+ # Build a chain with two small layers at the bottom
+ # and a larger barrier layer on top, producing a
+ # chain that violates the compaction invariant, since
+ # the two small layers would normally have been merged.
+ create_layers <<-\EOF &&
+ one 2
+ two 4
+ barrier 54
+ EOF
+
+ cp $midx_chain $midx_chain.before &&
+
+ # Running an incremental repack compacts the two
+ # small layers at the bottom of the chain as a
+ # separate step in the compaction plan.
+ test_commit another &&
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ test_line_count = 2 "$midx_chain" &&
+ git multi-pack-index verify
+ )
+'
+
+test_expect_success 'geometric rollup with surviving tip packs' '
+ git init geometric-rollup-with-surviving-tip-packs &&
+ (
+ cd geometric-rollup-with-surviving-tip-packs &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 1 &&
+ git config repack.midxsplitfactor 2 &&
+
+ # Create a pack large enough to anchor the geometric
+ # progression when small packs are added alongside it.
+ create_layer --message="big" 5 &&
+
+ test_line_count = 1 "$midx_chain" &&
+ cp $midx_chain $midx_chain.before &&
+
+ # Repack a small number of objects such that the
+ # progression is unbothered. Note that the existing pack
+ # is considered a repack candidate as the new layer
+ # threshold is set to 1.
+ test_commit small-1 &&
+ git repack -d &&
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ ! test_cmp $midx_chain.before $midx_chain &&
+ cp $midx_chain $midx_chain.before
+ )
+'
+
+test_expect_success 'kept packs are excluded from repack' '
+ git init kept-packs-excluded-from-repack &&
+ (
+ cd kept-packs-excluded-from-repack &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 1 &&
+ git config repack.midxsplitfactor 2 &&
+
+ # Create two equal-sized packs, marking one as kept.
+ for i in A B
+ do
+ test_commit "$i" && git repack -d || return 1
+ done &&
+
+ keep=$(ls $packdir/pack-*.idx | head -n 1) &&
+ touch "${keep%.idx}.keep" &&
+
+ # The kept pack is excluded as a repacking candidate
+ # entirely, so no rollup occurs as there is only one
+ # non-kept pack. A new MIDX layer is written containing
+ # that pack.
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ test-tool read-midx $objdir >actual &&
+ grep "^pack-.*\.idx$" actual >actual.packs &&
+ test_line_count = 1 actual.packs &&
+ test_grep ! "$keep" actual.packs &&
+
+ git multi-pack-index verify &&
+
+ # All objects (from both kept and non-kept packs)
+ # must still be accessible.
+ git fsck
+ )
+'
+
+test_expect_success 'incremental MIDX with --max-pack-size' '
+ git init incremental-midx-with--max-pack-size &&
+ (
+ cd incremental-midx-with--max-pack-size &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 1 &&
+ git config repack.midxsplitfactor 2 &&
+
+ create_layer --message="base" 1 &&
+
+ # Now add enough data that a small --max-pack-size will
+ # cause pack-objects to split its output. Create objects
+ # large enough to fill multiple packs.
+ test-tool genrandom foo 1M >big1 &&
+ test-tool genrandom bar 1M >big2 &&
+ git add big1 big2 &&
+ test_tick &&
+ git commit -a -m "big blobs" &&
+ git repack -d &&
+
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index --max-pack-size=1M &&
+
+ test_line_count = 1 "$midx_chain" &&
+ test-tool read-midx $objdir >actual &&
+ grep "^pack-.*\.idx$" actual >actual.packs &&
+ test_line_count -gt 1 actual.packs &&
+
+ git multi-pack-index verify
+ )
+'
+
+test_expect_success 'noop repack preserves valid MIDX chain' '
+ git init noop-repack-preserves-valid-midx-chain &&
+ (
+ cd noop-repack-preserves-valid-midx-chain &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 1 &&
+ git config repack.midxsplitfactor 2 &&
+
+ create_layer --message="base" 1 &&
+
+ git multi-pack-index verify &&
+ cp $midx_chain $midx_chain.before &&
+
+ # Running again with no new objects should not break
+ # the MIDX chain. It produces "Nothing new to pack."
+ git repack --geometric=2 -d --write-midx=incremental \
+ --write-bitmap-index &&
+
+ test_cmp $midx_chain.before $midx_chain &&
+
+ git multi-pack-index verify &&
+ git fsck
+ )
+'
+
+test_expect_success 'repack -ad removes stale incremental chain' '
+ git init repack--ad-removes-stale-incremental-chain &&
+ (
+ cd repack--ad-removes-stale-incremental-chain &&
+
+ git config maintenance.auto false &&
+ git config repack.midxnewlayerthreshold 1 &&
+ git config repack.midxsplitfactor 2 &&
+
+ create_layers <<-\EOF &&
+ one 1
+ two 1
+ EOF
+
+ test_path_is_file $midx_chain &&
+ test_line_count = 2 $midx_chain &&
+
+ git repack -ad &&
+
+ test_path_is_missing $packdir/multi-pack-index &&
+ test_dir_is_empty $midxdir
+ )
+'
+
+test_expect_success 'repack rejects invalid midxSplitFactor' '
+ test_when_finished "rm -fr bad-split-factor" &&
+ git init bad-split-factor &&
+ (
+ cd bad-split-factor &&
+ test_commit base &&
+
+ for v in 0 1 -1
+ do
+ test_must_fail git -c repack.midxSplitFactor=$v \
+ repack -d --geometric=2 --write-midx=incremental 2>err &&
+ test_grep "invalid value for --midx-split-factor" err ||
+ return 1
+ done
+ )
+'
+
+test_expect_success 'repack rejects invalid midxNewLayerThreshold' '
+ test_when_finished "rm -fr bad-layer-threshold" &&
+ git init bad-layer-threshold &&
+ (
+ cd bad-layer-threshold &&
+ test_commit base &&
+
+ for v in 0 -1
+ do
+ test_must_fail git -c repack.midxNewLayerThreshold=$v \
+ repack -d --geometric=2 --write-midx=incremental 2>err &&
+ test_grep "invalid value for --midx-new-layer-threshold" err ||
+ return 1
+ done
+ )
+'
+
+test_done
--
2.54.0.16.g1c05dfce579
^ permalink raw reply related
* [PATCH v3 16/16] repack: allow `--write-midx=incremental` without `--geometric`
From: Taylor Blau @ 2026-04-30 0:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1777507303.git.me@ttaylorr.com>
Previously, `--write-midx=incremental` required `--geometric` and would
die() without it. Relax this restriction so that incremental MIDX
repacking can be used independently.
Without `--geometric`, the behavior is append-only: a single new MIDX
layer is created containing whatever packs were written by the repack
and appended to the existing chain (or a new chain is started). Existing
layers are preserved as-is with no compaction or merging.
Implement this via a new repack_make_midx_append_plan() that builds a
plan consisting of a WRITE step for the freshly written packs followed
by COPY steps for every existing MIDX layer. The existing compaction
plan (repack_make_midx_compaction_plan) is used only when `--geometric`
is active.
Update the documentation to describe the behavior with and without
`--geometric`, and replace the test that enforced the old restriction
with one exercising append-only incremental MIDX repacking.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-repack.adoc | 19 +++++----
builtin/repack.c | 3 --
repack-midx.c | 64 +++++++++++++++++++++++++++--
t/t7705-repack-incremental-midx.sh | 65 +++++++++++++++++++++++++++---
4 files changed, 133 insertions(+), 18 deletions(-)
diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc
index 27a99cc46f4..72c42015e23 100644
--- a/Documentation/git-repack.adoc
+++ b/Documentation/git-repack.adoc
@@ -263,14 +263,19 @@ linkgit:git-multi-pack-index[1]).
`incremental`;;
Write an incremental MIDX chain instead of a single
- flat MIDX. This mode requires `--geometric`.
+ flat MIDX.
+
-The incremental mode maintains a chain of MIDX layers that is compacted
-over time using a geometric merging strategy. Each repack creates a new
-tip layer containing the newly written pack(s). Adjacent layers are then
-merged whenever the newer layer's object count exceeds
-`1/repack.midxSplitFactor` of the next deeper layer's count. Layers
-that do not meet this condition are retained as-is.
+Without `--geometric`, a new MIDX layer is appended to the existing
+chain (or a new chain is started) containing whatever packs were written
+by the repack. Existing layers are preserved as-is.
++
+When combined with `--geometric`, the incremental mode maintains a chain
+of MIDX layers that is compacted over time using a geometric merging
+strategy. Each repack creates a new tip layer containing the newly
+written pack(s). Adjacent layers are then merged whenever the newer
+layer's object count exceeds `1/repack.midxSplitFactor` of the next
+deeper layer's count. Layers that do not meet this condition are
+retained as-is.
+
The result is that newer (tip) layers tend to contain many small packs
with relatively few objects, while older (deeper) layers contain fewer,
diff --git a/builtin/repack.c b/builtin/repack.c
index 5ffa18e085e..1524a9c13ad 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -263,9 +263,6 @@ int cmd_repack(int argc,
if (pack_everything & PACK_CRUFT)
pack_everything |= ALL_INTO_ONE;
- if (write_midx == REPACK_WRITE_MIDX_INCREMENTAL && !geometry.split_factor)
- die(_("--write-midx=incremental requires --geometric"));
-
if (write_bitmaps < 0) {
if (write_midx == REPACK_WRITE_MIDX_NONE &&
(!(pack_everything & ALL_INTO_ONE) || !is_bare_repository()))
diff --git a/repack-midx.c b/repack-midx.c
index bb3bee03ace..799f2b03697 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -548,6 +548,60 @@ static void midx_compaction_step_release(struct midx_compaction_step *step)
free(step->csum);
}
+/*
+ * Build an append-only MIDX plan: a single WRITE step for the freshly
+ * written packs, plus COPY steps for every existing layer. No
+ * compaction or merging is performed.
+ */
+static void repack_make_midx_append_plan(struct repack_write_midx_opts *opts,
+ struct midx_compaction_step **steps_p,
+ size_t *steps_nr_p)
+{
+ struct multi_pack_index *m;
+ struct midx_compaction_step *steps = NULL;
+ struct midx_compaction_step *step;
+ size_t steps_nr = 0, steps_alloc = 0;
+
+ odb_reprepare(opts->existing->repo->objects);
+ m = get_multi_pack_index(opts->existing->source);
+
+ if (opts->names->nr) {
+ struct strbuf buf = STRBUF_INIT;
+ uint32_t i;
+
+ ALLOC_GROW(steps, st_add(steps_nr, 1), steps_alloc);
+
+ step = &steps[steps_nr++];
+ memset(step, 0, sizeof(*step));
+
+ step->type = MIDX_COMPACTION_STEP_WRITE;
+ string_list_init_dup(&step->u.write);
+
+ for (i = 0; i < opts->names->nr; i++) {
+ strbuf_reset(&buf);
+ strbuf_addf(&buf, "pack-%s.idx",
+ opts->names->items[i].string);
+ string_list_append(&step->u.write, buf.buf);
+ }
+
+ strbuf_release(&buf);
+ }
+
+ for (; m; m = m->base_midx) {
+ ALLOC_GROW(steps, st_add(steps_nr, 1), steps_alloc);
+
+ step = &steps[steps_nr++];
+ memset(step, 0, sizeof(*step));
+
+ step->type = MIDX_COMPACTION_STEP_COPY;
+ step->u.copy = m;
+ step->objects_nr = m->num_objects;
+ }
+
+ *steps_p = steps;
+ *steps_nr_p = steps_nr;
+}
+
static int repack_make_midx_compaction_plan(struct repack_write_midx_opts *opts,
struct midx_compaction_step **steps_p,
size_t *steps_nr_p)
@@ -904,9 +958,13 @@ static int write_midx_incremental(struct repack_write_midx_opts *opts)
goto done;
}
- if (repack_make_midx_compaction_plan(opts, &steps, &steps_nr) < 0) {
- ret = error(_("unable to generate compaction plan"));
- goto done;
+ if (opts->geometry->split_factor) {
+ if (repack_make_midx_compaction_plan(opts, &steps, &steps_nr) < 0) {
+ ret = error(_("unable to generate compaction plan"));
+ goto done;
+ }
+ } else {
+ repack_make_midx_append_plan(opts, &steps, &steps_nr);
}
for (i = 0; i < steps_nr; i++) {
diff --git a/t/t7705-repack-incremental-midx.sh b/t/t7705-repack-incremental-midx.sh
index 9e317ff6e8f..25a8c40e8ee 100755
--- a/t/t7705-repack-incremental-midx.sh
+++ b/t/t7705-repack-incremental-midx.sh
@@ -63,10 +63,36 @@ create_layers () {
done
}
-test_expect_success '--write-midx=incremental requires --geometric' '
- test_must_fail git repack --write-midx=incremental 2>err &&
+test_expect_success '--write-midx=incremental without --geometric' '
+ git init incremental-without-geometric &&
+ (
+ cd incremental-without-geometric &&
- test_grep -- "--write-midx=incremental requires --geometric" err
+ git config maintenance.auto false &&
+
+ test_commit first &&
+ git repack -d &&
+
+ test_commit second &&
+ git repack --write-midx=incremental &&
+
+ git multi-pack-index verify &&
+ test_line_count = 1 $midx_chain &&
+ cp $midx_chain $midx_chain.before &&
+
+ # A second repack appends a new layer without
+ # disturbing the existing one.
+ test_commit third &&
+ git repack --write-midx=incremental &&
+
+ git multi-pack-index verify &&
+ test_line_count = 2 $midx_chain &&
+ head -n 1 $midx_chain.before >expect &&
+ head -n 1 $midx_chain >actual &&
+ test_cmp expect actual &&
+
+ git fsck
+ )
'
test_expect_success 'below layer threshold, tip packs excluded' '
@@ -334,8 +360,7 @@ test_expect_success 'kept packs are excluded from repack' '
# entirely, so no rollup occurs as there is only one
# non-kept pack. A new MIDX layer is written containing
# that pack.
- git repack --geometric=2 -d --write-midx=incremental \
- --write-bitmap-index &&
+ git repack --geometric=2 -d --write-midx=incremental &&
test-tool read-midx $objdir >actual &&
grep "^pack-.*\.idx$" actual >actual.packs &&
@@ -433,6 +458,36 @@ test_expect_success 'repack -ad removes stale incremental chain' '
)
'
+test_expect_success 'repack -ad --write-midx=incremental is safe' '
+ git init ad-incremental-midx &&
+ (
+ cd ad-incremental-midx &&
+
+ git config maintenance.auto false &&
+
+ # Build a MIDX chain with multiple layers referencing
+ # distinct packs.
+ test_commit first &&
+ git repack -d &&
+
+ test_commit second &&
+ git repack -d --write-midx=incremental &&
+
+ git multi-pack-index verify &&
+ test_line_count = 1 $midx_chain &&
+
+ # Now do a full -ad repack. The new pack contains all
+ # objects, but any retained MIDX layers still reference
+ # the now-deleted packs.
+ test_commit third &&
+ git repack -ad --write-midx=incremental &&
+
+ git multi-pack-index verify &&
+ git fsck &&
+ git rev-list --all --objects >/dev/null
+ )
+'
+
test_expect_success 'repack rejects invalid midxSplitFactor' '
test_when_finished "rm -fr bad-split-factor" &&
git init bad-split-factor &&
--
2.54.0.16.g1c05dfce579
^ permalink raw reply related
* Re: [PATCH v3 5/5] format-rev: introduce builtin for on-demand pretty formatting
From: Kristoffer Haugsbakk @ 2026-04-30 6:23 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble
In-Reply-To: <374661c1-4676-4538-af24-0564f38469ca@app.fastmail.com>
On Wed, Apr 29, 2026, at 15:41, Kristoffer Haugsbakk wrote:
> On Wed, Apr 29, 2026, at 00:25, kristofferhaugsbakk@fastmail.com wrote:
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>[snip]
>
> CI returned an error.
>
> builtin/name-rev.c:893:25: error: ‘commit’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> 893 | get_format_rev(commit, &format_pp,
> &scratch_buf);
> |
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Now Im using `config.mak.dev`.
^ permalink raw reply
* [PATCH v3 0/6] ci: GitHub Actions updates (brought to you by Dependabot)
From: Johannes Schindelin via GitGitGadget @ 2026-04-30 7:34 UTC (permalink / raw)
To: git; +Cc: Christoph Grüninger, Johannes Schindelin
In-Reply-To: <pull.2097.v2.git.1777114720.gitgitgadget@gmail.com>
Dependabot (which my voice-typing software frequently mis-translates to "the
panda bot" 😉) is enabled in Git for Windows' fork of the git/git repository
to lighten the maintenance burden a little bit. Frequently, the updates are
not actually for Git for Windows' patches on top of git/git, but apply
directly to git/git.
Here is the latest batch of those updates, with heavily augmented commit
messages.
Changes since v2:
* Included the version bump for the freshly-updated
setup-git-for-windows-sdk GitHub Action (which now also requires Node.JS
24, at long last).
Changes since v1:
* Also bump mshick/add-pr-comment to the newest major version.
Johannes Schindelin (6):
ci: bump microsoft/setup-msbuild from v2 to v3
ci: bump actions/{upload,download}-artifact to v7 and v8
ci: bump actions/github-script from v8 to v9
ci: bump actions/checkout from v5 to v6
ci: bump git-for-windows/setup-git-for-windows-sdk from v1 to v2
l10n: bump mshick/add-pr-comment from v2 to v3
.github/workflows/check-style.yml | 2 +-
.github/workflows/check-whitespace.yml | 2 +-
.github/workflows/coverity.yml | 4 +-
.github/workflows/l10n.yml | 2 +-
.github/workflows/main.yml | 58 +++++++++++++-------------
5 files changed, 34 insertions(+), 34 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2097%2Fdscho%2Fdependabot-updates-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2097/dscho/dependabot-updates-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2097
Range-diff vs v2:
1: 0d2fdc1cf4 = 1: 0d2fdc1cf4 ci: bump microsoft/setup-msbuild from v2 to v3
2: 5d719b3729 = 2: 5d719b3729 ci: bump actions/{upload,download}-artifact to v7 and v8
3: bfbe0db67f = 3: bfbe0db67f ci: bump actions/github-script from v8 to v9
4: 5694ca1016 = 4: 5694ca1016 ci: bump actions/checkout from v5 to v6
-: ---------- > 5: c6e8df1eff ci: bump git-for-windows/setup-git-for-windows-sdk from v1 to v2
5: faa83723f4 = 6: b9ccb66405 l10n: bump mshick/add-pr-comment from v2 to v3
--
gitgitgadget
^ permalink raw reply
* [PATCH v3 1/6] ci: bump microsoft/setup-msbuild from v2 to v3
From: Johannes Schindelin via GitGitGadget @ 2026-04-30 7:34 UTC (permalink / raw)
To: git; +Cc: Christoph Grüninger, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2097.v3.git.1777534500.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The v2 of `microsoft/setup-msbuild` runs on Node.js 20, which GitHub
is phasing out of the Actions runners. v3 is a minimal release whose
only substantive change is moving the action's runtime to Node.js 24,
so that our Visual Studio build jobs keep working once Node.js 20 is
removed from the runners.
The risk of this bump is very low: v3 contains no functional changes
to the action itself -- it merely adds `msbuild.exe` to `PATH`, with
no change to command-line flags, inputs, outputs, or default tool
resolution. The only precondition is a recent-enough Actions Runner,
which the github.com-hosted runners already satisfy.
See also:
- Release notes: https://github.com/microsoft/setup-msbuild/releases
- Compare: https://github.com/microsoft/setup-msbuild/compare/v2...v3
Originally-authored-by: dependabot[bot] <support@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 6f3d94e3a6..0d3e0e42a4 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -186,7 +186,7 @@ jobs:
repository: git/git
definitionId: 9
- name: add msbuild to PATH
- uses: microsoft/setup-msbuild@v2
+ uses: microsoft/setup-msbuild@v3
- name: copy dlls to root
shell: cmd
run: compat\vcbuild\vcpkg_copy_dlls.bat release
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 3/6] ci: bump actions/github-script from v8 to v9
From: Johannes Schindelin via GitGitGadget @ 2026-04-30 7:34 UTC (permalink / raw)
To: git; +Cc: Christoph Grüninger, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2097.v3.git.1777534500.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The only use we have of `actions/github-script` is the "skip if the
commit or tree was already tested" step in `main.yml`, which checks
whether an identical tree-SHA was already built successfully. It
currently pins v8; v9 is the latest release.
What v9 changes:
- The `ACTIONS_ORCHESTRATION_ID` environment variable is now
appended to the HTTP user-agent string. This is transparent to
our script.
- A new injected `getOctokit` factory lets scripts create
additional authenticated clients in the same step without
importing `@actions/github`. We do not use it.
- Two breaking changes affect scripts that either call
`require('@actions/github')` (fails at runtime, because
`@actions/github` v9 is now ESM-only) or that shadow the
implicit `getOctokit` parameter via `const`/`let` (syntax
error). Our script does neither -- it only uses the pre-supplied
`github` REST client and `core` helpers -- so the upgrade is
safe.
Risk analysis: the step is advisory. It sets `enabled=' but skip'`
as an optimization to avoid re-running CI on a tree that was already
tested successfully. Even if the v9 upgrade broke the script, the
surrounding `try { ... } catch (e) { core.warning(e); }` block would
degrade it to a warning and CI would still run normally. In practice
the script continues to work identically on v9.
See also:
- Release notes: https://github.com/actions/github-script/releases
- Compare: https://github.com/actions/github-script/compare/v8...v9
Originally-authored-by: dependabot[bot] <support@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index da31b10c79..6d7f26e71e 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -63,7 +63,7 @@ jobs:
echo "skip_concurrent=$skip_concurrent" >>$GITHUB_OUTPUT
- name: skip if the commit or tree was already tested
id: skip-if-redundant
- uses: actions/github-script@v8
+ uses: actions/github-script@v9
if: steps.check-ref.outputs.enabled == 'yes'
with:
github-token: ${{secrets.GITHUB_TOKEN}}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 2/6] ci: bump actions/{upload,download}-artifact to v7 and v8
From: Johannes Schindelin via GitGitGadget @ 2026-04-30 7:34 UTC (permalink / raw)
To: git; +Cc: Christoph Grüninger, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2097.v3.git.1777534500.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
`actions/upload-artifact` and `actions/download-artifact` are tightly
coupled: the upload action writes artifact archives in a format that
the download action then reads. Because of this coupling, the two
actions should always be bumped together so that the artifact format
contract between them is satisfied.
All of our `actions/upload-artifact` uses are still on v5, with one
stray v4 occurrence. Keeping them on these versions would leave the
artifact-upload steps running on Node.js 20, which GitHub is phasing
out, and would eventually cause all upload steps to fail.
Going from v5 directly to v7 folds in two release bumps:
- v6 switches the action's default runtime from Node.js 20 to
Node.js 24 (v5 had preliminary Node 24 support but still defaulted
to Node 20). This is the main motivation for bumping now: it gets
us off the deprecated runtime.
- v7 adds two opt-in features: direct (unzipped) single-file uploads
via a new `archive: false` parameter, and an internal conversion of
the action to ESM to match the updated `@actions/*` packages.
Risk analysis: we never pass `archive`, so the zip-as-usual behavior
is unchanged. We also do not `require('@actions/*')` from any calling
workflow, so the ESM migration cannot affect us. The upload steps we
care about -- tracked files/build artifacts and failing-test
directories -- keep the same inputs (`name`, `path`) and outputs, so
the diff is purely the `@vN` identifier. The main precondition is a
recent Actions Runner (>= 2.327.1), which the github.com-hosted
runners used by our CI already satisfy.
While at it, align the one remaining `@v4` occurrence with the rest
so that every `upload-artifact` step uses the same version.
See also:
- Release notes: https://github.com/actions/upload-artifact/releases
- Compare: https://github.com/actions/upload-artifact/compare/v5...v7
We use `actions/download-artifact` to pass build artifacts between
the "windows-build" / "vs-build" / "windows-meson-build" jobs and
their corresponding test jobs. All callers are currently on v6;
bumping to v8 keeps this action in lockstep with the `upload-artifact`
bump above.
What v7 and v8 change:
- v7 switches the default runtime from Node.js 20 to Node.js 24 (v6
had preliminary Node 24 support but still defaulted to Node 20).
This is the main motivation: it gets us off the deprecated runtime.
- v8 makes three further changes:
* The package is converted to ESM (invisible to workflow authors).
* The action now checks the `Content-Type` header before
attempting to unzip a download, so that directly-uploaded
(unzipped) artifacts from `upload-artifact` v7 are downloaded
correctly.
* The `digest-mismatch` behaviour is changed from warn-and-
continue to a hard failure by default.
Risk analysis: defaulting hash-mismatch to a hard failure is
strictly safer than the previous warn-and-continue behaviour -- a
mismatch points to real corruption or tampering and should stop the
run. We download archives that the same workflow just uploaded, on
the same runner fleet, so false positives are not expected. Our
usage is limited to the `name` and `path` inputs, which are
unchanged between v6 and v8, so the diff is purely the `@vN`
identifier.
See also:
- Release notes: https://github.com/actions/download-artifact/releases
- Compare: https://github.com/actions/download-artifact/compare/v6...v8
Originally-authored-by: dependabot[bot] <support@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0d3e0e42a4..da31b10c79 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -123,7 +123,7 @@ jobs:
- name: zip up tracked files
run: git archive -o artifacts/tracked.tar.gz HEAD
- name: upload tracked files and build artifacts
- uses: actions/upload-artifact@v5
+ uses: actions/upload-artifact@v7
with:
name: windows-artifacts
path: artifacts
@@ -140,7 +140,7 @@ jobs:
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- name: download tracked files and build artifacts
- uses: actions/download-artifact@v6
+ uses: actions/download-artifact@v8
with:
name: windows-artifacts
path: ${{github.workspace}}
@@ -157,7 +157,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v5
+ uses: actions/upload-artifact@v7
with:
name: failed-tests-windows-${{ matrix.nr }}
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -208,7 +208,7 @@ jobs:
- name: zip up tracked files
run: git archive -o artifacts/tracked.tar.gz HEAD
- name: upload tracked files and build artifacts
- uses: actions/upload-artifact@v5
+ uses: actions/upload-artifact@v7
with:
name: vs-artifacts
path: artifacts
@@ -226,7 +226,7 @@ jobs:
steps:
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: download tracked files and build artifacts
- uses: actions/download-artifact@v6
+ uses: actions/download-artifact@v8
with:
name: vs-artifacts
path: ${{github.workspace}}
@@ -244,7 +244,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v5
+ uses: actions/upload-artifact@v7
with:
name: failed-tests-windows-vs-${{ matrix.nr }}
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -270,7 +270,7 @@ jobs:
shell: pwsh
run: meson compile -C build
- name: Upload build artifacts
- uses: actions/upload-artifact@v5
+ uses: actions/upload-artifact@v7
with:
name: windows-meson-artifacts
path: build
@@ -292,7 +292,7 @@ jobs:
shell: pwsh
run: pip install meson ninja
- name: Download build artifacts
- uses: actions/download-artifact@v6
+ uses: actions/download-artifact@v8
with:
name: windows-meson-artifacts
path: build
@@ -305,7 +305,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v4
+ uses: actions/upload-artifact@v7
with:
name: failed-tests-windows-meson-${{ matrix.nr }}
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -349,7 +349,7 @@ jobs:
run: ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v5
+ uses: actions/upload-artifact@v7
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
@@ -449,7 +449,7 @@ jobs:
run: sudo --preserve-env --set-home --user=builder ci/print-test-failures.sh
- name: Upload failed tests' directories
if: failure() && env.FAILED_TEST_ARTIFACTS != ''
- uses: actions/upload-artifact@v5
+ uses: actions/upload-artifact@v7
with:
name: failed-tests-${{matrix.vector.jobname}}
path: ${{env.FAILED_TEST_ARTIFACTS}}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 4/6] ci: bump actions/checkout from v5 to v6
From: Johannes Schindelin via GitGitGadget @ 2026-04-30 7:34 UTC (permalink / raw)
To: git; +Cc: Christoph Grüninger, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2097.v3.git.1777534500.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Every workflow currently pins `actions/checkout` to v5, which was
introduced primarily to move to the Node.js 24 runtime. v6 is the
next release and worth picking up so we stay on a maintained version
of the action.
The one behaviorally interesting change in v6:
`persist-credentials` now stores the helper credentials under
`$RUNNER_TEMP` instead of writing them directly into the local
`.git/config`. Two implications follow:
1. In the normal case this is an unambiguous improvement -- the
token no longer lands in `.git/config`, reducing the risk of
inadvertently leaking it through workspace archiving
(`upload-artifact` snapshots, cache entries, core dumps, ...).
2. Docker container actions require an Actions Runner of at least
v2.329.0 to find the credentials in their new location. The
github.com-hosted runners our CI uses are already past that
version, so this does not affect us. Downstream users running
self-hosted runners may need to update them before adopting
this version of the action.
Risk analysis: our checkout steps either check out the default
repository (no special credential requirements) or, in the `vs-build`
job, explicitly set `repository: microsoft/vcpkg` and
`path: compat/vcbuild/vcpkg`. Neither case relies on the precise
location of the persisted credentials -- subsequent steps interact
with the API via the runner-provided `GITHUB_TOKEN` directly -- so
the v6 credential-storage change is transparent to our workflows.
The diff is purely the `@vN` identifier; there are no input or
output changes.
See also:
- Release notes: https://github.com/actions/checkout/releases
- Changelog: https://github.com/actions/checkout/blob/main/CHANGELOG.md
- Compare: https://github.com/actions/checkout/compare/v5...v6
Originally-authored-by: dependabot[bot] <support@github.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/check-style.yml | 2 +-
.github/workflows/check-whitespace.yml | 2 +-
.github/workflows/coverity.yml | 2 +-
.github/workflows/main.yml | 24 ++++++++++++------------
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/.github/workflows/check-style.yml b/.github/workflows/check-style.yml
index 19a145d4ad..108a2de903 100644
--- a/.github/workflows/check-style.yml
+++ b/.github/workflows/check-style.yml
@@ -20,7 +20,7 @@ jobs:
jobname: ClangFormat
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
with:
fetch-depth: 0
diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml
index 928fd4cfe2..ea6f49f742 100644
--- a/.github/workflows/check-whitespace.yml
+++ b/.github/workflows/check-whitespace.yml
@@ -19,7 +19,7 @@ jobs:
check-whitespace:
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
with:
fetch-depth: 0
diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 3435baeca2..89bef26727 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -38,7 +38,7 @@ jobs:
COVERITY_LANGUAGE: cxx
COVERITY_PLATFORM: overridden-below
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: install minimal Git for Windows SDK
if: contains(matrix.os, 'windows')
uses: git-for-windows/setup-git-for-windows-sdk@v1
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 6d7f26e71e..0ea266f27c 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -112,7 +112,7 @@ jobs:
group: windows-build-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: build
shell: bash
@@ -173,10 +173,10 @@ jobs:
group: vs-build-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- uses: git-for-windows/setup-git-for-windows-sdk@v1
- name: initialize vcpkg
- uses: actions/checkout@v5
+ uses: actions/checkout@v6
with:
repository: 'microsoft/vcpkg'
path: 'compat/vcbuild/vcpkg'
@@ -258,7 +258,7 @@ jobs:
group: windows-meson-build-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- uses: actions/setup-python@v6
- name: Set up dependencies
shell: pwsh
@@ -286,7 +286,7 @@ jobs:
group: windows-meson-test-${{ matrix.nr }}-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- uses: actions/setup-python@v6
- name: Set up dependencies
shell: pwsh
@@ -341,7 +341,7 @@ jobs:
TEST_OUTPUT_DIRECTORY: ${{github.workspace}}/t
runs-on: ${{matrix.vector.pool}}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- run: ci/install-dependencies.sh
- run: ci/run-build-and-tests.sh
- name: print test failures
@@ -362,7 +362,7 @@ jobs:
CI_JOB_IMAGE: ubuntu-latest
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- run: ci/install-dependencies.sh
- run: ci/run-build-and-minimal-fuzzers.sh
dockerized:
@@ -439,7 +439,7 @@ jobs:
else
apt-get -q update && apt-get -q -y install git
fi
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- run: ci/install-dependencies.sh
- run: useradd builder --create-home
- run: chown -R builder .
@@ -464,7 +464,7 @@ jobs:
group: static-analysis-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- run: ci/install-dependencies.sh
- run: ci/run-static-analysis.sh
- run: ci/check-directional-formatting.bash
@@ -480,7 +480,7 @@ jobs:
group: rust-analysis-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- run: ci/install-dependencies.sh
- run: ci/run-rust-checks.sh
sparse:
@@ -494,7 +494,7 @@ jobs:
group: sparse-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- name: Install other dependencies
run: ci/install-dependencies.sh
- run: make sparse
@@ -510,6 +510,6 @@ jobs:
CI_JOB_IMAGE: ubuntu-latest
runs-on: ubuntu-latest
steps:
- - uses: actions/checkout@v5
+ - uses: actions/checkout@v6
- run: ci/install-dependencies.sh
- run: ci/test-documentation.sh
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 5/6] ci: bump git-for-windows/setup-git-for-windows-sdk from v1 to v2
From: Johannes Schindelin via GitGitGadget @ 2026-04-30 7:34 UTC (permalink / raw)
To: git; +Cc: Christoph Grüninger, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2097.v3.git.1777534500.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The v1 of `git-for-windows/setup-git-for-windows-sdk` runs on
Node.js 20, which GitHub is phasing out of the Actions runners.
v2 moves the action to Node.js 24 so that the CI jobs relying on
a Git for Windows SDK keep working once Node.js 20 is removed.
The risk is very low: v2 contains no functional changes to the
SDK setup itself, only the runtime upgrade. The action still
provisions the same minimal SDK and exposes the same outputs.
The sole precondition is a recent Actions Runner (>= 2.327.1),
which the github.com-hosted runners already satisfy.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/coverity.yml | 2 +-
.github/workflows/main.yml | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 89bef26727..58a78f1eb3 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -41,7 +41,7 @@ jobs:
- uses: actions/checkout@v6
- name: install minimal Git for Windows SDK
if: contains(matrix.os, 'windows')
- uses: git-for-windows/setup-git-for-windows-sdk@v1
+ uses: git-for-windows/setup-git-for-windows-sdk@v2
- run: ci/install-dependencies.sh
if: contains(matrix.os, 'ubuntu') || contains(matrix.os, 'macos')
env:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 0ea266f27c..3da5326f0b 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -113,7 +113,7 @@ jobs:
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- uses: actions/checkout@v6
- - uses: git-for-windows/setup-git-for-windows-sdk@v1
+ - uses: git-for-windows/setup-git-for-windows-sdk@v2
- name: build
shell: bash
env:
@@ -147,7 +147,7 @@ jobs:
- name: extract tracked files and build artifacts
shell: bash
run: tar xf artifacts.tar.gz && tar xf tracked.tar.gz
- - uses: git-for-windows/setup-git-for-windows-sdk@v1
+ - uses: git-for-windows/setup-git-for-windows-sdk@v2
- name: test
shell: bash
run: . /etc/profile && ci/run-test-slice.sh $((${{matrix.nr}} + 1)) 10
@@ -174,7 +174,7 @@ jobs:
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- uses: actions/checkout@v6
- - uses: git-for-windows/setup-git-for-windows-sdk@v1
+ - uses: git-for-windows/setup-git-for-windows-sdk@v2
- name: initialize vcpkg
uses: actions/checkout@v6
with:
@@ -224,7 +224,7 @@ jobs:
group: vs-test-${{ matrix.nr }}-${{ github.ref }}
cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }}
steps:
- - uses: git-for-windows/setup-git-for-windows-sdk@v1
+ - uses: git-for-windows/setup-git-for-windows-sdk@v2
- name: download tracked files and build artifacts
uses: actions/download-artifact@v8
with:
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 6/6] l10n: bump mshick/add-pr-comment from v2 to v3
From: Johannes Schindelin via GitGitGadget @ 2026-04-30 7:35 UTC (permalink / raw)
To: git; +Cc: Christoph Grüninger, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2097.v3.git.1777534500.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The l10n workflow uses `mshick/add-pr-comment` to post git-po-helper
reports as comments on translation pull requests. It was still pinned
to v2, which runs on Node.js 20. GitHub is phasing out the Node.js 20
runtime on Actions runners, so staying on v2 will eventually cause the
"Create comment in pull request for report" step to fail.
The sole breaking change in v3 is the switch from Node.js 20 to
Node.js 24 (https://github.com/mshick/add-pr-comment/releases/tag/v3.0.0).
The action's inputs and outputs are unchanged, so the upgrade is a
drop-in replacement. Subsequent v3.x releases added new opt-in
features (message truncation, retry with exponential backoff, file
attachments, commit comment support, "delete on status") but none of
them affect existing callers that do not opt in.
See also:
- Changelog: https://github.com/mshick/add-pr-comment/blob/main/CHANGELOG.md
- Compare: https://github.com/mshick/add-pr-comment/compare/v2...v3
Pointed-out-by: Christoph Grüninger <foss@grueninger.de>
Assisted-by: Claude Opus 4.6
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/l10n.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.github/workflows/l10n.yml b/.github/workflows/l10n.yml
index 95e55134bd..114a12a9e5 100644
--- a/.github/workflows/l10n.yml
+++ b/.github/workflows/l10n.yml
@@ -92,7 +92,7 @@ jobs:
cat git-po-helper.out
exit $exit_code
- name: Create comment in pull request for report
- uses: mshick/add-pr-comment@v2
+ uses: mshick/add-pr-comment@v3
if: >-
always() &&
github.event_name == 'pull_request_target' &&
--
gitgitgadget
^ permalink raw reply related
* git interactive rebase does not allow editing commits at once anymore
From: David Jordanes @ 2026-04-30 7:46 UTC (permalink / raw)
To: git@vger.kernel.org
Git version: git version 2.53.0.windows.2
OS: Windows 11
Steps to reproduce:
1. Create a dummy repo
2. Create a couple of commits
2. Run git rebase -i HEAD~2
Observed:
Lines in todo appear as:
pick <hash> # commit 1
pick <hash> # commit 2
After editing:
reword <hash> commit A
reword <hash> commit B
Then git loops each commit to edit.
Expected:
After editing, all changes should be applied at once (no loop).
Question:
Is this intended behavior? By whom and why this flow was decided? What problem it solves? If I have to rebase interactively 10 or 15 commits I have to go through all those commits one by one???
^ permalink raw reply
* Re: [Bug] fetch --deepen truncates history in v2.54.0
From: Mikael Magnusson @ 2026-04-30 9:10 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Owen Stephens, git
In-Reply-To: <CALnO6CBzd0coeyJ9B+EkGWsSNEVTdVLvcVmEraGNxnUm5wXy=g@mail.gmail.com>
On Wed, Apr 29, 2026 at 3:23 PM D. Ben Knoble <ben.knoble@gmail.com> wrote:
>
> On Wed, Apr 29, 2026 at 7:27 AM Owen Stephens <owen@owenstephens.co.uk> wrote:
> >
> > > What did you do before the bug happened? (Steps to reproduce your issue)
> >
> > Repeatedy called `git fetch --deepen 2` inside a shallow repo that was a
> > file:// clone of another repo. Once all commits had been fetched, a subsequent
> > `fetch --deepen` appears to "reset" the repo back to being shallow with a depth
> > of 2. A reproduction script is included below. This issue appears to have been
> > introduced in v2.54.0.
> >
> > > What did you expect to happen? (Expected behavior)
> >
> > I expected `git fetch --deepen` in a non-shallow repo with no upstream commits
> > to be a no-op.
>
> Here's the relevant part of git-fetch(1):
>
> --depth=<depth>
> Limit fetching to the specified number of commits from the tip of
> each remote branch history. If fetching to a shallow repository
> created by git clone with --depth=<depth> option (see git-clone(1)),
> deepen or shorten the history to the specified number of commits.
> Tags for the deepened commits are not fetched.
>
> --deepen=<depth>
> Similar to --depth, except it specifies the number of commits from
> the current shallow boundary instead of from the tip of each remote
> branch history.
>
> I can see how one might read this as implying that when fetching in a
> non-shallow repository, there's no effect, but I don't think the text
> explicitly says that. In fact, the first sentence under "--depth"
> (which is of course relevant for "--deepen") is unconditional.
One would assume that this 'shallow boundary' on a non-shallow
repository would be the *start* of the history, not the current tip,
and thus it would be a no-op. Especially if you consider the position
of this 'shallow boundary' throughout the process.
consider the repo
A-B-C-D-E
you have a shallow repo with
A-B*
where * marks the shallow boundary, after another fetch --deepen=2 we get
A-B-C-D*
and then
A-B-C-D-E*
you're proposing that it's reasonable that this should instead be
*A-B-C-D-E
such that another fetch gives us
A-B*
> So I'm not sure it should be a no-op.
I think it's pretty obvious that it should be.
> That said, it is possible the behavior changed between 2.53 and 2.54?
> I haven't tried to reproduce or bisect yet.
The mail you're replying to already answers this question.
--
Mikael Magnusson
^ permalink raw reply
* Re: [PATCH v3 5/5] format-rev: introduce builtin for on-demand pretty formatting
From: Kristoffer Haugsbakk @ 2026-04-30 9:21 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble
In-Reply-To: <V3_format-rev_new_builtin.66f@msgid.xyz>
On Wed, Apr 29, 2026, at 00:25, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>[snip]
> + peeled = deref_tag(the_repository, object, scratch_buf.buf, 0);
> + if (peeled && peeled->type == OBJ_COMMIT)
> + commit = (struct commit *)peeled;
> + if (!commit) {
> + fprintf(stderr, "Could not get commit for %s. Skipping.\n",
> + *argv);
s/*argv/scratch_buf.buf/
> + continue;
> + }
>[snip]
^ permalink raw reply
* Re: git interactive rebase does not allow editing commits at once anymore
From: Junio C Hamano @ 2026-04-30 9:29 UTC (permalink / raw)
To: David Jordanes; +Cc: git@vger.kernel.org
In-Reply-To: <DB7PR03MB3881199B8D12CC7A981ADF0CA8352@DB7PR03MB3881.eurprd03.prod.outlook.com>
David Jordanes <davidjordanes@outlook.com> writes:
> Git version: git version 2.53.0.windows.2
> OS: Windows 11
I do not do Windows, but I am curious about "anymore" part of your
message title. Are you reporting a regression?
^ permalink raw reply
* Re: [PATCH v3 6/9] update-ref: handle rejections while adding updates
From: Karthik Nayak @ 2026-04-30 9:52 UTC (permalink / raw)
To: Toon Claes, git; +Cc: ps
In-Reply-To: <87v7dagdjk.fsf@toon--20250203-5JQV3.mail-host-address-is-not-set>
[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]
Toon Claes <toon@iotcl.com> writes:
> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> @@ -289,22 +300,35 @@ static void parse_cmd_update(struct ref_transaction *transaction,
>> if (*next != line_termination)
>> die("update %s: extra input: %s", refname, next);
>>
>> - if (ref_transaction_update(transaction, refname,
>> - &new_oid, have_old ? &old_oid : NULL,
>> - NULL, NULL,
>> - update_flags | create_reflog_flag,
>> - msg, &err))
>> + tx_err = ref_transaction_update(transaction, refname,
>> + &new_oid, have_old ? &old_oid : NULL,
>> + NULL, NULL,
>> + update_flags | create_reflog_flag,
>> + msg, &err);
>> +
>> + /*
>> + * Generic errors are non-recoverable, so we cannot skip the update
>> + * or mark it as rejected.
>> + */
>> + if (tx_err == REF_TRANSACTION_ERROR_GENERIC)
>> die("%s", err.buf);
>>
>> + if (tx_err && opts->allow_update_failures)
>> + print_rejected_refs(refname, have_old ? &old_oid : NULL,
>> + &new_oid, NULL, NULL, tx_err, err.buf,
>> + NULL);
>
> I realize I've made this suggestion, but I think I've made a mistake.
> When opts->allow_update_failures is falsey and tx_err is truthy we
> should die also. Don't we?
>
Nice. I didn't think of that either.
> I'm not sure what the nicest way is to write this, but maybe:
>
> if (tx_err) {
> if (tx_err == REF_TRANSACTION_ERROR_GENERIC || !opts->allow_update_failures)
> die("%s", err.buf);
>
> print_rejected_refs(refname, have_old ? &old_oid : NULL,
> &new_oid, NULL, NULL, tx_err, err.buf,
> NULL);
> }
>
> How did test coverage not find this?
>
Because:
1. The function only returns `REF_TRANSACTION_ERROR_GENERIC` as of this
commit.
2. We only seem to be testing this scenario for batched updates.
3. I'll fix this and add some tests.
> --
> Cheers,
> Toon
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir
From: Shroom Moo @ 2026-04-30 10:02 UTC (permalink / raw)
To: git; +Cc: j6t, mlevedahl, Shroom Moo
In-Reply-To: <tencent_AEE968E8E785907BA55A383977C8968ED406@qq.com>
When git-gui is started from a directory that Git recognizes as a
valid repository but the working tree is not accessible (e.g., a
separated gitdir created by `git clone --separate-git-dir`, a bare
repository, or a case where the worktree directory was removed),
it previously called `rev-parse --show-toplevel` without error
handling, causing a fatal Tcl error ("this operation must be run
in a work tree").
Wrap the call in a `catch` and handle the failure as follows:
- For bare repositories, keep `_gitworktree` empty so that the
existing `is_bare` check shows "Cannot use bare repository" and
exits. No behavioral change.
- For non‑bare repositories, try to locate the worktree from the
parent directory using `git -C $parent rev-parse --show-toplevel`.
If the parent is a valid worktree, change to it; this covers the
legitimate case of starting git-gui from within the .git
subdirectory of a normal working tree.
- If the parent directory is not a worktree, refuse to start with
a clear error message. This prevents dangerous operations in a
separated gitdir, where ordinary Git commands like `git status`
would themselves refuse to run.
The approach intentionally avoids two pitfalls:
- Testing `--is-inside-git-dir` before calling `--show-toplevel`
would break the normal use case of starting git-gui from within
a .git subdirectory (where --show-toplevel would succeed).
- A simple “non‑bare” check after a failed --show-toplevel would
reject a normal repository whose worktree was only temporarily
removed.
The chosen method keeps the original behavior for bare repositories
and for regular working trees, fixes the crash, and properly blocks
separated gitdirs without a reachable worktree.
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
git-gui/git-gui.sh | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..2392282df3 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1169,7 +1169,28 @@ if {![file isdirectory $_gitdir]} {
load_config 0
apply_config
-set _gitworktree [git rev-parse --show-toplevel]
+if {[catch {set _gitworktree [git rev-parse --show-toplevel]}]} {
+ # For bare repositories, use the existing error handling
+ if {![catch {set bare [git rev-parse --is-bare-repository]}] && $bare eq {true}} {
+ set _gitworktree {}
+ } else {
+ # Non-bare: try to find the worktree from the parent directory
+ set parent [file dirname [pwd]]
+ # Cannot go higher than the root directory; leave _gitworktree empty
+ if {[file normalize $parent] eq [file normalize [pwd]]} {
+ # Already at the filesystem root; let existing paths cope
+ set _gitworktree {}
+ } elseif {![catch {
+ set _gitworktree [git -C $parent rev-parse --show-toplevel]
+ }]} {
+ cd $parent
+ } else {
+ catch {wm withdraw .}
+ error_popup [mc "Cannot start git-gui from inside the Git directory."]
+ exit 1
+ }
+ }
+}
if {$_prefix ne {}} {
if {$_gitworktree eq {}} {
--
2.52.0.windows.1
^ permalink raw reply related
* Re: [PATCH 2/3] http: attempt Negotiate auth in http.emptyAuth=auto mode
From: Matthew John Cheetham @ 2026-04-30 10:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <xmqqse8dz4pi.fsf@gitster.g>
[re-cc:ing the accidentially dropped mailing list]
On 2026-04-30 01:12, Junio C Hamano wrote:
> Matthew John Cheetham <mjcheetham@outlook.com> writes:
>
>> Agreed - the existing description is pretty opaque about what values it
>> actually takes. Should I add another patch to this series to spell out
>> the three values explicitly? How about something like this:
>>
>> http.emptyAuth::
>> Attempt authentication without seeking a username or
>> password. This can be used to attempt GSS-Negotiate
>> authentication without specifying a username in the URL,
>> as libcurl normally requires a username for
>> authentication. Possible values are:
>> +
>> --
>> * `auto` (default) - Send empty credentials only if the server's
>> 401 response advertises an authentication mechanism that
>> requires them (such as GSS-Negotiate); otherwise fall back to
>> prompting via the credential helper.
>> * `true` - Always send empty credentials on the very first
>> request, before receiving any 401 response from the server.
>> * `false` - Never send empty credentials. Mechanisms that
>> require empty credentials, such as GSS-Negotiate, will not
>> work.
>> --
>>
>> Does that read better?
>
> Surely. Thanks.
Submitted as v2
Thanks,
Matthew
^ permalink raw reply
* [PATCH v2 0/4] http: fix emptyAuth=auto for Negotiate/SPNEGO
From: Matthew John Cheetham via GitGitGadget @ 2026-04-30 10:54 UTC (permalink / raw)
To: git
Cc: gitster, johannes.schindelin, Matthew John Cheetham,
Matthew John Cheetham
In-Reply-To: <pull.2087.git.1776331259.gitgitgadget@gmail.com>
When a server advertises Negotiate (SPNEGO) authentication alongside Basic,
the "auto" mode of http.emptyAuth should allow libcurl to attempt Kerberos
authentication using the system ticket cache before falling back to
credential_fill(). Currently this never happens due to an interaction
between two older features.
The Negotiate-stripping logic from 4dbe66464b (remote-curl: fall back to
Basic auth if Negotiate fails, 2015-01-08) removes CURLAUTH_GSSNEGOTIATE on
the first 401, before the auto-detection from 40a18fc77c (http: add an
"auto" mode for http.emptyauth, 2017-02-25) gets a chance to see it as an
"exotic" method. The result is that auto mode silently degrades to the same
behavior as emptyAuth=false for any server whose only non-Basic/Digest
method is Negotiate, forcing Kerberos users to manually set
http.emptyAuth=true to get seamless ticket-based authentication.
This series fixes the interaction by delaying the Negotiate stripping in
auto mode by one round-trip, giving empty auth a chance to use the system
Kerberos ticket. If there is no valid ticket, Negotiate is stripped on the
second 401 and we fall through to credential_fill() as before. The true and
false modes are unchanged.
Patch 1: Extract a http_reauth_prepare() helper from the three retry paths
that call credential_fill() on HTTP_REAUTH. Pure refactor, no behavior
change.
Patch 2: Delay the GSSNEGOTIATE stripping in auto mode and teach
http_reauth_prepare() to skip credential_fill() when empty auth should be
attempted first.
Patch 3: Add tests verifying that auto mode produces an extra round-trip
(empty auth attempt) compared to false mode, using the existing
nph-custom-auth.sh CGI infrastructure.
Patch 4: Update http.emptyAuth documentation to clarify possible values
(true, false, and auto).
There is a trade-off in auto mode: when a server advertises Negotiate but
the client has no valid Kerberos ticket, there is one extra round-trip
compared to the current behavior. This matches the trade-off already
documented in 40a18fc77c. Users who want to avoid it can set
http.emptyAuth=false.
Note: this patch series was taken early into Git for Windows for the
2.54.0-rc2 release.
https://github.com/git-for-windows/git/commit/8e94b65c003783d7d7b09d9fccdf06a1363e347c
----------------------------------------------------------------------------
Update in v2:
* Add patch 4 to clarify the available options for http.emptyAuth in the
config documentation.
Matthew John Cheetham (4):
http: extract http_reauth_prepare() from retry paths
http: attempt Negotiate auth in http.emptyAuth=auto mode
t5563: add tests for http.emptyAuth with Negotiate
doc: clarify http.emptyAuth values
Documentation/config/http.adoc | 13 +++++-
http.c | 32 ++++++++++++++-
http.h | 6 +++
remote-curl.c | 4 +-
t/t5563-simple-http-auth.sh | 74 ++++++++++++++++++++++++++++++++++
5 files changed, 124 insertions(+), 5 deletions(-)
base-commit: 2b39a27d40682c09ac1c031f099ee602061597cd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2087%2Fmjcheetham%2Fspnego-fix-upstream-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2087/mjcheetham/spnego-fix-upstream-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2087
Range-diff vs v1:
1: 49488cc7d4 = 1: 49488cc7d4 http: extract http_reauth_prepare() from retry paths
2: f175294459 = 2: f175294459 http: attempt Negotiate auth in http.emptyAuth=auto mode
3: 650acab79e = 3: 650acab79e t5563: add tests for http.emptyAuth with Negotiate
-: ---------- > 4: e0f236767f doc: clarify http.emptyAuth values
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 1/4] http: extract http_reauth_prepare() from retry paths
From: Matthew John Cheetham via GitGitGadget @ 2026-04-30 10:54 UTC (permalink / raw)
To: git
Cc: gitster, johannes.schindelin, Matthew John Cheetham,
Matthew John Cheetham, Matthew John Cheetham
In-Reply-To: <pull.2087.v2.git.1777546472.gitgitgadget@gmail.com>
From: Matthew John Cheetham <mjcheetham@outlook.com>
All three HTTP retry paths (http_request_recoverable, post_rpc,
probe_rpc) call credential_fill() directly when handling
HTTP_REAUTH. Extract this into a helper function so that a
subsequent commit can add pre-fill logic (such as attempting
empty-auth before prompting) in one place.
No functional change.
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
http.c | 7 ++++++-
http.h | 6 ++++++
remote-curl.c | 4 ++--
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/http.c b/http.c
index d8d016891b..f208e0ad82 100644
--- a/http.c
+++ b/http.c
@@ -665,6 +665,11 @@ static void init_curl_http_auth(CURL *result)
}
}
+void http_reauth_prepare(int all_capabilities)
+{
+ credential_fill(the_repository, &http_auth, all_capabilities);
+}
+
/* *var must be free-able */
static void var_override(char **var, char *value)
{
@@ -2398,7 +2403,7 @@ static int http_request_recoverable(const char *url,
sleep(retry_delay);
}
} else if (ret == HTTP_REAUTH) {
- credential_fill(the_repository, &http_auth, 1);
+ http_reauth_prepare(1);
}
ret = http_request(url, result, target, options);
diff --git a/http.h b/http.h
index f9ee888c3e..729c51904d 100644
--- a/http.h
+++ b/http.h
@@ -76,6 +76,12 @@ extern int http_is_verbose;
extern ssize_t http_post_buffer;
extern struct credential http_auth;
+/**
+ * Prepare for an HTTP re-authentication retry. This fills credentials
+ * via credential_fill() so the next request can include them.
+ */
+void http_reauth_prepare(int all_capabilities);
+
extern char curl_errorstr[CURL_ERROR_SIZE];
enum http_follow_config {
diff --git a/remote-curl.c b/remote-curl.c
index aba60d5712..affdb880f7 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -946,7 +946,7 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece
do {
err = probe_rpc(rpc, &results);
if (err == HTTP_REAUTH)
- credential_fill(the_repository, &http_auth, 0);
+ http_reauth_prepare(0);
} while (err == HTTP_REAUTH);
if (err != HTTP_OK)
return -1;
@@ -1068,7 +1068,7 @@ retry:
rpc->any_written = 0;
err = run_slot(slot, NULL);
if (err == HTTP_REAUTH && !large_request) {
- credential_fill(the_repository, &http_auth, 0);
+ http_reauth_prepare(0);
curl_slist_free_all(headers);
goto retry;
}
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/4] http: attempt Negotiate auth in http.emptyAuth=auto mode
From: Matthew John Cheetham via GitGitGadget @ 2026-04-30 10:54 UTC (permalink / raw)
To: git
Cc: gitster, johannes.schindelin, Matthew John Cheetham,
Matthew John Cheetham, Matthew John Cheetham
In-Reply-To: <pull.2087.v2.git.1777546472.gitgitgadget@gmail.com>
From: Matthew John Cheetham <mjcheetham@outlook.com>
When a server advertises Negotiate (SPNEGO) authentication, the
"auto" mode of http.emptyAuth should detect this as an "exotic"
method and proactively send empty credentials, allowing libcurl to
use the system Kerberos ticket without prompting the user.
However, two features interact to prevent this from working:
The Negotiate-stripping logic, introduced in 4dbe66464b
(remote-curl: fall back to Basic auth if Negotiate fails,
2015-01-08), removes CURLAUTH_GSSNEGOTIATE from the allowed
methods on the first 401 response. The empty-auth auto-detection,
introduced in 40a18fc77c (http: add an "auto" mode for
http.emptyauth, 2017-02-25), then checks the remaining methods
for anything "exotic" -- but Negotiate has already been removed,
so auto mode never activates for servers whose only non-Basic/Digest
method is Negotiate (e.g., Apache with mod_auth_kerb offering
Basic + Negotiate).
Fix this by delaying the Negotiate stripping in auto mode: on the
first 401, keep Negotiate in the allowed methods so that auto mode
can detect it and retry with empty credentials. If that attempt
fails (no valid Kerberos ticket), strip Negotiate on the second 401
and fall through to credential_fill() as usual.
To support this, also teach http_reauth_prepare() to skip
credential_fill() when empty auth is about to be attempted, since
filling real credentials would bypass the empty-auth mechanism.
The true and false modes are unchanged: true sends empty credentials
on the very first request (before any 401), and false never sends
them.
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
http.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/http.c b/http.c
index f208e0ad82..1c7ea32ef2 100644
--- a/http.c
+++ b/http.c
@@ -138,6 +138,7 @@ static unsigned long empty_auth_useless =
CURLAUTH_BASIC
| CURLAUTH_DIGEST_IE
| CURLAUTH_DIGEST;
+static int empty_auth_try_negotiate;
static struct curl_slist *pragma_header;
static struct string_list extra_http_headers = STRING_LIST_INIT_DUP;
@@ -667,6 +668,17 @@ static void init_curl_http_auth(CURL *result)
void http_reauth_prepare(int all_capabilities)
{
+ /*
+ * If we deferred stripping Negotiate to give empty auth a
+ * chance (auto mode), skip credential_fill on this retry so
+ * that init_curl_http_auth() sends empty credentials and
+ * libcurl can attempt Negotiate with the system ticket cache.
+ */
+ if (empty_auth_try_negotiate &&
+ !http_auth.password && !http_auth.credential &&
+ (http_auth_methods & CURLAUTH_GSSNEGOTIATE))
+ return;
+
credential_fill(the_repository, &http_auth, all_capabilities);
}
@@ -1895,7 +1907,18 @@ static int handle_curl_result(struct slot_results *results)
http_proactive_auth = PROACTIVE_AUTH_NONE;
return HTTP_NOAUTH;
} else {
- http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+ if (curl_empty_auth == -1 &&
+ !empty_auth_try_negotiate &&
+ (results->auth_avail & CURLAUTH_GSSNEGOTIATE)) {
+ /*
+ * In auto mode, give Negotiate a chance via
+ * empty auth before stripping it. If it fails,
+ * we will strip it on the next 401.
+ */
+ empty_auth_try_negotiate = 1;
+ } else {
+ http_auth_methods &= ~CURLAUTH_GSSNEGOTIATE;
+ }
if (results->auth_avail) {
http_auth_methods &= results->auth_avail;
http_auth_methods_restricted = 1;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 3/4] t5563: add tests for http.emptyAuth with Negotiate
From: Matthew John Cheetham via GitGitGadget @ 2026-04-30 10:54 UTC (permalink / raw)
To: git
Cc: gitster, johannes.schindelin, Matthew John Cheetham,
Matthew John Cheetham, Matthew John Cheetham
In-Reply-To: <pull.2087.v2.git.1777546472.gitgitgadget@gmail.com>
From: Matthew John Cheetham <mjcheetham@outlook.com>
Add tests exercising the interaction between http.emptyAuth and
servers that advertise Negotiate (SPNEGO) authentication.
Verify that auto mode gives Negotiate a chance via empty auth
(resulting in two 401 responses before falling through to
credential_fill with Basic credentials), and that false mode
strips Negotiate immediately (only one 401 response).
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
t/t5563-simple-http-auth.sh | 74 +++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index 0063581615..a7d475dd68 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -719,4 +719,78 @@ test_expect_success 'access using three-legged auth' '
EOF
'
+test_lazy_prereq SPNEGO 'curl --version | grep -qi "SPNEGO\|GSS-API\|Kerberos\|negotiate"'
+
+test_expect_success SPNEGO 'http.emptyAuth=auto attempts Negotiate before credential_fill' '
+ test_when_finished "per_test_cleanup" &&
+
+ set_credential_reply get <<-EOF &&
+ username=alice
+ password=secret-passwd
+ EOF
+
+ # Basic base64(alice:secret-passwd)
+ cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+ id=1 creds=Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+ EOF
+
+ cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+ id=1 status=200
+ id=default response=WWW-Authenticate: Negotiate
+ id=default response=WWW-Authenticate: Basic realm="example.com"
+ EOF
+
+ test_config_global credential.helper test-helper &&
+ GIT_TRACE_CURL="$TRASH_DIRECTORY/trace-auto" \
+ git -c http.emptyAuth=auto \
+ ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+ # In auto mode with a Negotiate+Basic server, there should be
+ # three 401 responses: (1) initial no-auth request, (2) empty-auth
+ # retry where Negotiate fails (no Kerberos ticket), (3) libcurl
+ # internal Negotiate retry. The fourth attempt uses Basic
+ # credentials from credential_fill and succeeds.
+ grep "HTTP/[0-9.]* 401" "$TRASH_DIRECTORY/trace-auto" >actual_401s &&
+ test_line_count = 3 actual_401s &&
+
+ expect_credential_query get <<-EOF
+ capability[]=authtype
+ capability[]=state
+ protocol=http
+ host=$HTTPD_DEST
+ wwwauth[]=Negotiate
+ wwwauth[]=Basic realm="example.com"
+ EOF
+'
+
+test_expect_success SPNEGO 'http.emptyAuth=false skips Negotiate' '
+ test_when_finished "per_test_cleanup" &&
+
+ set_credential_reply get <<-EOF &&
+ username=alice
+ password=secret-passwd
+ EOF
+
+ # Basic base64(alice:secret-passwd)
+ cat >"$HTTPD_ROOT_PATH/custom-auth.valid" <<-EOF &&
+ id=1 creds=Basic YWxpY2U6c2VjcmV0LXBhc3N3ZA==
+ EOF
+
+ cat >"$HTTPD_ROOT_PATH/custom-auth.challenge" <<-EOF &&
+ id=1 status=200
+ id=default response=WWW-Authenticate: Negotiate
+ id=default response=WWW-Authenticate: Basic realm="example.com"
+ EOF
+
+ test_config_global credential.helper test-helper &&
+ GIT_TRACE_CURL="$TRASH_DIRECTORY/trace-false" \
+ git -c http.emptyAuth=false \
+ ls-remote "$HTTPD_URL/custom_auth/repo.git" &&
+
+ # With emptyAuth=false, Negotiate is stripped immediately and
+ # credential_fill is called right away. Only one 401 response.
+ grep "HTTP/[0-9.]* 401" "$TRASH_DIRECTORY/trace-false" >actual_401s &&
+ test_line_count = 1 actual_401s
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 4/4] doc: clarify http.emptyAuth values
From: Matthew John Cheetham via GitGitGadget @ 2026-04-30 10:54 UTC (permalink / raw)
To: git
Cc: gitster, johannes.schindelin, Matthew John Cheetham,
Matthew John Cheetham, Matthew John Cheetham
In-Reply-To: <pull.2087.v2.git.1777546472.gitgitgadget@gmail.com>
From: Matthew John Cheetham <mjcheetham@outlook.com>
The existing description of http.emptyAuth explains the purpose of the
setting but never says what values it accepts. Readers have to infer
from context (or read the source) that it takes 'true', 'false', or
'auto', and what each one means.
Document the three accepted values explicitly:
* 'auto' (the default) only sends empty credentials when the server's
401 response advertises a mechanism that requires them, such as
GSS-Negotiate. This matches the long-standing auto-detection
behaviour added in 40a18fc77c (http: add an "auto" mode for
http.emptyauth, 2017-02-25).
* 'true' unconditionally sends empty credentials on the very first
request, before any 401 response, for callers that know they want
this behaviour up front.
* 'false' disables the feature entirely; mechanisms that depend on
empty credentials, such as GSS-Negotiate, will not work in this
mode.
Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
---
Documentation/config/http.adoc | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc
index 849c89f36c..792a71b413 100644
--- a/Documentation/config/http.adoc
+++ b/Documentation/config/http.adoc
@@ -59,7 +59,18 @@ http.emptyAuth::
Attempt authentication without seeking a username or password. This
can be used to attempt GSS-Negotiate authentication without specifying
a username in the URL, as libcurl normally requires a username for
- authentication.
+ authentication. Possible values are:
++
+--
+* `auto` (default) - Send empty credentials only if the server's 401 response
+ advertises an authentication mechanism that requires them (such as
+ GSS-Negotiate); otherwise fall back to prompting via the credential helper.
+* `true` - Always send empty credentials on the very first request, before
+ receiving any 401 response from the server.
+* `false` - Never send empty credentials. Mechanisms that require
+ empty credentials or an explicit username, such as GSS-Negotiate, will not
+ work.
+--
http.proactiveAuth::
Attempt authentication without first making an unauthenticated attempt and
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox