* [RFC PATCH 08/10] pack-objects: introduce '--stdin-packs=follow-reachable'
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
Introduce a new '--stdin-packs=follow-reachable' mode. Like
'--stdin-packs=follow', this mode recognizes the '!' (excluded-open)
pack prefix and halts at '^' (excluded-closed) packs.
Unlike 'follow', which eagerly includes all objects from listed packs
and then walks reachability to rescue additional objects, the new
'follow-reachable' mode uses reference tips as its traversal starting
points and only includes objects that are both reachable AND belong to
an included pack (or are reachable from a commit or tag in one):
- Objects in included packs: added to the output if reachable.
- Objects reachable from included-pack commits but in unknown packs:
added to the output (rescued).
- Objects in excluded-open ('!') packs: not included, but the traversal
continues through them.
- Objects in excluded-closed ('^') packs: not included, and the
traversal halts.
The implementation uses a two-phase approach:
1. In the first phase, commits and tags in included packs (and loose,
when --unpacked is given) are marked with a flag bit
(IN_INCLUDED_PACK). A commit-only walk from ref tips then identifies
which marked objects are reachable, halting at excluded-closed
packs.
2. In the second phase, every reachable marked object (from the
previous step) becomes a tip for a full object traversal whose
`show_object_pack_hint()` and `show_commit_pack_hint()` callbacks
add discovered objects (obeying the usual constraints imposed by
`want_object_in_pack()`).
When '--unpacked' is given, reachable loose objects are included in the
output while unreachable loose objects are left alone. This is achieved
by marking loose commits and tags with IN_INCLUDED_PACK during the first
phase, so the pre-walk discovers them naturally.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-pack-objects.adoc | 17 +++
builtin/pack-objects.c | 185 +++++++++++++++++++++++--
t/t5331-pack-objects-stdin.sh | 201 ++++++++++++++++++++++++++++
3 files changed, 393 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 8a27aa19fd3..d7b2e39e76c 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -113,6 +113,23 @@ This mode is useful, for example, to resurrect once-unreachable
objects found in cruft packs to generate packs which are closed under
reachability up to the boundary set by the excluded packs.
+
+When `mode` is "follow-reachable", the same pack prefixes are recognized
+as in "follow" (`!` for excluded-open, `^` for excluded-closed). However,
+instead of including all objects from included packs, only objects that
+are reachable from reference tips AND belong to an included pack (or are
+reachable from a commit in one) are included. Objects in excluded-open
+packs are traversed but not included; objects in excluded-closed packs
+halt the traversal.
++
+This mode is designed for geometric repacking with cruft packs, where
+the output pack should contain only reachable objects so that unreachable
+ones can be collected separately.
++
+When `--unpacked` is given alongside `--stdin-packs=follow-reachable`,
+reachable loose objects are also included in the output pack, while
+unreachable loose objects are left alone. This includes both loose
+commits and annotated tag objects.
++
Incompatible with `--revs`, or options that imply `--revs` (such as
`--all`), with the exception of `--unpacked`, which is compatible.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 29e43abb51e..5d96757b645 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -290,6 +290,7 @@ enum stdin_packs_mode {
STDIN_PACKS_MODE_NONE,
STDIN_PACKS_MODE_STANDARD,
STDIN_PACKS_MODE_FOLLOW,
+ STDIN_PACKS_MODE_FOLLOW_REACHABLE,
};
/**
@@ -3835,7 +3836,8 @@ static void show_object_pack_hint(struct object *object, const char *name,
void *data)
{
enum stdin_packs_mode mode = *(enum stdin_packs_mode *)data;
- if (mode == STDIN_PACKS_MODE_FOLLOW) {
+ if (mode == STDIN_PACKS_MODE_FOLLOW ||
+ mode == STDIN_PACKS_MODE_FOLLOW_REACHABLE) {
if (object->type == OBJ_BLOB &&
!odb_has_object(the_repository->objects, &object->oid, 0))
return;
@@ -3866,7 +3868,8 @@ static void show_commit_pack_hint(struct commit *commit, void *data)
{
enum stdin_packs_mode mode = *(enum stdin_packs_mode *)data;
- if (mode == STDIN_PACKS_MODE_FOLLOW) {
+ if (mode == STDIN_PACKS_MODE_FOLLOW ||
+ mode == STDIN_PACKS_MODE_FOLLOW_REACHABLE) {
show_object_pack_hint((struct object *)commit, "", data);
return;
}
@@ -3933,6 +3936,156 @@ static int stdin_packs_include_check(struct commit *commit, void *data)
return stdin_packs_include_check_obj((struct object *)commit, data);
}
+/*
+ * Flag bit set on commits that belong to an included pack during
+ * '--stdin-packs=follow-reachable'. Used by the pre-walk to
+ * identify which reachable commits should be tips for the main
+ * object traversal.
+ */
+#define IN_INCLUDED_PACK (1u<<11)
+
+static int mark_included_pack_tip(const struct object_id *oid,
+ struct packed_git *p,
+ uint32_t pos,
+ void *data)
+{
+ struct rev_info *main_revs = data;
+ off_t ofs = nth_packed_object_offset(p, pos);
+ enum object_type type;
+ struct object_info oi = OBJECT_INFO_INIT;
+ struct object *obj;
+
+ oi.typep = &type;
+ if (packed_object_info(p, ofs, &oi) < 0)
+ return 0;
+ if (type != OBJ_COMMIT && type != OBJ_TAG)
+ return 0;
+
+ obj = parse_object(the_repository, oid);
+ if (!obj)
+ return 0;
+
+ obj->flags |= IN_INCLUDED_PACK;
+
+ if (type == OBJ_TAG && main_revs)
+ add_pending_object(main_revs, obj, "");
+ return 0;
+}
+
+static int mark_loose_object_tip(const struct object_id *oid,
+ struct object_info *oi UNUSED,
+ void *data)
+{
+ struct rev_info *main_revs = data;
+ struct object *obj;
+ enum object_type type;
+
+ type = odb_read_object_info(the_repository->objects, oid, NULL);
+ if (type != OBJ_COMMIT && type != OBJ_TAG)
+ return 0;
+
+ obj = parse_object(the_repository, oid);
+ if (!obj)
+ return 0;
+
+ obj->flags |= IN_INCLUDED_PACK;
+
+ if (type == OBJ_TAG && main_revs)
+ add_pending_object(main_revs, obj, "");
+
+ return 0;
+}
+
+static int add_ref_to_pending(const struct reference *ref, void *cb_data)
+{
+ struct rev_info *revs = cb_data;
+ struct object *object;
+
+ object = parse_object(the_repository, ref->oid);
+ if (!object)
+ return 0;
+
+ add_pending_object(revs, object, "");
+ return 0;
+}
+
+static void stdin_packs_add_reachable_pack_entries(struct string_list *keys,
+ struct rev_info *revs,
+ int rev_list_unpacked)
+{
+ struct rev_info pre_walk;
+ struct commit *commit;
+ struct string_list_item *item;
+
+ /*
+ * Phase 1: mark commits in included packs, then walk from
+ * ref tips to discover which of them are reachable. The walk
+ * halts at excluded-closed packs (via no_kept_objects) and
+ * continues through excluded-open ones.
+ *
+ * Also set include_check on the outer revs so that phase 2
+ * (the main object traversal) halts at closed packs.
+ */
+ revs->include_check = stdin_packs_include_check;
+ revs->include_check_obj = stdin_packs_include_check_obj;
+
+ for_each_string_list_item(item, keys) {
+ struct stdin_pack_info *info = item->util;
+ if (info->kind & STDIN_PACK_INCLUDE)
+ for_each_object_in_pack(info->p,
+ mark_included_pack_tip,
+ revs,
+ ODB_FOR_EACH_OBJECT_PACK_ORDER);
+ }
+
+ if (rev_list_unpacked) {
+ /*
+ * With '--stdin-packs=follow-reachable', specifying
+ * '--unpacked' instructs pack-objects to pack any loose
+ * objects which are reachable.
+ *
+ * Pretend as if all loose objects are in an included
+ * pack in order to make them eligible for packing.
+ */
+ struct odb_source *source = revs->repo->objects->sources;
+ for (; source; source = source->next) {
+ struct odb_source_files *files = odb_source_files_downcast(source);
+ struct odb_for_each_object_options opts = { 0 };
+ if (local)
+ opts.flags |= ODB_FOR_EACH_OBJECT_LOCAL_ONLY;
+
+ odb_source_for_each_object(&files->loose->base, NULL,
+ mark_loose_object_tip,
+ revs, &opts);
+ }
+ }
+
+ repo_init_revisions(the_repository, &pre_walk, NULL);
+ pre_walk.no_kept_objects = 1;
+ pre_walk.keep_pack_cache_flags |= KEPT_PACK_IN_CORE;
+ pre_walk.ignore_missing_links = 1;
+
+ refs_for_each_ref(get_main_ref_store(the_repository),
+ add_ref_to_pending, &pre_walk);
+
+ if (prepare_revision_walk(&pre_walk))
+ die(_("revision walk setup failed"));
+
+ /*
+ * Phase 2 tips: every reachable commit that is in an
+ * included pack becomes a starting point for the main
+ * object traversal.
+ */
+ while ((commit = get_revision(&pre_walk)) != NULL) {
+ if (commit->object.flags & IN_INCLUDED_PACK)
+ add_pending_oid(revs, NULL,
+ &commit->object.oid, 0);
+ }
+
+ reset_revision_walk();
+ release_revisions(&pre_walk);
+}
+
static void stdin_packs_add_all_pack_entries(struct string_list *keys,
struct rev_info *revs)
{
@@ -3962,7 +4115,9 @@ static void stdin_packs_add_all_pack_entries(struct string_list *keys,
}
static void stdin_packs_add_pack_entries(struct strmap *packs,
- struct rev_info *revs)
+ struct rev_info *revs,
+ enum stdin_packs_mode mode,
+ int rev_list_unpacked)
{
struct string_list keys = STRING_LIST_INIT_NODUP;
struct hashmap_iter iter;
@@ -3983,13 +4138,18 @@ static void stdin_packs_add_pack_entries(struct strmap *packs,
*/
QSORT(keys.items, keys.nr, pack_mtime_cmp);
- stdin_packs_add_all_pack_entries(&keys, revs);
+ if (mode == STDIN_PACKS_MODE_FOLLOW_REACHABLE)
+ stdin_packs_add_reachable_pack_entries(&keys, revs,
+ rev_list_unpacked);
+ else
+ stdin_packs_add_all_pack_entries(&keys, revs);
string_list_clear(&keys, 0);
}
static void stdin_packs_read_input(struct rev_info *revs,
- enum stdin_packs_mode mode)
+ enum stdin_packs_mode mode,
+ int rev_list_unpacked)
{
struct strbuf buf = STRBUF_INIT;
struct strmap packs = STRMAP_INIT;
@@ -4004,7 +4164,9 @@ static void stdin_packs_read_input(struct rev_info *revs,
continue;
else if (*key == '^')
kind = STDIN_PACK_EXCLUDE_CLOSED;
- else if (*key == '!' && mode == STDIN_PACKS_MODE_FOLLOW)
+ else if (*key == '!' &&
+ (mode == STDIN_PACKS_MODE_FOLLOW ||
+ mode == STDIN_PACKS_MODE_FOLLOW_REACHABLE))
kind = STDIN_PACK_EXCLUDE_OPEN;
if (kind != STDIN_PACK_INCLUDE)
@@ -4069,7 +4231,7 @@ static void stdin_packs_read_input(struct rev_info *revs,
info->p = p;
}
- stdin_packs_add_pack_entries(&packs, revs);
+ stdin_packs_add_pack_entries(&packs, revs, mode, rev_list_unpacked);
strbuf_release(&buf);
strmap_clear(&packs, 1);
@@ -4109,7 +4271,8 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
/* avoids adding objects in excluded packs */
ignore_packed_keep_in_core = 1;
- if (mode == STDIN_PACKS_MODE_FOLLOW) {
+ if (mode == STDIN_PACKS_MODE_FOLLOW ||
+ mode == STDIN_PACKS_MODE_FOLLOW_REACHABLE) {
/*
* In '--stdin-packs=follow' mode, additionally ignore
* objects in excluded-open packs to prevent them from
@@ -4117,8 +4280,8 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
*/
ignore_packed_keep_in_core_open = 1;
}
- stdin_packs_read_input(&revs, mode);
- if (rev_list_unpacked)
+ stdin_packs_read_input(&revs, mode, rev_list_unpacked);
+ if (rev_list_unpacked && mode != STDIN_PACKS_MODE_FOLLOW_REACHABLE)
add_unreachable_loose_objects(&revs);
if (prepare_revision_walk(&revs))
@@ -5027,6 +5190,8 @@ static int parse_stdin_packs_mode(const struct option *opt, const char *arg,
*mode = STDIN_PACKS_MODE_STANDARD;
else if (!strcmp(arg, "follow"))
*mode = STDIN_PACKS_MODE_FOLLOW;
+ else if (!strcmp(arg, "follow-reachable"))
+ *mode = STDIN_PACKS_MODE_FOLLOW_REACHABLE;
else
die(_("invalid value for '%s': '%s'"), opt->long_name, arg);
diff --git a/t/t5331-pack-objects-stdin.sh b/t/t5331-pack-objects-stdin.sh
index c74b5861af3..443d855291a 100755
--- a/t/t5331-pack-objects-stdin.sh
+++ b/t/t5331-pack-objects-stdin.sh
@@ -520,4 +520,205 @@ test_expect_success '--stdin-packs with !-delimited pack without follow' '
)
'
+test_expect_success '--stdin-packs=follow-reachable excludes unreachable objects' '
+ test_when_finished "rm -fr repo" &&
+
+ git init repo &&
+ (
+ cd repo &&
+ git config set maintenance.auto false &&
+
+ git branch -M main &&
+
+ # Create the following commit structure:
+ #
+ # A <-- B <-- C (main)
+ # ^
+ # \
+ # U (unreachable, no ref)
+ test_commit A &&
+ test_commit B &&
+ test_commit U &&
+ U_TIP="$(git rev-parse HEAD)" &&
+ git reset --hard HEAD^ &&
+ git tag -d U &&
+ git reflog expire --all --expire=all &&
+
+ test_commit C &&
+
+ A="$(echo A | git pack-objects --revs $packdir/pack)" &&
+ B="$(echo A..B | git pack-objects --revs $packdir/pack)" &&
+ C="$(echo B..C | git pack-objects --revs $packdir/pack)" &&
+ U="$(echo "$U_TIP" | git pack-objects $packdir/pack)" &&
+
+ git prune-packed &&
+
+ # Include packs A and C, exclude B as open (since B
+ # may not have closure), leave U as unknown.
+ #
+ # With follow-reachable:
+ # - objects from A and C are included (reachable from
+ # main, through excluded-open B, and in included
+ # packs)
+ # - objects from B are excluded (excluded-open)
+ # - objects from U are NOT included (not reachable
+ # from any ref, even though the pack exists)
+ P=$(git pack-objects --stdin-packs=follow-reachable \
+ $packdir/pack <<-EOF
+ pack-$A.pack
+ !pack-$B.pack
+ pack-$C.pack
+ EOF
+ ) &&
+
+ objects_in_packs $A $C >expect &&
+ objects_in_packs $P >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--stdin-packs=follow-reachable with open-excluded packs' '
+ test_when_finished "rm -fr repo" &&
+
+ git init repo &&
+ (
+ cd repo &&
+ git config set maintenance.auto false &&
+
+ git branch -M main &&
+
+ # Create the following commit structure:
+ #
+ # A <-- B <-- C <-- D (main)
+ #
+ # Pack each commit separately, then use follow-reachable
+ # with B excluded-open and A excluded-closed. Since B is
+ # open, the traversal continues through it, but since A
+ # is closed, it halts there.
+ test_commit A &&
+ test_commit B &&
+ test_commit C &&
+ test_commit D &&
+
+ A="$(echo A | git pack-objects --revs $packdir/pack)" &&
+ B="$(echo A..B | git pack-objects --revs $packdir/pack)" &&
+ C="$(echo B..C | git pack-objects --revs $packdir/pack)" &&
+ D="$(echo C..D | git pack-objects --revs $packdir/pack)" &&
+
+ git prune-packed &&
+
+ # Include C and D, B excluded-open, A excluded-closed.
+ #
+ # The traversal starts at main (D), walks:
+ # D (included) -> C (included) -> B (open, continue
+ # but do not include) -> A (closed, halt).
+ #
+ # Objects from C and D are in the output (reachable,
+ # included). B.t is also rescued (reachable via
+ # C^{tree} or similar). A and its objects are NOT
+ # (behind the closed boundary).
+ P=$(git pack-objects --stdin-packs=follow-reachable \
+ $packdir/pack <<-EOF
+ pack-$C.pack
+ pack-$D.pack
+ !pack-$B.pack
+ ^pack-$A.pack
+ EOF
+ ) &&
+
+ objects_in_packs $C $D >expect &&
+ objects_in_packs $P >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--stdin-packs=follow-reachable with --unpacked and loose objects' '
+ test_when_finished "rm -fr repo" &&
+
+ git init repo &&
+ (
+ cd repo &&
+ git config set maintenance.auto false &&
+
+ git branch -M main &&
+
+ test_commit A &&
+ test_commit B &&
+
+ A="$(echo A | git pack-objects --revs $packdir/pack)" &&
+ B="$(echo A..B | git pack-objects --revs $packdir/pack)" &&
+
+ git prune-packed &&
+
+ # Create a reachable loose commit on top of B.
+ test_commit C &&
+
+ # Create an unreachable loose object.
+ unreachable="$(echo "unreachable" | git hash-object -w --stdin)" &&
+
+ # Include A and B, no excluded packs. With --unpacked,
+ # the reachable loose objects from C should be included
+ # in the output but the unreachable blob should not.
+ P=$(git pack-objects --stdin-packs=follow-reachable \
+ --unpacked $packdir/pack <<-EOF
+ pack-$A.pack
+ pack-$B.pack
+ EOF
+ ) &&
+
+ # The output should contain objects from A, B, and C.
+ {
+ objects_in_packs $A $B &&
+ git rev-list --objects --no-object-names B..C
+ } >expect.raw &&
+ sort expect.raw >expect &&
+
+ objects_in_packs $P >actual &&
+
+ # The unreachable blob should NOT be in the output.
+ ! grep $unreachable actual &&
+
+ test_cmp expect actual
+ )
+'
+
+test_expect_success '--stdin-packs=follow-reachable with --unpacked and loose annotated tag' '
+ test_when_finished "rm -fr repo" &&
+
+ git init repo &&
+ (
+ cd repo &&
+ git config set maintenance.auto false &&
+
+ git branch -M main &&
+
+ test_commit A &&
+
+ A="$(echo A | git pack-objects --revs $packdir/pack)" &&
+
+ git prune-packed &&
+
+ # Create a loose annotated tag pointing at A.
+ git tag -a -m "annotated" annotated-tag A &&
+ tag_oid="$(git rev-parse annotated-tag)" &&
+
+ P=$(git pack-objects --stdin-packs=follow-reachable \
+ --unpacked $packdir/pack <<-EOF
+ pack-$A.pack
+ EOF
+ ) &&
+
+ # The output should contain objects from A plus the
+ # loose annotated tag object.
+ {
+ objects_in_packs $A &&
+ echo $tag_oid
+ } >expect.raw &&
+ sort expect.raw >expect &&
+
+ objects_in_packs $P >actual &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 07/10] pack-objects: extract `stdin_packs_add_all_pack_entries()`
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
Extract the pack enumeration loop from stdin_packs_add_pack_entries()
into a separate stdin_packs_add_all_pack_entries() helper, and have the
caller dispatch to it based on the stdin_packs_mode.
This prepares for a subsequent commit which will introduce an alternate
code path for '--stdin-packs=follow-reachable' that determines the set
of objects to include via a reachability walk rather than eagerly adding
all objects from included packs.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/pack-objects.c | 49 ++++++++++++++++++++++++------------------
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 27048bbb4dd..29e43abb51e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3933,30 +3933,12 @@ static int stdin_packs_include_check(struct commit *commit, void *data)
return stdin_packs_include_check_obj((struct object *)commit, data);
}
-static void stdin_packs_add_pack_entries(struct strmap *packs,
- struct rev_info *revs)
+static void stdin_packs_add_all_pack_entries(struct string_list *keys,
+ struct rev_info *revs)
{
- struct string_list keys = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
- struct hashmap_iter iter;
- struct strmap_entry *entry;
- strmap_for_each_entry(packs, &iter, entry) {
- struct stdin_pack_info *info = entry->value;
- if (!info->p)
- die(_("could not find pack '%s'"), entry->key);
-
- string_list_append(&keys, entry->key)->util = info;
- }
-
- /*
- * Order packs by ascending mtime; use QSORT directly to access the
- * string_list_item's ->util pointer, which string_list_sort() does not
- * provide.
- */
- QSORT(keys.items, keys.nr, pack_mtime_cmp);
-
- for_each_string_list_item(item, &keys) {
+ for_each_string_list_item(item, keys) {
struct stdin_pack_info *info = item->util;
if (info->kind & STDIN_PACK_EXCLUDE_OPEN) {
@@ -3977,6 +3959,31 @@ static void stdin_packs_add_pack_entries(struct strmap *packs,
revs,
ODB_FOR_EACH_OBJECT_PACK_ORDER);
}
+}
+
+static void stdin_packs_add_pack_entries(struct strmap *packs,
+ struct rev_info *revs)
+{
+ struct string_list keys = STRING_LIST_INIT_NODUP;
+ struct hashmap_iter iter;
+ struct strmap_entry *entry;
+
+ strmap_for_each_entry(packs, &iter, entry) {
+ struct stdin_pack_info *info = entry->value;
+ if (!info->p)
+ die(_("could not find pack '%s'"), entry->key);
+
+ string_list_append(&keys, entry->key)->util = info;
+ }
+
+ /*
+ * Order packs by ascending mtime; use QSORT directly to access the
+ * string_list_item's ->util pointer, which string_list_sort() does not
+ * provide.
+ */
+ QSORT(keys.items, keys.nr, pack_mtime_cmp);
+
+ stdin_packs_add_all_pack_entries(&keys, revs);
string_list_clear(&keys, 0);
}
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 02/10] repack: extract `locate_existing_pack()` helper
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
Factor out the lookup from `existing_packs_retain_cruft()` that converts
a pack basename to a `string_list_item` into a reusable static helper
function, `locate_existing_pack()`.
A subsequent commit will introduce a new function which will need to
perform this same lookup against a different `string_list`.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
repack.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/repack.c b/repack.c
index 571dabb665e..986c74ac7e8 100644
--- a/repack.c
+++ b/repack.c
@@ -226,21 +226,32 @@ static void existing_packs_mark_for_deletion_1(const struct git_hash_algo *algop
}
}
+static struct string_list_item *locate_existing_pack(struct string_list *list,
+ struct packed_git *p)
+{
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list_item *item;
+
+ strbuf_addstr(&buf, pack_basename(p));
+ strbuf_strip_suffix(&buf, ".pack");
+
+ item = string_list_lookup(list, buf.buf);
+
+ strbuf_release(&buf);
+
+ return item;
+}
+
void existing_packs_retain_cruft(struct existing_packs *existing,
struct packed_git *cruft)
{
- struct strbuf buf = STRBUF_INIT;
struct string_list_item *item;
- strbuf_addstr(&buf, pack_basename(cruft));
- strbuf_strip_suffix(&buf, ".pack");
-
- item = string_list_lookup(&existing->cruft_packs, buf.buf);
+ item = locate_existing_pack(&existing->cruft_packs, cruft);
if (!item)
BUG("could not find cruft pack '%s'", pack_basename(cruft));
existing_packs_mark_retained(item);
- strbuf_release(&buf);
}
void existing_packs_mark_for_deletion(struct existing_packs *existing,
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 06/10] repack-geometry: drop unused redundant-pack removal
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
The previous commit stopped using pack_geometry_remove_redundant() when
deleting packs after a geometric repack. The existing_packs machinery now
handles the same removal after geometric packs are marked for deletion.
Remove the unused geometry-specific helper and its declaration.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
repack-geometry.c | 44 --------------------------------------------
repack.h | 5 -----
2 files changed, 49 deletions(-)
diff --git a/repack-geometry.c b/repack-geometry.c
index 2064683dcfe..c75fa508612 100644
--- a/repack-geometry.c
+++ b/repack-geometry.c
@@ -245,50 +245,6 @@ struct packed_git *pack_geometry_preferred_pack(struct pack_geometry *geometry)
return NULL;
}
-static void remove_redundant_packs(struct packed_git **pack,
- uint32_t pack_nr,
- struct string_list *names,
- struct existing_packs *existing,
- const char *packdir,
- bool wrote_incremental_midx)
-{
- const struct git_hash_algo *algop = existing->repo->hash_algo;
- struct strbuf buf = STRBUF_INIT;
- uint32_t i;
-
- for (i = 0; i < pack_nr; i++) {
- struct packed_git *p = pack[i];
- if (string_list_has_string(names, hash_to_hex_algop(p->hash,
- algop)))
- continue;
-
- strbuf_reset(&buf);
- strbuf_addstr(&buf, pack_basename(p));
- strbuf_strip_suffix(&buf, ".pack");
-
- if ((p->pack_keep) ||
- (string_list_has_string(&existing->kept_packs, buf.buf)))
- continue;
-
- repack_remove_redundant_pack(existing->repo, packdir, buf.buf,
- wrote_incremental_midx);
- }
-
- strbuf_release(&buf);
-}
-
-void pack_geometry_remove_redundant(struct pack_geometry *geometry,
- struct string_list *names,
- struct existing_packs *existing,
- const char *packdir,
- bool wrote_incremental_midx)
-{
- remove_redundant_packs(geometry->pack, geometry->split,
- names, existing, packdir, wrote_incremental_midx);
- remove_redundant_packs(geometry->promisor_pack, geometry->promisor_split,
- names, existing, packdir, wrote_incremental_midx);
-}
-
void pack_geometry_release(struct pack_geometry *geometry)
{
if (!geometry)
diff --git a/repack.h b/repack.h
index 90c89630ef8..4295829cea0 100644
--- a/repack.h
+++ b/repack.h
@@ -134,11 +134,6 @@ void pack_geometry_init(struct pack_geometry *geometry,
const struct pack_objects_args *args);
void pack_geometry_split(struct pack_geometry *geometry);
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,
- bool wrote_incremental_midx);
void pack_geometry_release(struct pack_geometry *geometry);
struct tempfile;
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 05/10] repack: delete geometric packs via existing_packs
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
Now that packs above the geometric split are marked as retained, teach
geometric repacks to use the existing_packs deletion machinery instead of
calling pack_geometry_remove_redundant().
This lets geometric repacks share the same mark-then-remove path as
all-into-one repacks: packs below the split are marked for deletion, and
packs above the split are ignored because they were retained earlier.
When doing a geometric repack without --combine-cruft-below-size, retain
all cruft packs before marking anything for deletion. Geometric repacks do
not rewrite cruft packs in that mode, so the common deletion path must not
remove them.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 11 +++++------
repack.c | 8 ++++++++
repack.h | 1 +
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index 66b46b86896..dfb6fed231d 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -574,10 +574,13 @@ int cmd_repack(int argc,
packtmp);
/* End of pack replacement. */
- if (delete_redundant && pack_everything & ALL_INTO_ONE) {
+ if (delete_redundant) {
if (write_midx == REPACK_WRITE_MIDX_INCREMENTAL)
existing_packs_retain_midx_packs(&existing, &geometry);
- existing_packs_mark_for_deletion(&existing, &names);
+ if (geometry.split_factor && !combine_cruft_below_size)
+ existing_packs_retain_all_cruft(&existing);
+ if (pack_everything & ALL_INTO_ONE || geometry.split_factor)
+ existing_packs_mark_for_deletion(&existing, &names);
}
if (write_midx != REPACK_WRITE_MIDX_NONE) {
@@ -609,10 +612,6 @@ int cmd_repack(int argc,
existing_packs_remove_redundant(&existing, packdir,
wrote_incremental_midx);
- if (geometry.split_factor)
- pack_geometry_remove_redundant(&geometry, &names,
- &existing, packdir,
- wrote_incremental_midx);
if (show_progress)
opts |= PRUNE_PACKED_VERBOSE;
prune_packed_objects(opts);
diff --git a/repack.c b/repack.c
index c7b79a3c113..90797561954 100644
--- a/repack.c
+++ b/repack.c
@@ -242,6 +242,14 @@ static struct string_list_item *locate_existing_pack(struct string_list *list,
return item;
}
+void existing_packs_retain_all_cruft(struct existing_packs *existing)
+{
+ struct string_list_item *item;
+
+ for_each_string_list_item(item, &existing->cruft_packs)
+ existing_packs_mark_retained(item);
+}
+
void existing_packs_retain_cruft(struct existing_packs *existing,
struct packed_git *cruft)
{
diff --git a/repack.h b/repack.h
index f0d082df9e8..90c89630ef8 100644
--- a/repack.h
+++ b/repack.h
@@ -81,6 +81,7 @@ void existing_packs_collect(struct existing_packs *existing,
const struct string_list *extra_keep);
int existing_packs_has_non_kept(const struct existing_packs *existing);
int existing_pack_is_marked_for_deletion(struct string_list_item *item);
+void existing_packs_retain_all_cruft(struct existing_packs *existing);
void existing_packs_retain_cruft(struct existing_packs *existing,
struct packed_git *cruft);
void existing_packs_retain_from_geometry(struct existing_packs *existing,
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 04/10] repack: teach MIDX retention about geometric rollups
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
When writing an incremental MIDX, existing_packs_retain_midx_packs()
marks packs in the existing MIDX chain as retained. This keeps them from
being deleted by the later existing_packs deletion pass, since retained
MIDX layers may still refer to those packs.
Geometric repacks need a narrower rule. Packs below the split are rolled
up into the newly-written pack, and should remain eligible for deletion
even if the old MIDX chain mentions them. Packs above the split were
marked as retained by the previous commit.
Teach existing_packs_retain_midx_packs() to skip packs which are part of
the geometric rollup. This does not change the current caller's behavior,
since geometric repacks do not yet use the existing_packs deletion path.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 2 +-
repack.c | 43 +++++++++++++++++++++++++++++++++++++++++--
repack.h | 3 ++-
3 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index ce979d86d96..66b46b86896 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -576,7 +576,7 @@ int cmd_repack(int argc,
if (delete_redundant && pack_everything & ALL_INTO_ONE) {
if (write_midx == REPACK_WRITE_MIDX_INCREMENTAL)
- existing_packs_retain_midx_packs(&existing);
+ existing_packs_retain_midx_packs(&existing, &geometry);
existing_packs_mark_for_deletion(&existing, &names);
}
diff --git a/repack.c b/repack.c
index 9b3cb425431..c7b79a3c113 100644
--- a/repack.c
+++ b/repack.c
@@ -292,6 +292,39 @@ void existing_packs_mark_for_deletion(struct existing_packs *existing,
&existing->cruft_packs);
}
+static int pack_geometry_contains_pack(struct packed_git **packs,
+ uint32_t packs_nr,
+ const char *base)
+{
+ struct strbuf buf = STRBUF_INIT;
+ uint32_t i;
+
+ for (i = 0; i < packs_nr; i++) {
+ strbuf_reset(&buf);
+ strbuf_addstr(&buf, pack_basename(packs[i]));
+ strbuf_strip_suffix(&buf, ".pack");
+
+ if (!strcmp(buf.buf, base)) {
+ strbuf_release(&buf);
+ return 1;
+ }
+ }
+
+ strbuf_release(&buf);
+ return 0;
+}
+
+static int pack_geometry_contains_rollup(const struct pack_geometry *geometry,
+ const char *base)
+{
+ if (!geometry || !geometry->split_factor)
+ return 0;
+
+ return pack_geometry_contains_pack(geometry->pack, geometry->split, base) ||
+ pack_geometry_contains_pack(geometry->promisor_pack,
+ geometry->promisor_split, base);
+}
+
/*
* Mark every pack that is referenced by the existing MIDX chain as
* retained, so that a subsequent call to
@@ -300,9 +333,12 @@ void existing_packs_mark_for_deletion(struct existing_packs *existing,
* 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.
+ * freshly-written pack supersedes them. When doing a geometric repack,
+ * packs below the split are rewritten into the new MIDX tip and should
+ * remain eligible for deletion.
*/
-void existing_packs_retain_midx_packs(struct existing_packs *existing)
+void existing_packs_retain_midx_packs(struct existing_packs *existing,
+ const struct pack_geometry *geometry)
{
struct string_list_item *item;
struct strbuf buf = STRBUF_INIT;
@@ -315,6 +351,9 @@ void existing_packs_retain_midx_packs(struct existing_packs *existing)
strbuf_strip_suffix(&buf, ".pack");
strbuf_strip_suffix(&buf, ".idx");
+ if (pack_geometry_contains_rollup(geometry, buf.buf))
+ continue;
+
found = string_list_lookup(&existing->non_kept_packs, buf.buf);
if (found)
existing_packs_mark_retained(found);
diff --git a/repack.h b/repack.h
index bb4c944d0cb..f0d082df9e8 100644
--- a/repack.h
+++ b/repack.h
@@ -87,7 +87,8 @@ void existing_packs_retain_from_geometry(struct existing_packs *existing,
const struct pack_geometry *geometry);
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_retain_midx_packs(struct existing_packs *existing,
+ const struct pack_geometry *geometry);
void existing_packs_remove_redundant(struct existing_packs *existing,
const char *packdir,
bool wrote_incremental_midx);
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 03/10] repack: mark geometric progression of packs as retained
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
In non-geometric repacks, any packs which repack wishes to delete are
handled via the `existing_packs` struct, which has a mechanism to retain
would-be-deleted packs (e.g., if we happened to write a new pack
identical to one otherwise marked for deletion).
In geometric repacks, repack removes any rewritten packs (alternatively,
any packs which were combined in order to restore a geometric
progression) by enumerating them via `pack_geometry_remove_redundant()`.
Prepare to use the `existing_packs` deletion machinery for geometric
repacks by marking any non-kept packs above the geometric split line as
retained. Do the same for promisor packs, which have their own split
point.
This commit only records which packs the later deletion pass must keep;
it does not change which packs are written or removed.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/repack.c | 2 ++
repack.c | 27 +++++++++++++++++++++++++++
repack.h | 3 +++
3 files changed, 32 insertions(+)
diff --git a/builtin/repack.c b/builtin/repack.c
index 1524a9c13ad..ce979d86d96 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -325,6 +325,8 @@ int cmd_repack(int argc,
}
pack_geometry_init(&geometry, &existing, &po_args);
pack_geometry_split(&geometry);
+
+ existing_packs_retain_from_geometry(&existing, &geometry);
}
prepare_pack_objects(&cmd, &po_args, packtmp);
diff --git a/repack.c b/repack.c
index 986c74ac7e8..9b3cb425431 100644
--- a/repack.c
+++ b/repack.c
@@ -254,6 +254,33 @@ void existing_packs_retain_cruft(struct existing_packs *existing,
existing_packs_mark_retained(item);
}
+static void existing_packs_retain_non_kept(struct existing_packs *existing,
+ struct packed_git *p)
+{
+ struct string_list_item *item;
+
+ if (!p->pack_local)
+ return;
+
+ item = locate_existing_pack(&existing->non_kept_packs, p);
+ if (!item)
+ BUG("could not find non-kept pack '%s'", pack_basename(p));
+
+ existing_packs_mark_retained(item);
+}
+
+void existing_packs_retain_from_geometry(struct existing_packs *existing,
+ const struct pack_geometry *geometry)
+{
+ uint32_t i;
+
+ for (i = geometry->split; i < geometry->pack_nr; i++)
+ existing_packs_retain_non_kept(existing, geometry->pack[i]);
+ for (i = geometry->promisor_split; i < geometry->promisor_pack_nr; i++)
+ existing_packs_retain_non_kept(existing,
+ geometry->promisor_pack[i]);
+}
+
void existing_packs_mark_for_deletion(struct existing_packs *existing,
struct string_list *names)
diff --git a/repack.h b/repack.h
index f9fbc895f02..bb4c944d0cb 100644
--- a/repack.h
+++ b/repack.h
@@ -54,6 +54,7 @@ int finish_pack_objects_cmd(const struct git_hash_algo *algop,
struct repository;
struct packed_git;
+struct pack_geometry;
struct existing_packs {
struct repository *repo;
@@ -82,6 +83,8 @@ int existing_packs_has_non_kept(const struct existing_packs *existing);
int existing_pack_is_marked_for_deletion(struct string_list_item *item);
void existing_packs_retain_cruft(struct existing_packs *existing,
struct packed_git *cruft);
+void existing_packs_retain_from_geometry(struct existing_packs *existing,
+ const struct pack_geometry *geometry);
void existing_packs_mark_for_deletion(struct existing_packs *existing,
struct string_list *names);
void existing_packs_retain_midx_packs(struct existing_packs *existing);
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 01/10] repack: unconditionally exclude non-kept packs
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1782500507.git.me@ttaylorr.com>
In `write_cruft_pack()`, we handle excluding objects found in non-kept
packs from being included in the cruft pack via two code paths:
* When using '--combine-cruft-below-size' (provided that we are not
expiring cruft objects), we use the aptly-named
`combine_small_cruft_packs()` function.
* In all other cases, we handle it directly in the 'else' branch of the
same conditional.
Simplify this by moving the non-kept pack exclusion out of the
conditional entirely, so that non-kept packs are always excluded
regardless of whether we are combining small cruft packs or not.
This is a preparatory refactor for a subsequent change that will use the
pack_geometry struct when available to determine which non-kept packs to
exclude.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
repack-cruft.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/repack-cruft.c b/repack-cruft.c
index 0653e887923..6a040e98017 100644
--- a/repack-cruft.c
+++ b/repack-cruft.c
@@ -9,7 +9,6 @@ static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size,
{
struct packed_git *p;
struct strbuf buf = STRBUF_INIT;
- size_t i;
repo_for_each_pack(existing->repo, p) {
if (!(p->is_cruft && p->pack_local))
@@ -30,10 +29,6 @@ static void combine_small_cruft_packs(FILE *in, off_t combine_cruft_below_size,
}
}
- for (i = 0; i < existing->non_kept_packs.nr; i++)
- fprintf(in, "-%s.pack\n",
- existing->non_kept_packs.items[i].string);
-
strbuf_release(&buf);
}
@@ -80,15 +75,14 @@ int write_cruft_pack(const struct write_pack_opts *opts,
in = xfdopen(cmd.in, "w");
for_each_string_list_item(item, names)
fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
- if (combine_cruft_below_size && !cruft_expiration) {
+ if (combine_cruft_below_size && !cruft_expiration)
combine_small_cruft_packs(in, combine_cruft_below_size,
existing);
- } else {
- for_each_string_list_item(item, &existing->non_kept_packs)
- fprintf(in, "-%s.pack\n", item->string);
+ else
for_each_string_list_item(item, &existing->cruft_packs)
fprintf(in, "-%s.pack\n", item->string);
- }
+ for_each_string_list_item(item, &existing->non_kept_packs)
+ fprintf(in, "-%s.pack\n", item->string);
for_each_string_list_item(item, &existing->kept_packs)
fprintf(in, "%s.pack\n", item->string);
fclose(in);
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply related
* [RFC PATCH 00/10] repack: combine '--geometric' and '--cruft'
From: Taylor Blau @ 2026-06-26 19:02 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
First, a short note. This series is an RFC because I have not had the
chance to review and test it as thoroughly as I normally would, and
because we are deep in the -rc phase.
I wanted to get this series off my backlog since I have decided to leave
GitHub at the end of the month for a new role. I will still be
contributing to Git in my new role (which I will start in the early part
of July), but wanted to get this off my backlog nonetheless.
This series teaches `git repack` how to combine `--geometric` and
`--cruft`.
Today these two modes are mutually exclusive, since `--cruft` implies
`-a`, and `-a` is fundamentally incompatible with `--geometric`. As a
result, repositories have to choose between keeping reachable objects in
a geometric progression of packs and collecting unreachable objects into
cruft packs.
The goal of this series is to to be able to do both simultaneously. When
both options are given, 'git repack' rolls up the selected non-cruft
packs as usual while collecting unreachable objects separately into a
cruft pack. That means a command like
$ git repack -d --geometric=2 --cruft --combine-cruft-below-size=1G
will keep reachable non-cruft packs in a geometric progression, while
combining sufficiently-small cruft packs (along with newly-discovered
unreachable objects) into a fresh cruft pack.
The series is structured roughly as follows:
* The first two patches prepare the cruft pack machinery for the later
changes by making non-kept pack exclusion unconditional and
extracting a helper for looking up packs in an `existing_packs` list.
* The next four patches route geometric pack deletion through the
common `existing_packs` machinery. They mark packs above the
geometric split as retained, teach incremental-MIDX retention not to
keep packs that are being rolled up, switch geometric repacks over to
the common deletion path, and then remove the old geometry-specific
deletion helper.
* The next three patches teach `pack-objects` the new pieces needed by
this mode. The main addition is `--stdin-packs=follow-reachable`,
which walks from reference tips and includes only reachable objects
from the selected packs, while still allowing traversal through
excluded-open packs and stopping at excluded-closed ones. The
following patch teaches that mode to use `--refs-snapshot`, so that
`pack-objects` and the MIDX bitmap writer can agree on the same set
of tips.
* The final patch wires everything together in `git repack`, including
teaching the cruft writer how to interpret the geometric split when
choosing which packs to include or exclude.
Thanks in advance for your review!
Taylor Blau (10):
repack: unconditionally exclude non-kept packs
repack: extract `locate_existing_pack()` helper
repack: mark geometric progression of packs as retained
repack: teach MIDX retention about geometric rollups
repack: delete geometric packs via existing_packs
repack-geometry: drop unused redundant-pack removal
pack-objects: extract `stdin_packs_add_all_pack_entries()`
pack-objects: introduce '--stdin-packs=follow-reachable'
pack-objects: support '--refs-snapshot' with 'follow-reachable'
repack: support combining '--geometric' with '--cruft'
Documentation/git-pack-objects.adoc | 25 +++
Documentation/git-repack.adoc | 11 ++
builtin/pack-objects.c | 276 ++++++++++++++++++++++++----
builtin/repack.c | 38 ++--
repack-cruft.c | 29 ++-
repack-geometry.c | 44 -----
repack.c | 101 +++++++++-
repack.h | 15 +-
t/t5331-pack-objects-stdin.sh | 201 ++++++++++++++++++++
t/t7704-repack-cruft.sh | 251 +++++++++++++++++++++++++
10 files changed, 878 insertions(+), 113 deletions(-)
base-commit: ab776a62a78576513ee121424adb19597fbb7613
--
2.55.0.rc2.10.g29e31820dce
^ permalink raw reply
* Re: [PATCH RFC v2 2/2] Move libgit.a sources into separate "lib/" directory
From: Junio C Hamano @ 2026-06-26 18:50 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Patrick Steinhardt, git, brian m. carlson, Elijah Newren,
Derrick Stolee, Phillip Wood
In-Reply-To: <32bb1cf6-1e37-dc0c-dfb2-e78a30763342@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> - path: 'compat/vcbuild/vcpkg'
>> + path: 'lib/compat/vcbuild/vcpkg'
>> - name: download vcpkg artifacts
>> uses: git-for-windows/get-azure-pipelines-artifact@v0
>> with:
>
> Please also adopt:
>
> -- snip --
> From 1d09a51d426bd3592e4f4b0331f7715ab3b5d502 Mon Sep 17 00:00:00 2001
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> Date: Fri, 26 Jun 2026 14:39:19 +0200
> Subject: [PATCH] fixup??? Move libgit.a sources into separate "lib/" directory
>
> Turns out that there was one path that was forgotten to be adjusted.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> .github/workflows/main.yml | 1 +
> 1 file changed, 1 insertion(+)
Thanks. Queued at the tip of the topic for now, but I trust Patrick
will include/squash it in the next iteration.
^ permalink raw reply
* Re: [PATCH v3 0/8] commit-reach: terminate merge-base walk when one side is exhausted
From: Junio C Hamano @ 2026-06-26 18:43 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Kristofer Karlsson via GitGitGadget, git, Derrick Stolee,
Elijah Newren
In-Reply-To: <CAL71e4OT0brcmbNXBzKpZuxTh3=R0j+zgxWmV4S-weT3q=vpvQ@mail.gmail.com>
Kristofer Karlsson <krka@spotify.com> writes:
> On Fri, 26 Jun 2026 at 18:36, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> I am getting this failure standalone, when applied on the same base
>> as where v2 was applied earlier. For now I'll eject it from 'seen'.
>>
>
> Yes, I am sorry about that. I submitted it after having missed running t6600
> and accidentally having introduced a bug. I tried to self-report it to avoid
> wasting your time, but I only did so in the relevant v3 patch and
> not this main message.
Ah, I see. Mistakes happen, and do not need to rush, as
collaboration is asynchronous around here anyway, and we may read
our e-mails in different order ;-)
Thanks. It would have been a more troubling experience if only my
set-up were seeing the issue, but I am glad that is not the case.
^ permalink raw reply
* Re: [PATCH v5 0/4] history: add squash subcommand to fold a range
From: Harald Nordgren @ 2026-06-26 18:36 UTC (permalink / raw)
To: Junio C Hamano
Cc: Phillip Wood, phillip.wood, Harald Nordgren via GitGitGadget, git,
Patrick Steinhardt
In-Reply-To: <xmqqh5mpcsc0.fsf@gitster.g>
Interesting, I will take a look at handling multiple args.
Harald
^ permalink raw reply
* Re: [PATCH v5 3/3] replay: offer an option to linearize the commit topology
From: Junio C Hamano @ 2026-06-26 17:10 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Elijah Newren, Johannes Schindelin
In-Reply-To: <20260626-toon-git-replay-drop-merges-v5-3-5e120738b9d0@iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> Documentation/git-replay.adoc | 8 ++++-
> builtin/replay.c | 6 +++-
> replay.c | 50 ++++++++++++++++----------
> replay.h | 5 +++
> t/t3650-replay-basics.sh | 84 ++++++++++++++++++++++++++++++++++++++++++-
> 5 files changed, 132 insertions(+), 21 deletions(-)
"replay --linearize" behaves differently from the flattening rebase
in a case where X and Y that forked from A are merged at Z, and we
ask to flatten the history leading to Z, doesn't it?
A----X
\ \
Y----Z (tip)
A typical flattening rebase would rewrite X to X', Y to Y', while
dropping Z, and would leave us a flattened history, like
A---X'---Y' (updated tip, the order of X' and Y' may be swapped)
I may be misreading the logic, but doesn't "replay --linearize"
instead produce
A----X' (dangling)
\
Y' (tip -- Z is dropped and gets mapped)
and leave X' dangling (or Y'; the point is that only one of them
will survive), never incorporating it in the resulting history?
> + if (commit->parents && commit->parents->next) {
> + if (!opts->linearize)
> + die(_("replaying merge commits is not supported yet!"));
> + /*
> + * Drop the merge commit: do not pick it, leave
> + * `last_commit` unchanged, and fall through to the
> + * rest of the loop. As a result:
> + * - the merge commit is mapped to `last_commit` in
> + * `replayed_commits`, this will become the parent for
> + * the child commits.
> + * - refs previously pointing to the merge commit are
> + * rewritten to point to the previous non-merge commit.
> + */
> + } else {
> + /*
> + * pick_regular_commit() looks up the parent of `commit` in
> + * `replayed_commits` to determine the ancestor to replay onto.
> + * The `default_base` parameter is used when no ancestor is found,
> + * which happens for the first commit in the revision range.
> + * When reverting, commits are replayed in reverse order, so the
> + * lookup never succeeds, and we need to pass `last_commit`.
> + */
> + struct commit *base = onto;
> + if (mode == REPLAY_MODE_REVERT)
> + base = last_commit;
> +
> + last_commit = pick_regular_commit(revs->repo, commit, base,
> + replayed_commits,
> + &merge_opt, &result,
> + mode, opts->empty);
> + }
> +
> if (!last_commit)
> break;
Immediately after this hunk beyond the post-context are these lines.
/* Record commit -> last_commit mapping */
put_mapped_commit(replayed_commits, commit, last_commit);
Let's imagine X gets processed first. X (and other commits on its
branch) gets replayed, last_commit is set to X' (which is the
rewritten X). replayed_commits mapping holds X->X' mapping.
Then let's imagine the history leading to Y is replayed next.
last_commit becomes Y', and Y->Y' mapping is stored in
replayed_commits.
Finally, we see Z. We are going to _drop_ it. last_commit is left
unchanged, pointing at Y'. Then last_commit (i.e., Y') is used as
the merge commit Z maps to (i.e., correctly dropping Z).
Any descendants of Z, if any, will be grafted as descendants of Y'.
If X did not have any descendants other than Z in the rewritten part
of the history, then X' (and commits leading to it) would be lost,
no?
This "loss of the other branch" may be an inherent characteristic of
this feature (i.e., I do not think it is necessarily a bug, and it
may even be that the "bug" is in the way I am reading the patch),
but then I wonder if the user may want to have control over which
side branch should survive, perhaps? It would probably need to be
documented, and a test or two to cast this behaviour in stone.
> diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
> index 3353bc4a4d..34c038eab9 100755
> --- a/t/t3650-replay-basics.sh
> +++ b/t/t3650-replay-basics.sh
> @@ -52,8 +52,12 @@ test_expect_success 'setup' '
The pre-context here has
git switch --detach topic4 &&
test_commit N &&
test_commit O &&
git switch -c topic-with-merge topic4 &&
> test_merge P O --no-ff &&
> git switch main &&
The above does prepare topic-with-merge branch, but ...
> +test_expect_success 'replay to rebase merge commit with --linearize' '
> + git replay --ref-action=print --linearize \
> + --onto main I..topic-with-merge >result &&
... this does not really exersize linearizing replay in a typical
mergy history. P merges O with --no-ff because otherwise there
won't be a merge, since O is a descendant of the commit "test_merge
P O" runs on (i.e., topic4 == topic-with-merge).
topic4 --- N --- O
\ \
.-----------P
So, as long as O is replayed later than the parent of N (which is
true), O' will be the surviving tip (corresponds to Y' that the
dropped Z was mapped to in the earlier example), and nothing gets
orphaned, I think.
Perhaps a test to try a real merge may look something like this.
diff --git c/t/t3650-replay-basics.sh w/t/t3650-replay-basics.sh
index 34c038eab9..bb737f729a 100755
--- c/t/t3650-replay-basics.sh
+++ w/t/t3650-replay-basics.sh
@@ -647,4 +647,37 @@ test_expect_success 'replay with --linearize to rebase multiple divergent branch
test_cmp expect actual
'
+test_expect_success 'replay with --linearize of a divergent merge drops one branch' '
+ git switch -c topic-divergent-base main &&
+ test_commit base &&
+ # Fork 1: base -> X
+ git switch -c topic-divergent-x &&
+ test_commit X &&
+ # Fork 2: base -> Y
+ git switch topic-divergent-base &&
+ git switch -c topic-divergent-y &&
+ test_commit Y &&
+ # Merge them at Z
+ git switch topic-divergent-x &&
+ test_merge Z topic-divergent-y --no-ff &&
+
+ # History is now:
+ #
+ # X - Z (topic-divergent-x)
+ # / /
+ # base - Y
+ #
+
+ git replay --ref-action=print --linearize \
+ --onto main topic-divergent-base..topic-divergent-x >result &&
+ test_line_count = 1 result &&
+ tip=$(cut -f 3 -d " " result) &&
+ # Get the commits replayed onto main
+ git log --format=%s main..$tip >actual &&
+ # We expect exactly one commit to be replayed (either X or Y)
+ # because the other one is left dangling due to the merge being dropped.
+ test_line_count = 1 actual &&
+ test_grep "^[XY]$" actual
+'
+
test_done
^ permalink raw reply related
* Re: [PATCH GSoC v14 07/13] connect: refactor packet writing
From: Karthik Nayak @ 2026-06-26 17:03 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260625-ps-eric-work-rebase-v14-7-09f7ffe21a53@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 743 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
The subject is bit too generic no? Maybe we can talk about the function?
Perhaps:
connect: make `write_fetch_command_and_capabilities()` more generic
> Refactor `write_fetch_command_and_capabilities()`, enabling it to serve
> both fetch and additional commands.
>
> In this context, "command" refers to the "operations" supported by
> Git's wire protocol https://git-scm.com/docs/protocol-v2, such as a Git
> subcommand (e.g., git-fetch(1)) or a server-side operation like
> "object-info" as implemented in commit a2ba162
> (object-info: support for retrieving object info, 2021-04-20).
>
> Refactor the function signature to accept a command instead of the
> hardcoded "fetch".
[snip]
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH GSoC v14 06/13] fetch-pack: move function to connect.c
From: Karthik Nayak @ 2026-06-26 16:57 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260625-ps-eric-work-rebase-v14-6-09f7ffe21a53@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5192 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
Nit: could we mention the function in the subject?
> write_fetch_command_and_capabilities will be refactored in a
> subsequent
Super Nit: Some parts of your patches use backticks for quoting code or
filenames and others skip the convention. It would be nice to be
consistent.
> commit where it will become a more general-purpose function, making it
> more accessible to additional commands in the future.
>
> Move `write_fetch_command_and_capabilities()` to `connect.c`, where
> there are similar purpose functions.
>
> Because string_list is only used as a pointer, use a forward
> declaration [1].
>
> [1]: https://lore.kernel.org/git/Z0RIqUAoEob8lGfM@pks.im/
>
> Helped-by: Jonathan Tan <jonathantanmy@google.com>
> Helped-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> connect.c | 34 ++++++++++++++++++++++++++++++++++
> connect.h | 4 ++++
> fetch-pack.c | 34 ----------------------------------
> 3 files changed, 38 insertions(+), 34 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 47e39d2a73..1dced8e632 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -700,6 +700,40 @@ int server_supports(const char *feature)
> return !!server_feature_value(feature, NULL);
> }
>
> +void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> + const struct string_list *server_options)
> +{
> + const char *hash_name;
> + int advertise_sid;
> +
> + repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
> +
> + ensure_server_supports_v2("fetch");
> + packet_buf_write(req_buf, "command=fetch");
> + if (server_supports_v2("agent"))
> + packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
> + if (advertise_sid && server_supports_v2("session-id"))
> + packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
> + if (server_options && server_options->nr) {
> + ensure_server_supports_v2("server-option");
> + for (size_t i = 0; i < server_options->nr; i++)
> + packet_buf_write(req_buf, "server-option=%s",
> + server_options->items[i].string);
> + }
> +
> + if (server_feature_v2("object-format", &hash_name)) {
> + const unsigned int hash_algo = hash_algo_by_name(hash_name);
> + if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
> + die(_("mismatched algorithms: client %s; server %s"),
> + the_hash_algo->name, hash_name);
> + packet_buf_write(req_buf, "object-format=%s", the_hash_algo->name);
> + } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1_LEGACY) {
> + die(_("the server does not support algorithm '%s'"),
> + the_hash_algo->name);
> + }
> + packet_buf_delim(req_buf);
> +}
> +
> static const char *url_scheme_name(enum url_scheme scheme)
> {
> switch (scheme) {
> diff --git a/connect.h b/connect.h
> index aa482a37fb..c4f6ea4b0a 100644
> --- a/connect.h
> +++ b/connect.h
> @@ -34,4 +34,8 @@ void check_stateless_delimiter(int stateless_rpc,
> struct packet_reader *reader,
> const char *error);
>
> +struct string_list;
> +void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> + const struct string_list *server_options);
> +
> #endif
> diff --git a/fetch-pack.c b/fetch-pack.c
> index ad07603755..4a8a70b5f3 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1376,40 +1376,6 @@ static int add_haves(struct fetch_negotiator *negotiator,
> return haves_added;
> }
>
> -static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> - const struct string_list *server_options)
> -{
> - const char *hash_name;
> - int advertise_sid;
> -
> - repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
> -
> - ensure_server_supports_v2("fetch");
> - packet_buf_write(req_buf, "command=fetch");
> - if (server_supports_v2("agent"))
> - packet_buf_write(req_buf, "agent=%s", git_user_agent_sanitized());
> - if (advertise_sid && server_supports_v2("session-id"))
> - packet_buf_write(req_buf, "session-id=%s", trace2_session_id());
> - if (server_options && server_options->nr) {
> - ensure_server_supports_v2("server-option");
> - for (size_t i = 0; i < server_options->nr; i++)
> - packet_buf_write(req_buf, "server-option=%s",
> - server_options->items[i].string);
> - }
> -
> - if (server_feature_v2("object-format", &hash_name)) {
> - const unsigned int hash_algo = hash_algo_by_name(hash_name);
> - if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
> - die(_("mismatched algorithms: client %s; server %s"),
> - the_hash_algo->name, hash_name);
> - packet_buf_write(req_buf, "object-format=%s", the_hash_algo->name);
> - } else if (hash_algo_by_ptr(the_hash_algo) != GIT_HASH_SHA1_LEGACY) {
> - die(_("the server does not support algorithm '%s'"),
> - the_hash_algo->name);
> - }
> - packet_buf_delim(req_buf);
> -}
> -
> static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
> struct fetch_pack_args *args,
> const struct ref *wants, struct oidset *common,
>
> --
> 2.54.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH GSoC v14 05/13] fetch-pack: prepare function to be moved
From: Karthik Nayak @ 2026-06-26 16:54 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon, Jonathan Tan, Calvin Wan
In-Reply-To: <20260625-ps-eric-work-rebase-v14-5-09f7ffe21a53@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3287 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
The subject doesn't really give much insight into what the patch does.
Perhaps something like:
fetch-pack: use repo config in `write_fetch_command_and_capabilities()`
fetch-pack: drop static variable use in
`write_fetch_command_and_capabilities()`
> `write_fetch_command_and_capabilities()` will be refactored and moved in
> subsequent commits where it will become a more general-purpose function,
> making it more accessible to additional commands in the future.
>
> To move `write_fetch_command_and_capabilities()` to `connect.c`, we
> previously need to adjust how `advertise_sid` is managed. Currently in
I don't think 'previously' makes sense here.
> `fetch_pack.c`, `advertise_sid` is a static variable, modified using
> `repo_config_get_bool()`.
>
Perhaps:
To move `write_fetch_command_and_capabilities()` to `connect.c`,
drop the usage of file static variable `advertise_sid` within the
function. Currently, `advertise_sid` is modified...
>
> Initialize `advertise_sid` at the begining by directly using
> `repo_config_get_bool()`. This change is safe because:
>
> In the original `fetch-pack.c` code, there are only two places that write
> `advertise_sid`:
>
This needs to be modified no? This is from the prev patch, where we
moved and refactored in the same patch, this no longer is the case.
> 1. In function `do_fetch_pack()`:
> if (!server_supports("session_id"))
> advertise_sid = 0;
> 2. In function `fetch_pack_config()`:
> repo_config_get_bool("transfer.advertisesid", &advertise_sid);
>
> About 1, since `do_fetch_pack()` is only relevant for protocol v1, this
> assignment can be ignored, as `write_fetch_command_and_capabilities()`
> is only used in v2.
>
> About 2, `repo_config_get_bool()` is from `config.h` and it's an
> out-of-box dependency of `connect.c`, so we can reuse it directly.
>
> Helped-by: Jonathan Tan <jonathantanmy@google.com>
> Helped-by: Christian Couder <chriscool@tuxfamily.org>
> Signed-off-by: Calvin Wan <calvinwan@google.com>
> Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> fetch-pack.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fetch-pack.c b/fetch-pack.c
> index f13951d154..ad07603755 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -1380,6 +1380,9 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> const struct string_list *server_options)
> {
> const char *hash_name;
> + int advertise_sid;
> +
> + repo_config_get_bool(the_repository, "transfer.advertisesid", &advertise_sid);
>
> ensure_server_supports_v2("fetch");
> packet_buf_write(req_buf, "command=fetch");
> @@ -1395,7 +1398,7 @@ static void write_fetch_command_and_capabilities(struct strbuf *req_buf,
> }
>
> if (server_feature_v2("object-format", &hash_name)) {
> - int hash_algo = hash_algo_by_name(hash_name);
> + const unsigned int hash_algo = hash_algo_by_name(hash_name);
>
Agreed with Chandra, this needs to be assessed.
> if (hash_algo_by_ptr(the_hash_algo) != hash_algo)
> die(_("mismatched algorithms: client %s; server %s"),
> the_hash_algo->name, hash_name);
>
> --
> 2.54.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v5 1/3] replay: add helper to put entry into mapped_commits
From: Junio C Hamano @ 2026-06-26 16:50 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Elijah Newren
In-Reply-To: <20260626-toon-git-replay-drop-merges-v5-1-5e120738b9d0@iotcl.com>
Toon Claes <toon@iotcl.com> writes:
> +static void put_mapped_commit(kh_oid_map_t *replayed_commits,
> + struct commit *commit,
> + struct commit *new_commit)
> +{
> + khint_t pos;
> + int ret;
> +
> + pos = kh_put_oid_map(replayed_commits, commit->object.oid, &ret);
> + if (ret == 0)
> + BUG("Duplicate rewritten commit: %s\n",
Please do not add terminating LF at the end of single-liner messages
that use our print infrastructure, like BUG(), warning(), error(),
and die(), as the machinery adds one for you.
Other than that, looking good.
^ permalink raw reply
* Re: [PATCH v3 0/8] commit-reach: terminate merge-base walk when one side is exhausted
From: Kristofer Karlsson @ 2026-06-26 16:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: Kristofer Karlsson via GitGitGadget, git, Derrick Stolee,
Elijah Newren
In-Reply-To: <xmqqjyrl8di0.fsf@gitster.g>
On Fri, 26 Jun 2026 at 18:36, Junio C Hamano <gitster@pobox.com> wrote:
>
> I am getting this failure standalone, when applied on the same base
> as where v2 was applied earlier. For now I'll eject it from 'seen'.
>
Yes, I am sorry about that. I submitted it after having missed running t6600
and accidentally having introduced a bug. I tried to self-report it to avoid
wasting your time, but I only did so in the relevant v3 patch and
not this main message.
I will be more careful for v4 (and onwards)
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH GSoC v14 02/13] git-compat-util: add strtoul_szt() with error handling
From: Karthik Nayak @ 2026-06-26 16:43 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: chandrapratap3519, chriscool, eric.peijian, gitster, jltobler,
peff, toon
In-Reply-To: <20260625-ps-eric-work-rebase-v14-2-09f7ffe21a53@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> From: Eric Ju <eric.peijian@gmail.com>
>
> We already have strtoul_ui() and similar functions that provide proper
> error handling using strtoul from the standard library. However,
> there isn't currently a variant that returns an unsigned long.
>
Shouldn't this say returns a 'size_t'?
> This variant is needed in a subsequent commit to enable returning an
> size_t with proper error handling.
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> git-compat-util.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 8809776407..7f417f1acf 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -975,6 +975,26 @@ static inline int strtoul_ui(char const *s, int base, unsigned int *result)
> return 0;
> }
>
> +/*
> + * Convert a string to a size_t using the standard library's strtoul, with
> + * additional error handling to ensure robustness.
> + */
> +static inline int strtoul_szt(char const *s, int base, size_t *result)
> +{
> + unsigned long ul;
> + char *p;
> +
> + errno = 0;
> + /* negative values would be accepted by strtoul */
> + if (strchr(s, '-'))
> + return -1;
> + ul = strtoul(s, &p, base);
> + if (errno || *p || p == s)
> + return -1;
> + *result = ul;
This converts unsigned long to size_t, but that is fine.
Looks good.
> + return 0;
> +}
> +
> static inline int strtol_i(char const *s, int base, int *result)
> {
> long ul;
>
> --
> 2.54.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
* Re: [PATCH v3 7/8] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson @ 2026-06-26 16:41 UTC (permalink / raw)
To: Derrick Stolee; +Cc: Kristofer Karlsson via GitGitGadget, git, Elijah Newren
In-Reply-To: <cb82cb80-291b-4a36-ae83-f308560c844b@gmail.com>
On Fri, 26 Jun 2026 at 16:32, Derrick Stolee <stolee@gmail.com> wrote:
>
> > I have to note that I accidentally pushed this version before noticing
> > that it now fails for a subset of commit-graph modes.
> > Apologies for that - I will rework the logic here later
> > to preserve the behavior better.
>
> And do we catch this with a test case? I'm hoping that you discovered
> this error through the test suite, even if you submitted the series a
> little early.
(I missed replying to this message initially, sorry)
Yes exactly - it was caught by t6600 but somehow I missed running it before
submitting it (so I noticed it in the GGG CI instead)
So the existing tests are good, I only wish I could be equally reliable as them.
Thanks,
Kristofer
^ permalink raw reply
* [PATCH] history: streamline message preparation and plug file stream leak
From: Junio C Hamano @ 2026-06-26 16:38 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Johannes Schindelin
An early part of fill_commit_mmessage() function uses write_file_buf()
to write out what was prepared in a strbuf, which is primarily meant
for use by callers that have their own message prepared fully and
called as the last thing to flush it to the destination file.
However, the function then opens a file stream in append mode to
further write into it. It may have been understandable if this was
a later addition, but it seems it came from a single commit,
d205234c (builtin/history: implement "reword" subcommand,
2026-01-13), which is somewhat puzzling, but anyway...
Just open the file stream upfront for writing, write the message
the function has in the strbuf, and then keep writing whatever it
wants to write to the same open file stream.
And do not forget to close the stream. We are about to pass the
resulting file to an external editor, and on some systems, notably
Windows, you are not supposed to keep a file open while expecting
another program to access it.
Diagnosed-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* As the initial one was written and sent as "how about doing a bit
more thorough job while we are at it?" response to a posted patch
found in <pull.2158.git.1782412427801.gitgitgadget@gmail.com>,
this is a tested and merge-ready cersion that I consider "v1".
builtin/history.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 8dcb9a6046..f17ec049c0 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -41,11 +41,6 @@ static int fill_commit_message(struct repository *repo,
" empty message aborts the commit.\n");
struct wt_status s;
- strbuf_addstr(out, default_message);
- strbuf_addch(out, '\n');
- strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str);
- write_file_buf(path, out->buf, out->len);
-
wt_status_prepare(repo, &s);
FREE_AND_NULL(s.branch);
s.ahead_behind_flags = AHEAD_BEHIND_QUICK;
@@ -57,14 +52,20 @@ static int fill_commit_message(struct repository *repo,
s.whence = FROM_COMMIT;
s.committable = 1;
- s.fp = fopen(git_path_commit_editmsg(), "a");
+ s.fp = fopen(path, "w");
if (!s.fp)
- return error_errno(_("could not open '%s'"), git_path_commit_editmsg());
+ return error_errno(_("could not open '%s'"), path);
+
+ strbuf_addstr(out, default_message);
+ strbuf_addch(out, '\n');
+ strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str);
+ fwrite(out->buf, 1, out->len, s.fp);
wt_status_collect_changes_trees(&s, old_tree, new_tree);
wt_status_print(&s);
wt_status_collect_free_buffers(&s);
string_list_clear_func(&s.change, change_data_free);
+ fclose(s.fp);
strbuf_reset(out);
if (launch_editor(path, out, NULL)) {
--
2.55.0-rc2-177-gc9430f6415
^ permalink raw reply related
* Re: [PATCH v3 0/8] commit-reach: terminate merge-base walk when one side is exhausted
From: Junio C Hamano @ 2026-06-26 16:36 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget
Cc: git, Derrick Stolee, Elijah Newren, Kristofer Karlsson
In-Reply-To: <pull.2149.v3.git.1782479286.gitgitgadget@gmail.com>
"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> Changes since v2:
>
> * New patch 8/8: moved the min_generation termination check and the
> last_gen monotonicity assertion into paint_queue_get(), consolidating
> halt conditions. commit_graph_generation() is now called once per
> dequeued commit and shared across all checks.
>
> * Widened the generation-monotonicity BUG assertion to fire
> unconditionally, not only when min_generation is set. The side-exhaustion
> optimization depends on correct generation ordering, so the assertion
> should always be active. This is a behavior change: the BUG() now fires
> for any generation ordering violation, regardless of the caller.
>
> * Moved all halt conditions inside paint_queue_get() with the "pop first"
> form: pop, check, then decrement counters. This keeps the optimization
> commit's diff minimal (just inserting the new checks between pop and
> decrement).
>
> * Shortened the doc comment on paint_queue_get() to describe what it does
> rather than how. Inline comments on each return NULL explain the specific
> halt condition.
>
> * Replaced the manual commit-graph setup in the step-count test with
> run_all_modes, which now sets GIT_TRACE2_EVENT per mode and produces
> trace-mode-{none,full,half,no-gdat}.txt files.
>
> * Added a test_paint_down_steps helper for concise 4-mode step assertions
> with diagnostic output on mismatch (prints "expected X, got Y" instead of
> a silent grep failure).
>
> * Added step-count assertions to the single-walk edge-case tests:
> in_merge_bases_many:self, pending-stale, infinity-both-sides,
> mixed-finite-infinity.
>
> * Included step counts alongside wall-clock times in the benchmark tables.
I am getting this failure standalone, when applied on the same base
as where v2 was applied earlier. For now I'll eject it from 'seen'.
expecting success of 6600.12 'get_merge_bases_many':
cat >input <<-\EOF &&
A:commit-5-7
X:commit-4-8
X:commit-6-6
X:commit-8-3
EOF
{
echo "get_merge_bases_many(A,X):" &&
git rev-parse commit-5-6 \
commit-4-7 | sort
} >expect &&
test_all_modes get_merge_bases_many
BUG: commit-reach.c:152: bad generation skip 9 > 8 at 772be737f220103f706a39013c0d115009feefec
not ok 12 - get_merge_bases_many
#
# cat >input <<-\EOF &&
# A:commit-5-7
# X:commit-4-8
^ permalink raw reply
* Re: [PATCH RFC v2 2/2] Move libgit.a sources into separate "lib/" directory
From: Johannes Schindelin @ 2026-06-26 16:01 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, brian m. carlson, Junio C Hamano, Elijah Newren,
Derrick Stolee, Phillip Wood
In-Reply-To: <20260622-pks-libgit-in-subdir-v2-2-cb946c51ee7b@pks.im>
Hi Patrick,
On Mon, 22 Jun 2026, Patrick Steinhardt wrote:
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index cf341d74db..a8402babd9 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -179,7 +179,7 @@ jobs:
> uses: actions/checkout@v6
> with:
> repository: 'microsoft/vcpkg'
> - path: 'compat/vcbuild/vcpkg'
> + path: 'lib/compat/vcbuild/vcpkg'
> - name: download vcpkg artifacts
> uses: git-for-windows/get-azure-pipelines-artifact@v0
> with:
Please also adopt:
-- snip --
From 1d09a51d426bd3592e4f4b0331f7715ab3b5d502 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 26 Jun 2026 14:39:19 +0200
Subject: [PATCH] fixup??? Move libgit.a sources into separate "lib/" directory
Turns out that there was one path that was forgotten to be adjusted.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
.github/workflows/main.yml | 1 +
1 file changed, 1 insertion(+)
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 29d2057bde4b..57ad4ba64f67 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -188,6 +188,7 @@ jobs:
with:
repository: git/git
definitionId: 9
+ path: lib/compat
- name: add msbuild to PATH
uses: microsoft/setup-msbuild@v3
- name: copy dlls to root
-- snap --
This is needed to fix the `vs-build` job, see
https://github.com/git-for-windows/git/actions/runs/28242360731 for proof
that it's now working.
Thank you,
Johannes
> @@ -189,11 +189,11 @@ jobs:
> uses: microsoft/setup-msbuild@v3
> - name: copy dlls to root
> shell: cmd
> - run: compat\vcbuild\vcpkg_copy_dlls.bat release
> + run: lib\compat\vcbuild\vcpkg_copy_dlls.bat release
> - name: generate Visual Studio solution
> shell: bash
> run: |
> - cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \
> + cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/lib/compat/vcbuild/vcpkg/installed/x64-windows \
> -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
> - name: MSBuild
> run: msbuild git.sln -property:Configuration=Release -property:Platform=x64 -maxCpuCount:4 -property:PlatformToolset=v142
> @@ -201,7 +201,7 @@ jobs:
> shell: bash
> env:
> MSVC: 1
> - VCPKG_ROOT: ${{github.workspace}}\compat\vcbuild\vcpkg
> + VCPKG_ROOT: ${{github.workspace}}\lib\compat\vcbuild\vcpkg
> run: |
> mkdir -p artifacts &&
> eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts NO_GETTEXT=YesPlease 2>&1 | grep ^tar)"
> [...]
^ permalink raw reply related
* [PATCH] rust: validate object map insert algorithms
From: Feng Wu via GitGitGadget @ 2026-06-26 15:58 UTC (permalink / raw)
To: git; +Cc: Feng Wu, Feng Wu
From: Feng Wu <wufengwufengwufeng@gmail.com>
The loose object map stores entries keyed by the repository's storage
hash and the compatible hash. ObjectMap::insert() accepts its two object
IDs in either order, but it currently checks only whether oid1 uses the
compatible hash algorithm. If it does not, oid2 is assumed to be the
compatible ID without validating oid2's algorithm.
That means callers can pass two IDs with the same algorithm, or an ID
using an unknown algorithm, and have one of them silently treated as the
storage ID. This does not match the map invariant that each entry must
contain exactly one storage hash and one compatible hash.
Make the invariant explicit by decoding both object ID algorithms and
rejecting unknown or mismatched pairs before inserting anything. Introduce
ObjectMapInsertError with InvalidHashAlgorithm and MismatchedAlgorithms
variants for clear error reporting.
Update the existing tests to unwrap successful insertions, and add tests
for same-algorithm and unknown-algorithm inputs.
Signed-off-by: Feng Wu <wufengwufengwufeng@gmail.com>
---
rust: validate object map insert algorithms
ObjectMap::insert() accepts a storage OID and a compatible OID in either
order, but it currently checks whether oid1 uses the compatible
algorithm, and if not, assumes oid2 is the compatible one without
validating oid2.
That means inputs with two OIDs using the same hash algorithm, or an OID
using an unknown hash algorithm, are accepted and one of them is
silently treated as the storage OID. This breaks the object map
invariant that each entry must contain exactly one storage hash and one
compatible hash.
Teach ObjectMap::insert() to decode and validate both OID algorithms
before inserting anything. The function now accepts only the two valid
permutations: (storage, compat) and (compat, storage). Unknown
algorithms and mismatched algorithm pairs are rejected via
ObjectMapInsertError.
The tests cover successful insertion in either order, same-algorithm
input, and unknown-algorithm input.
Tested with:
* cargo fmt --all -- --check
* git diff --check
* cargo test
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2350%2Fwufengwind%2Fobject-map-insert-validation-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2350/wufengwind/object-map-insert-validation-v1
Pull-Request: https://github.com/git/git/pull/2350
src/loose.rs | 79 ++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 73 insertions(+), 6 deletions(-)
diff --git a/src/loose.rs b/src/loose.rs
index 24accf9c33..8f6c1fb40e 100644
--- a/src/loose.rs
+++ b/src/loose.rs
@@ -510,6 +510,17 @@ pub struct ObjectMap {
batch: Option<ObjectMemoryMap>,
}
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum ObjectMapInsertError {
+ InvalidHashAlgorithm(u32),
+ MismatchedAlgorithms {
+ storage: HashAlgorithm,
+ compat: HashAlgorithm,
+ oid1: HashAlgorithm,
+ oid2: HashAlgorithm,
+ },
+}
+
impl ObjectMap {
/// Create a new `ObjectMap` with the given hash algorithms.
///
@@ -585,19 +596,39 @@ impl ObjectMap {
///
/// If `write` is true and there is a batch started, write the object into the batch as well as
/// into the memory map.
- pub fn insert(&mut self, oid1: &ObjectID, oid2: &ObjectID, kind: MapType, write: bool) {
- let (compat_oid, storage_oid) =
- if HashAlgorithm::from_u32(oid1.algo) == Some(self.mem.compat) {
+ pub fn insert(
+ &mut self,
+ oid1: &ObjectID,
+ oid2: &ObjectID,
+ kind: MapType,
+ write: bool,
+ ) -> Result<(), ObjectMapInsertError> {
+ let oid1_algo = HashAlgorithm::from_u32(oid1.algo)
+ .ok_or(ObjectMapInsertError::InvalidHashAlgorithm(oid1.algo))?;
+ let oid2_algo = HashAlgorithm::from_u32(oid2.algo)
+ .ok_or(ObjectMapInsertError::InvalidHashAlgorithm(oid2.algo))?;
+
+ let (storage_oid, compat_oid) =
+ if oid1_algo == self.mem.storage && oid2_algo == self.mem.compat {
(oid1, oid2)
- } else {
+ } else if oid1_algo == self.mem.compat && oid2_algo == self.mem.storage {
(oid2, oid1)
+ } else {
+ return Err(ObjectMapInsertError::MismatchedAlgorithms {
+ storage: self.mem.storage,
+ compat: self.mem.compat,
+ oid1: oid1_algo,
+ oid2: oid2_algo,
+ });
};
+
Self::insert_into(&mut self.mem, storage_oid, compat_oid, kind);
if write {
if let Some(ref mut batch) = self.batch {
Self::insert_into(batch, storage_oid, compat_oid, kind);
}
}
+ Ok(())
}
fn insert_into(
@@ -729,9 +760,9 @@ mod tests {
if *swap {
// Insert the item into the batch arbitrarily based on the type. This tests that
// we can specify either order and we'll do the right thing.
- map.insert(&s256, &s1, *kind, write);
+ map.insert(&s256, &s1, *kind, write).unwrap();
} else {
- map.insert(&s1, &s256, *kind, write);
+ map.insert(&s1, &s256, *kind, write).unwrap();
}
}
@@ -873,6 +904,42 @@ mod tests {
);
}
+ #[test]
+ fn refuses_insert_with_mismatched_algorithms() {
+ let mut map = ObjectMap::new(HashAlgorithm::SHA256, HashAlgorithm::SHA1);
+ let entries = test_entries();
+ let s256 = sha256_oid(entries[0].2);
+ let s256_other = sha256_oid(entries[1].2);
+ let s1 = sha1_oid(entries[0].1);
+ let s1_other = sha1_oid(entries[1].1);
+
+ assert!(map.insert(&s256, &s1, MapType::LooseObject, false).is_ok());
+ assert!(matches!(
+ map.insert(&s256, &s256_other, MapType::LooseObject, false),
+ Err(super::ObjectMapInsertError::MismatchedAlgorithms { .. })
+ ));
+ assert!(matches!(
+ map.insert(&s1, &s1_other, MapType::LooseObject, false),
+ Err(super::ObjectMapInsertError::MismatchedAlgorithms { .. })
+ ));
+ }
+
+ #[test]
+ fn refuses_insert_with_unknown_algorithm() {
+ let mut map = ObjectMap::new(HashAlgorithm::SHA256, HashAlgorithm::SHA1);
+ let entries = test_entries();
+ let s1 = sha1_oid(entries[0].1);
+ let invalid_oid = ObjectID {
+ hash: [0xffu8; 32],
+ algo: 99,
+ };
+
+ assert_eq!(
+ map.insert(&invalid_oid, &s1, MapType::LooseObject, false),
+ Err(super::ObjectMapInsertError::InvalidHashAlgorithm(99))
+ );
+ }
+
#[test]
fn looks_up_known_oids_correctly() {
let map = test_map(false);
base-commit: ab776a62a78576513ee121424adb19597fbb7613
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2 2/2] environment: move excludes_file into repo_config_values
From: Junio C Hamano @ 2026-06-26 15:43 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, cirnovskyv, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <20260626075037.532164-3-cat@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
> Continue the libification effort by moving the 'excludes_file' global
> variable into 'struct repo_config_values'.
>
> Since 'excludes_file' is a dynamically allocated string (char *), it
> requires proper memory management. Introduce repo_config_values_clear()
> to safely free the heap memory when repository instance is destroyed.
>
> Note:
>
> - 'if (repo != the_repository)' fallback logic is temporarily added
> in both the getter and the clear function. This prevents calling
> repo_config_values() on uninitialized submodules, which triggers BUG().
>
> - 'attribute_file' is another string variable that was migrated
> earlier. Its FREE_AND_NULL() call is also added to
> repo_config_values_clear().
OK. I think the placement of the new member in repo_config_values
in this round, moving to the spot next to existing attributes_file,
makes more sense than the previous round, too.
Looking good. Shall we mark it as ready for 'next' now?
^ permalink raw reply
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