* [PATCH v4 16/16] repack: allow `--write-midx=incremental` without `--geometric`
From: Taylor Blau @ 2026-05-19 15:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt
In-Reply-To: <cover.1779206239.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 4f5deeb97bf..b6b1de71805 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.175.g8bd0ec98dc3
^ permalink raw reply related
* [PATCH 0/8] pack-bitmap-write: speed up bitmap generation
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
Note to the maintainer:
* This series is based on 'tb/pseudo-merge-bugfixes', with
'ps/clang-w-glibc-2.43-and-_Generic' merged in. I suggest queueing it
as 'tb/bitmap-build-performance'.
The latter merge is only to avoid the current Clang/glibc 2.43 CI
breakage, and is unrelated to the bitmap changes themselves.
This series improves the performance of reachability bitmap generation,
focusing on very large repositories and the penalty to generate
pseudo-merge reachability bitmaps.
The first few patches address hot paths in the ordinary bitmap build:
- pass object positions into `fill_bitmap_tree()` so callers can avoid
redundant lookups,
- check subtree bits before recursing, which avoids many no-op
`fill_bitmap_tree()` calls,
- reuse already-stored selected bitmaps when `fill_bitmap_commit()`
reaches a selected ancestor, and
- add a small direct-mapped cache from object IDs to bitmap positions
to avoid repeated pack/MIDX lookups while filling bitmaps.
On the large repository that I have been using to benchmark these
changes (~4.8M commits and ~57M total objects), the no-pseudo-merge
bitmap generation case drops **from ~612.5 seconds to ~294.1 seconds**.
The next patch sorts selected bitmaps before choosing XOR offsets. This
does not change bitmap selection/coverage, but in the same repository it
shrinks the generated bitmap file **from ~635.5 MiB to ~176.4 MiB** by
putting related ancestor/descendant bitmaps close enough together for
the XOR search window to find them.
The final two patches focus on pseudo-merge bitmaps. The existing code
feeds pseudo-merges into the same maximal-commit selection machinery as
ordinary selected commits. That machinery works well for real history,
but not pseudo-merges.
Instead, this series builds ordinary selected bitmaps first, then builds
pseudo-merge bitmaps afterwards. The later pseudo-merge fill can still
reuse stored selected ancestor bitmaps, and can also reuse an existing
on-disk pseudo-merge bitmap when the parent set matches.
With the coarse pseudo-merge configuration used for testing:
[bitmapPseudoMerge "all"]
pattern=refs/
threshold=now
stableSize=10000000
maxMerges=8
, the optimized no-pseudo-merge case takes ~294.1 seconds, while the
**pseudo-merge case takes ~328.4 seconds**. Before the final change, the
same pseudo-merge configuration took ~575.0 seconds.
On our testing repository, it is faster at the end of this series to
generate bitmaps with pseudo-merges (~328 seconds as above) than it is
to generate bitmaps without pseudo-merges at the start of this series
(~612 seconds).
Thanks in advance for your review!
Taylor Blau (8):
pack-bitmap: pass object position to `fill_bitmap_tree()`
pack-bitmap: check subtree bits before recursing
pack-bitmap: reuse stored selected bitmaps
pack-bitmap: consolidate `find_object_pos()` success path
pack-bitmap: cache object positions during fill
pack-bitmap: sort bitmaps before XORing
pack-bitmap: remember pseudo-merge parents
pack-bitmap: build pseudo-merge bitmaps after regular bitmaps
pack-bitmap-write.c | 431 +++++++++++++++++++++++++++++++++++++-------
pack-bitmap.h | 7 +
2 files changed, 377 insertions(+), 61 deletions(-)
base-commit: c3d7ca7d982efc3a848fd85f34e867cfc0a99479
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply
* [PATCH 1/8] pack-bitmap: pass object position to `fill_bitmap_tree()`
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
In the following commit, callers of `fill_bitmap_tree()` will be
required to check the bit corresponding to their tree before calling
that function. That change will reduce the overhead of setting up and
tearing down stack frames for trees whose bits are already set.
To prepare for that change, have callers pass in the tree's bit position
in `fill_bitmap_tree()`, which will make the next commit easier to read.
In the meantime, this change has a surprising and measurable benefit
during bitmap generation, particularly on very large repositories.
When processing sub-trees within `fill_bitmap_tree()`, the preimage of
this patch did the following:
while (tree_entry(&desc, entry)) {
switch (object_type(entry.mode)) {
case OBJ_TREE:
if (fill_bitmap_tree(writer, bitmap,
lookup_tree(writer->repo,
&entry.oid)) < 0) {
/* ... */
}
/* ... */
}
}
, first performing the object lookup via `lookup_tree()`, and then
locating its bit position within the recursive call. This patch
effectively reorders those two calls so that we first discover the
sub-tree's bit position, *then* load its tree.
By reordering these two operations, we spend fewer CPU cycles per
instruction, likely due to improved CPU dependency/cache/pipeline
behavior. Comparing the results of: running `perf stat` before and after
this commit, we have:
+--------------+-------------+-------------+-------------------+
| | HEAD^ | HEAD | Delta |
+--------------+-------------+-------------+-------------------+
| elapsed | 612.5 s | 582.4 s | -30.1 s (-4.9%) |
| cycles | 2,857.3 B | 2,713.3 B | -144.0 B (-5.0%) |
| instructions | 2,413.2 B | 2,415.5 B | +2.3 B (+0.1%) |
| CPI | 1.184 | 1.123 | -0.061 (-5.1%) |
+--------------+-------------+-------------+-------------------+
In a large repository with ~4.8M commit, and ~37.1M tree objects this
change improves timing from ~612.5 seconds down to ~582.4 seconds, or a
~4.9% improvement. More importantly, the number of CPU cycles spent
dropped off significantly as a result of this commit, lowering our
cycles-per-instruction ratio by about ~5.1%.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 1c8070f99c0..2d5ff8fd406 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -456,10 +456,10 @@ static void bitmap_builder_clear(struct bitmap_builder *bb)
static int fill_bitmap_tree(struct bitmap_writer *writer,
struct bitmap *bitmap,
- struct tree *tree)
+ struct tree *tree,
+ uint32_t pos)
{
int found;
- uint32_t pos;
struct tree_desc desc;
struct name_entry entry;
@@ -467,9 +467,6 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
* If our bit is already set, then there is nothing to do. Both this
* tree and all of its children will be set.
*/
- pos = find_object_pos(writer, &tree->object.oid, &found);
- if (!found)
- return -1;
if (bitmap_get(bitmap, pos))
return 0;
bitmap_set(bitmap, pos);
@@ -482,8 +479,12 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
while (tree_entry(&desc, &entry)) {
switch (object_type(entry.mode)) {
case OBJ_TREE:
+ pos = find_object_pos(writer, &entry.oid, &found);
+ if (!found)
+ return -1;
if (fill_bitmap_tree(writer, bitmap,
- lookup_tree(writer->repo, &entry.oid)) < 0)
+ lookup_tree(writer->repo,
+ &entry.oid), pos) < 0)
return -1;
break;
case OBJ_BLOB:
@@ -575,8 +576,14 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
}
while (tree_queue->nr) {
- if (fill_bitmap_tree(writer, ent->bitmap,
- prio_queue_get(tree_queue)) < 0)
+ struct tree *t = prio_queue_get(tree_queue);
+ int found;
+
+ pos = find_object_pos(writer, &t->object.oid, &found);
+ if (!found)
+ return -1;
+
+ if (fill_bitmap_tree(writer, ent->bitmap, t, pos) < 0)
return -1;
}
return 0;
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH 2/8] pack-bitmap: check subtree bits before recursing
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
In the previous commit, we adjusted the callers of `fill_bitmap_tree()`
to pass in the bit position of the tree they wish to fill.
This commit makes use of that information at the call site to avoid
setting up a stack frame for fill_bitmap_tree() entirely whenever a
tree's bit position is already set.
Since this is such a hot path, the avoided cost of setting up and
tearing down stack frames for each noop'd call to `fill_bitmap_tree()`
is significant:
+--------------+-------------+-------------+-------------------+
| | HEAD^ | HEAD | Delta |
+--------------+-------------+-------------+-------------------+
| elapsed | 582.4 s | 562.8 s | -19.6 s (-3.4%) |
| cycles | 2,713.3 B | 2,621.3 B | -92.0 B (-3.4%) |
| instructions | 2,415.5 B | 2,348.9 B | -66.6 B (-2.8%) |
| CPI | 1.123 | 1.116 | -0.007 (-0.7%) |
+--------------+-------------+-------------+-------------------+
In the same repository as in the previous commit, our timings dropped
from ~582.4 seconds down to ~562.77 seconds.
While the cycles-per-instruction ratio is basically unchanged, we
execute significantly fewer instructions, and correspondingly fewer
cycles.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 2d5ff8fd406..72610397020 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -463,12 +463,6 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
struct tree_desc desc;
struct name_entry entry;
- /*
- * If our bit is already set, then there is nothing to do. Both this
- * tree and all of its children will be set.
- */
- if (bitmap_get(bitmap, pos))
- return 0;
bitmap_set(bitmap, pos);
if (repo_parse_tree(writer->repo, tree) < 0)
@@ -482,6 +476,15 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
pos = find_object_pos(writer, &entry.oid, &found);
if (!found)
return -1;
+ if (bitmap_get(bitmap, pos)) {
+ /*
+ * If our bit is already set, then there
+ * is nothing to do. Both this tree and
+ * all of its children will be set.
+ */
+ break;
+ }
+
if (fill_bitmap_tree(writer, bitmap,
lookup_tree(writer->repo,
&entry.oid), pos) < 0)
@@ -582,6 +585,14 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
pos = find_object_pos(writer, &t->object.oid, &found);
if (!found)
return -1;
+ if (bitmap_get(ent->bitmap, pos)) {
+ /*
+ * If our bit is already set, then there is
+ * nothing to do. Both this tree and all of its
+ * children will be set.
+ */
+ continue;
+ }
if (fill_bitmap_tree(writer, ent->bitmap, t, pos) < 0)
return -1;
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH 3/8] pack-bitmap: reuse stored selected bitmaps
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
When `fill_bitmap_commit()` reaches an ancestor that was selected for
its own bitmap and processed earlier, its object closure is already
stored in `writer->bitmaps` as an EWAH bitmap. As a result, walking
through that commit's tree and parents again is redundant.
Teach `fill_bitmap_commit()` to notice that case. For non-root commits in
the walk, look for a stored selected bitmap and OR it into the bitmap
being built. If one exists, skip the commit, its tree, and its parents.
Building bitmaps from scratch on the same test repository from the
previous commits yields a significant speed-up:
+------------------+-------------+-------------+---------------------+
| | HEAD^ | HEAD | Delta |
+------------------+-------------+-------------+---------------------+
| elapsed | 562.8 s | 324.8 s | -237.9 s (-42.3%) |
| cycles | 2,621.3 B | 1,508.6 B | -1,112.7 B (-42.4%) |
| instructions | 2,348.9 B | 1,436.6 B | -912.3 B (-38.8%) |
| CPI | 1.116 | 1.050 | -0.066 (-5.9%) |
+------------------+-------------+-------------+---------------------+
In our testing repository, there are 1,261 commits selected for bitmap
coverage, and 1,382 maximal commits induced as a result of that. Of the
1,382 calls made to `fill_bitmap_commit()` (one per maximal commit), 131
of them can be short-circuited at some point during their traversal as a
consequence of this change.
In large repositories where the cost of filling the bitmap for any
individual commit is large, being able to short-circuit even ~9.5% of
the calls to `fill_bitmap_commit()` results in a significant savings.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 72610397020..651ad467469 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -509,6 +509,9 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
static int reused_bitmaps_nr;
static int reused_pseudo_merge_bitmaps_nr;
+static int fill_bitmap_commit_calls_nr;
+static int fill_bitmap_commit_found_ancestor_nr;
+
static int fill_bitmap_commit(struct bitmap_writer *writer,
struct bb_commit *ent,
struct commit *commit,
@@ -519,6 +522,9 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
{
int found;
uint32_t pos;
+
+ fill_bitmap_commit_calls_nr++;
+
if (!ent->bitmap)
ent->bitmap = bitmap_new();
@@ -553,6 +559,28 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
bitmap_free(remapped);
}
+ /*
+ * If we encounter an ancestor for which we have already
+ * computed a bitmap during this build (i.e. a regular
+ * selected commit processed earlier in topo order), we can
+ * short-circuit the walk: its stored bitmap already covers
+ * the commit itself, its tree, and all of its ancestors.
+ */
+ if (c != commit) {
+ khiter_t hash_pos = kh_get_oid_map(writer->bitmaps,
+ c->object.oid);
+ if (hash_pos != kh_end(writer->bitmaps)) {
+ struct bitmapped_commit *stored =
+ kh_value(writer->bitmaps, hash_pos);
+ if (stored && stored->bitmap) {
+ fill_bitmap_commit_found_ancestor_nr++;
+ bitmap_or_ewah(ent->bitmap,
+ stored->bitmap);
+ continue;
+ }
+ }
+ }
+
/*
* Mark ourselves and queue our tree. The commit
* walk ensures we cover all parents.
@@ -692,6 +720,12 @@ int bitmap_writer_build(struct bitmap_writer *writer)
trace2_data_intmax("pack-bitmap-write", writer->repo,
"building_bitmaps_pseudo_merge_reused",
reused_pseudo_merge_bitmaps_nr);
+ trace2_data_intmax("pack-bitmap-write", writer->repo,
+ "fill_bitmap_commit_calls_nr",
+ fill_bitmap_commit_calls_nr);
+ trace2_data_intmax("pack-bitmap-write", writer->repo,
+ "fill_bitmap_commit_found_ancestor_nr",
+ fill_bitmap_commit_found_ancestor_nr);
stop_progress(&writer->progress);
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH 4/8] pack-bitmap: consolidate `find_object_pos()` success path
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
Both sides of `find_object_pos()` report success in the same way by
setting the optional `found` out-parameter and return the resolved
bitmap position.
Prepare for adding more bookkeeping around object-position lookups by
storing the result in a local `pos` variable and sharing the success
return path between the packlist and MIDX cases.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 651ad467469..6483fdc7daf 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -224,23 +224,22 @@ static uint32_t find_object_pos(struct bitmap_writer *writer,
if (writer->midx)
base_objects = writer->midx->num_objects +
writer->midx->num_objects_in_base;
-
- if (found)
- *found = 1;
- return oe_in_pack_pos(writer->to_pack, entry) + base_objects;
+ pos = oe_in_pack_pos(writer->to_pack, entry) + base_objects;
} else if (writer->midx) {
- uint32_t at, pos;
+ uint32_t at;
if (!bsearch_midx(oid, writer->midx, &at))
goto missing;
if (midx_to_pack_pos(writer->midx, at, &pos) < 0)
goto missing;
-
- if (found)
- *found = 1;
- return pos;
+ } else {
+ goto missing;
}
+ if (found)
+ *found = 1;
+ return pos;
+
missing:
if (found)
*found = 0;
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH 5/8] pack-bitmap: cache object positions during fill
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
The previous commits removed some redundant work from bitmap generation
by avoiding unnecessary tree recursion and by reusing selected bitmaps
that have already been computed.
Even with those changes in place, there is still an extremely hot path
from `fill_bitmap_commit()` and `fill_bitmap_tree()` to translate object
IDs into their corresponding bit positions in order to generate their
bitmaps.
In a small repository, this overhead is not significant. However, in a
very large repository (e.g., the one that we have been using as a
benchmark over the past several commits with ~57M total objects), the
overhead of locating object bit positions (often repeatedly) adds up
significantly.
Combat this by adding a small, direct-mapped cache to the bitmap writer
which maps object IDs to their corresponding bit positions. Size the
cache according to the number of objects being written, with fixed lower
and upper bounds so small repositories do not pay for a large table and
large repositories can avoid most repeated packlist and MIDX lookups.
On my machine with (a somewhat outdated) GCC 15.2.0, each entry in the
cache is 40 bytes wide:
$ pahole -C bitmap_pos_cache_entry pack-bitmap-write.o
struct bitmap_pos_cache_entry {
struct object_id oid; /* 0 36 */
uint32_t pos; /* 36 4 */
/* size: 40, cachelines: 1, members: 2 */
/* last cacheline: 40 bytes */
};
, and we will allocate up to 2^21 entries for a maximum total of 80 MiB
of cache overhead.
In our example repository from above and in earlier commits, this
results in a ~9.4% reduction in runtime relative to the previous commit:
+------------------+-------------+-------------+---------------------+
| | HEAD^ | HEAD | Delta |
+------------------+-------------+-------------+---------------------+
| elapsed | 324.8 s | 294.1 s | -30.7 s (-9.4%) |
| cycles | 1,508.6 B | 1,365.5 B | -143.0 B (-9.5%) |
| instructions | 1,436.6 B | 1,389.8 B | -46.9 B (-3.3%) |
| CPI | 1.050 | 0.983 | -0.068 (-6.4%) |
+------------------+-------------+-------------+---------------------+
When generating bitmaps on this repository (to produce the above
timings), the cache grew to its maximum size of 80 MiB, and resulted in
1.024B cache hits and 59.957M cache misses.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 89 ++++++++++++++++++++++++++++++++++++++++++++-
pack-bitmap.h | 7 ++++
2 files changed, 95 insertions(+), 1 deletion(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 6483fdc7daf..4b6fb07edd7 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -89,6 +89,7 @@ void bitmap_writer_free(struct bitmap_writer *writer)
ewah_free(writer->tags);
kh_destroy_oid_map(writer->bitmaps);
+ free(writer->pos_cache);
kh_foreach_value(writer->pseudo_merge_commits, idx,
free_pseudo_merge_commit_idx(idx));
@@ -213,14 +214,92 @@ void bitmap_writer_push_commit(struct bitmap_writer *writer,
writer->selected_nr++;
}
+struct bitmap_pos_cache_entry {
+ struct object_id oid;
+ uint32_t pos;
+};
+
+#define BITMAP_POS_MIN_CACHE_SIZE (1U << 10)
+#define BITMAP_POS_MAX_CACHE_SIZE (1U << 21)
+#define BITMAP_POS_CACHE_VALID (1U << 31)
+
+static void bitmap_writer_init_pos_cache(struct bitmap_writer *writer)
+{
+ if (writer->pos_cache)
+ return;
+
+ writer->pos_cache_nr = BITMAP_POS_MIN_CACHE_SIZE;
+
+ while (writer->pos_cache_nr < writer->to_pack->nr_objects &&
+ writer->pos_cache_nr < BITMAP_POS_MAX_CACHE_SIZE)
+ writer->pos_cache_nr <<= 1;
+
+ CALLOC_ARRAY(writer->pos_cache, writer->pos_cache_nr);
+}
+
+static size_t bitmap_writer_pos_cache_slot(struct bitmap_writer *writer,
+ const struct object_id *oid)
+{
+ return oidhash(oid) & (writer->pos_cache_nr - 1);
+}
+
+static bool bitmap_writer_pos_cache_valid(struct bitmap_writer *writer,
+ size_t slot)
+{
+ return !!(writer->pos_cache[slot].pos & BITMAP_POS_CACHE_VALID);
+}
+
+static int find_cached_object_pos(struct bitmap_writer *writer,
+ const struct object_id *oid, uint32_t *pos)
+{
+ size_t slot = bitmap_writer_pos_cache_slot(writer, oid);
+
+ if (bitmap_writer_pos_cache_valid(writer, slot) &&
+ oideq(&writer->pos_cache[slot].oid, oid)) {
+ writer->pos_cache_hits++;
+ *pos = writer->pos_cache[slot].pos & ~BITMAP_POS_CACHE_VALID;
+ return 1;
+ }
+
+ writer->pos_cache_misses++;
+ return 0;
+}
+
+static uint32_t store_cached_object_pos(struct bitmap_writer *writer,
+ const struct object_id *oid,
+ uint32_t pos)
+{
+ size_t slot;
+
+ if (pos & BITMAP_POS_CACHE_VALID)
+ return pos; /* too large to cache */
+
+ slot = bitmap_writer_pos_cache_slot(writer, oid);
+
+ oidcpy(&writer->pos_cache[slot].oid, oid);
+ writer->pos_cache[slot].pos = pos | BITMAP_POS_CACHE_VALID;
+
+ return pos;
+}
+
static uint32_t find_object_pos(struct bitmap_writer *writer,
const struct object_id *oid, int *found)
{
struct object_entry *entry;
+ uint32_t pos;
+
+ bitmap_writer_init_pos_cache(writer);
+
+ if (find_cached_object_pos(writer, oid, &pos)) {
+ if (found)
+ *found = 1;
+ return pos;
+ }
entry = packlist_find(writer->to_pack, oid);
if (entry) {
uint32_t base_objects = 0;
+
if (writer->midx)
base_objects = writer->midx->num_objects +
writer->midx->num_objects_in_base;
@@ -238,7 +317,7 @@ static uint32_t find_object_pos(struct bitmap_writer *writer,
if (found)
*found = 1;
- return pos;
+ return store_cached_object_pos(writer, oid, pos);
missing:
if (found)
@@ -661,6 +740,10 @@ int bitmap_writer_build(struct bitmap_writer *writer)
writer->progress = start_progress(writer->repo,
"Building bitmaps",
writer->selected_nr);
+
+ writer->pos_cache_hits = 0;
+ writer->pos_cache_misses = 0;
+
trace2_region_enter("pack-bitmap-write", "building_bitmaps_total",
writer->repo);
@@ -725,6 +808,10 @@ int bitmap_writer_build(struct bitmap_writer *writer)
trace2_data_intmax("pack-bitmap-write", writer->repo,
"fill_bitmap_commit_found_ancestor_nr",
fill_bitmap_commit_found_ancestor_nr);
+ trace2_data_intmax("pack-bitmap-write", writer->repo,
+ "bitmap_pos_cache_hits", writer->pos_cache_hits);
+ trace2_data_intmax("pack-bitmap-write", writer->repo,
+ "bitmap_pos_cache_misses", writer->pos_cache_misses);
stop_progress(&writer->progress);
diff --git a/pack-bitmap.h b/pack-bitmap.h
index a95e1c2d115..19a86554579 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -132,6 +132,8 @@ int bitmap_has_oid_in_uninteresting(struct bitmap_index *, const struct object_i
off_t get_disk_usage_from_bitmap(struct bitmap_index *, struct rev_info *);
+struct bitmap_pos_cache_entry;
+
struct bitmap_writer {
struct repository *repo;
struct ewah_bitmap *commits;
@@ -143,6 +145,11 @@ struct bitmap_writer {
struct packing_data *to_pack;
struct multi_pack_index *midx; /* if appending to a MIDX chain */
+ struct bitmap_pos_cache_entry *pos_cache;
+ size_t pos_cache_nr;
+ uint64_t pos_cache_hits;
+ uint64_t pos_cache_misses;
+
struct bitmapped_commit *selected;
unsigned int selected_nr, selected_alloc;
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH 6/8] pack-bitmap: sort bitmaps before XORing
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
Reachability bitmaps may be stored as XORs against nearby bitmaps, up to
10 away. However, when callers provide selected commits in an arbitrary
order, the writer may miss good ancestor/descendant pairs and produce
much larger bitmap files without changing query coverage.
Sort the selected bitmaps in date order (from oldest to newest) before
computing XOR offsets, leaving pseudo-merge bitmaps alone (which we will
deal with separately in following commits).
On our same testing repository from previous commits, this change shrunk
our selection of 1,261 bitmaps from ~635.46 MiB to 176.4 MiB for a
~72.24% reduction in the on-disk size of our *.bitmap file. The time to
generate the smaller bitmap file decreased by ~3.69 seconds, though this
is likely mostly noise.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 4b6fb07edd7..66282ea14b5 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -327,11 +327,40 @@ static uint32_t find_object_pos(struct bitmap_writer *writer,
return 0;
}
+static int bitmapped_commit_date_cmp(const void *_a, const void *_b)
+{
+ const struct bitmapped_commit *a = _a;
+ const struct bitmapped_commit *b = _b;
+
+ if (a->commit->date < b->commit->date)
+ return -1;
+ if (a->commit->date > b->commit->date)
+ return 1;
+ return 0;
+}
+
static void compute_xor_offsets(struct bitmap_writer *writer)
{
static const int MAX_XOR_OFFSET_SEARCH = 10;
int i, next = 0;
+ int nr = bitmap_writer_nr_selected_commits(writer);
+
+ if (nr > 1) {
+ QSORT(writer->selected, nr, bitmapped_commit_date_cmp);
+
+ for (i = 0; i < nr; i++) {
+ struct bitmapped_commit *stored = &writer->selected[i];
+ khiter_t hash_pos = kh_get_oid_map(writer->bitmaps,
+ stored->commit->object.oid);
+
+ if (hash_pos == kh_end(writer->bitmaps))
+ BUG("selected commit missing from bitmap map: %s",
+ oid_to_hex(&stored->commit->object.oid));
+
+ kh_value(writer->bitmaps, hash_pos) = stored;
+ }
+ }
while (next < writer->selected_nr) {
struct bitmapped_commit *stored = &writer->selected[next];
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH 7/8] pack-bitmap: remember pseudo-merge parents
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
write_pseudo_merges() currently builds an array of temporary bitmaps for
the parent set of each pseudo-merge, then serializes those bitmaps later
while writing the extension.
Move those parent bitmaps onto the corresponding bitmapped_commit
entries instead. This keeps the on-disk output unchanged, but gives the
parent bitmap the same lifetime and access pattern that later changes
will use when pseudo-merge object bitmaps are built before the write
step.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 66282ea14b5..8200aed6101 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -32,6 +32,7 @@ struct bitmapped_commit {
struct commit *commit;
struct ewah_bitmap *bitmap;
struct ewah_bitmap *write_as;
+ struct ewah_bitmap *pseudo_merge_parents;
int flags;
int xor_offset;
uint32_t commit_pos;
@@ -102,6 +103,7 @@ void bitmap_writer_free(struct bitmap_writer *writer)
if (bc->write_as != bc->bitmap)
ewah_free(bc->write_as);
ewah_free(bc->bitmap);
+ ewah_free(bc->pseudo_merge_parents);
}
free(writer->selected);
}
@@ -210,6 +212,7 @@ void bitmap_writer_push_commit(struct bitmap_writer *writer,
writer->selected[writer->selected_nr].write_as = NULL;
writer->selected[writer->selected_nr].flags = 0;
writer->selected[writer->selected_nr].pseudo_merge = pseudo_merge;
+ writer->selected[writer->selected_nr].pseudo_merge_parents = NULL;
writer->selected_nr++;
}
@@ -1004,42 +1007,47 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
struct hashfile *f)
{
struct oid_array commits = OID_ARRAY_INIT;
- struct bitmap **commits_bitmap = NULL;
off_t *pseudo_merge_ofs = NULL;
off_t start, table_start, next_ext;
uint32_t base = bitmap_writer_nr_selected_commits(writer);
size_t i, j = 0;
- CALLOC_ARRAY(commits_bitmap, writer->pseudo_merges_nr);
CALLOC_ARRAY(pseudo_merge_ofs, writer->pseudo_merges_nr);
for (i = 0; i < writer->pseudo_merges_nr; i++) {
struct bitmapped_commit *merge = &writer->selected[base + i];
struct commit_list *p;
+ struct bitmap *parents = bitmap_new();
if (!merge->pseudo_merge)
BUG("found non-pseudo merge commit at %"PRIuMAX, (uintmax_t)i);
- commits_bitmap[i] = bitmap_new();
-
for (p = merge->commit->parents; p; p = p->next)
- bitmap_set(commits_bitmap[i],
+ bitmap_set(parents,
find_object_pos(writer, &p->item->object.oid,
NULL));
+
+ merge->pseudo_merge_parents = bitmap_to_ewah(parents);
+ bitmap_free(parents);
}
start = hashfile_total(f);
for (i = 0; i < writer->pseudo_merges_nr; i++) {
- struct ewah_bitmap *commits_ewah = bitmap_to_ewah(commits_bitmap[i]);
+ struct bitmapped_commit *merge = &writer->selected[base + i];
+
+ if (!merge->pseudo_merge)
+ BUG("found non-pseudo merge commit at %"PRIuMAX, (uintmax_t)i);
+
+ if (!merge->pseudo_merge_parents)
+ BUG("missing pseudo-merge parents bitmap for commit %s",
+ oid_to_hex(&merge->commit->object.oid));
pseudo_merge_ofs[i] = hashfile_total(f);
- dump_bitmap(f, commits_ewah);
+ dump_bitmap(f, merge->pseudo_merge_parents);
dump_bitmap(f, writer->selected[base+i].write_as);
-
- ewah_free(commits_ewah);
}
next_ext = st_add(hashfile_total(f),
@@ -1122,12 +1130,8 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
hashwrite_be64(f, table_start - start);
hashwrite_be64(f, hashfile_total(f) - start + sizeof(uint64_t));
- for (i = 0; i < writer->pseudo_merges_nr; i++)
- bitmap_free(commits_bitmap[i]);
-
oid_array_clear(&commits);
free(pseudo_merge_ofs);
- free(commits_bitmap);
}
static int table_cmp(const void *_va, const void *_vb, void *_data)
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH 8/8] pack-bitmap: build pseudo-merge bitmaps after regular bitmaps
From: Taylor Blau @ 2026-05-19 16:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779207127.git.me@ttaylorr.com>
When generating bitmaps, `bitmap_builder_init()` starts with an initial
selection of commits to receive bitmap coverage, and then determines a
set of "maximal" commits based on its input.
Commit 089f751360f (pack-bitmap-write: build fewer intermediate bitmaps,
2020-12-08) has extensive details, but the gist is as follows:
Each selected commit starts with one commit_mask bit in its "commit
mask" bitmap. Then, we walk the first-parent history in topological
order and OR each commit's mask into its (first) parent. Whenever that
OR results in the parent having more bits set, the child is deemed to be
non-maximal, and the frontier is pushed further back along the first
parent history.
That approach works extremely well for ordinary selected commits, whose
first-parent histories often describe real sharing between the bitmaps
we are going to write.
It struggles, however, to efficiently generate pseudo-merge bitmaps.
Unlike ordinary commits for which the above algorithm is designed,
pseudo-merges don't represent any "real" commit in history, just a
grouping of non-bitmapped reference tips. In that sense, their first
parent is just a part of a larger set, and treating them like ordinary
selected commits imposes a significant slow-down when generating bitmaps
with pseudo-merges enabled.
Consider partitioning all non-bitmapped reference tips into eight
individual pseudo-merges via the following configuration:
[bitmapPseudoMerge "all"]
pattern=refs/
threshold=now
stableSize=10000000
maxMerges=8
, the cost of generating a bitmap from scratch rises significantly:
+------------------+-----------------+---------------+---------------------+
| | no pseudo-merge | pseudo-merges | Delta |
| | | (HEAD^) | |
+------------------+-----------------+---------------+---------------------+
| elapsed | 294.1 s | 575.0 s | +280.9 s (+95.5%) |
| cycles | 1,365.5 B | 2,686.9 B | +1,321.4 B (+96.8%) |
| instructions | 1,389.8 B | 2,546.6 B | +1,156.8 B (+83.2%) |
| CPI | 0.983 | 1.055 | +0.073 (+7.4%) |
+------------------+-----------------+---------------+---------------------+
This is a particularly poor trade-off, because the time saved by these
pseudo-merges during, e.g.,
$ git rev-list --count --all --objects --use-bitmap-index
is only:
$ hyperfine -L v true,false -n 'pseudo-merges: {v}' '
GIT_TEST_USE_PSEUDO_MERGES={v} git.compile rev-list --count \
--objects --all --use-bitmap-index
'
Benchmark 1: pseudo-merges: true
Time (mean ± σ): 2.613 s ± 0.012 s [User: 2.308 s, System: 0.305 s]
Range (min … max): 2.594 s … 2.633 s 10 runs
Benchmark 2: pseudo-merges: false
Time (mean ± σ): 52.205 s ± 0.170 s [User: 51.500 s, System: 0.697 s]
Range (min … max): 51.956 s … 52.458 s 10 runs
Summary
pseudo-merges: true ran
19.98 ± 0.11 times faster than pseudo-merges: false
In other words, we pay a nearly ~5 minute penalty to generate
pseudo-merge bitmaps, but only save ~50 seconds during traversal.
The problem stems from injecting pseudo-merges into the bitmap builder
as if they were normal commits. The maximal commit selection algorithm
was simply not designed for that case, and performs predictably poorly.
The only reason we reused the maximal commit selection routine for
pseudo-merges alongside regular non-pseudo-merge commits is because we
represent them both as commit objects (where the pseudo-merge commits
just represent a made-up commit as opposed to one that actually exists
in a repository's object store).
Instead, build the regular selected commit bitmaps first, considering
only non-pseudo-merge commits in `bitmap_builder_init()`. Once those
bitmaps have been stored, build each pseudo-merge bitmap separately and
attach its parent and object bitmaps to the corresponding pseudo-merge
entry before writing the extension.
This keeps the regular bitmap build shaped like the no-pseudo-merge
case. The later pseudo-merge fill can still stop at stored selected
ancestor bitmaps, so it does not have to rewalk each pseudo-merge
closure from scratch.
When an existing bitmap has the same pseudo-merge parent set, reuse and
remap that whole pseudo-merge bitmap before falling back to
fill_bitmap_commit(). This preserves the benefit of stable pseudo-merges
while keeping the on-disk format and reader behavior unchanged.
As a result, the overhead cost for generating pseudo-merges in the above
configuration is much smaller:
+------------------+-----------------+---------------+-------------------+
| | no pseudo-merge | pseudo-merges | Delta |
| | | (HEAD) | |
+------------------+-----------------+---------------+-------------------+
| elapsed | 294.1 s | 328.4 s | +34.3 s (+11.7%) |
| cycles | 1,365.5 B | 1,529.3 B | +163.7 B (+12.0%) |
| instructions | 1,389.8 B | 1,552.8 B | +163.0 B (+11.7%) |
| CPI | 0.983 | 0.985 | +0.002 (+0.2%) |
+------------------+-----------------+---------------+-------------------+
Recall that at the start of this series, generating reachability bitmaps
took 612.5 seconds *without* pseudo-merges. With this commit, it is
still ~46.38% *faster* to generate reachability bitmaps *with*
pseudo-merges than it was to generate bitmaps wihtout them at the
beginning of this series.
The changes to implement this are mostly straightforward. We exclude
pseudo-merge commits from the existing bitmap generation, and walk over
them in a separate pass, by either reusing an existing on-disk
pseudo-merge, or passing the pseudo-merge commit itself back to the
existing routine in `fill_bitmap_commit()`.
(Note that the routine to build pseudo-merge bitmaps is the same both
before and after this change, the difference is only that we do not let
psuedo-merges participate in determining the set of maximal commits.)
The only wrinkle is that `fill_bitmap_commit()` must be taught to not
expect that all tree objects have been parsed, which is the case for any
portion of history reachable by one or more pseudo-merge(s), but not by
any non-pseudo-merge commit selected for bitmapping.
Now that we have decoupled how we generate pseudo-merges from their
representation, the following commits will improve the API around
specifying pseudo-merge groupings during bitmap generation.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 210 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 174 insertions(+), 36 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 8200aed6101..1bcb3f98a42 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -446,13 +446,17 @@ static void bitmap_builder_init(struct bitmap_builder *bb,
revs.topo_order = 1;
revs.first_parent_only = 1;
- for (i = 0; i < writer->selected_nr; i++) {
+ for (i = 0; i < bitmap_writer_nr_selected_commits(writer); i++) {
struct bitmapped_commit *bc = &writer->selected[i];
struct bb_commit *ent = bb_data_at(&bb->data, bc->commit);
+ if (bc->pseudo_merge)
+ BUG("unexpected pseudo-merge at %"PRIuMAX,
+ (uintmax_t)i);
+
ent->selected = 1;
ent->maximal = 1;
- ent->pseudo_merge = bc->pseudo_merge;
+ ent->pseudo_merge = 0;
ent->idx = i;
ent->commit_mask = bitmap_new();
@@ -618,6 +622,8 @@ static int fill_bitmap_tree(struct bitmap_writer *writer,
static int reused_bitmaps_nr;
static int reused_pseudo_merge_bitmaps_nr;
+static int pseudo_merge_bitmap_nr;
+static int pseudo_merge_bitmap_parents;
static int fill_bitmap_commit_calls_nr;
static int fill_bitmap_commit_found_ancestor_nr;
@@ -631,8 +637,12 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
const uint32_t *mapping)
{
int found;
+ int from_pseudo_merge = commit->object.flags & BITMAP_PSEUDO_MERGE;
uint32_t pos;
+ if (ent->pseudo_merge)
+ BUG("unexpected pseudo-merge commit in fill_bitmap_commit()");
+
fill_bitmap_commit_calls_nr++;
if (!ent->bitmap)
@@ -648,10 +658,7 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
struct ewah_bitmap *old;
struct bitmap *remapped = bitmap_new();
- if (commit->object.flags & BITMAP_PSEUDO_MERGE)
- old = pseudo_merge_bitmap_for_commit(old_bitmap, c);
- else
- old = bitmap_for_commit(old_bitmap, c);
+ old = bitmap_for_commit(old_bitmap, c);
/*
* If this commit has an old bitmap, then translate that
* bitmap and add its bits to this one. No need to walk
@@ -660,10 +667,7 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
if (old && !rebuild_bitmap(mapping, old, remapped)) {
bitmap_or(ent->bitmap, remapped);
bitmap_free(remapped);
- if (commit->object.flags & BITMAP_PSEUDO_MERGE)
- reused_pseudo_merge_bitmaps_nr++;
- else
- reused_bitmaps_nr++;
+ reused_bitmaps_nr++;
continue;
}
bitmap_free(remapped);
@@ -696,12 +700,32 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
* walk ensures we cover all parents.
*/
if (!(c->object.flags & BITMAP_PSEUDO_MERGE)) {
+ struct tree *tree;
+
+ if (from_pseudo_merge && !c->object.parsed) {
+ /*
+ * Commits reachable from selected
+ * non-pseudo-merges are already parsed
+ * by the regular bitmap build.
+ *
+ * However, pseudo-merge fills can also
+ * reach commits that were not covered
+ * there, so parse any such leftovers
+ * before reading their tree or parents.
+ */
+ if (repo_parse_commit(writer->repo, c))
+ return -1;
+ }
+
pos = find_object_pos(writer, &c->object.oid, &found);
if (!found)
return -1;
bitmap_set(ent->bitmap, pos);
- prio_queue_put(tree_queue,
- repo_get_commit_tree(writer->repo, c));
+
+ tree = repo_get_commit_tree(writer->repo, c);
+ if (!tree)
+ return -1;
+ prio_queue_put(tree_queue, tree);
}
for (p = c->parents; p; p = p->next) {
@@ -738,6 +762,137 @@ static int fill_bitmap_commit(struct bitmap_writer *writer,
return 0;
}
+static int reuse_pseudo_merge_bitmap(struct bitmap_index *old_bitmap,
+ const uint32_t *mapping,
+ struct commit *merge,
+ struct ewah_bitmap **out)
+{
+ struct ewah_bitmap *old;
+ struct bitmap *remapped;
+
+ if (!old_bitmap || !mapping)
+ return 0;
+
+ old = pseudo_merge_bitmap_for_commit(old_bitmap, merge);
+ if (!old)
+ return 0;
+
+ remapped = bitmap_new();
+ if (rebuild_bitmap(mapping, old, remapped) < 0) {
+ bitmap_free(remapped);
+ return 0;
+ }
+
+ *out = bitmap_to_ewah(remapped);
+ bitmap_free(remapped);
+ reused_pseudo_merge_bitmaps_nr++;
+ return 1;
+}
+
+static int build_pseudo_merge_bitmap(struct bitmap_writer *writer,
+ struct bitmap_index *old_bitmap,
+ const uint32_t *mapping,
+ struct commit *merge,
+ struct ewah_bitmap **out)
+{
+ struct bb_commit ent = { 0 };
+ struct prio_queue queue = { NULL };
+ struct prio_queue tree_queue = { NULL };
+ unsigned parents = commit_list_count(merge->parents);
+ int ret;
+
+ ent.bitmap = bitmap_new();
+
+ pseudo_merge_bitmap_nr++;
+ pseudo_merge_bitmap_parents += parents;
+
+ if (reuse_pseudo_merge_bitmap(old_bitmap, mapping, merge, out)) {
+ ret = 0;
+ goto done;
+ }
+
+ ret = fill_bitmap_commit(writer, &ent, merge, &queue, &tree_queue,
+ old_bitmap, mapping);
+
+ if (!ret)
+ *out = bitmap_to_ewah(ent.bitmap);
+
+done:
+ bitmap_free(ent.bitmap);
+ clear_prio_queue(&queue);
+ clear_prio_queue(&tree_queue);
+
+ return ret;
+}
+
+static int build_pseudo_merge_bitmaps(struct bitmap_writer *writer,
+ struct bitmap_index *old_bitmap,
+ const uint32_t *mapping,
+ int *nr_stored)
+{
+ size_t i = bitmap_writer_nr_selected_commits(writer);
+ int ret = 0;
+
+ if (!writer->pseudo_merges_nr)
+ return 0;
+
+ trace2_region_enter("pack-bitmap-write", "building_pseudo_merge_bitmaps",
+ writer->repo);
+
+ for (; i < writer->selected_nr; i++) {
+ struct bitmapped_commit *merge = &writer->selected[i];
+ struct commit_list *p;
+ struct bitmap *parents = bitmap_new();
+ struct ewah_bitmap *objects = NULL;
+
+ if (!merge->pseudo_merge)
+ BUG("found non-pseudo merge commit at %"PRIuMAX,
+ (uintmax_t)i);
+
+ for (p = merge->commit->parents; p; p = p->next) {
+ int found;
+ uint32_t pos = find_object_pos(writer,
+ &p->item->object.oid,
+ &found);
+ if (!found) {
+ bitmap_free(parents);
+ ret = -1;
+ goto done;
+ }
+ bitmap_set(parents, pos);
+ }
+
+ merge->pseudo_merge_parents = bitmap_to_ewah(parents);
+ bitmap_free(parents);
+
+ if (build_pseudo_merge_bitmap(writer, old_bitmap, mapping,
+ merge->commit, &objects) < 0) {
+ ret = -1;
+ goto done;
+ }
+ merge->bitmap = objects;
+
+ (*nr_stored)++;
+ display_progress(writer->progress, *nr_stored);
+ }
+
+done:
+ trace2_region_leave("pack-bitmap-write", "building_pseudo_merge_bitmaps",
+ writer->repo);
+
+ trace2_data_intmax("pack-bitmap-write", writer->repo,
+ "pseudo_merge_bitmap_nr",
+ pseudo_merge_bitmap_nr);
+ trace2_data_intmax("pack-bitmap-write", writer->repo,
+ "building_bitmaps_pseudo_merge_reused",
+ reused_pseudo_merge_bitmaps_nr);
+ trace2_data_intmax("pack-bitmap-write", writer->repo,
+ "pseudo_merge_bitmap_parents",
+ pseudo_merge_bitmap_parents);
+
+ return ret;
+}
+
static void store_selected(struct bitmap_writer *writer,
struct bb_commit *ent, struct commit *commit)
{
@@ -821,6 +976,10 @@ int bitmap_writer_build(struct bitmap_writer *writer)
bitmap_free(ent->bitmap);
ent->bitmap = NULL;
}
+ if (closed &&
+ build_pseudo_merge_bitmaps(writer, old_bitmap, mapping,
+ &nr_stored) < 0)
+ closed = 0;
clear_prio_queue(&queue);
clear_prio_queue(&tree_queue);
bitmap_builder_clear(&bb);
@@ -831,9 +990,6 @@ int bitmap_writer_build(struct bitmap_writer *writer)
writer->repo);
trace2_data_intmax("pack-bitmap-write", writer->repo,
"building_bitmaps_reused", reused_bitmaps_nr);
- trace2_data_intmax("pack-bitmap-write", writer->repo,
- "building_bitmaps_pseudo_merge_reused",
- reused_pseudo_merge_bitmaps_nr);
trace2_data_intmax("pack-bitmap-write", writer->repo,
"fill_bitmap_commit_calls_nr",
fill_bitmap_commit_calls_nr);
@@ -1015,23 +1171,6 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
CALLOC_ARRAY(pseudo_merge_ofs, writer->pseudo_merges_nr);
- for (i = 0; i < writer->pseudo_merges_nr; i++) {
- struct bitmapped_commit *merge = &writer->selected[base + i];
- struct commit_list *p;
- struct bitmap *parents = bitmap_new();
-
- if (!merge->pseudo_merge)
- BUG("found non-pseudo merge commit at %"PRIuMAX, (uintmax_t)i);
-
- for (p = merge->commit->parents; p; p = p->next)
- bitmap_set(parents,
- find_object_pos(writer, &p->item->object.oid,
- NULL));
-
- merge->pseudo_merge_parents = bitmap_to_ewah(parents);
- bitmap_free(parents);
- }
-
start = hashfile_total(f);
for (i = 0; i < writer->pseudo_merges_nr; i++) {
@@ -1040,14 +1179,13 @@ static void write_pseudo_merges(struct bitmap_writer *writer,
if (!merge->pseudo_merge)
BUG("found non-pseudo merge commit at %"PRIuMAX, (uintmax_t)i);
- if (!merge->pseudo_merge_parents)
- BUG("missing pseudo-merge parents bitmap for commit %s",
+ if (!merge->pseudo_merge_parents || !merge->bitmap)
+ BUG("missing pseudo-merge bitmap for commit %s",
oid_to_hex(&merge->commit->object.oid));
pseudo_merge_ofs[i] = hashfile_total(f);
-
dump_bitmap(f, merge->pseudo_merge_parents);
- dump_bitmap(f, writer->selected[base+i].write_as);
+ dump_bitmap(f, merge->bitmap);
}
next_ext = st_add(hashfile_total(f),
--
2.54.0.rc1.84.g30ce254312c
^ permalink raw reply related
* [PATCH v6 0/8] fetch: rework negotiation tip options
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee
In-Reply-To: <pull.2085.v5.git.1779135575.gitgitgadget@gmail.com>
Fetch negotiation aims to find enough information from haves and wants such
that the server can be reasonably confident that it will send all necessary
objects and not too many "extra" objects that the client already has.
However, this can break down if there are too many references, since Git
truncates the list of haves based on a few factors (a 256 count limit or the
server sending an ACK at the right time).
We already have the --negotiation-tip feature to focus the set of references
that are used in negotiation, but I feel like this is designed backwards.
I'd rather that we have a way to say "this is an important set of refs, but
feel free to add more refs if needed" than "only use these refs for
negotiation".
Here's an example that demonstrates the problem. In an internal monorepo,
developers work off of the 'main' branch so there are thousands of user
branches that each add a few commits different from the 'main' branch.
However, there is also a long-lived 'release' branch. This branch has a
first-parent history that is parallel to 'main' and each of those commits is
a merge whose second parent is a commit from 'main' that had a successful CI
run. There are additional changes in the 'release' branch merge commits that
add some changelog data, so there is a nontrivial set of novel blob content
in that branch and not just a different set of commits.
The problem we had was that our georeplication system was regularly fetching
from the origin and trying to get all data from all reachable branches. When
the 'release' branch updated, the client would run out of haves before
advertising its copy of the 'release' branch, but it would still list the
new 'release' tip as a want. The server would then think that the client had
never fetched that branch before and would send all of the changelog data
from the whole history of the repo. (This led to a lot of downstream
problems; we mitigated by setting a refspec that stopped fetching the
'release' branch, but this is not ideal.)
What I'd like is a mechanism to say "always advertise the client's version
of 'main' and 'release' but also opportunistically include some user
branches".
Based on my understanding, the '--negotiation-tip' option is close but not
quite what I want. I could have the client only advertise 'release' and
'main' and never advertise any user branches. But then we'd download all
content from each user branch every time it updates. Perhaps this would
happen even with opportunistic inclusion of more haves, but I'd like to
explore this area more.
There's also an issue that the '--negotiation-tip' feature doesn't seem to
have a config key that enables it without CLI arguments. This is something
that we could consider independently.
This patch series adds a new '--negotiation-include' option that does what I
want: it makes sure that these references are included as 'have's during
negotiation. In order to help clarify the difference between this and
'--negotiation-tip', I first create a synonym called
'--negotiation-restrict'.
Both of these options get 'remote.*.negotiation(Include|Restrict)' config
options that enable their behavior by default.
During development, I had briefly considered only using config values, but
that required some strange changes to care about the remote name in the
transport layer. This was most different in the 'git push' integration. When
I discovered the '--negotiation-tip' feature during the process, that gave
me a clear pattern to follow with the addition of a config on top.
Updates in v2
=============
This version is a near-complete rewrite based on feedback around the names
of the previous option and config. The --negotiation-restrict option is new
and the ability to set it via config is also new.
I did try to be more careful around translatable error messages, too.
Updates in v3
=============
* --negotiation-tip is now an alias of --negotiation-restrict.
* More translatable strings use %s to isolate non-translatable options from
translatable words.
* The string_list named negotiation_tip is now renamed to
negotiation_restrict.
* The config options now allow an empty value to reset the list.
* The --negotiation-require option is now called --negotiation-include.
* Similarly, the config option is renamed and all code references.
* The included haves now mark their commits as COMMON so commits that they
can reach are not included in the negotiation walk if they are reached
from the restricted commits.
* The ref iterators are more careful about failing on bad references (ref
exists but object doesn't) and ignoring missing references (perhaps
config is erroneous?).
* When sending tips during push negotiation, use the --negotiation-restrict
option instead of -tip.
Updates in v4
=============
Thanks, Matthew, for the detailed review! There are some big changes in this
version.
* Expanded commit message to cite the commit that introduced the bug
(3f763ddf28).
* Renamed --negotiation-tip to --negotiation-restrict throughout docs/code
(including send-pack.c, transport-helper.c, builtin/pull.c). Added
OPT_ALIAS in git-pull.
* Switched config parsing to use parse_transport_option() helper. Removed
git push from docs (not implemented yet). Restructured --negotiate-only
validation flow.
* NEW Patch 5: Added have_sent() interface to negotiators, so included
haves can be de-duplicated properly by the negotiation algorithm.
* Replaced COMMON flag hack with negotiator->have_sent() calls. Moved
ref-pattern resolution into builtin/fetch.c (add_negotiation_tips()) so
fetch-pack receives pre-resolved oid_array instead of string_list. Added
test for --negotiation-tip ignoring missing refs. Added
duplicate-avoidance test for v0. Accepts commit hashes in addition to ref
names/globs.
* Use parse_transport_option() for config. Updated docs to mention commit
hashes. Removed git push from config docs. Fixed test to use correct
restrict/include combinations.
* In the last patch, add doc notes that remote config values also apply
during git push with push.negotiate, now that they are integrated by that
change.
Updates in v5
=============
Responded to small comments.
Updates in v6
=============
Corrected reviewed-by annotations in commit messages.
Thanks, -Stolee
Derrick Stolee (8):
t5516: fix test order flakiness
fetch: add --negotiation-restrict option
transport: rename negotiation_tips
remote: add remote.*.negotiationRestrict config
negotiator: add have_sent() interface
fetch: add --negotiation-include option for negotiation
remote: add remote.*.negotiationInclude config
send-pack: pass negotiation config in push
Documentation/config/fetch.adoc | 2 +-
Documentation/config/remote.adoc | 49 ++++++++
Documentation/fetch-options.adoc | 29 ++++-
builtin/fetch.c | 87 +++++++++++---
builtin/pull.c | 6 +-
fetch-negotiator.h | 9 ++
fetch-pack.c | 99 +++++++++++++---
fetch-pack.h | 10 +-
negotiator/default.c | 8 ++
negotiator/noop.c | 7 ++
negotiator/skipping.c | 8 ++
remote.c | 10 ++
remote.h | 2 +
send-pack.c | 39 +++++--
send-pack.h | 2 +
t/t5510-fetch.sh | 191 +++++++++++++++++++++++++++++++
t/t5516-fetch-push.sh | 32 +++++-
t/t5702-protocol-v2.sh | 4 +-
transport-helper.c | 5 +-
transport.c | 20 +++-
transport.h | 7 +-
21 files changed, 564 insertions(+), 62 deletions(-)
base-commit: 6e8d538aab8fe4dd07ba9fb87b5c7edcfa5706ad
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2085%2Fderrickstolee%2Fmust-have-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2085/derrickstolee/must-have-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/2085
Range-diff vs v5:
1: 538913a327 ! 1: c8c422f646 t5516: fix test order flakiness
@@ Commit message
Use 'sort -k 3' to match the actual number of columns in the output.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## t/t5516-fetch-push.sh ##
2: 580aa58943 ! 2: ac3e8f74d9 fetch: add --negotiation-restrict option
@@ Commit message
translatable with the option name inserted by formatting. At least one
of these messages will be reused later for a new option.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## Documentation/config/fetch.adoc ##
3: eee0543647 ! 3: 5206640b8b transport: rename negotiation_tips
@@ Commit message
Also update the string_list used to store the inputs from command-line
options.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## builtin/fetch.c ##
4: 63c675e93e ! 4: eec0f90e02 remote: add remote.*.negotiationRestrict config
@@ Commit message
An empty value resets the value list to allow ignoring earlier config
values, such as those that might be set in system or global config.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## Documentation/config/remote.adoc ##
5: d423c56283 ! 5: 840db1d957 negotiator: add have_sent() interface
@@ Commit message
common, so the implementation is quite simple. This logic will be exercised
in the next change.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## fetch-negotiator.h ##
6: e86c9791e2 ! 6: 62e5ef1a4b fetch: add --negotiation-include option for negotiation
@@ Commit message
Also add --negotiation-include to 'git pull' passthrough options.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## Documentation/fetch-options.adoc ##
7: e5714115b5 ! 7: 05a4b69b9b remote: add remote.*.negotiationInclude config
@@ Commit message
list to allow ignoring earlier config values, such as those that might be
set in system or global config.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## Documentation/config/remote.adoc ##
8: ed0be32e2c ! 8: c69ca2e919 send-pack: pass negotiation config in push
@@ Commit message
are passed as --negotiation-include to ensure their tips are always
sent as 'have' lines during push negotiation.
- Reviewed-by: Matthew John Cheetham <mcheetham@outlook.com>
+ Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
## Documentation/config/remote.adoc ##
--
gitgitgadget
^ permalink raw reply
* [PATCH v6 1/8] t5516: fix test order flakiness
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The 'fetch follows tags by default' test sorts using 'sort -k 4', but
for-each-ref output only has 3 columns. This relies on sort treating records
with fewer fields as having an empty fourth field, which may produce
unstable results depending on locale. This appears to be an accident added
in 3f763ddf28 (fetch: set remote/HEAD if it does not exist, 2024-11-22).
Use 'sort -k 3' to match the actual number of columns in the output.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
t/t5516-fetch-push.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 29e2f17608..ac8447f21e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1349,7 +1349,7 @@ test_expect_success 'fetch follows tags by default' '
git for-each-ref >tmp1 &&
sed -n "p; s|refs/heads/main$|refs/remotes/origin/main|p" tmp1 |
sed -n "p; s|refs/heads/main$|refs/remotes/origin/HEAD|p" |
- sort -k 4 >../expect
+ sort -k 3 >../expect
) &&
test_when_finished "rm -rf dst" &&
git init dst &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 2/8] fetch: add --negotiation-restrict option
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The --negotiation-tip option to 'git fetch' and 'git pull' allows users
to specify that they want to focus negotiation on a small set of
references. This is a _restriction_ on the negotiation set, helping to
focus the negotiation when the ref count is high. However, it doesn't
allow for the ability to opportunistically select references beyond that
list.
This subtle detail that this is a 'maximum set' and not a 'minimum set'
is not immediately clear from the option name. This makes it more
complicated to add a new option that provides the complementary behavior
of a minimum set.
For now, create a new synonym option, --negotiation-restrict, that
behaves identically to --negotiation-tip. Update the documentation to
make it clear that this new name is the preferred option, but we keep
the old name for compatibility. Mark --negotiation-tip as an alias of the
new, preferred option.
Update a few warning messages with the new option, but also make them
translatable with the option name inserted by formatting. At least one
of these messages will be reused later for a new option.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/fetch.adoc | 2 +-
Documentation/fetch-options.adoc | 6 +++++-
builtin/fetch.c | 13 ++++++++-----
builtin/pull.c | 3 ++-
send-pack.c | 2 +-
t/t5510-fetch.sh | 25 +++++++++++++++++++++++++
t/t5702-protocol-v2.sh | 4 ++--
transport-helper.c | 3 ++-
8 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/Documentation/config/fetch.adoc b/Documentation/config/fetch.adoc
index cd40db0cad..04ac90912d 100644
--- a/Documentation/config/fetch.adoc
+++ b/Documentation/config/fetch.adoc
@@ -76,7 +76,7 @@
default is `skipping`. Unknown values will cause `git fetch` to
error out.
+
-See also the `--negotiate-only` and `--negotiation-tip` options to
+See also the `--negotiate-only` and `--negotiation-restrict` options to
linkgit:git-fetch[1].
`fetch.showForcedUpdates`::
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 81a9d7f9bb..d39cecb446 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -49,6 +49,7 @@ the current repository has the same history as the source repository.
`.git/shallow`. This option updates `.git/shallow` and accepts such
refs.
+`--negotiation-restrict=(<commit>|<glob>)`::
`--negotiation-tip=(<commit>|<glob>)`::
By default, Git will report, to the server, commits reachable
from all local refs to find common commits in an attempt to
@@ -58,6 +59,9 @@ the current repository has the same history as the source repository.
local ref is likely to have commits in common with the
upstream ref being fetched.
+
+`--negotiation-restrict` is the preferred name for this option;
+`--negotiation-tip` is accepted as a synonym.
++
This option may be specified more than once; if so, Git will report
commits reachable from any of the given commits.
+
@@ -71,7 +75,7 @@ configuration variables documented in linkgit:git-config[1], and the
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
- ancestors of the provided `--negotiation-tip=` arguments,
+ ancestors of the provided `--negotiation-restrict=` arguments,
which we have in common with the server.
+
This is incompatible with `--recurse-submodules=(yes|on-demand)`.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4795b2a13c..fc950fe35b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1558,8 +1558,8 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
refs_for_each_ref_ext(get_main_ref_store(the_repository),
add_oid, oids, &opts);
if (old_nr == oids->nr)
- warning("ignoring --negotiation-tip=%s because it does not match any refs",
- s);
+ warning(_("ignoring %s=%s because it does not match any refs"),
+ "--negotiation-restrict", s);
}
smart_options->negotiation_tips = oids;
}
@@ -1599,7 +1599,8 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
if (transport->smart_options)
add_negotiation_tips(transport->smart_options);
else
- warning("ignoring --negotiation-tip because the protocol does not support it");
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-restrict");
}
return transport;
}
@@ -2565,8 +2566,9 @@ int cmd_fetch(int argc,
N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
OPT_IPVERSION(&family),
- OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
+ OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
N_("report that we have only objects reachable from this object")),
+ OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
OPT_BOOL(0, "negotiate-only", &negotiate_only,
N_("do not fetch a packfile; instead, print ancestors of negotiation tips")),
OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
@@ -2657,7 +2659,8 @@ int cmd_fetch(int argc,
}
if (negotiate_only && !negotiation_tip.nr)
- die(_("--negotiate-only needs one or more --negotiation-tip=*"));
+ die(_("%s needs one or more %s"), "--negotiate-only",
+ "--negotiation-restrict=*");
if (deepen_relative) {
if (deepen_relative < 0)
diff --git a/builtin/pull.c b/builtin/pull.c
index 7e67fdce97..cc6ce485fc 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -996,9 +996,10 @@ int cmd_pull(int argc,
OPT_PASSTHRU('6', "ipv6", &opt_ipv6, NULL,
N_("use IPv6 addresses only"),
PARSE_OPT_NOARG),
- OPT_PASSTHRU_ARGV(0, "negotiation-tip", &opt_fetch, N_("revision"),
+ OPT_PASSTHRU_ARGV(0, "negotiation-restrict", &opt_fetch, N_("revision"),
N_("report that we have only objects reachable from this object"),
0),
+ OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
N_("check for forced-updates on all updated branches")),
OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
diff --git a/send-pack.c b/send-pack.c
index 67d6987b1c..3d5d36ba3b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -447,7 +447,7 @@ static void get_commons_through_negotiation(struct repository *r,
strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
for (ref = remote_refs; ref; ref = ref->next) {
if (!is_null_oid(&ref->new_oid)) {
- strvec_pushf(&child.args, "--negotiation-tip=%s",
+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
oid_to_hex(&ref->new_oid));
nr_negotiation_tip++;
}
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5dcb4b51a4..dc3ce56d84 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1460,6 +1460,31 @@ EOF
test_cmp fatal-expect fatal-actual
'
+test_expect_success '--negotiation-restrict limits "have" lines sent' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 --negotiation-restrict=beta_1 \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
+test_expect_success '--negotiation-restrict understands globs' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=*_1 \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
+test_expect_success '--negotiation-restrict and --negotiation-tip can be mixed' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-tip=beta_1 \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index f826ac46a5..9f6cf4142d 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -869,14 +869,14 @@ setup_negotiate_only () {
test_commit -C client three
}
-test_expect_success 'usage: --negotiate-only without --negotiation-tip' '
+test_expect_success 'usage: --negotiate-only without --negotiation-restrict' '
SERVER="server" &&
URI="file://$(pwd)/server" &&
setup_negotiate_only "$SERVER" "$URI" &&
cat >err.expect <<-\EOF &&
- fatal: --negotiate-only needs one or more --negotiation-tip=*
+ fatal: --negotiate-only needs one or more --negotiation-restrict=*
EOF
test_must_fail git -c protocol.version=2 -C client fetch \
diff --git a/transport-helper.c b/transport-helper.c
index 4d95d84f9e..dd78d40668 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -755,7 +755,8 @@ static int fetch_refs(struct transport *transport,
}
if (data->transport_options.negotiation_tips)
- warning("Ignoring --negotiation-tip because the protocol does not support it.");
+ warning(_("ignoring %s because the protocol does not support it."),
+ "--negotiation-restrict");
if (data->fetch)
return fetch_with_fetch(transport, nr_heads, to_fetch);
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 3/8] transport: rename negotiation_tips
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The previous change added the --negotiation-restrict synonym for the
--negotiation-tip option for 'git fetch'. In anticipation of adding a new
option that behaves similarly but with distinct changes to its behavior,
rename the internal representation of this data from 'negotiation_tips' to
'negotiation_restrict_tips'.
The 'tips' part is kept because this is an oid_array in the transport layer.
This requires the builtin to handle parsing refs into collections of oids so
the transport layer can handle this cleaner form of the data.
Also update the string_list used to store the inputs from command-line
options.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
builtin/fetch.c | 18 +++++++++---------
fetch-pack.c | 18 +++++++++---------
fetch-pack.h | 4 ++--
transport-helper.c | 2 +-
transport.c | 10 +++++-----
transport.h | 4 ++--
6 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc950fe35b..2ba0051d52 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -98,7 +98,7 @@ static struct transport *gtransport;
static struct transport *gsecondary;
static struct refspec refmap = REFSPEC_INIT_FETCH;
static struct string_list server_options = STRING_LIST_INIT_DUP;
-static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
+static struct string_list negotiation_restrict = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
@@ -1534,13 +1534,13 @@ static int add_oid(const struct reference *ref, void *cb_data)
return 0;
}
-static void add_negotiation_tips(struct git_transport_options *smart_options)
+static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
{
struct oid_array *oids = xcalloc(1, sizeof(*oids));
int i;
- for (i = 0; i < negotiation_tip.nr; i++) {
- const char *s = negotiation_tip.items[i].string;
+ for (i = 0; i < negotiation_restrict.nr; i++) {
+ const char *s = negotiation_restrict.items[i].string;
struct refs_for_each_ref_options opts = {
.pattern = s,
};
@@ -1561,7 +1561,7 @@ static void add_negotiation_tips(struct git_transport_options *smart_options)
warning(_("ignoring %s=%s because it does not match any refs"),
"--negotiation-restrict", s);
}
- smart_options->negotiation_tips = oids;
+ smart_options->negotiation_restrict_tips = oids;
}
static struct transport *prepare_transport(struct remote *remote, int deepen,
@@ -1595,9 +1595,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER, spec);
set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
}
- if (negotiation_tip.nr) {
+ if (negotiation_restrict.nr) {
if (transport->smart_options)
- add_negotiation_tips(transport->smart_options);
+ add_negotiation_restrict_tips(transport->smart_options);
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-restrict");
@@ -2566,7 +2566,7 @@ int cmd_fetch(int argc,
N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
OPT_IPVERSION(&family),
- OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_tip, N_("revision"),
+ OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_restrict, N_("revision"),
N_("report that we have only objects reachable from this object")),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
OPT_BOOL(0, "negotiate-only", &negotiate_only,
@@ -2658,7 +2658,7 @@ int cmd_fetch(int argc,
config.display_format = DISPLAY_FORMAT_PORCELAIN;
}
- if (negotiate_only && !negotiation_tip.nr)
+ if (negotiate_only && !negotiation_restrict.nr)
die(_("%s needs one or more %s"), "--negotiate-only",
"--negotiation-restrict=*");
diff --git a/fetch-pack.c b/fetch-pack.c
index 6ecd468ef7..baf239adf9 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -291,21 +291,21 @@ static int next_flush(int stateless_rpc, int count)
}
static void mark_tips(struct fetch_negotiator *negotiator,
- const struct oid_array *negotiation_tips)
+ const struct oid_array *negotiation_restrict_tips)
{
struct refs_for_each_ref_options opts = {
.flags = REFS_FOR_EACH_INCLUDE_BROKEN,
};
int i;
- if (!negotiation_tips) {
+ if (!negotiation_restrict_tips) {
refs_for_each_ref_ext(get_main_ref_store(the_repository),
rev_list_insert_ref_oid, negotiator, &opts);
return;
}
- for (i = 0; i < negotiation_tips->nr; i++)
- rev_list_insert_ref(negotiator, &negotiation_tips->oid[i]);
+ for (i = 0; i < negotiation_restrict_tips->nr; i++)
+ rev_list_insert_ref(negotiator, &negotiation_restrict_tips->oid[i]);
return;
}
@@ -355,7 +355,7 @@ static int find_common(struct fetch_negotiator *negotiator,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
- mark_tips(negotiator, args->negotiation_tips);
+ mark_tips(negotiator, args->negotiation_restrict_tips);
for_each_cached_alternate(negotiator, insert_one_alternate_object);
fetching = 0;
@@ -1728,7 +1728,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
else
state = FETCH_SEND_REQUEST;
- mark_tips(negotiator, args->negotiation_tips);
+ mark_tips(negotiator, args->negotiation_restrict_tips);
for_each_cached_alternate(negotiator,
insert_one_alternate_object);
break;
@@ -2177,7 +2177,7 @@ static void clear_common_flag(struct oidset *s)
}
}
-void negotiate_using_fetch(const struct oid_array *negotiation_tips,
+void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
@@ -2195,13 +2195,13 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips,
timestamp_t min_generation = GENERATION_NUMBER_INFINITY;
fetch_negotiator_init(the_repository, &negotiator);
- mark_tips(&negotiator, negotiation_tips);
+ mark_tips(&negotiator, negotiation_restrict_tips);
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
- oid_array_for_each((struct oid_array *) negotiation_tips,
+ oid_array_for_each((struct oid_array *) negotiation_restrict_tips,
add_to_object_array,
&nt_object_array);
diff --git a/fetch-pack.h b/fetch-pack.h
index 9d3470366f..6c70c942c2 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -21,7 +21,7 @@ struct fetch_pack_args {
* If not NULL, during packfile negotiation, fetch-pack will send "have"
* lines only with these tips and their ancestors.
*/
- const struct oid_array *negotiation_tips;
+ const struct oid_array *negotiation_restrict_tips;
unsigned deepen_relative:1;
unsigned quiet:1;
@@ -89,7 +89,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
* In the capability advertisement that has happened prior to invoking this
* function, the "wait-for-done" capability must be present.
*/
-void negotiate_using_fetch(const struct oid_array *negotiation_tips,
+void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
diff --git a/transport-helper.c b/transport-helper.c
index dd78d40668..f4388da766 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -754,7 +754,7 @@ static int fetch_refs(struct transport *transport,
set_helper_option(transport, "filter", spec);
}
- if (data->transport_options.negotiation_tips)
+ if (data->transport_options.negotiation_restrict_tips)
warning(_("ignoring %s because the protocol does not support it."),
"--negotiation-restrict");
diff --git a/transport.c b/transport.c
index 107f4fa5dc..a3051f6733 100644
--- a/transport.c
+++ b/transport.c
@@ -463,7 +463,7 @@ static int fetch_refs_via_pack(struct transport *transport,
args.refetch = data->options.refetch;
args.stateless_rpc = transport->stateless_rpc;
args.server_options = transport->server_options;
- args.negotiation_tips = data->options.negotiation_tips;
+ args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
args.reject_shallow_remote = transport->smart_options->reject_shallow;
if (!data->finished_handshake) {
@@ -491,7 +491,7 @@ static int fetch_refs_via_pack(struct transport *transport,
warning(_("server does not support wait-for-done"));
ret = -1;
} else {
- negotiate_using_fetch(data->options.negotiation_tips,
+ negotiate_using_fetch(data->options.negotiation_restrict_tips,
transport->server_options,
transport->stateless_rpc,
data->fd,
@@ -979,9 +979,9 @@ static int disconnect_git(struct transport *transport)
finish_connect(data->conn);
}
- if (data->options.negotiation_tips) {
- oid_array_clear(data->options.negotiation_tips);
- free(data->options.negotiation_tips);
+ if (data->options.negotiation_restrict_tips) {
+ oid_array_clear(data->options.negotiation_restrict_tips);
+ free(data->options.negotiation_restrict_tips);
}
list_objects_filter_release(&data->options.filter_options);
oid_array_clear(&data->extra_have);
diff --git a/transport.h b/transport.h
index 892f19454a..cdeb33c16f 100644
--- a/transport.h
+++ b/transport.h
@@ -40,13 +40,13 @@ struct git_transport_options {
/*
* This is only used during fetch. See the documentation of
- * negotiation_tips in struct fetch_pack_args.
+ * negotiation_restrict_tips in struct fetch_pack_args.
*
* This field is only supported by transports that support connect or
* stateless_connect. Set this field directly instead of using
* transport_set_option().
*/
- struct oid_array *negotiation_tips;
+ struct oid_array *negotiation_restrict_tips;
/*
* If allocated, whenever transport_fetch_refs() is called, add known
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 4/8] remote: add remote.*.negotiationRestrict config
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
In a previous change, the --negotiation-restrict command-line option of 'git
fetch' was added as a synonym of --negotiation-tip. Both of these options
restrict the set of 'haves' the client can send as part of negotiation.
This was previously not available via a configuration option. Add a new
'remote.<name>.negotiationRestrict' multi-valued config option that updates
'git fetch <name>' to use these restrictions by default.
If the user provides even one --negotiation-restrict argument, then the
config is ignored.
An empty value resets the value list to allow ignoring earlier config
values, such as those that might be set in system or global config.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 18 ++++++++++++++++++
builtin/fetch.c | 28 +++++++++++++++++++++-------
remote.c | 5 +++++
remote.h | 1 +
t/t5510-fetch.sh | 26 ++++++++++++++++++++++++++
5 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 91e46f66f5..4dcf81fbce 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -107,6 +107,24 @@ priority configuration file (e.g. `.git/config` in a repository) to clear
the values inherited from a lower priority configuration files (e.g.
`$HOME/.gitconfig`).
+remote.<name>.negotiationRestrict::
+ When negotiating with this remote during `git fetch`, restrict the
+ commits advertised as "have" lines to only those reachable from refs
+ matching the given patterns. This multi-valued config option behaves
+ like `--negotiation-restrict` on the command line.
++
+Each value is either an exact ref name (e.g. `refs/heads/release`) or a
+glob pattern (e.g. `refs/heads/release/*`). The pattern syntax is the
+same as for `--negotiation-restrict`.
++
+These config values are used as defaults for the `--negotiation-restrict`
+command-line option. If `--negotiation-restrict` (or its synonym
+`--negotiation-tip`) is specified on the command line, then the config
+values are not used.
++
+Blank values signal to ignore all previous values, allowing a reset of
+the list from broader config scenarios.
+
remote.<name>.followRemoteHEAD::
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
when fetching using the configured refspecs of a remote.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2ba0051d52..a957739f37 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1601,6 +1601,19 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-restrict");
+ } else if (remote->negotiation_restrict.nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, &remote->negotiation_restrict)
+ string_list_append(&negotiation_restrict, item->string);
+ if (transport->smart_options)
+ add_negotiation_restrict_tips(transport->smart_options);
+ else {
+ struct strbuf config_name = STRBUF_INIT;
+ strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
+ warning(_("ignoring %s because the protocol does not support it"),
+ config_name.buf);
+ strbuf_release(&config_name);
+ }
}
return transport;
}
@@ -2658,10 +2671,6 @@ int cmd_fetch(int argc,
config.display_format = DISPLAY_FORMAT_PORCELAIN;
}
- if (negotiate_only && !negotiation_restrict.nr)
- die(_("%s needs one or more %s"), "--negotiate-only",
- "--negotiation-restrict=*");
-
if (deepen_relative) {
if (deepen_relative < 0)
die(_("negative depth in --deepen is not supported"));
@@ -2749,14 +2758,19 @@ int cmd_fetch(int argc,
if (!remote)
die(_("must supply remote when using --negotiate-only"));
gtransport = prepare_transport(remote, 1, &filter_options);
- if (gtransport->smart_options) {
- gtransport->smart_options->acked_commits = &acked_commits;
- } else {
+
+ if (!gtransport->smart_options) {
warning(_("protocol does not support --negotiate-only, exiting"));
result = 1;
trace2_region_leave("fetch", "negotiate-only", the_repository);
goto cleanup;
}
+ if (!gtransport->smart_options->negotiation_restrict_tips)
+ die(_("%s needs one or more %s"), "--negotiate-only",
+ "--negotiation-restrict=*");
+
+ gtransport->smart_options->acked_commits = &acked_commits;
+
if (server_options.nr)
gtransport->server_options = &server_options;
result = transport_fetch_refs(gtransport, NULL);
diff --git a/remote.c b/remote.c
index 7ca2a6501b..620086e16e 100644
--- a/remote.c
+++ b/remote.c
@@ -152,6 +152,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
refspec_init_push(&ret->push);
refspec_init_fetch(&ret->fetch);
string_list_init_dup(&ret->server_options);
+ string_list_init_dup(&ret->negotiation_restrict);
ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
remote_state->remotes_alloc);
@@ -179,6 +180,7 @@ static void remote_clear(struct remote *remote)
FREE_AND_NULL(remote->http_proxy);
FREE_AND_NULL(remote->http_proxy_authmethod);
string_list_clear(&remote->server_options, 0);
+ string_list_clear(&remote->negotiation_restrict, 0);
}
static void add_merge(struct branch *branch, const char *name)
@@ -562,6 +564,9 @@ static int handle_config(const char *key, const char *value,
} else if (!strcmp(subkey, "serveroption")) {
return parse_transport_option(key, value,
&remote->server_options);
+ } else if (!strcmp(subkey, "negotiationrestrict")) {
+ return parse_transport_option(key, value,
+ &remote->negotiation_restrict);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
diff --git a/remote.h b/remote.h
index fc052945ee..e6ec37c393 100644
--- a/remote.h
+++ b/remote.h
@@ -117,6 +117,7 @@ struct remote {
char *http_proxy_authmethod;
struct string_list server_options;
+ struct string_list negotiation_restrict;
enum follow_remote_head_settings follow_remote_head;
const char *no_warn_branch;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index dc3ce56d84..eff3ce8e2d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1485,6 +1485,32 @@ test_expect_success '--negotiation-restrict and --negotiation-tip can be mixed'
check_negotiation_tip
'
+test_expect_success 'remote.<name>.negotiationRestrict used as default' '
+ setup_negotiation_tip server server 0 &&
+
+ # test the reset of the list on an empty value
+ git -C client config --add remote.origin.negotiationRestrict alpha_2 &&
+ git -C client config --add remote.origin.negotiationRestrict "" &&
+ git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
+ git -C client config --add remote.origin.negotiationRestrict beta_1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
+test_expect_success 'CLI --negotiation-restrict overrides remote config' '
+ setup_negotiation_tip server server 0 &&
+ git -C client config --add remote.origin.negotiationRestrict alpha_1 &&
+ git -C client config --add remote.origin.negotiationRestrict beta_1 &&
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ origin alpha_s beta_s &&
+ test_grep "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep ! "fetch> have $BETA_1" trace
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 5/8] negotiator: add have_sent() interface
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
In a future change, we will introduce a capability to choose specific commit
OIDs as 'have's in fetch negotiation, with the ability to have the
negotiator choose more 'have's to increase coverage beyond that required
core set. The negotiator works to avoid emitting 'have's that can reach each
other, but that logic is hidden beneath the negotiator's iterator function
pointer ('next'). We need a way to communicate to the negotiator that we
have picked a 'have' so it could incorporate that into its logic.
Add a have_sent() method to the fetch_negotiator interface. This is the
signal that allows the negotiator to track the commit as already shown and
can perform the proper bookkeeping to avoid emitting those objects or
anything they can reach.
For our non-trivial negotiators, it is sufficient to mark these commits as
common, so the implementation is quite simple. This logic will be exercised
in the next change.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
fetch-negotiator.h | 9 +++++++++
negotiator/default.c | 8 ++++++++
negotiator/noop.c | 7 +++++++
negotiator/skipping.c | 8 ++++++++
4 files changed, 32 insertions(+)
diff --git a/fetch-negotiator.h b/fetch-negotiator.h
index e348905a1f..6ca422a064 100644
--- a/fetch-negotiator.h
+++ b/fetch-negotiator.h
@@ -47,6 +47,15 @@ struct fetch_negotiator {
*/
int (*ack)(struct fetch_negotiator *, struct commit *);
+ /*
+ * Inform the negotiator that this commit has already been sent as
+ * a "have" line outside of the negotiator's control. The negotiator
+ * should avoid outputting it from next() and may use it to optimize
+ * further negotiation (e.g., by treating it and its ancestors as
+ * common).
+ */
+ void (*have_sent)(struct fetch_negotiator *, struct commit *);
+
void (*release)(struct fetch_negotiator *);
/* internal use */
diff --git a/negotiator/default.c b/negotiator/default.c
index 116dedcf83..05ab616f39 100644
--- a/negotiator/default.c
+++ b/negotiator/default.c
@@ -175,6 +175,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
return known_to_be_common;
}
+static void have_sent(struct fetch_negotiator *n, struct commit *c)
+{
+ if (repo_parse_commit(the_repository, c))
+ return;
+ mark_common(n->data, c, 0, 0);
+}
+
static void release(struct fetch_negotiator *n)
{
clear_prio_queue(&((struct negotiation_state *)n->data)->rev_list);
@@ -188,6 +195,7 @@ void default_negotiator_init(struct fetch_negotiator *negotiator)
negotiator->add_tip = add_tip;
negotiator->next = next;
negotiator->ack = ack;
+ negotiator->have_sent = have_sent;
negotiator->release = release;
negotiator->data = CALLOC_ARRAY(ns, 1);
ns->rev_list.compare = compare_commits_by_commit_date;
diff --git a/negotiator/noop.c b/negotiator/noop.c
index 65e3c20008..edf1b456f3 100644
--- a/negotiator/noop.c
+++ b/negotiator/noop.c
@@ -29,6 +29,12 @@ static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
return 0;
}
+static void have_sent(struct fetch_negotiator *n UNUSED,
+ struct commit *c UNUSED)
+{
+ /* nothing to do */
+}
+
static void release(struct fetch_negotiator *n UNUSED)
{
/* nothing to release */
@@ -40,6 +46,7 @@ void noop_negotiator_init(struct fetch_negotiator *negotiator)
negotiator->add_tip = add_tip;
negotiator->next = next;
negotiator->ack = ack;
+ negotiator->have_sent = have_sent;
negotiator->release = release;
negotiator->data = NULL;
}
diff --git a/negotiator/skipping.c b/negotiator/skipping.c
index 0a272130fb..69472c58e1 100644
--- a/negotiator/skipping.c
+++ b/negotiator/skipping.c
@@ -243,6 +243,13 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
return known_to_be_common;
}
+static void have_sent(struct fetch_negotiator *n, struct commit *c)
+{
+ if (repo_parse_commit(the_repository, c))
+ return;
+ mark_common(n->data, c);
+}
+
static void release(struct fetch_negotiator *n)
{
struct data *data = n->data;
@@ -259,6 +266,7 @@ void skipping_negotiator_init(struct fetch_negotiator *negotiator)
negotiator->add_tip = add_tip;
negotiator->next = next;
negotiator->ack = ack;
+ negotiator->have_sent = have_sent;
negotiator->release = release;
negotiator->data = CALLOC_ARRAY(data, 1);
data->rev_list.compare = compare;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 6/8] fetch: add --negotiation-include option for negotiation
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Add a new --negotiation-include option to 'git fetch', which ensures
that certain ref tips are always sent as 'have' lines during fetch
negotiation, regardless of what the negotiation algorithm selects.
This is useful when the repository has a large number of references, so
the normal negotiation algorithm truncates the list. This is especially
important in repositories with long parallel commit histories. For
example, a repo could have a 'dev' branch for development and a
'release' branch for released versions. If the 'dev' branch isn't
selected for negotiation, then it's not a big deal because there are
many in-progress development branches with a shared history. However, if
'release' is not selected for negotiation, then the server may think
that this is the first time the client has asked for that reference,
causing a full download of its parallel commit history (and any extra
data that may be unique to that branch). This is based on a real example
where certain fetches would grow to 60+ GB when a release branch
updated.
This option is a complement to --negotiation-restrict, which reduces the
negotiation ref set to a specific list. In the earlier example, using
--negotiation-restrict to focus the negotiation to 'dev' and 'release'
would avoid those problematic downloads, but would still not allow
advertising potentially-relevant user branches. In this way, the
'include' version solves the problem I mention while allowing
negotiation to pick other references opportunistically. The two options
can also be combined to allow the best of both worlds.
The argument may be an exact ref name or a glob pattern. Non-existent
refs are silently ignored. This behavior is also updated in the ref matching
logic for the related --negotiation-restrict option to match.
The implementation outputs the requested objects as haves before the
negotiator performs its own algorithm to choose the next haves. Use the new
have_sent() interface to signal these have commits were sent before engaging
with the negotiator's next() iterator.
Also add --negotiation-include to 'git pull' passthrough options.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/fetch-options.adoc | 19 +++++++
builtin/fetch.c | 38 ++++++++++---
builtin/pull.c | 3 ++
fetch-pack.c | 81 +++++++++++++++++++++++++---
fetch-pack.h | 6 ++-
t/t5510-fetch.sh | 91 ++++++++++++++++++++++++++++++++
transport.c | 8 ++-
transport.h | 5 +-
8 files changed, 232 insertions(+), 19 deletions(-)
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index d39cecb446..7b897a7202 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -73,6 +73,25 @@ See also the `fetch.negotiationAlgorithm` and `push.negotiate`
configuration variables documented in linkgit:git-config[1], and the
`--negotiate-only` option below.
+`--negotiation-include=(<commit>|<glob>)`::
+ Ensure that the commits at the given tips are always sent as "have"
+ lines during fetch negotiation, regardless of what the negotiation
+ algorithm selects. This is useful to guarantee that common
+ history reachable from specific refs is always considered, even
+ when `--negotiation-restrict` restricts the set of tips or when
+ the negotiation algorithm would otherwise skip them.
++
+This option may be specified more than once; if so, each commit is sent
+unconditionally.
++
+The argument may be an exact ref name (e.g. `refs/heads/release`), an
+object hash, or a glob pattern (e.g. `refs/heads/release/{asterisk}`).
+The pattern syntax is the same as for `--negotiation-restrict`.
++
+If `--negotiation-restrict` is used, the have set is first restricted by
+that option and then increased to include the tips specified by
+`--negotiation-include`.
+
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
ancestors of the provided `--negotiation-restrict=` arguments,
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a957739f37..ba56e9022b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -99,6 +99,7 @@ static struct transport *gsecondary;
static struct refspec refmap = REFSPEC_INIT_FETCH;
static struct string_list server_options = STRING_LIST_INIT_DUP;
static struct string_list negotiation_restrict = STRING_LIST_INIT_NODUP;
+static struct string_list negotiation_include = STRING_LIST_INIT_NODUP;
struct fetch_config {
enum display_format display_format;
@@ -1534,23 +1535,29 @@ static int add_oid(const struct reference *ref, void *cb_data)
return 0;
}
-static void add_negotiation_restrict_tips(struct git_transport_options *smart_options)
+static void add_negotiation_tips(struct string_list *input_list,
+ struct oid_array **output_list,
+ const char *argname)
{
struct oid_array *oids = xcalloc(1, sizeof(*oids));
int i;
- for (i = 0; i < negotiation_restrict.nr; i++) {
- const char *s = negotiation_restrict.items[i].string;
+ for (i = 0; i < input_list->nr; i++) {
+ const char *s = input_list->items[i].string;
struct refs_for_each_ref_options opts = {
.pattern = s,
};
int old_nr;
if (!has_glob_specials(s)) {
struct object_id oid;
+
+ /* Ignore missing reference. */
if (repo_get_oid(the_repository, s, &oid))
- die(_("%s is not a valid object"), s);
+ continue;
+ /* Fail on missing object pointed by ref. */
if (!odb_has_object(the_repository->objects, &oid, 0))
die(_("the object %s does not exist"), s);
+
oid_array_append(oids, &oid);
continue;
}
@@ -1559,9 +1566,9 @@ static void add_negotiation_restrict_tips(struct git_transport_options *smart_op
add_oid, oids, &opts);
if (old_nr == oids->nr)
warning(_("ignoring %s=%s because it does not match any refs"),
- "--negotiation-restrict", s);
+ argname, s);
}
- smart_options->negotiation_restrict_tips = oids;
+ *output_list = oids;
}
static struct transport *prepare_transport(struct remote *remote, int deepen,
@@ -1597,7 +1604,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
}
if (negotiation_restrict.nr) {
if (transport->smart_options)
- add_negotiation_restrict_tips(transport->smart_options);
+ add_negotiation_tips(&negotiation_restrict,
+ &transport->smart_options->negotiation_restrict_tips,
+ "--negotiation-restrict");
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-restrict");
@@ -1606,7 +1615,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
for_each_string_list_item(item, &remote->negotiation_restrict)
string_list_append(&negotiation_restrict, item->string);
if (transport->smart_options)
- add_negotiation_restrict_tips(transport->smart_options);
+ add_negotiation_tips(&negotiation_restrict,
+ &transport->smart_options->negotiation_restrict_tips,
+ "--negotiation-restrict");
else {
struct strbuf config_name = STRBUF_INIT;
strbuf_addf(&config_name, "remote.%s.negotiationRestrict", remote->name);
@@ -1615,6 +1626,15 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
strbuf_release(&config_name);
}
}
+ if (negotiation_include.nr) {
+ if (transport->smart_options)
+ add_negotiation_tips(&negotiation_include,
+ &transport->smart_options->negotiation_include_tips,
+ "--negotiation-include");
+ else
+ warning(_("ignoring %s because the protocol does not support it"),
+ "--negotiation-include");
+ }
return transport;
}
@@ -2582,6 +2602,8 @@ int cmd_fetch(int argc,
OPT_STRING_LIST(0, "negotiation-restrict", &negotiation_restrict, N_("revision"),
N_("report that we have only objects reachable from this object")),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
+ OPT_STRING_LIST(0, "negotiation-include", &negotiation_include, N_("revision"),
+ N_("ensure this ref is always sent as a negotiation have")),
OPT_BOOL(0, "negotiate-only", &negotiate_only,
N_("do not fetch a packfile; instead, print ancestors of negotiation tips")),
OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
diff --git a/builtin/pull.c b/builtin/pull.c
index cc6ce485fc..d49b09114a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1000,6 +1000,9 @@ int cmd_pull(int argc,
N_("report that we have only objects reachable from this object"),
0),
OPT_ALIAS(0, "negotiation-tip", "negotiation-restrict"),
+ OPT_PASSTHRU_ARGV(0, "negotiation-include", &opt_fetch, N_("revision"),
+ N_("ensure this ref is always sent as a negotiation have"),
+ 0),
OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
N_("check for forced-updates on all updated branches")),
OPT_PASSTHRU(0, "set-upstream", &set_upstream, NULL,
diff --git a/fetch-pack.c b/fetch-pack.c
index baf239adf9..96071434b8 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -25,6 +25,7 @@
#include "oidset.h"
#include "packfile.h"
#include "odb.h"
+#include "object-name.h"
#include "path.h"
#include "connected.h"
#include "fetch-negotiator.h"
@@ -332,6 +333,21 @@ static void send_filter(struct fetch_pack_args *args,
}
}
+static void add_oids_to_set(const struct oid_array *array,
+ struct oidset *set)
+{
+ if (!array)
+ return;
+
+ for (size_t i = 0; i < array->nr; i++) {
+ struct object_id *oid = &array->oid[i];
+ if (!odb_has_object(the_repository->objects, oid, 0))
+ die(_("the object %s does not exist"), oid_to_hex(oid));
+
+ oidset_insert(set, oid);
+ }
+}
+
static int find_common(struct fetch_negotiator *negotiator,
struct fetch_pack_args *args,
int fd[2], struct object_id *result_oid,
@@ -347,6 +363,7 @@ static int find_common(struct fetch_negotiator *negotiator,
struct strbuf req_buf = STRBUF_INIT;
size_t state_len = 0;
struct packet_reader reader;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
if (args->stateless_rpc && multi_ack == 1)
die(_("the option '%s' requires '%s'"), "--stateless-rpc", "multi_ack_detailed");
@@ -474,6 +491,27 @@ static int find_common(struct fetch_negotiator *negotiator,
trace2_region_enter("fetch-pack", "negotiation_v0_v1", the_repository);
flushes = 0;
retval = -1;
+
+ /* Send unconditional haves from --negotiation-include */
+ add_oids_to_set(args->negotiation_include_tips,
+ &negotiation_include_oids);
+ if (oidset_size(&negotiation_include_oids)) {
+ struct oidset_iter iter;
+ oidset_iter_init(&negotiation_include_oids, &iter);
+
+ while ((oid = oidset_iter_next(&iter))) {
+ struct commit *commit;
+ packet_buf_write(&req_buf, "have %s\n",
+ oid_to_hex(oid));
+ print_verbose(args, "have %s", oid_to_hex(oid));
+ count++;
+
+ commit = lookup_commit(the_repository, oid);
+ if (commit)
+ negotiator->have_sent(negotiator, commit);
+ }
+ }
+
while ((oid = negotiator->next(negotiator))) {
packet_buf_write(&req_buf, "have %s\n", oid_to_hex(oid));
print_verbose(args, "have %s", oid_to_hex(oid));
@@ -584,6 +622,7 @@ done:
flushes++;
}
strbuf_release(&req_buf);
+ oidset_clear(&negotiation_include_oids);
if (!got_ready || !no_done)
consume_shallow_list(args, &reader);
@@ -1305,11 +1344,27 @@ static void add_common(struct strbuf *req_buf, struct oidset *common)
static int add_haves(struct fetch_negotiator *negotiator,
struct strbuf *req_buf,
- int *haves_to_send)
+ int *haves_to_send,
+ struct oidset *negotiation_include_oids)
{
int haves_added = 0;
const struct object_id *oid;
+ /* Send unconditional haves from --negotiation-include */
+ if (negotiation_include_oids) {
+ struct oidset_iter iter;
+ oidset_iter_init(negotiation_include_oids, &iter);
+
+ while ((oid = oidset_iter_next(&iter))) {
+ struct commit *commit = lookup_commit(the_repository, oid);
+ if (commit) {
+ packet_buf_write(req_buf, "have %s\n",
+ oid_to_hex(oid));
+ negotiator->have_sent(negotiator, commit);
+ }
+ }
+ }
+
while ((oid = negotiator->next(negotiator))) {
packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
if (++haves_added >= *haves_to_send)
@@ -1358,7 +1413,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
struct fetch_pack_args *args,
const struct ref *wants, struct oidset *common,
int *haves_to_send, int *in_vain,
- int sideband_all, int seen_ack)
+ int sideband_all, int seen_ack,
+ struct oidset *negotiation_include_oids)
{
int haves_added;
int done_sent = 0;
@@ -1413,7 +1469,8 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
/* Add all of the common commits we've found in previous rounds */
add_common(&req_buf, common);
- haves_added = add_haves(negotiator, &req_buf, haves_to_send);
+ haves_added = add_haves(negotiator, &req_buf, haves_to_send,
+ negotiation_include_oids);
*in_vain += haves_added;
trace2_data_intmax("negotiation_v2", the_repository, "haves_added", haves_added);
trace2_data_intmax("negotiation_v2", the_repository, "in_vain", *in_vain);
@@ -1657,6 +1714,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
struct ref *ref = copy_ref_list(orig_ref);
enum fetch_state state = FETCH_CHECK_LOCAL;
struct oidset common = OIDSET_INIT;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
struct packet_reader reader;
int in_vain = 0, negotiation_started = 0;
int negotiation_round = 0;
@@ -1729,6 +1787,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
state = FETCH_SEND_REQUEST;
mark_tips(negotiator, args->negotiation_restrict_tips);
+ add_oids_to_set(args->negotiation_include_tips,
+ &negotiation_include_oids);
for_each_cached_alternate(negotiator,
insert_one_alternate_object);
break;
@@ -1747,7 +1807,8 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
&common,
&haves_to_send, &in_vain,
reader.use_sideband,
- seen_ack)) {
+ seen_ack,
+ &negotiation_include_oids)) {
trace2_region_leave_printf("negotiation_v2", "round",
the_repository, "%d",
negotiation_round);
@@ -1883,6 +1944,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
negotiator->release(negotiator);
oidset_clear(&common);
+ oidset_clear(&negotiation_include_oids);
return ref;
}
@@ -2181,12 +2243,14 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
- struct oidset *acked_commits)
+ struct oidset *acked_commits,
+ const struct oid_array *negotiation_include_tips)
{
struct fetch_negotiator negotiator;
struct packet_reader reader;
struct object_array nt_object_array = OBJECT_ARRAY_INIT;
struct strbuf req_buf = STRBUF_INIT;
+ struct oidset negotiation_include_oids = OIDSET_INIT;
int haves_to_send = INITIAL_FLUSH;
int in_vain = 0;
int seen_ack = 0;
@@ -2197,6 +2261,9 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
fetch_negotiator_init(the_repository, &negotiator);
mark_tips(&negotiator, negotiation_restrict_tips);
+ add_oids_to_set(negotiation_include_tips,
+ &negotiation_include_oids);
+
packet_reader_init(&reader, fd[0], NULL, 0,
PACKET_READ_CHOMP_NEWLINE |
PACKET_READ_DIE_ON_ERR_PACKET);
@@ -2221,7 +2288,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
packet_buf_write(&req_buf, "wait-for-done");
- haves_added = add_haves(&negotiator, &req_buf, &haves_to_send);
+ haves_added = add_haves(&negotiator, &req_buf, &haves_to_send,
+ &negotiation_include_oids);
in_vain += haves_added;
if (!haves_added || (seen_ack && in_vain >= MAX_IN_VAIN))
last_iteration = 1;
@@ -2273,6 +2341,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
clear_common_flag(acked_commits);
object_array_clear(&nt_object_array);
+ oidset_clear(&negotiation_include_oids);
negotiator.release(&negotiator);
strbuf_release(&req_buf);
}
diff --git a/fetch-pack.h b/fetch-pack.h
index 6c70c942c2..6d0dec7f41 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -19,9 +19,10 @@ struct fetch_pack_args {
/*
* If not NULL, during packfile negotiation, fetch-pack will send "have"
- * lines only with these tips and their ancestors.
+ * lines for all _include_ tips and then a subset of the _restrict_ tips.
*/
const struct oid_array *negotiation_restrict_tips;
+ const struct oid_array *negotiation_include_tips;
unsigned deepen_relative:1;
unsigned quiet:1;
@@ -93,7 +94,8 @@ void negotiate_using_fetch(const struct oid_array *negotiation_restrict_tips,
const struct string_list *server_options,
int stateless_rpc,
int fd[],
- struct oidset *acked_commits);
+ struct oidset *acked_commits,
+ const struct oid_array *negotiation_include_tips);
/*
* Print an appropriate error message for each sought ref that wasn't
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index eff3ce8e2d..bc2e2af959 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1460,6 +1460,16 @@ EOF
test_cmp fatal-expect fatal-actual
'
+test_expect_success '--negotiation-tip ignores missing refs and invalid hashes' '
+ setup_negotiation_tip server server 0 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-tip=alpha_1 --negotiation-tip=beta_1 \
+ --negotiation-tip=no-such-ref \
+ --negotiation-tip=invalid-hash \
+ origin alpha_s beta_s &&
+ check_negotiation_tip
+'
+
test_expect_success '--negotiation-restrict limits "have" lines sent' '
setup_negotiation_tip server server 0 &&
GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
@@ -1511,6 +1521,87 @@ test_expect_success 'CLI --negotiation-restrict overrides remote config' '
test_grep ! "fetch> have $BETA_1" trace
'
+test_expect_success '--negotiation-include includes configured refs as haves' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ test_grep "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success '--negotiation-include works with glob patterns' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include="refs/tags/beta_*" \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep "fetch> have $BETA_2" trace
+'
+
+test_expect_success '--negotiation-include is additive with negotiation' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success '--negotiation-include ignores non-existent refs silently' '
+ setup_negotiation_tip server server 0 &&
+
+ git -C client fetch --quiet \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/nonexistent \
+ origin alpha_s beta_s 2>err &&
+ test_must_be_empty err
+'
+
+test_expect_success '--negotiation-include avoids duplicates with negotiator' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/alpha_1 \
+ origin alpha_s beta_s &&
+
+ test_grep "fetch> have $ALPHA_1" trace >matches &&
+ test_line_count = 1 matches
+'
+
+test_expect_success '--negotiation-include avoids duplicates with v0' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client \
+ -c protocol.version=0 fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/alpha_1 \
+ origin alpha_s beta_s &&
+
+ test_grep "fetch> have $ALPHA_1" trace >matches &&
+ test_line_count = 1 matches
+'
+
test_expect_success SYMLINKS 'clone does not get confused by a D/F conflict' '
git init df-conflict &&
(
diff --git a/transport.c b/transport.c
index a3051f6733..fa54928966 100644
--- a/transport.c
+++ b/transport.c
@@ -464,6 +464,7 @@ static int fetch_refs_via_pack(struct transport *transport,
args.stateless_rpc = transport->stateless_rpc;
args.server_options = transport->server_options;
args.negotiation_restrict_tips = data->options.negotiation_restrict_tips;
+ args.negotiation_include_tips = data->options.negotiation_include_tips;
args.reject_shallow_remote = transport->smart_options->reject_shallow;
if (!data->finished_handshake) {
@@ -495,7 +496,8 @@ static int fetch_refs_via_pack(struct transport *transport,
transport->server_options,
transport->stateless_rpc,
data->fd,
- data->options.acked_commits);
+ data->options.acked_commits,
+ data->options.negotiation_include_tips);
ret = 0;
}
goto cleanup;
@@ -983,6 +985,10 @@ static int disconnect_git(struct transport *transport)
oid_array_clear(data->options.negotiation_restrict_tips);
free(data->options.negotiation_restrict_tips);
}
+ if (data->options.negotiation_include_tips) {
+ oid_array_clear(data->options.negotiation_include_tips);
+ free(data->options.negotiation_include_tips);
+ }
list_objects_filter_release(&data->options.filter_options);
oid_array_clear(&data->extra_have);
oid_array_clear(&data->shallow);
diff --git a/transport.h b/transport.h
index cdeb33c16f..97d905ecc0 100644
--- a/transport.h
+++ b/transport.h
@@ -40,13 +40,14 @@ struct git_transport_options {
/*
* This is only used during fetch. See the documentation of
- * negotiation_restrict_tips in struct fetch_pack_args.
+ * these member names in struct fetch_pack_args.
*
- * This field is only supported by transports that support connect or
+ * These fields are only supported by transports that support connect or
* stateless_connect. Set this field directly instead of using
* transport_set_option().
*/
struct oid_array *negotiation_restrict_tips;
+ struct oid_array *negotiation_include_tips;
/*
* If allocated, whenever transport_fetch_refs() is called, add known
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 7/8] remote: add remote.*.negotiationInclude config
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Add a new 'remote.<name>.negotiationInclude' multi-valued config option that
provides default values for --negotiation-include when no
--negotiation-include arguments are specified over the command line. This
is a mirror of how 'remote.<name>.negotiationRestrict' specifies defaults
for the --negotiation-restrict arguments.
Each value is either an exact ref name or a glob pattern whose tips should
always be sent as 'have' lines during negotiation. The config values are
resolved through the same resolve_negotiation_include() codepath as the CLI
options.
This option is additive with the normal negotiation process: the negotiation
algorithm still runs and advertises its own selected commits, but the refs
matching the config are sent unconditionally on top of those heuristically
selected commits.
Similar to the negotiationRestrict config, an empty value resets the value
list to allow ignoring earlier config values, such as those that might be
set in system or global config.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 25 ++++++++++++++++
Documentation/fetch-options.adoc | 4 +++
builtin/fetch.c | 12 ++++++++
remote.c | 5 ++++
remote.h | 1 +
t/t5510-fetch.sh | 49 ++++++++++++++++++++++++++++++++
6 files changed, 96 insertions(+)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 4dcf81fbce..1951df154e 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -125,6 +125,31 @@ values are not used.
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
+remote.<name>.negotiationInclude::
+ When negotiating with this remote during `git fetch`, the client
+ advertises a list of commits that exist locally. In repos with
+ many references, this list of "haves" can be truncated. Depending
+ on data shape, dropping certain references may be expensive. This
+ multi-valued config option specifies references, commit hashes,
+ or ref pattern globs whose tips should always be sent as "have"
+ commits during fetch negotiation with this remote.
++
+Each value is either an exact ref name (e.g. `refs/heads/release`), a
+commit hash, or a glob pattern (e.g. `refs/heads/release/*`). The
+pattern syntax is the same as for `--negotiation-include`.
++
+These config values are used as defaults for the `--negotiation-include`
+command-line option. If `--negotiation-include` is specified on the
+command line, then the config values are not used.
++
+This option is additive with the normal negotiation process: the
+negotiation algorithm still runs and advertises its own selected commits,
+but the refs matching `remote.<name>.negotiationInclude` are sent
+unconditionally on top of those heuristically selected commits.
++
+Blank values signal to ignore all previous values, allowing a reset of
+the list from broader config scenarios.
+
remote.<name>.followRemoteHEAD::
How linkgit:git-fetch[1] should handle updates to `remotes/<name>/HEAD`
when fetching using the configured refspecs of a remote.
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 7b897a7202..8074004377 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -91,6 +91,10 @@ The pattern syntax is the same as for `--negotiation-restrict`.
If `--negotiation-restrict` is used, the have set is first restricted by
that option and then increased to include the tips specified by
`--negotiation-include`.
++
+If this option is not specified on the command line, then any
+`remote.<name>.negotiationInclude` config values for the current remote
+are used instead.
`--negotiate-only`::
Do not fetch anything from the server, and instead print the
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ba56e9022b..1af6500c1d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1634,6 +1634,18 @@ static struct transport *prepare_transport(struct remote *remote, int deepen,
else
warning(_("ignoring %s because the protocol does not support it"),
"--negotiation-include");
+ } else if (remote->negotiation_include.nr) {
+ if (transport->smart_options) {
+ add_negotiation_tips(&remote->negotiation_include,
+ &transport->smart_options->negotiation_include_tips,
+ "--negotiation-include");
+ } else {
+ struct strbuf config_name = STRBUF_INIT;
+ strbuf_addf(&config_name, "remote.%s.negotiationInclude", remote->name);
+ warning(_("ignoring %s because the protocol does not support it"),
+ config_name.buf);
+ strbuf_release(&config_name);
+ }
}
return transport;
}
diff --git a/remote.c b/remote.c
index 620086e16e..6fb5758820 100644
--- a/remote.c
+++ b/remote.c
@@ -153,6 +153,7 @@ static struct remote *make_remote(struct remote_state *remote_state,
refspec_init_fetch(&ret->fetch);
string_list_init_dup(&ret->server_options);
string_list_init_dup(&ret->negotiation_restrict);
+ string_list_init_dup(&ret->negotiation_include);
ALLOC_GROW(remote_state->remotes, remote_state->remotes_nr + 1,
remote_state->remotes_alloc);
@@ -181,6 +182,7 @@ static void remote_clear(struct remote *remote)
FREE_AND_NULL(remote->http_proxy_authmethod);
string_list_clear(&remote->server_options, 0);
string_list_clear(&remote->negotiation_restrict, 0);
+ string_list_clear(&remote->negotiation_include, 0);
}
static void add_merge(struct branch *branch, const char *name)
@@ -567,6 +569,9 @@ static int handle_config(const char *key, const char *value,
} else if (!strcmp(subkey, "negotiationrestrict")) {
return parse_transport_option(key, value,
&remote->negotiation_restrict);
+ } else if (!strcmp(subkey, "negotiationinclude")) {
+ return parse_transport_option(key, value,
+ &remote->negotiation_include);
} else if (!strcmp(subkey, "followremotehead")) {
const char *no_warn_branch;
if (!strcmp(value, "never"))
diff --git a/remote.h b/remote.h
index e6ec37c393..d8809b6991 100644
--- a/remote.h
+++ b/remote.h
@@ -118,6 +118,7 @@ struct remote {
struct string_list server_options;
struct string_list negotiation_restrict;
+ struct string_list negotiation_include;
enum follow_remote_head_settings follow_remote_head;
const char *no_warn_branch;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index bc2e2af959..33f61ac12a 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -1587,6 +1587,55 @@ test_expect_success '--negotiation-include avoids duplicates with negotiator' '
test_line_count = 1 matches
'
+test_expect_success 'remote.<name>.negotiationInclude used as default for --negotiation-include' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ # test the reset of the list on an empty value
+ git -C client config --add remote.origin.negotiationInclude refs/tags/alpha_1 &&
+ git -C client config --add remote.origin.negotiationInclude "" &&
+ git -C client config --add remote.origin.negotiationInclude refs/tags/beta_1 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=beta_2 \
+ origin alpha_s beta_s &&
+
+ ALPHA_1=$(git -C client rev-parse alpha_1) &&
+ test_grep ! "fetch> have $ALPHA_1" trace &&
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace
+'
+
+test_expect_success 'remote.<name>.negotiationInclude works with glob patterns' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ git -C client config --add remote.origin.negotiationInclude "refs/tags/beta_*" &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep "fetch> have $BETA_2" trace
+'
+
+test_expect_success 'CLI --negotiation-include overrides remote.<name>.negotiationInclude' '
+ test_when_finished rm -f trace &&
+ setup_negotiation_tip server server 0 &&
+
+ git -C client config --add remote.origin.negotiationInclude refs/tags/beta_2 &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git -C client fetch \
+ --negotiation-restrict=alpha_1 \
+ --negotiation-include=refs/tags/beta_1 \
+ origin alpha_s beta_s &&
+
+ BETA_1=$(git -C client rev-parse beta_1) &&
+ test_grep "fetch> have $BETA_1" trace &&
+ BETA_2=$(git -C client rev-parse beta_2) &&
+ test_grep ! "fetch> have $BETA_2" trace
+'
+
test_expect_success '--negotiation-include avoids duplicates with v0' '
test_when_finished rm -f trace &&
setup_negotiation_tip server server 0 &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 8/8] send-pack: pass negotiation config in push
From: Derrick Stolee via GitGitGadget @ 2026-05-19 16:24 UTC (permalink / raw)
To: git; +Cc: gitster, ps, Matthew John Cheetham, Derrick Stolee,
Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
When push.negotiate is enabled, 'git push' spawns a child 'git fetch
--negotiate-only' process to find common commits. Pass
--negotiation-include and --negotiation-restrict options from the
'remote.<name>.negotiationInclude' and
'remote.<name>.negotiationRestrict' config keys to this child process.
When negotiationRestrict is configured, it replaces the default
behavior of using all remote refs as negotiation tips. This allows
the user to control which local refs are used for push negotiation.
When negotiationInclude is configured, the specified ref patterns
are passed as --negotiation-include to ensure their tips are always
sent as 'have' lines during push negotiation.
Reviewed-by: Matthew John Cheetham <mjcheetham@outlook.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/config/remote.adoc | 6 ++++++
send-pack.c | 37 ++++++++++++++++++++++++++------
send-pack.h | 2 ++
t/t5516-fetch-push.sh | 30 ++++++++++++++++++++++++++
transport.c | 2 ++
5 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/Documentation/config/remote.adoc b/Documentation/config/remote.adoc
index 1951df154e..eb9c8a3c48 100644
--- a/Documentation/config/remote.adoc
+++ b/Documentation/config/remote.adoc
@@ -122,6 +122,9 @@ command-line option. If `--negotiation-restrict` (or its synonym
`--negotiation-tip`) is specified on the command line, then the config
values are not used.
+
+These values also influence negotiation during `git push` if
+`push.negotiate` is enabled.
++
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
@@ -147,6 +150,9 @@ negotiation algorithm still runs and advertises its own selected commits,
but the refs matching `remote.<name>.negotiationInclude` are sent
unconditionally on top of those heuristically selected commits.
+
+These values also influence negotiation during `git push` if
+`push.negotiate` is enabled.
++
Blank values signal to ignore all previous values, allowing a reset of
the list from broader config scenarios.
diff --git a/send-pack.c b/send-pack.c
index 3d5d36ba3b..d18e030ce8 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -433,28 +433,48 @@ static void reject_invalid_nonce(const char *nonce, int len)
static void get_commons_through_negotiation(struct repository *r,
const char *url,
+ const struct string_list *negotiation_include,
+ const struct string_list *negotiation_restrict,
const struct ref *remote_refs,
struct oid_array *commons)
{
struct child_process child = CHILD_PROCESS_INIT;
const struct ref *ref;
int len = r->hash_algo->hexsz + 1; /* hash + NL */
- int nr_negotiation_tip = 0;
+ int nr_negotiation = 0;
child.git_cmd = 1;
child.no_stdin = 1;
child.out = -1;
strvec_pushl(&child.args, "fetch", "--negotiate-only", NULL);
- for (ref = remote_refs; ref; ref = ref->next) {
- if (!is_null_oid(&ref->new_oid)) {
+
+ if (negotiation_restrict && negotiation_restrict->nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, negotiation_restrict)
strvec_pushf(&child.args, "--negotiation-restrict=%s",
- oid_to_hex(&ref->new_oid));
- nr_negotiation_tip++;
+ item->string);
+ nr_negotiation = negotiation_restrict->nr;
+ } else {
+ for (ref = remote_refs; ref; ref = ref->next) {
+ if (!is_null_oid(&ref->new_oid)) {
+ strvec_pushf(&child.args, "--negotiation-restrict=%s",
+ oid_to_hex(&ref->new_oid));
+ nr_negotiation++;
+ }
}
}
+
+ if (negotiation_include && negotiation_include->nr) {
+ struct string_list_item *item;
+ for_each_string_list_item(item, negotiation_include)
+ strvec_pushf(&child.args, "--negotiation-include=%s",
+ item->string);
+ nr_negotiation += negotiation_include->nr;
+ }
+
strvec_push(&child.args, url);
- if (!nr_negotiation_tip) {
+ if (!nr_negotiation) {
child_process_clear(&child);
return;
}
@@ -528,7 +548,10 @@ int send_pack(struct repository *r,
repo_config_get_bool(r, "push.negotiate", &push_negotiate);
if (push_negotiate) {
trace2_region_enter("send_pack", "push_negotiate", r);
- get_commons_through_negotiation(r, args->url, remote_refs, &commons);
+ get_commons_through_negotiation(r, args->url,
+ args->negotiation_include,
+ args->negotiation_restrict,
+ remote_refs, &commons);
trace2_region_leave("send_pack", "push_negotiate", r);
}
diff --git a/send-pack.h b/send-pack.h
index c5ded2d200..13850c98bb 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -18,6 +18,8 @@ struct repository;
struct send_pack_args {
const char *url;
+ const struct string_list *negotiation_include;
+ const struct string_list *negotiation_restrict;
unsigned verbose:1,
quiet:1,
porcelain:1,
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index ac8447f21e..177cbc6c75 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -254,6 +254,36 @@ test_expect_success 'push with negotiation does not attempt to fetch submodules'
! grep "Fetching submodule" err
'
+test_expect_success 'push with negotiation and remote.<name>.negotiationInclude' '
+ test_when_finished rm -rf negotiation_include &&
+ mk_empty negotiation_include &&
+ git push negotiation_include $the_first_commit:refs/remotes/origin/first_commit &&
+ test_commit -C negotiation_include unrelated_commit &&
+ git -C negotiation_include config receive.hideRefs refs/remotes/origin/first_commit &&
+ test_when_finished "rm event" &&
+ GIT_TRACE2_EVENT="$(pwd)/event" \
+ git -c protocol.version=2 -c push.negotiate=1 \
+ -c remote.negotiation_include.negotiationInclude=refs/heads/main \
+ push negotiation_include refs/heads/main:refs/remotes/origin/main &&
+ test_grep \"key\":\"total_rounds\" event &&
+ grep_wrote 2 event # 1 commit, 1 tree
+'
+
+test_expect_success 'push with negotiation and remote.<name>.negotiationRestrict' '
+ test_when_finished rm -rf negotiation_restrict &&
+ mk_empty negotiation_restrict &&
+ git push negotiation_restrict $the_first_commit:refs/remotes/origin/first_commit &&
+ test_commit -C negotiation_restrict unrelated_commit &&
+ git -C negotiation_restrict config receive.hideRefs refs/remotes/origin/first_commit &&
+ test_when_finished "rm event" &&
+ GIT_TRACE2_EVENT="$(pwd)/event" \
+ git -c protocol.version=2 -c push.negotiate=1 \
+ -c remote.negotiation_restrict.negotiationRestrict=refs/heads/main \
+ push negotiation_restrict refs/heads/main:refs/remotes/origin/main &&
+ test_grep \"key\":\"total_rounds\" event &&
+ grep_wrote 2 event # 1 commit, 1 tree
+'
+
test_expect_success 'push without wildcard' '
mk_empty testrepo &&
diff --git a/transport.c b/transport.c
index fa54928966..a2d8958cb8 100644
--- a/transport.c
+++ b/transport.c
@@ -921,6 +921,8 @@ static int git_transport_push(struct transport *transport, struct ref *remote_re
args.atomic = !!(flags & TRANSPORT_PUSH_ATOMIC);
args.push_options = transport->push_options;
args.url = transport->url;
+ args.negotiation_include = &transport->remote->negotiation_include;
+ args.negotiation_restrict = &transport->remote->negotiation_restrict;
if (flags & TRANSPORT_PUSH_CERT_ALWAYS)
args.push_cert = SEND_PACK_PUSH_CERT_ALWAYS;
--
gitgitgadget
^ permalink raw reply related
* Re: What's cooking in git.git (May 2026, #04)
From: Taylor Blau @ 2026-05-19 16:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqv7clbizy.fsf@gitster.g>
On Mon, May 18, 2026 at 10:32:01AM +0900, Junio C Hamano wrote:
> * tb/incremental-midx-part-3.3 (2026-04-29) 16 commits
> - repack: allow `--write-midx=incremental` without `--geometric`
> - repack: introduce `--write-midx=incremental`
> - repack: implement incremental MIDX repacking
> - packfile: ensure `close_pack_revindex()` frees in-memory revindex
> - builtin/repack.c: convert `--write-midx` to an `OPT_CALLBACK`
> - repack-geometry: prepare for incremental MIDX repacking
> - repack-midx: extract `repack_fill_midx_stdin_packs()`
> - repack-midx: factor out `repack_prepare_midx_command()`
> - midx: expose `midx_layer_contains_pack()`
> - repack: track the ODB source via existing_packs
> - midx: support custom `--base` for incremental MIDX writes
> - midx: introduce `--no-write-chain-file` for incremental MIDX writes
> - midx: use `strvec` for `keep_hashes`
> - midx: build `keep_hashes` array in order
> - midx: use `strset` for retained MIDX files
> - midx-write: handle noop writes when converting incremental chains
>
> The repacking code has been refactored and compaction of MIDX layers
> have been implemented, and incremental strategy that does not require
> all-into-one repacking has been introduced.
>
> Waiting for response(s) to review comment(s).
> cf. <agTw579yuy4iHoMq@szeder.dev>
> cf. <20260513230825.GA1378716@coredump.intra.peff.net>
> source: <cover.1777507303.git.me@ttaylorr.com>
Apologies, I didn't realize you were waiting on these until seeing this
WC report. I sent an extremely tiny reroll
https://lore.kernel.org/git/cover.1779206239.git.me@ttaylorr.com/
that addresses the two outstanding comments you linked. They are very
minor changes, and queueing either version of the series would be
equally fine IMHO.
Thanks,
Taylor
^ permalink raw reply
* [PATCH 0/9] Add support for an external command for fetching notes
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
Hi,
This series teaches the notes display machinery to obtain note text from a
long-lived external helper configured by `notes.externalCommand`.
The motivation is mentioned in the main commit message (PATCH 7/9).
The helper protocol is intentionally narrow. Git starts the command once,
sends one commit object ID per request, and expects either:
<object-id> missing
<object-id> ok <n>
<n bytes of UTF-8 note text>
with the documented trailing newlines. The command is read only from protected
configuration, so an untrusted repository cannot make ordinary note display run
arbitrary commands. If the helper cannot be started, times out, exits, or sends
an invalid response, Git warns once, disables it for the rest of the process,
and continues without external notes.
Users can control from command line too with `--external-notes` and
`--no-external-notes`. The semantics are close to `--notes=<ref>`:
`--external-notes` implies naming an explicit notes source by itself, while
`--external-notes --notes` combines it with the default notes refs, and
`--external-notes --notes=<ref>` combines it with specific notes refs. The
series also adds `notes.externalCommandName`, `notes.externalCommandTimeoutMs`,
and the opt-in `notes.externalCommandForGrep` knob for installations that want
external notes to participate in `--grep` matching.
Because this puts an external process on the log-formatting path, the series
also adds the small support pieces needed to keep that boundary bounded:
timeout/deadline variants of the read helpers, a timeout-aware command
finisher, and cleanup that escalates if the helper does not exit promptly.
Testing: https://github.com/siddhpant/git/actions/runs/26107938855
Thanks,
Siddh
Siddh Raman Pant (9):
Documentation/git-range-diff: add missing notes options in synopsis
notes: convert raw arg in format_display_notes() to bool
wrapper: add sleep_nanosec
run-command: add support for timeout in command finisher
wrapper: add support for timeout and deadline in read helpers
t3301: cover generic displayed notes behavior
notes: support an external command to display notes
Documentation: document external notes command options
t: add tests for external notes command
Documentation/config/notes.adoc | 57 +++
Documentation/git-format-patch.adoc | 11 +-
Documentation/git-range-diff.adoc | 8 +-
Documentation/pretty-options.adoc | 9 +
Makefile | 2 +
builtin/log.c | 17 +-
builtin/name-rev.c | 9 +-
builtin/range-diff.c | 2 +
contrib/completion/git-completion.bash | 4 +-
log-tree.c | 10 +-
meson.build | 1 +
notes-external.c | 330 ++++++++++++++
notes-external.h | 19 +
notes.c | 244 ++++++++---
notes.h | 32 +-
revision.c | 32 +-
run-command.c | 92 +++-
run-command.h | 13 +
strbuf.c | 26 +-
strbuf.h | 4 +
t/helper/meson.build | 1 +
t/helper/test-external-notes | 64 +++
t/helper/test-notes-external-config-reset.c | 20 +
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/lib-notes.sh | 19 +
t/t3206-range-diff.sh | 68 +++
t/t3301-notes.sh | 461 ++++++++++++++++++++
t/t6120-describe.sh | 17 +
wrapper.c | 188 +++++++-
wrapper.h | 24 +
31 files changed, 1702 insertions(+), 84 deletions(-)
create mode 100644 notes-external.c
create mode 100644 notes-external.h
create mode 100755 t/helper/test-external-notes
create mode 100644 t/helper/test-notes-external-config-reset.c
create mode 100644 t/lib-notes.sh
--
2.53.0
^ permalink raw reply
* [PATCH 1/9] Documentation/git-range-diff: add missing notes options in synopsis
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
Documentation/git-range-diff.adoc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 880557084533..5cc5e2ed5673 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -11,7 +11,7 @@ SYNOPSIS
git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
[--no-dual-color] [--creation-factor=<factor>]
[--left-only | --right-only] [--diff-merges=<format>]
- [--remerge-diff]
+ [--remerge-diff] [--no-notes | --notes[=<ref>]]
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
[[--] <path>...]
--
2.53.0
^ permalink raw reply related
* [PATCH 4/9] run-command: add support for timeout in command finisher
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
A called command may not respond to the initial signal and will get
stuck in finish_command() -> wait_or_whine().
So let's add timeout support into the finisher so that if a deadline
occurs, we can send a force-kill signal.
The force-kill signal is in the argument because a program may trap a
signal, so it is the responsibility of caller to pass the correct kill
signal.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
run-command.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++----
run-command.h | 13 ++++++++
2 files changed, 98 insertions(+), 7 deletions(-)
diff --git a/run-command.c b/run-command.c
index c146a56532a1..60b84610d1f0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -554,16 +554,63 @@ static inline void set_cloexec(int fd)
fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
}
-static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
+#define NS_IN_10MS 10000000ULL /* 10 ms = 10^-2 s = 10^(9-2) ns = 10^7 ns */
+
+/* If timeout_ns == 0, no timeout happens (the timeout path is not taken). */
+static int wait_or_whine_timeout(pid_t pid, const char *argv0, int in_signal,
+ uint64_t timeout_ns)
{
int status, code = -1;
pid_t waiting;
int failed_errno = 0;
+ int flags = timeout_ns ? WNOHANG : 0;
+ bool timed_out = false;
+ uint64_t deadline_ns = getnanotime() + timeout_ns;
+
+ while(1) {
+ uint64_t current_time_ns, remaining_ns;
+ waiting = waitpid(pid, &status, flags);
+
+ /* Retry if interrupted. */
+ if (waiting < 0 && errno == EINTR)
+ continue;
+
+ /* Break if exited. */
+ if (waiting)
+ break;
+
+ /* If no timeout is specified, retry till it exits. */
+ if (!timeout_ns)
+ continue;
- while ((waiting = waitpid(pid, &status, 0)) < 0 && errno == EINTR)
- ; /* nothing */
+ current_time_ns = getnanotime();
+
+ /* If we are past the deadline, set errno and break. */
+ if (deadline_ns <= current_time_ns) {
+ errno = ETIMEDOUT;
+ timed_out = true;
+ break;
+ }
+
+ /**
+ * Retry after a sleep(min(remaining, default_chunk)).
+ *
+ * We don't blindly sleep for the entire remaining time because
+ * the process can exit early.
+ *
+ * The subtraction of uint64_t is safe here since we have
+ * already established that deadline_ns > current_time_ns.
+ */
+ remaining_ns = deadline_ns - current_time_ns;
+ sleep_nanosec(remaining_ns < NS_IN_10MS ?
+ remaining_ns : NS_IN_10MS);
+ }
- if (waiting < 0) {
+ if (timed_out) {
+ failed_errno = errno;
+ if (!in_signal)
+ error_errno("waitpid for %s timed out", argv0);
+ } else if (waiting < 0) {
failed_errno = errno;
if (!in_signal)
error_errno("waitpid for %s failed", argv0);
@@ -587,13 +634,28 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
error("waitpid is confused (%s)", argv0);
}
- if (!in_signal)
+ /**
+ * Signal handlers use the cleanup list while reaping children, so only
+ * non-signal waiters (in_signal != 0) should update it.
+ *
+ * In case of a timeout, we keep the child registered since it is
+ * actually not reaped so removing would be wrong. It is the
+ * responsibility of the caller to detect the timeout and do cleanup,
+ * like sending a kill signal using this function without a timeout.
+ */
+ if (!in_signal && !timed_out)
clear_child_for_cleanup(pid);
errno = failed_errno;
return code;
}
+/* Non-timeout wrapper for compatibility. */
+static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
+{
+ return wait_or_whine_timeout(pid, argv0, in_signal, 0);
+}
+
static void trace_add_env(struct strbuf *dst, const char *const *deltaenv)
{
struct string_list envs = STRING_LIST_INIT_DUP;
@@ -989,15 +1051,31 @@ int start_command(struct child_process *cmd)
return 0;
}
-int finish_command(struct child_process *cmd)
+/* See comment in the header file for executive summary. */
+int finish_command_with_timeout(struct child_process *cmd, uint64_t timeout_ns,
+ int signal_on_timeout)
{
- int ret = wait_or_whine(cmd->pid, cmd->args.v[0], 0);
+ int ret = wait_or_whine_timeout(cmd->pid, cmd->args.v[0], 0,
+ timeout_ns);
+
+ if (timeout_ns && ret < 0 && errno == ETIMEDOUT) {
+ kill(cmd->pid, signal_on_timeout);
+ ret = wait_or_whine(cmd->pid, cmd->args.v[0], 0);
+ }
+
trace2_child_exit(cmd, ret);
child_process_clear(cmd);
invalidate_lstat_cache();
return ret;
}
+/* Non-timeout wrapper for compatibility. */
+int finish_command(struct child_process *cmd)
+{
+ return finish_command_with_timeout(cmd, 0, 0);
+}
+
+
int finish_command_in_signal(struct child_process *cmd)
{
int ret = wait_or_whine(cmd->pid, cmd->args.v[0], 1);
diff --git a/run-command.h b/run-command.h
index 8ca496d7bdeb..cb1c8ba4ec01 100644
--- a/run-command.h
+++ b/run-command.h
@@ -215,6 +215,19 @@ int start_command(struct child_process *);
*/
int finish_command(struct child_process *);
+/**
+ * Wait for the completion of a sub-process that was started with
+ * start_command(), but uptil a given timeout duration timeout_ns.
+ *
+ * If it has not exited after timeout_ns, signal_on_timeout is sent to the
+ * process. We don't enforce a timeout for the second wait after sending
+ * the signal (as the process cleanup needs to happen), so it will block there.
+ *
+ * If timeout_ns == 0, no timeout happens and signal_on_timeout is ignored.
+ */
+int finish_command_with_timeout(struct child_process *cmd, uint64_t timeout_ns,
+ int signal_on_timeout);
+
int finish_command_in_signal(struct child_process *);
/**
--
2.53.0
^ permalink raw reply related
* [PATCH 5/9] wrapper: add support for timeout and deadline in read helpers
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
Add read helpers which allow a caller to enforce a timeout per read,
and a deadline for the read in case multiple reads have to be done
under a common timeout.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
strbuf.c | 26 +++++++++-
strbuf.h | 4 ++
wrapper.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++----
wrapper.h | 23 +++++++++
4 files changed, 182 insertions(+), 10 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 3e04addc22fe..b3fc7c624aa2 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -749,13 +749,15 @@ int strbuf_getline_nul(struct strbuf *sb, FILE *fp)
return strbuf_getdelim(sb, fp, '\0');
}
-int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
+static int strbuf_getwholeline_fd_with(struct strbuf *sb, int fd, int term,
+ xread_cb_t xread_cb,
+ void *cb_data)
{
strbuf_reset(sb);
while (1) {
char ch;
- ssize_t len = xread(fd, &ch, 1);
+ ssize_t len = xread_cb(fd, &ch, 1, cb_data);
if (len <= 0)
return EOF;
strbuf_addch(sb, ch);
@@ -765,6 +767,26 @@ int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
return 0;
}
+int strbuf_getwholeline_fd_deadline(struct strbuf *sb, int fd, int term,
+ uint64_t deadline_ns)
+{
+ return strbuf_getwholeline_fd_with(sb, fd, term, xread_deadline_fn,
+ &deadline_ns);
+}
+
+int strbuf_getwholeline_fd_timeout(struct strbuf *sb, int fd, int term,
+ int timeout_ms)
+{
+ return strbuf_getwholeline_fd_with(sb, fd, term, xread_timeout_fn,
+ &timeout_ms);
+}
+
+/* Non-timeout version for compatibility. */
+int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
+{
+ return strbuf_getwholeline_fd_timeout(sb, fd, term, 0);
+}
+
ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
{
int fd;
diff --git a/strbuf.h b/strbuf.h
index 06e284f9cca4..f896da1277a6 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -535,6 +535,10 @@ int strbuf_appendwholeline(struct strbuf *sb, FILE *file, int term);
* descriptor.
*/
int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term);
+int strbuf_getwholeline_fd_timeout(struct strbuf *sb, int fd, int term,
+ int timeout_ms);
+int strbuf_getwholeline_fd_deadline(struct strbuf *sb, int fd, int term,
+ uint64_t deadline_ns);
/**
* Set the buffer to the path of the current working directory.
diff --git a/wrapper.c b/wrapper.c
index 1349255f1eb4..3e0d65724e47 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -9,6 +9,7 @@
#include "parse.h"
#include "gettext.h"
#include "strbuf.h"
+#include "trace.h"
#include "trace2.h"
#include <time.h>
@@ -221,28 +222,129 @@ static int handle_nonblock(int fd, short poll_events, int err)
return 1;
}
-/*
- * xread() is the same a read(), but it automatically restarts read()
- * operations with a recoverable error (EAGAIN and EINTR). xread()
+static int wait_for_fd(int fd, short poll_events, int timeout_ms)
+{
+ struct pollfd pfd;
+
+ if (timeout_ms < 0) {
+ /* Negative timeout makes no sense. */
+ errno = EINVAL;
+ return -1;
+ }
+
+ pfd.fd = fd;
+ pfd.events = poll_events;
+
+ while(1) {
+ int ret = poll(&pfd, 1, timeout_ms);
+
+ if (ret <= 0) {
+ /* Retry if interrupted. */
+ if (ret < 0 && errno == EINTR)
+ continue;
+
+ /* Set errno if timeout happened. */
+ if (ret == 0)
+ errno = ETIMEDOUT;
+
+ return -1;
+ }
+
+ /* Invalid FD passed. */
+ if (pfd.revents & POLLNVAL) {
+ errno = EBADF;
+ return -1;
+ }
+
+ /* Some error happened. */
+ if (pfd.revents & POLLERR) {
+ errno = EIO;
+ return -1;
+ }
+
+ /* HangUp => We are ready to consume output till EOF. */
+ if (pfd.revents & (poll_events | POLLHUP))
+ return 0;
+ }
+}
+
+/**
+ * xread_timeout() is the same as read(), but it automatically restarts read()
+ * operations with a recoverable error (EAGAIN and EINTR). xread_timeout()
* DOES NOT GUARANTEE that "len" bytes is read even if the data is available.
+ *
+ * Fails with ETIMEDOUT when no bytes become available within timeout_ms
+ * milliseconds. A zero timeout disables timeout handling, so reads can
+ * block until the file descriptor is readable. Negative timeouts are invalid.
*/
-ssize_t xread(int fd, void *buf, size_t len)
+ssize_t xread_timeout(int fd, void *buf, size_t len, int timeout_ms)
{
ssize_t nr;
+
if (len > MAX_IO_SIZE)
len = MAX_IO_SIZE;
+
while (1) {
+ if (timeout_ms && wait_for_fd(fd, POLLIN, timeout_ms))
+ return -1;
+
nr = read(fd, buf, len);
+
if (nr < 0) {
if (errno == EINTR)
continue;
- if (handle_nonblock(fd, POLLIN, errno))
- continue;
+
+ if (timeout_ms) {
+ if (errno == EAGAIN || errno == EWOULDBLOCK)
+ continue;
+ } else {
+ if (handle_nonblock(fd, POLLIN, errno))
+ continue;
+ }
}
+
return nr;
}
}
+/* Non-timeout version for compatibility. */
+ssize_t xread(int fd, void *buf, size_t len)
+{
+ return xread_timeout(fd, buf, len, 0);
+}
+
+static int remaining_timeout_ms(uint64_t deadline_ns)
+{
+ uint64_t now, remaining_ns;
+
+ if (!deadline_ns)
+ return 0;
+
+ now = getnanotime();
+ if (now >= deadline_ns) {
+ errno = ETIMEDOUT;
+ return -1;
+ }
+
+ remaining_ns = deadline_ns - now;
+ return (int)((remaining_ns + 999999ULL) / 1000000ULL);
+}
+
+/* (deadline_ns = 0) disables the deadline and short-circuits to xread(). */
+ssize_t xread_deadline(int fd, void *buf, size_t len, uint64_t deadline_ns)
+{
+ int timeout_ms;
+
+ if (deadline_ns == 0)
+ return xread(fd, buf, len);
+
+ timeout_ms = remaining_timeout_ms(deadline_ns);
+ if (timeout_ms < 0)
+ return -1;
+
+ return xread_timeout(fd, buf, len, timeout_ms);
+}
+
/*
* xwrite() is the same a write(), but it automatically restarts write()
* operations with a recoverable error (EAGAIN and EINTR). xwrite() DOES NOT
@@ -284,13 +386,15 @@ ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
}
}
-ssize_t read_in_full(int fd, void *buf, size_t count)
+static ssize_t read_in_full_with(int fd, void *buf, size_t count,
+ xread_cb_t xread_cb,
+ void *cb_data)
{
char *p = buf;
ssize_t total = 0;
while (count > 0) {
- ssize_t loaded = xread(fd, p, count);
+ ssize_t loaded = xread_cb(fd, p, count, cb_data);
if (loaded < 0)
return -1;
if (loaded == 0)
@@ -303,6 +407,25 @@ ssize_t read_in_full(int fd, void *buf, size_t count)
return total;
}
+ssize_t read_in_full_deadline(int fd, void *buf, size_t count,
+ uint64_t deadline_ns)
+{
+ return read_in_full_with(fd, buf, count, xread_deadline_fn,
+ &deadline_ns);
+}
+
+ssize_t read_in_full_timeout(int fd, void *buf, size_t count, int timeout_ms)
+{
+ return read_in_full_with(fd, buf, count, xread_timeout_fn,
+ &timeout_ms);
+}
+
+/* Non-timeout version for compatibility. */
+ssize_t read_in_full(int fd, void *buf, size_t count)
+{
+ return read_in_full_timeout(fd, buf, count, 0);
+}
+
ssize_t write_in_full(int fd, const void *buf, size_t count)
{
const char *p = buf;
diff --git a/wrapper.h b/wrapper.h
index c39992893a81..f8592599216a 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -15,6 +15,8 @@ const char *mmap_os_err(void);
void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
int xopen(const char *path, int flags, ...);
ssize_t xread(int fd, void *buf, size_t len);
+ssize_t xread_timeout(int fd, void *buf, size_t len, int timeout_ms);
+ssize_t xread_deadline(int fd, void *buf, size_t len, uint64_t deadline_ns);
ssize_t xwrite(int fd, const void *buf, size_t len);
ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
int xdup(int fd);
@@ -44,9 +46,30 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode);
int git_mkstemp_mode(char *pattern, int mode);
ssize_t read_in_full(int fd, void *buf, size_t count);
+ssize_t read_in_full_timeout(int fd, void *buf, size_t count, int timeout_ms);
+ssize_t read_in_full_deadline(int fd, void *buf, size_t count,
+ uint64_t deadline_ns);
ssize_t write_in_full(int fd, const void *buf, size_t count);
ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
+typedef ssize_t xread_cb_t(int fd, void *buf, size_t len, const void *cb_data);
+
+static inline ssize_t xread_timeout_fn(int fd, void *buf, size_t len,
+ const void *cb_data)
+{
+ const int *timeout_ms = cb_data;
+
+ return xread_timeout(fd, buf, len, *timeout_ms);
+}
+
+static inline ssize_t xread_deadline_fn(int fd, void *buf, size_t len,
+ const void *cb_data)
+{
+ const uint64_t *deadline_ns = cb_data;
+
+ return xread_deadline(fd, buf, len, *deadline_ns);
+}
+
static inline ssize_t write_str_in_full(int fd, const char *str)
{
return write_in_full(fd, str, strlen(str));
--
2.53.0
^ permalink raw reply related
* [PATCH 6/9] t3301: cover generic displayed notes behavior
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
Displayed notes already participate in common log behavior.
Add explicit coverage for raw notes formatting, --no-notes
suppression, explicit notes refs, and --grep matching before
teaching external notes to feed the same display path.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
t/t3301-notes.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index d6c50460d086..27439010dfbc 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -885,6 +885,30 @@ test_expect_success '--show-notes=ref accumulates' '
test_cmp expect-both-reversed actual
'
+test_expect_success 'displayed notes honor raw notes formatting' '
+ git show -s --format=%N >actual &&
+ test_grep "^order test$" actual &&
+ ! grep "Notes" actual
+'
+
+test_expect_success 'displayed notes are suppressed by --no-notes' '
+ git log --no-notes -1 >actual &&
+ test_cmp expect-not-other actual
+'
+
+test_expect_success 'explicit notes ref replaces default displayed notes' '
+ git log --notes=other -1 >actual &&
+ test_cmp expect-other actual
+'
+
+test_expect_success 'displayed notes are used for grep matching' '
+ commit=$(git rev-parse HEAD) &&
+ git log --grep="order test" -1 >actual &&
+ test_grep "^commit $commit$" actual &&
+ git log --no-notes --grep="order test" -1 >actual &&
+ test_must_be_empty actual
+'
+
test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
test_config core.notesRef refs/notes/other &&
echo "Note on a tree" >expect &&
--
2.53.0
^ 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