* [PATCH v6 4/4] history: re-edit a squash with every message
From: Harald Nordgren via GitGitGadget @ 2026-06-28 8:29 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2337.v6.git.git.1782635349.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
By default "git history squash" reuses the oldest commit's message.
When --reedit-message is given it only reopened that one message, so the
messages of the folded-in commits were lost.
Gather the messages of every commit in the range, oldest first, and use
them as the editor template when re-editing, mirroring how "git rebase
-i" presents a squash.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Documentation/git-history.adoc | 5 +--
builtin/history.c | 61 +++++++++++++++++++++++++++++++++-
t/t3455-history-squash.sh | 37 +++++++++++++++++++++
3 files changed, 100 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 123ad5d4bc..8d4398ab1b 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -114,8 +114,9 @@ arguments to linkgit:git-rev-list[1], so several arguments may be given,
for example `@~3.. ^topic` to additionally exclude what is already on
`topic`.
+
-The oldest commit's message and authorship are preserved by default,
-unless you specify `--reedit-message`. A merge commit inside the range is
+The oldest commit's message and authorship are preserved by default. With
+`--reedit-message`, an editor opens pre-filled with the messages of all the
+folded commits so you can combine them. A merge commit inside the range is
folded like any other, but the range must have a single base, so a range
that reaches more than one entry point (for example a side branch that
forked before the range and was later merged into it) is rejected.
diff --git a/builtin/history.c b/builtin/history.c
index 5a1b42c063..1c31ea9118 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -1097,6 +1097,56 @@ static int find_interior_ref(const struct reference *ref, void *cb_data)
return 0;
}
+static int build_squash_message(struct repository *repo,
+ struct commit *base,
+ struct commit *tip,
+ struct strbuf *out)
+{
+ struct rev_info revs;
+ struct commit *commit;
+ struct strvec args = STRVEC_INIT;
+ int n = 0, ret;
+
+ repo_init_revisions(repo, &revs, NULL);
+ strvec_push(&args, "ignored");
+ strvec_push(&args, "--reverse");
+ strvec_push(&args, "--topo-order");
+ strvec_pushf(&args, "%s..%s", oid_to_hex(&base->object.oid),
+ oid_to_hex(&tip->object.oid));
+ setup_revisions_from_strvec(&args, &revs, NULL);
+
+ if (prepare_revision_walk(&revs) < 0) {
+ ret = error(_("error preparing revisions"));
+ goto out;
+ }
+
+ while ((commit = get_revision(&revs))) {
+ const char *message, *body;
+ struct strbuf one = STRBUF_INIT;
+
+ message = repo_logmsg_reencode(repo, commit, NULL, NULL);
+ find_commit_subject(message, &body);
+ strbuf_addstr(&one, body);
+ strbuf_trim_trailing_newline(&one);
+
+ if (n++)
+ strbuf_addch(out, '\n');
+ strbuf_addbuf(out, &one);
+ strbuf_addch(out, '\n');
+
+ strbuf_release(&one);
+ repo_unuse_commit_buffer(repo, commit, message);
+ }
+
+ ret = 0;
+
+out:
+ reset_revision_walk();
+ release_revisions(&revs);
+ strvec_clear(&args);
+ return ret;
+}
+
static int cmd_history_squash(int argc,
const char **argv,
const char *prefix,
@@ -1121,6 +1171,7 @@ static int cmd_history_squash(int argc,
OPT_END(),
};
struct strbuf reflog_msg = STRBUF_INIT;
+ struct strbuf message = STRBUF_INIT;
struct oidset interior = OIDSET_INIT;
struct commit *base, *oldest, *tip, *rewritten;
const struct object_id *base_tree_oid, *tip_tree_oid;
@@ -1160,6 +1211,12 @@ static int cmd_history_squash(int argc,
}
}
+ if (flags & COMMIT_TREE_EDIT_MESSAGE) {
+ ret = build_squash_message(repo, base, tip, &message);
+ if (ret < 0)
+ goto out;
+ }
+
ret = setup_revwalk(repo, action, tip, &revs);
if (ret < 0)
goto out;
@@ -1168,7 +1225,8 @@ static int cmd_history_squash(int argc,
tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid;
commit_list_append(base, &parents);
- ret = commit_tree_ext(repo, "squash", oldest, NULL, parents,
+ ret = commit_tree_ext(repo, "squash", oldest,
+ message.len ? message.buf : NULL, parents,
base_tree_oid, tip_tree_oid, &rewritten, flags);
if (ret < 0) {
ret = error(_("failed writing squashed commit"));
@@ -1189,6 +1247,7 @@ static int cmd_history_squash(int argc,
out:
strbuf_release(&reflog_msg);
+ strbuf_release(&message);
oidset_clear(&interior);
commit_list_free(parents);
release_revisions(&revs);
diff --git a/t/t3455-history-squash.sh b/t/t3455-history-squash.sh
index 94ee54eb24..5ef6768826 100755
--- a/t/t3455-history-squash.sh
+++ b/t/t3455-history-squash.sh
@@ -164,6 +164,43 @@ test_expect_success 'preserves authorship of the oldest commit' '
test_cmp expect actual
'
+test_expect_success '--reedit-message offers every folded-in message' '
+ git reset --hard start &&
+ echo b >file &&
+ git add file &&
+ git commit -m "re-one subject" -m "re-one body line" &&
+ test_commit --no-tag re-two file c &&
+ test_commit re-three file d &&
+
+ write_script editor <<-\EOF &&
+ cp "$1" buffer &&
+ echo combined >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ git history squash --reedit-message start.. &&
+
+ test_grep "re-one subject" buffer &&
+ test_grep "re-one body line" buffer &&
+ test_grep re-two buffer &&
+ test_grep re-three buffer &&
+ git log --format="%s" -1 >actual &&
+ echo combined >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '--reedit-message aborts on an empty message' '
+ git reset --hard three &&
+ head_before=$(git rev-parse HEAD) &&
+
+ write_script editor <<-\EOF &&
+ >"$1"
+ EOF
+ test_set_editor "$(pwd)/editor" &&
+ test_must_fail git history squash --reedit-message start.. &&
+
+ test_cmp_rev "$head_before" HEAD
+'
+
test_expect_success '--dry-run predicts the rewrite without performing it' '
git reset --hard three &&
head_before=$(git rev-parse HEAD) &&
--
gitgitgadget
^ permalink raw reply related
* Re: git-diff in a worktree is an order of magnitude slower?
From: Jeff King @ 2026-06-28 8:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: D. Ben Knoble, Git
In-Reply-To: <xmqqik7a4vhp.fsf@gitster.g>
On Mon, Jun 22, 2026 at 05:20:34AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Yes, though that implies comparing the index and file mtimes with
> > nanosecond precision. We have that precision stored (at least
> > when the system supports it) but I'm not sure if that comparison would
> > run afoul of the reasons USE_NSEC was not the default in the first
> > place.
> >
> > I guess not? The problem there is that the nanosecond portion would
> > sometimes get wiped if the entry was dropped from the kernel's in-memory
> > cache. And then stat-matching would not work. But if we are talking
> > about strictly asking "is this mtime later than that mtime", then I
> > think the worst case is that we fall back to the current behavior.
>
> Right, and you are right to point out that for the purpose of
> comparing mtimes of files' and the index file, this would make it
> unworkable. I can imagine that a file and the index may have been
> written within the same millisecond but we can tell that the former
> slightly earlier than the latter (or the other way around) with
> nanoseconds resolution, then only one of the two lose the sub millisecond
> resolution but not the other due to its in-core inode evicted out of
> the cache. Depending on which one survives (and keeps a non-zero
> sub millisecond part), they can compare differently.
Hmm, yeah. I was thinking there might be some mitigating factor because
we're comparing stat information that is stored in the index, and not
against a fresh stat() call. But that's not true.
We are using stat() information stored in the index from a file that was
written in the same second as that index (otherwise we do not care about
nanoseconds at all). But the index does not store its own mtime. At the
time of reading, we will fstat() it fresh, so we may see the truncated
mtime value.
In that case we'd always see the index as older than it really is. Which
I think does fail in the favorable direction for us (we assume the
too-new file is possibly racy and err on the side of caution).
But I don't think it rules out seeing the truncation in the other
direction. The original index write would have to be done in the same
second that the tracked file is written (because we only care about
nanoseconds when that is true). So it implies that the tracked file was
written, had its inode evicted, and then was re-read from disk all in
the same second that the index is being written, and the index inode
itself is never evicted. That seems unlikely but not impossible.
Anyway, it's all sufficiently scary that I think it should stay
conditional on USE_NSEC. I do suspect that USE_NSEC is safe at least on
Linux these days (see my response to Patrick elsewhere).
-Peff
^ permalink raw reply
* Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values
From: Junio C Hamano @ 2026-06-28 8:40 UTC (permalink / raw)
To: Tian Yuchen
Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <eabb8169-2c13-4961-9b21-f44b1fa66f70@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
>> Wouldn't we rather want to try to be more strict and say
>>
>> if (!repo || !repo->initialized)
>> BUG("repo must be an initialied repository");
>>
>> here? Aren't all the callers of this function supposed to be
>> dealing with an already initialized repository?
>
> That makes sense, but from my point of view...
>
> 'repo_config_values()' already has a check for 'repo->initialized'. If
> we're absolutely certain that the 'repo' is initialized, wouldn't it be
> better to simply remove all the checks inside the getter and leave the
> judgment to 'repo_config_values()'?
Yes, that was what I was getting at ;-).
^ permalink raw reply
* Re: [PATCH] meson: wire up USE_NSEC build knob
From: Jeff King @ 2026-06-28 8:48 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: D. Ben Knoble, git, brian m . carlson, Junio C Hamano,
Ramsay Jones
In-Reply-To: <20260628081806.GA3594700@coredump.intra.peff.net>
On Sun, Jun 28, 2026 at 04:18:07AM -0400, Jeff King wrote:
> I tried with a vfat mount, and it also works: we don't have nanoseconds
> either before or after. That makes sense, and implies that modern Linux
> will always be OK (because it limits the cached VFS response to what the
> underlying filesystem can handle).
>
> So...maybe this is just a non-issue these days, at least on Linux?
Oh, I also ran across this old thread:
https://public-inbox.org/git/5605D88A.20104%40gmail.com/
that implies similar:
* In-core file times may not be properly rounded to on-disk
precision, causing spurious file time changes when the cache is
refreshed from disk. This was fixed for typical Unix file systems
in kernel 2.6.11. The fix for CEPH, CIFS, NTFS, UFS and FUSE will
be in kernel 4.3. There's no fix for FAT-based file systems yet.
I also tested with CIFS on my system and it is fine. It looks like FAT
systems were fixed since 2015. ;)
But there is another interesting question raised there, which is how
different implementations may interact (e.g., two versions of Git
without and without USE_NSEC, or JGit which may have to use
millisecond-resolution APIs, etc). It should all work correctly as long
as each implementation consistently uses its own resolution (so JGit
would have to compare in millisecond-space and treat ties as racy). And
I think that is _probably_ what is happening now, since we already store
nanoseconds unconditionally (and only use them with USE_NSEC).
Though the opposite case is a performance problem but not a correctness
one: if JGit writes out an index with milliseconds and USE_NSEC Git
tries to read it, we will consider everything stat-dirty and re-read the
contents.
I don't know if these would be a problem in practice or not, but it's an
interesting potential gotcha. And one that nobody may have noticed,
because probably hardly anybody bothers to build with USE_NSEC now.
-Peff
^ permalink raw reply
* [PATCH] reftable: fix unlikely leak on API error
From: Jeff King @ 2026-06-28 9:03 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
If the reftable writer sees a bogus block size, we return with
REFTABLE_API_ERROR, leaking the reftable_writer struct we previously
allocated. Originally this case was a BUG(), but it became a regular
return in 445f9f4f35 (reftable: stop using `BUG()` in trivial cases,
2025-02-18).
We could obviously fix it by calling "reftable_free(wp)". But we can
observe that we never use the allocated "wp" until after we've validated
the input options. So let's just bump the allocation down. That fixes
the leak, and I think makes the flow of the function more logical
(we validate our inputs before doing any work).
Signed-off-by: Jeff King <peff@peff.net>
---
Noticed by Coverity as a "new" problem, though it has been there for
over a year. Presumably the nearby changes from 44f46f2be5 (reftable:
split up write options, 2026-06-25) confused it. There's a backlog of
hundreds of Coverity problems, most of which are garbage, so I tend to
only look at the ones it marks as new.
reftable/writer.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/reftable/writer.c b/reftable/writer.c
index 0133b64975..1bd4aa388b 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -152,16 +152,16 @@ int reftable_writer_new(struct reftable_writer **out,
struct reftable_write_options opts = {0};
struct reftable_writer *wp;
- wp = reftable_calloc(1, sizeof(*wp));
- if (!wp)
- return REFTABLE_OUT_OF_MEMORY_ERROR;
-
if (_opts)
opts = *_opts;
options_set_defaults(&opts);
if (opts.block_size >= (1 << 24))
return REFTABLE_API_ERROR;
+ wp = reftable_calloc(1, sizeof(*wp));
+ if (!wp)
+ return REFTABLE_OUT_OF_MEMORY_ERROR;
+
reftable_buf_init(&wp->block_writer_data.last_key);
reftable_buf_init(&wp->last_key);
reftable_buf_init(&wp->scratch);
--
2.55.0.rc2.353.gf769b6597e
^ permalink raw reply related
* Re: [PATCH] reftable: fix unlikely leak on API error
From: Jeff King @ 2026-06-28 9:06 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt
In-Reply-To: <20260628090314.GA661068@coredump.intra.peff.net>
On Sun, Jun 28, 2026 at 05:03:14AM -0400, Jeff King wrote:
> Noticed by Coverity as a "new" problem, though it has been there for
> over a year. Presumably the nearby changes from 44f46f2be5 (reftable:
> split up write options, 2026-06-25) confused it. There's a backlog of
> hundreds of Coverity problems, most of which are garbage, so I tend to
> only look at the ones it marks as new.
This does conflict textually with 44f46f2be5, which adds a new line
nearby. Resolving like:
diff --cc reftable/writer.c
index f850e9d599,1bd4aa388b..d969a6a021
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@@ -161,9 -158,10 +157,13 @@@ int reftable_writer_new(struct reftable
if (opts.block_size >= (1 << 24))
return REFTABLE_API_ERROR;
+ if (!hash_id)
+ hash_id = REFTABLE_HASH_SHA1;
+
+ wp = reftable_calloc(1, sizeof(*wp));
+ if (!wp)
+ return REFTABLE_OUT_OF_MEMORY_ERROR;
+
reftable_buf_init(&wp->block_writer_data.last_key);
reftable_buf_init(&wp->last_key);
reftable_buf_init(&wp->scratch);
makes sense to me, as it keeps the hash_id setting with the "opts"
setup.
-Peff
^ permalink raw reply
* [GIT PULL] l10n updates for Git 2.55.0
From: Jiang Xin @ 2026-06-28 11:35 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Git List, Aindriú Mac Giolla Eoin,
Alexander Shopov, Arkadii Yakovets, Bagas Sanjaya,
Dimitriy Ryazantcev, Emir SARI, Emir SARI, Jean-Noël Avila,
lilydjwg, Lumynous, Matteo Beniamino, Mikel Forcada,
Mikel Forcada, Peter Krefting, Ralf Thielow,
Vũ Tiến Hưng, Yi-Jyun Pan
Hi Junio,
Please pull the following l10n updates for Git 2.55.0.
The following changes since commit 6c3d7b73556db708feb3b16232fab1efc4353428:
Merge branch 'ps/t4216-tap-fix' (2026-06-25 19:49:01 -0700)
are available in the Git repository at:
git@github.com:git-l10n/git-po.git tags/l10n-2.55.0-v1
for you to fetch changes up to 08621c32d5536babd139ab1a9086349b3672edd6:
Merge branch '2.55-uk-pr' of github.com:arkid15r/git-ukrainian-l10n (2026-06-28 19:25:08 +0800)
----------------------------------------------------------------
l10n-2.55.0-v1
-----BEGIN PGP SIGNATURE-----
iQJPBAABCAA5FiEE37vMEzKDqYvVxs51k24VDd1FMtUFAmpBBXgbFIAAAAAABAAO
bWFudTIsMi41KzEuMTIsMCwzAAoJEJNuFQ3dRTLVKN8QAJfOm5oM21vsH6ONo4QO
u89j0ynacQI0rvQNLa9yGrR7vZumPQQKETwBulWTZLpQ0BrWev69LwpTZFVjQBp0
JxXvUW5FiHKQx+tSPT2SeNkMR3eHWxEFcyCP3QMnAbV5GFRRTnOr9ajRVI/b3Fi3
5xP2dgQW0F8oiuMAX/6osqwjjqO8qAAxtnX/+ecw2KPQ8ddgJsWPdAkWnAG7Ctnu
ff5jxOJ4ECAvsV5eywr2Ea1MOT032nsRX6Yaf6JIfdpU2oZzhCaVXgJwVkjKn/8c
mdEvaEZGSXPn969PEV3bsUEUEV7kgBt5wdWHRX5bGjwfXv/MKVnXZz/OQjlIa/pP
Pzpbi8jRSVCNXBi1kOpqVIOP3yndrYPh103juue5LEwyxsctmmvOtvB2t8Q3pY6G
/baSLDKp8GGi8x0h61D+lGAToK05YWafx95pZlxuwdd27ShJWaE6cknMkirJl9yD
IkihgCWxrCZmRX32YstZsV3FrvsT1GUwLEXm31IHfFRLp+76oVympgQh1r2qttCo
zMRPdo8ECZ9jwZ7OeCjshmAzgGzhuiwURZnYaPG/BC9wKqPDRmaUp8oKSjYreNgy
IuBUt/bOJO/y8ut6KZvMhpbJnOlULrOnHWDWqm2IfY+7+PLSD6uoodtbWf7ISUeP
k4eqg5pJX/q0t+uo8I0Z9Xqb
=M5pa
-----END PGP SIGNATURE-----
----------------------------------------------------------------
Aindriú Mac Giolla Eoin (1):
l10n: ga.po: update for Git 2.55
Alexander Shopov (1):
l10n: bg.po: Updated Bulgarian translation (6322t)
Arkadii Yakovets (1):
l10n: uk: add 2.55 translation
Bagas Sanjaya (1):
l10n: po-id for 2.55
Emir SARI (1):
l10n: tr: Update Turkish translations
Jean-Noël Avila (2):
l10n: fr: version 2.55
l10n: fr: mass fix of typos
Jiang Xin (12):
l10n: AGENTS.md: add quotation mark preservation guidelines
Merge branch 'master' of github.com:mbeniamino/git-po
Merge branch 'master' of github.com:nafmo/git-l10n-sv
Merge branch 'fr_v2.55' of github.com:jnavila/git
Merge branch 'master' of github.com:alshopov/git-po
Merge branch 'po-id' of github.com:bagasme/git-po
Merge branch 'tr-l10n' of github.com:bitigchi/git-po
Merge branch 'zh_CN-2.55' of github.com:lilydjwg/git-po
Merge branch 'ca-20260624-b' of github.com:Softcatala/git-po
Merge branch 'l10n/zh-TW/2026-06-26' of github.com:l10n-tw/git-po
Merge branch 'l10n-ga-2.55' of github.com:aindriu80/git-po
Merge branch '2.55-uk-pr' of github.com:arkid15r/git-ukrainian-l10n
Lumynous (1):
l10n: zh-TW.po: Update Chinese (Traditional) translation
Matteo Beniamino (1):
l10n: it: fix italian usage messages alignment
Mikel Forcada (1):
l10n: ca.po: update Catalan translation
Peter Krefting (1):
l10n: sv.po: Update Swedish translation
lilydjwg (2):
l10n: TEAMS: change Simplified Chinese team leader
l10n: zh_CN: updated translation for 2.55
po/AGENTS.md | 51 +-
po/TEAMS | 6 +-
po/bg.po | 796 ++++++++++++++++----
po/ca.po | 1421 +++++++++++++++++++++++-----------
po/fr.po | 836 ++++++++++++++------
po/ga.po | 864 ++++++++++++++++-----
po/id.po | 2322 ++++++++++++++++++++++++++++++++++++++++++--------------
po/it.po | 2 +-
po/sv.po | 784 ++++++++++++++-----
po/tr.po | 752 +++++++++++++-----
po/uk.po | 2384 +++++++++++++++++++++++++++++++++++++++++++---------------
po/zh_CN.po | 1034 ++++++++++++++++++-------
po/zh_TW.po | 2261 +++++++++++++++++++++++++++++++------------------------
13 files changed, 9725 insertions(+), 3788 deletions(-)
--
Jiang Xin
^ permalink raw reply
* Re: [PATCH v5 0/3] Teach git-replay(1) to linearize merge commits
From: Johannes Schindelin @ 2026-06-28 12:20 UTC (permalink / raw)
To: Toon Claes; +Cc: git, Elijah Newren
In-Reply-To: <20260626-toon-git-replay-drop-merges-v5-0-5e120738b9d0@iotcl.com>
Hi Toon,
On Fri, 26 Jun 2026, Toon Claes wrote:
> - (BIGGEST CHANGE) When working on a refactor to undo the enum->bool
> patch, I extended the code comments to explain how things work. This
> made me realize the use of the "replayed_base" was incorrect when
> multiple branches are rebased with --onto. This is fixed now and a
> test is added for this scenario.
I am not quite certain that this results in the desired outcome when
working with a single branch that contains a merge commit. Take for
example this topology (master~2..master at the time of writing):
* 6c3d7b73556d Merge branch 'ps/t4216-tap-fix'
|\
| * f0411a4c717e t4216: fix no-op test that breaks TAP output
* | ab776a62a785 Git 2.55-rc2
o | 1ea786d14a1b Merge branch 'hn/macos-linker-warning'
/
o 08b6ae38c602 t4216: test changed path filters with high bit paths
Running `git replay --linearize --onto master~2 master~2..master` used to
result in this:
* 3ec7cc3e73c0 t4216: fix no-op test that breaks TAP output
* 8dca9f98dc05 Git 2.55-rc2
o 1ea786d14a1b Merge branch 'hn/macos-linker-warning'
which is what I would expect. But now, due to the dropped `replayed_base`,
that tip commit is replayed directly on top of `onto` and the first
replayed commit ("Git 2.55-rc2") is simply (and inadvertently) dropped:
* 5e4899a3e03c t4216: fix no-op test that breaks TAP output
o 1ea786d14a1b Merge branch 'hn/macos-linker-warning'
I had originally introduced that `replayed_base` specifically to prevent
this commit-dropping.
As to the question what should happen if multiple branches are replayed at
the same time with `--linearize`: This is a very tricky problem. Naively,
one would want all of those branches to be linearized _individually_. But
that idea breaks down when you replay three branches, two of them with
distinct commits, and the third branch a merge of the first two:
* Branch C: merge branches A and B
|\
| * Branch B
* | Branch A
|/
o onto
What should the replayed branch C look like? Should it have A' and B' in
that order? I.e. share the rewritten commit with the replayed branch A?
But then B' could not be the replayed B because that needs to be directly
on top of onto.
So I fear that the `replayed_base` design _is_ needed, and the only way
`git replay --linearize` can work with multiple branches is by linearizing
all of the replayed commits into one single, linear commit topology.
Obviously, there are ways one could _try_ to rescue the previous idea, so
that at least replaying just branches A and B would keep the replayed
commits non-reachable from each other, but I strongly suspect that any
such design will invariably surprise users in nasty ways when the logic
has to fall back to the simple idea I outlined anyway.
Ciao,
Johannes
^ permalink raw reply
* [PATCH v4 0/8] commit-reach: terminate merge-base walk when one side is exhausted
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson
In-Reply-To: <pull.2149.v3.git.1782479286.gitgitgadget@gmail.com>
commit-reach: terminate merge-base walk when one paint side is exhausted
Optimize paint_down_to_common() for merge-base queries that hit large
one-sided histories.
When the walk from one side reaches a commit with a very low generation
number that the other side never paints, the walk is forced to drain most of
the graph. A common trigger is a repository import that grafts a separate
history with its own root, but any merge that introduces a low-generation
commit never painted by the other side has the same effect.
A new merge-base candidate can only be discovered when exclusive PARENT1 and
PARENT2 paint meet. This series teaches paint_down_to_common() to stop as
soon as one side has no exclusive commits left in the queue; once one side
is exhausted, no further candidates can appear.
origin/HEAD o o PR HEAD
| |
(import) o :
/ \ /
| o merge-base
| |
: : (~2.5M commits)
| |
import root main root
In the RFC thread [1], Derrick Stolee provided a criss-cross counterexample
that sharpened the halt condition, and Elijah Newren independently
discovered the same optimization and shared an implementation in PR #2150
[2]. Patches 2-3 incorporate test cases from Elijah's branch.
This series implements the optimization only after the walk enters the
finite-generation region, where generation ordering guarantees that paint on
visited commits is final.
Patch layout:
1/8 Documentation/technical: add paint-down-to-common doc 2/8 t6600: add
test cases for side-exhaustion edge cases 3/8 t6099, t6600: add
side-exhaustion regression tests 4/8 commit-reach: add trace2
instrumentation to paint_down_to_common() 5/8 commit-reach: introduce struct
paint_state with per-side counters 6/8 commit-reach: remove unused
nonstale_queue dedup wrappers 7/8 commit-reach: terminate merge-base walk
when one paint side is exhausted 8/8 commit-reach: move min_generation check
into paint_queue_get()
Benchmarks
Step counts are deterministic (measured via trace2_data_intmax added in
patch 4). Wall-clock times are best-of-11 runs.
2.6M-commit monorepo with commit-graph:
steps wall-clock
merge-base --all (across import) 2143438 -> 3 3.67s -> 5ms
merge-base --all (1000 apart) 2692915 -> 1035 4.41s -> 7ms
merge-base --all (5000 apart) 2692915 -> 6401 4.45s -> 13ms
merge-base --all (HEAD vs import) 2698872 -> 45960 4.50s -> 79ms
merge-tree (across import) 2143438 -> 3 4.42s -> 11ms
git.git (88k commits, commit-graph):
steps wall-clock
merge-base --all v2.0.0 v2.55.0-rc1 72264 -> 44589 110ms -> 68ms
merge-base --all HEAD HEAD~1000 9891 -> 3828 18ms -> 10ms
merge-base --all HEAD HEAD~10000 72303 -> 41487 101ms -> 50ms
Changes since v3:
* Fixed BUG assertion that was accidentally made unconditional in v3:
restored the min_generation guard so it only fires when generation-based
ordering is active.
* Moved generation cutoff and single-result termination conditions into the
documentation in patch 1/8, since they describe existing behavior.
* Renamed paint_state counter fields for clarity: p1_count ->
parent1_count, p2_count -> parent2_count, pending_merge_bases ->
mb_candidate_count. Changed counter types from int to size_t. (Suggested
by Rene Scharfe.)
Changes since v2:
* New patch 8/8: moved the min_generation termination check and the
last_gen monotonicity assertion into paint_queue_get(), consolidating
halt conditions. commit_graph_generation() is now called once per
dequeued commit and shared across all checks.
* Moved all halt conditions inside paint_queue_get() with the "pop first"
form: pop, check, then decrement counters. This keeps the optimization
commit's diff minimal (just inserting the new checks between pop and
decrement).
* Shortened the doc comment on paint_queue_get() to describe what it does
rather than how. Inline comments on each return NULL explain the specific
halt condition.
* Replaced the manual commit-graph setup in the step-count test with
run_all_modes, which now sets GIT_TRACE2_EVENT per mode and produces
trace-mode-{none,full,half,no-gdat}.txt files.
* Added a test_paint_down_steps helper for concise 4-mode step assertions
with diagnostic output on mismatch (prints "expected X, got Y" instead of
a silent grep failure).
* Added step-count assertions to the single-walk edge-case tests:
in_merge_bases_many:self, pending-stale, infinity-both-sides,
mixed-finite-infinity.
* Included step counts alongside wall-clock times in the benchmark tables.
Changes since v1:
* Reordered patches: documentation first (describing the existing
algorithm), tests before code changes, so they demonstrate passing with
old logic first.
* Dropped the ahead_behind decoupling patch. paint_state is now a NEW
struct alongside nonstale_queue instead of replacing it. ahead_behind()
is completely untouched.
* Removed nonstale_queue_put_dedup() and nonstale_queue_get_dedup() (dead
code after the conversion) in a separate commit.
* Renamed: struct paint_queue -> paint_state, field pq -> queue,
paint_count_add/remove -> paint_count_update (single function with signed
delta parameter).
* Split the old paint_count_transition (which handled both old and new
flags in one call) into separate remove/add calls with a signed delta.
This eliminates the need for the case 0 handler (which tracked "not in
the queue") and allows an exhaustive switch on (PARENT1 | PARENT2 |
STALE) that documents all valid flag combinations, with BUG() in default.
* Added trace2_data_intmax() instrumentation to report the number of
commits visited per paint walk (separate commit), with deterministic
step-count assertions in t6600.
* Expanded switch statements to multi-line format per .clang-format.
* Used !count style throughout instead of count == 0.
* Updated technical documentation alongside code changes.
[1]
https://lore.kernel.org/git/CAL71e4Ps-2_0+uuZu43N9pFnXBemoAohPs_eyRJf8taXHJPAXQ@mail.gmail.com/T/#u
[2] https://github.com/gitgitgadget/git/pull/2150
Elijah Newren (1):
t6600: add test cases for side-exhaustion edge cases
Kristofer Karlsson (7):
Documentation/technical: add paint-down-to-common doc
t6099, t6600: add side-exhaustion regression tests
commit-reach: add trace2 instrumentation to paint_down_to_common()
commit-reach: introduce struct paint_state with per-side counters
commit-reach: remove unused nonstale_queue dedup wrappers
commit-reach: terminate merge-base walk when one paint side is
exhausted
commit-reach: move min_generation check into paint_queue_get()
Documentation/Makefile | 1 +
Documentation/technical/meson.build | 1 +
.../technical/paint-down-to-common.adoc | 149 ++++++++++++++
commit-reach.c | 147 ++++++++++----
t/meson.build | 1 +
t/t6099-merge-base-side-exhaustion.sh | 82 ++++++++
t/t6600-test-reach.sh | 181 ++++++++++++++++--
7 files changed, 513 insertions(+), 49 deletions(-)
create mode 100644 Documentation/technical/paint-down-to-common.adoc
create mode 100755 t/t6099-merge-base-side-exhaustion.sh
base-commit: 6c3d7b73556db708feb3b16232fab1efc4353428
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2149%2Fspkrka%2Fside-exhaust-pr-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2149/spkrka/side-exhaust-pr-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/2149
Range-diff vs v3:
1: 2593866bce ! 1: 3efb095b03 Documentation/technical: add paint-down-to-common doc
@@ Documentation/technical/paint-down-to-common.adoc (new)
+ 1. The queue is empty.
+ 2. `max_nonstale` has been dequeued, meaning the queue only contains
+ STALE entries.
++ 3. Generation cutoff: the dequeued commit's generation is below
++ a caller-supplied `min_generation` threshold.
++ 4. Single result: the caller only needs one merge base, one has
++ been found, and the walk has entered the finite-generation
++ region.
+
+Stale entry condition
+~~~~~~~~~~~~~~~~~~~~~
@@ Documentation/technical/paint-down-to-common.adoc (new)
+`remove_redundant()` handles that as a post-processing step, so it
+is safe to exit early.
+
++Generation cutoff
++~~~~~~~~~~~~~~~~~
++Some callers (notably `remove_redundant()`) supply a `min_generation`
++threshold -- the minimum generation of the input commits. No merge
++base can have a generation below this threshold, so the walk
++terminates as soon as it dequeues such a commit.
++
++Single result
++~~~~~~~~~~~~~
++When only one merge base is needed and the walk is in the
++finite-generation region, the first candidate found is necessarily
++the highest-generation common ancestor. No remaining commit in the
++queue can be a descendant of this candidate (generation ordering
++guarantees children are visited first), so it cannot be redundant
++and the walk can stop immediately.
++
+Related documentation
+---------------------
+
2: 9efc084850 = 2: 1a0154b406 t6600: add test cases for side-exhaustion edge cases
3: 14b0d86b93 = 3: 017bf156c5 t6099, t6600: add side-exhaustion regression tests
4: 2592264cda = 4: df3b090a2b commit-reach: add trace2 instrumentation to paint_down_to_common()
5: e82e0c72b6 ! 5: fed9f2c368 commit-reach: introduce struct paint_state with per-side counters
@@ Documentation/technical/paint-down-to-common.adoc: re-enqueued is bounded by the
- 2. `max_nonstale` has been dequeued, meaning the queue only contains
- STALE entries.
+ 2. The queue contains only stale entries.
-
- Stale entry condition
- ~~~~~~~~~~~~~~~~~~~~~
+ 3. Generation cutoff: the dequeued commit's generation is below
+ a caller-supplied `min_generation` threshold.
+ 4. Single result: the caller only needs one merge base, one has
## commit-reach.c ##
@@ commit-reach.c: static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
@@ commit-reach.c: static struct commit *nonstale_queue_get_dedup(struct nonstale_q
+ */
+struct paint_state {
+ struct prio_queue queue;
-+ int p1_count;
-+ int p2_count;
-+ int pending_merge_bases;
++ size_t parent1_count;
++ size_t parent2_count;
++ size_t mb_candidate_count;
+};
+
+static void paint_count_update(struct paint_state *state,
@@ commit-reach.c: static struct commit *nonstale_queue_get_dedup(struct nonstale_q
+{
+ switch (flags & (PARENT1 | PARENT2 | STALE)) {
+ case PARENT1:
-+ state->p1_count += delta;
++ state->parent1_count += delta;
+ break;
+
+ case PARENT2:
-+ state->p2_count += delta;
++ state->parent2_count += delta;
+ break;
+
+ case PARENT1 | PARENT2:
-+ state->pending_merge_bases += delta;
++ state->mb_candidate_count += delta;
+ break;
+
+ case PARENT1 | PARENT2 | STALE:
@@ commit-reach.c: static struct commit *nonstale_queue_get_dedup(struct nonstale_q
+
+ commit->object.flags &= ~ENQUEUED;
+
-+ if (!state->p1_count && !state->p2_count &&
-+ !state->pending_merge_bases)
++ if (!state->parent1_count && !state->parent2_count &&
++ !state->mb_candidate_count)
+ return NULL;
+
+ paint_count_update(state, commit->object.flags, -1);
6: e6181bf3c1 = 6: 4db485b48a commit-reach: remove unused nonstale_queue dedup wrappers
7: f3572a8a89 ! 7: 4506780649 commit-reach: terminate merge-base walk when one paint side is exhausted
@@ Commit message
commit-reach: terminate merge-base walk when one paint side is exhausted
Add an early termination check to paint_down_to_common() using the
- per-side counters introduced earlier. Once the walk enters the
+ per-side counters introduced earlier. Once the walk enters the
finite-generation region, terminate early when one side's exclusive
count drops to zero -- no new merge-base can form without both paint
sides meeting.
@@ Commit message
The INFINITY gate ensures correctness: commits without a commit-graph
entry have GENERATION_NUMBER_INFINITY and are ordered by commit date,
- which is not topologically reliable. The optimization only fires
+ which is not topologically reliable. The optimization only fires
once the walk enters the finite-generation region where ordering
guarantees hold.
- Widen the existing generation-monotonicity BUG assertion to fire
- unconditionally, not only when min_generation is set. The
- side-exhaustion optimization depends on correct generation ordering,
- so the assertion should always be active.
-
Step counts measured with trace2 on git.git with commit-graph:
merge-base --all v2.0.0 v2.55.0-rc1:
@@ Commit message
## Documentation/technical/paint-down-to-common.adoc ##
@@ Documentation/technical/paint-down-to-common.adoc: ends when one of the following conditions holds:
-
- 1. The queue is empty.
- 2. The queue contains only stale entries.
-+ 3. Side exhaustion: no pure PARENT1 or pure PARENT2 commits
+ 4. Single result: the caller only needs one merge base, one has
+ been found, and the walk has entered the finite-generation
+ region.
++ 5. Side exhaustion: no pure PARENT1 or pure PARENT2 commits
+ remain in the queue, no pending merge-base candidates exist,
+ and the walk has entered the finite-generation region.
@@ Documentation/technical/paint-down-to-common.adoc: existing candidates by provin
+commit-date ordering can violate this guarantee, so the check is
+skipped.
+
- Related documentation
- ---------------------
-
+ Generation cutoff
+ ~~~~~~~~~~~~~~~~~
+ Some callers (notably `remove_redundant()`) supply a `min_generation`
## commit-reach.c ##
@@ commit-reach.c: static void paint_queue_put(struct paint_state *state,
@@ commit-reach.c: static struct commit *paint_queue_get(struct paint_state *state)
commit->object.flags &= ~ENQUEUED;
-- if (!state->p1_count && !state->p2_count &&
-- !state->pending_merge_bases)
+- if (!state->parent1_count && !state->parent2_count &&
+- !state->mb_candidate_count)
- return NULL;
-+ if (!state->pending_merge_bases) {
++ if (!state->mb_candidate_count) {
+ /* only stale entries remain */
-+ if (!state->p1_count && !state->p2_count)
++ if (!state->parent1_count && !state->parent2_count)
+ return NULL;
+
+ /* one side is exhausted */
-+ if ((!state->p1_count || !state->p2_count) &&
++ if ((!state->parent1_count || !state->parent2_count) &&
+ commit_graph_generation(commit) < GENERATION_NUMBER_INFINITY)
+ return NULL;
+ }
paint_count_update(state, commit->object.flags, -1);
return commit;
-@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
- timestamp_t generation = commit_graph_generation(commit);
- steps++;
-
-- if (min_generation && generation > last_gen)
-+ if (generation > last_gen)
- BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
- generation, last_gen,
- oid_to_hex(&commit->object.oid));
## t/t6600-test-reach.sh ##
@@ t/t6600-test-reach.sh: test_expect_success 'in_merge_bases_many:self' '
8: 4b9f192d98 ! 8: 8dd15d44e6 commit-reach: move min_generation check into paint_queue_get()
@@ Commit message
Move last_gen into struct paint_state so that
commit_graph_generation() is called exactly once per dequeued commit
and the result is shared across all termination checks and the
- monotonicity BUG assertion. The loop body in paint_down_to_common()
- reads state.last_gen instead of recomputing the generation number.
+ monotonicity BUG assertion.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
- ## Documentation/technical/paint-down-to-common.adoc ##
-@@ Documentation/technical/paint-down-to-common.adoc: ends when one of the following conditions holds:
- 3. Side exhaustion: no pure PARENT1 or pure PARENT2 commits
- remain in the queue, no pending merge-base candidates exist,
- and the walk has entered the finite-generation region.
-+ 4. Generation cutoff: the dequeued commit's generation is below
-+ a caller-supplied `min_generation` threshold.
-
- Stale entry condition
- ~~~~~~~~~~~~~~~~~~~~~
-@@ Documentation/technical/paint-down-to-common.adoc: time and an exhausted side cannot reappear. In the INFINITY region,
- commit-date ordering can violate this guarantee, so the check is
- skipped.
-
-+Generation cutoff
-+~~~~~~~~~~~~~~~~~
-+Some callers (notably `remove_redundant()`) supply a `min_generation`
-+threshold -- the minimum generation of the input commits. No merge
-+base can have a generation below this threshold, so the walk
-+terminates as soon as it dequeues such a commit.
-+
- Related documentation
- ---------------------
-
-
## commit-reach.c ##
@@ commit-reach.c: struct paint_state {
- int p1_count;
- int p2_count;
- int pending_merge_bases;
+ size_t parent1_count;
+ size_t parent2_count;
+ size_t mb_candidate_count;
+ timestamp_t min_generation;
+ timestamp_t last_gen;
};
@@ commit-reach.c: static void paint_queue_put(struct paint_state *state,
commit->object.flags &= ~ENQUEUED;
+ generation = commit_graph_generation(commit);
+
-+ if (generation > state->last_gen)
++ if (state->min_generation && generation > state->last_gen)
+ BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
+ generation, state->last_gen,
+ oid_to_hex(&commit->object.oid));
@@ commit-reach.c: static void paint_queue_put(struct paint_state *state,
+ if (generation < state->min_generation)
+ return NULL;
- if (!state->pending_merge_bases) {
+ if (!state->mb_candidate_count) {
/* only stale entries remain */
@@ commit-reach.c: static struct commit *paint_queue_get(struct paint_state *state)
/* one side is exhausted */
- if ((!state->p1_count || !state->p2_count) &&
+ if ((!state->parent1_count || !state->parent2_count) &&
- commit_graph_generation(commit) < GENERATION_NUMBER_INFINITY)
+ generation < GENERATION_NUMBER_INFINITY)
return NULL;
@@ commit-reach.c: static int paint_down_to_common(struct repository *r,
- timestamp_t generation = commit_graph_generation(commit);
steps++;
-- if (generation > last_gen)
+- if (min_generation && generation > last_gen)
- BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
- generation, last_gen,
- oid_to_hex(&commit->object.oid));
--
gitgitgadget
^ permalink raw reply
* [PATCH v4 1/8] Documentation/technical: add paint-down-to-common doc
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add a technical document describing the paint_down_to_common()
algorithm used for merge-base computation, covering the paint
walk, generation number regions, and termination conditions.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
Documentation/Makefile | 1 +
Documentation/technical/meson.build | 1 +
.../technical/paint-down-to-common.adoc | 135 ++++++++++++++++++
commit-reach.c | 6 +-
4 files changed, 142 insertions(+), 1 deletion(-)
create mode 100644 Documentation/technical/paint-down-to-common.adoc
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2699f0b24a..f8dea4b395 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -129,6 +129,7 @@ TECH_DOCS += technical/long-running-process-protocol
TECH_DOCS += technical/multi-pack-index
TECH_DOCS += technical/packfile-uri
TECH_DOCS += technical/pack-heuristics
+TECH_DOCS += technical/paint-down-to-common
TECH_DOCS += technical/parallel-checkout
TECH_DOCS += technical/partial-clone
TECH_DOCS += technical/platform-support
diff --git a/Documentation/technical/meson.build b/Documentation/technical/meson.build
index ec07088c57..9ce11d5e48 100644
--- a/Documentation/technical/meson.build
+++ b/Documentation/technical/meson.build
@@ -18,6 +18,7 @@ articles = [
'multi-pack-index.adoc',
'packfile-uri.adoc',
'pack-heuristics.adoc',
+ 'paint-down-to-common.adoc',
'parallel-checkout.adoc',
'partial-clone.adoc',
'platform-support.adoc',
diff --git a/Documentation/technical/paint-down-to-common.adoc b/Documentation/technical/paint-down-to-common.adoc
new file mode 100644
index 0000000000..a4dfcba038
--- /dev/null
+++ b/Documentation/technical/paint-down-to-common.adoc
@@ -0,0 +1,135 @@
+Merge-Base Computation and paint_down_to_common()
+==================================================
+
+The function `paint_down_to_common()` in `commit-reach.c` computes merge
+bases by walking the commit graph backwards from two sets of tips and
+finding where their ancestry meets.
+
+Use cases
+---------
+
+Computing merge bases is used in two different ways:
+
+ 1. *Finding all merge bases* (`merge-base --all`, `merge-tree`,
+ `merge`, `rebase`). A merge base is a common ancestor that is
+ not itself an ancestor of another common ancestor.
+
+ 2. *Ancestry checks* (`in_merge_bases`, used by `merge-base
+ --is-ancestor`, `branch -d`, `fetch`). These ask: "is commit A
+ an ancestor of commit B?" If a common ancestor equals one of the
+ inputs, that input is necessarily the only merge base -- no other
+ common ancestor can be both as recent and not an ancestor of it.
+
+Both use cases share the same algorithm and implementation.
+
+Algorithm
+---------
+
+Given a commit `one` and a set of commits `twos[]`, the walk paints
+commits with two colors:
+
+ - PARENT1: reachable from `one`
+ - PARENT2: reachable from any commit in `twos[]`
+
+The walk uses a priority queue ordered by generation number (falling
+back to commit date when generation numbers are unavailable). Each
+step dequeues the highest-priority commit (this is when we say a
+commit is "visited") and propagates its paint flags to its parents,
+enqueuing them if they gained new flags. When a commit receives
+both PARENT1 and PARENT2, it is a merge-base candidate. A candidate
+gains the STALE flag so its ancestors propagate staleness -- any
+deeper common ancestor is necessarily redundant.
+
+INFINITY and finite generation regions
+--------------------------------------
+
+The commit-graph stores a generation number for each commit. Commits
+not in the commit-graph have generation `GENERATION_NUMBER_INFINITY`. The
+graph is closed under reachability: if a commit is in the graph, all
+its ancestors are too. This partitions the commit graph into two regions:
+
+....
+ +---------------------------------------+
+ | INFINITY region |
+ | generation = INFINITY |
+ | queue order: heuristic (commit date) |
+ +---------------------------------------+
+ |
+ v
+ +---------------------------------------+
+ | Finite region |
+ | generation = finite |
+ | queue order: topological |
+ +---------------------------------------+
+....
+
+When the commit-graph is enabled, the INFINITY region is typically
+very small -- it only contains commits added since the last
+commit-graph refresh.
+
+All reachable INFINITY-generation commits are visited before any
+finite-generation commit, because INFINITY is larger than any finite
+value. Once the walk crosses into the finite region, it stays there.
+
+In the finite region, generation ordering guarantees topological
+traversal: children are always visited before their parents. This
+means that paint on already-visited commits is final -- no future
+traversal step can add paint to them.
+
+In the INFINITY region, commit-date ordering can violate this: a
+parent with a later date can be visited before a child with an earlier
+date. Paint flags are therefore NOT final at visit time, and a
+commit visited with only one side's paint may later gain the other.
+
+Paint flags are only added, never removed. Since each flag can be set
+at most once per commit, the number of times a commit can be
+re-enqueued is bounded by the number of flag transitions.
+
+Termination
+-----------
+
+The walk uses a `nonstale_queue` wrapper around `prio_queue` that
+tracks `max_nonstale`: the lowest-priority non-stale commit enqueued
+so far. Once that commit is dequeued, every remaining entry is known
+to be STALE and the loop terminates. Specifically, the main loop
+ends when one of the following conditions holds:
+
+ 1. The queue is empty.
+ 2. `max_nonstale` has been dequeued, meaning the queue only contains
+ STALE entries.
+ 3. Generation cutoff: the dequeued commit's generation is below
+ a caller-supplied `min_generation` threshold.
+ 4. Single result: the caller only needs one merge base, one has
+ been found, and the walk has entered the finite-generation
+ region.
+
+Stale entry condition
+~~~~~~~~~~~~~~~~~~~~~
+Once all queued entries are stale, no new merge-base candidates can
+be discovered -- that requires at least one non-stale commit from
+each side meeting. Continuing the walk could still invalidate
+existing candidates by proving one is an ancestor of another, but
+`remove_redundant()` handles that as a post-processing step, so it
+is safe to exit early.
+
+Generation cutoff
+~~~~~~~~~~~~~~~~~
+Some callers (notably `remove_redundant()`) supply a `min_generation`
+threshold -- the minimum generation of the input commits. No merge
+base can have a generation below this threshold, so the walk
+terminates as soon as it dequeues such a commit.
+
+Single result
+~~~~~~~~~~~~~
+When only one merge base is needed and the walk is in the
+finite-generation region, the first candidate found is necessarily
+the highest-generation common ancestor. No remaining commit in the
+queue can be a descendant of this candidate (generation ordering
+guarantees children are visited first), so it cannot be redundant
+and the walk can stop immediately.
+
+Related documentation
+---------------------
+
+ - `Documentation/technical/commit-graph.adoc` -- generation numbers
+ and the reachability closure property.
diff --git a/commit-reach.c b/commit-reach.c
index 5df471a313..a9483759e0 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -96,7 +96,11 @@ static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
return commit;
}
-/* all input commits in one and twos[] must have been parsed! */
+/*
+ * See Documentation/technical/paint-down-to-common.adoc
+ *
+ * All input commits in one and twos[] must have been parsed!
+ */
static int paint_down_to_common(struct repository *r,
struct commit *one, int n,
struct commit **twos,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 2/8] t6600: add test cases for side-exhaustion edge cases
From: Elijah Newren via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson, Elijah Newren
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
Add test cases to t6600-test-reach.sh that exercise edge cases in the
side-exhaustion optimization for paint_down_to_common():
- in_merge_bases_many:self: commit is both A and one of the X inputs
- get_merge_bases_many:duplicate-twos: duplicate entries in X list
- get_merge_bases_many:pending-stale: STALE transition on an
already-painted commit (ps-* diamond topology)
- get_merge_bases_many:infinity-both-sides: both tips outside the
commit-graph with non-monotonic dates (pi-* topology)
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
t/t6600-test-reach.sh | 111 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 111 insertions(+)
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index b5b314e570..c2e091aad1 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -49,6 +49,62 @@ test_expect_success 'setup' '
git tag -a -m "$x-$i" tag-$x-$i commit-$x-$i || return 1
done
done &&
+
+ # Build a small side topology to exercise the (PARENT1|PARENT2) ->
+ # (PARENT1|PARENT2|STALE) transition in paint_down_to_common(); the
+ # 10x10 grid above does not exercise it because no merge-base candidate
+ # there is a descendant of another, so STALE never reaches a
+ # still-pending candidate.
+ #
+ # ps-X
+ # /|\
+ # / | \
+ # ps-Z ps-B ps-W
+ # | / \ |
+ # | / \ |
+ # |/ \|
+ # ps-T1 ps-T2
+ #
+ # where ps-T1=merge(ps-Z,ps-B), ps-T2=merge(ps-W,ps-B), so
+ # merge-base(ps-T1,ps-T2) = ps-B. During the walk, ps-X transitions
+ # to (PARENT1|PARENT2) via ps-Z and ps-W before ps-B is dequeued;
+ # then the STALE-walk from ps-B transitions ps-X to
+ # (PARENT1|PARENT2|STALE).
+ git checkout --orphan ps-orphan &&
+ test_commit ps-X &&
+ git checkout -b ps-B-br ps-X && test_commit ps-B &&
+ git checkout -b ps-Z-br ps-X && test_commit ps-Z &&
+ git checkout -b ps-W-br ps-X && test_commit ps-W &&
+ git checkout -b ps-T1 ps-Z &&
+ git merge --no-ff -m ps-T1 ps-B &&
+ git checkout -b ps-T2 ps-W &&
+ git merge --no-ff -m ps-T2 ps-B &&
+
+ # Build a side topology that lives entirely outside the half
+ # commit-graph and has non-monotonic commit dates, to exercise the
+ # INFINITY-gate in paint_down_to_common. With both tips outside
+ # the graph, generation is INFINITY and the queue falls back to
+ # commit-date order, which here is non-monotonic.
+ #
+ # pi-X (date 500, PARENT1 tip) --> pi-P, pi-D
+ # pi-D (date 480) --> pi-C
+ # pi-C (date 200) --> pi-B
+ # pi-B (date 100, PARENT2 tip) --> pi-P
+ # pi-P (date 450, root)
+ #
+ # merge-base(pi-X, pi-B) = pi-B (it is an ancestor of pi-X and is
+ # itself one of the queried tips).
+ git checkout --orphan pi-orphan &&
+ test_commit --date "@450 +0000" pi-P &&
+ test_commit --date "@100 +0000" pi-B &&
+ test_commit --date "@200 +0000" pi-C &&
+ test_commit --date "@480 +0000" pi-D &&
+ GIT_AUTHOR_DATE="@500 +0000" GIT_COMMITTER_DATE="@500 +0000" \
+ git commit-tree -p pi-D -p pi-P -m pi-X pi-D^{tree} >pi-X-oid &&
+ pi_x="$(cat pi-X-oid)" &&
+ git branch -f pi-X-br "$pi_x" &&
+ git tag pi-X "$pi_x" &&
+
git commit-graph write --reachable &&
mv .git/objects/info/commit-graph commit-graph-full &&
chmod u+w commit-graph-full &&
@@ -146,6 +202,16 @@ test_expect_success 'in_merge_bases_many:miss-heuristic' '
test_all_modes in_merge_bases_many
'
+test_expect_success 'in_merge_bases_many:self' '
+ cat >input <<-\EOF &&
+ A:commit-6-8
+ X:commit-5-9
+ X:commit-6-8
+ EOF
+ echo "in_merge_bases_many(A,X):1" >expect &&
+ test_all_modes in_merge_bases_many
+'
+
test_expect_success 'is_descendant_of:hit' '
cat >input <<-\EOF &&
A:commit-5-7
@@ -183,6 +249,51 @@ test_expect_success 'get_merge_bases_many' '
test_all_modes get_merge_bases_many
'
+test_expect_success 'get_merge_bases_many:duplicate-twos' '
+ cat >input <<-\EOF &&
+ A:commit-5-7
+ X:commit-4-8
+ X:commit-4-8
+ X:commit-6-6
+ X:commit-6-6
+ X:commit-8-3
+ EOF
+ {
+ echo "get_merge_bases_many(A,X):" &&
+ git rev-parse commit-5-6 \
+ commit-4-7 | sort
+ } >expect &&
+ test_all_modes get_merge_bases_many
+'
+
+test_expect_success 'get_merge_bases_many:pending-stale' '
+ # Exercises the (PARENT1|PARENT2) -> (...|STALE) transition path in
+ # paint_down_to_common(). See the topology comment in the setup test.
+ cat >input <<-\EOF &&
+ A:ps-T1
+ X:ps-T2
+ EOF
+ {
+ echo "get_merge_bases_many(A,X):" &&
+ git rev-parse ps-B
+ } >expect &&
+ test_all_modes get_merge_bases_many
+'
+
+test_expect_success 'get_merge_bases_many:infinity-both-sides' '
+ # Exercises the push-time INFINITY-gate in paint_down_to_common(). See
+ # the pi-* topology comment in the setup test.
+ cat >input <<-\EOF &&
+ A:pi-X
+ X:pi-B
+ EOF
+ {
+ echo "get_merge_bases_many(A,X):" &&
+ git rev-parse pi-B
+ } >expect &&
+ test_all_modes get_merge_bases_many
+'
+
test_expect_success 'reduce_heads' '
cat >input <<-\EOF &&
X:commit-1-10
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 3/8] t6099, t6600: add side-exhaustion regression tests
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add t6099 to test the case where multiple merge-base candidates exist
and one is an ancestor of another. This exercises the side-exhaustion
optimization in paint_down_to_common together with the
remove_redundant safety net in get_merge_bases_many_0.
Add a mixed finite/INFINITY test to t6600 where one tip is outside
the commit-graph (INFINITY generation) and the other is inside.
This exercises the region transition: the walk starts in the
INFINITY region where side-exhaustion is disabled, then crosses
into the finite region where it can fire.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
t/meson.build | 1 +
t/t6099-merge-base-side-exhaustion.sh | 82 +++++++++++++++++++++++++++
t/t6600-test-reach.sh | 25 ++++++++
3 files changed, 108 insertions(+)
create mode 100755 t/t6099-merge-base-side-exhaustion.sh
diff --git a/t/meson.build b/t/meson.build
index 3219264fe7..ee6ebdffb9 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -786,6 +786,7 @@ integration_tests = [
't6041-bisect-submodule.sh',
't6050-replace.sh',
't6060-merge-index.sh',
+ 't6099-merge-base-side-exhaustion.sh',
't6100-rev-list-in-order.sh',
't6101-rev-parse-parents.sh',
't6102-rev-list-unexpected-objects.sh',
diff --git a/t/t6099-merge-base-side-exhaustion.sh b/t/t6099-merge-base-side-exhaustion.sh
new file mode 100755
index 0000000000..4f1e0d50ef
--- /dev/null
+++ b/t/t6099-merge-base-side-exhaustion.sh
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+test_description='merge-base with ancestor among merge-base candidates
+
+Test that merge-base --all correctly handles cases where
+multiple merge-base candidates exist and one is an ancestor
+of another. The side-exhaustion optimization in
+paint_down_to_common may exit before STALE propagation
+removes the ancestor, but remove_redundant catches it.
+
+Graph shape (parents are below children):
+
+ A ----------- X
+ |\ /|
+ | B---------/ |
+ | | |
+ e2 \ f2
+ | | |
+ e1 d1 f1
+ \ | /
+ \ | /
+ \| /
+ C
+
+A and X are the two tips.
+B and C are both reachable from A and X.
+B reaches C through d1.
+Only B should appear in merge-base --all output.
+'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
+
+test_expect_success 'setup ancestor merge-base candidate' '
+ test_commit C &&
+
+ git checkout -b d-chain HEAD &&
+ test_commit d1 &&
+ test_commit B &&
+
+ git checkout -b e-path C &&
+ test_commit e1 &&
+ test_commit e2 &&
+
+ git checkout -b f-path C &&
+ test_commit f1 &&
+ test_commit f2 &&
+
+ git checkout -b branch-A e-path &&
+ test_merge A B &&
+
+ git checkout -b branch-X f-path &&
+ test_merge X B &&
+
+ git commit-graph write --reachable
+'
+
+test_expect_success 'merge-base --all excludes ancestor candidate' '
+ git rev-parse B >expected &&
+ git merge-base --all A X >actual &&
+ test_cmp expected actual
+'
+
+test_expect_success 'merge-base (single) finds shallowest' '
+ git rev-parse B >expected &&
+ git merge-base A X >actual &&
+ test_cmp expected actual
+'
+
+# Without commit-graph: generation numbers are INFINITY,
+# side-exhaustion optimization does not fire.
+test_expect_success 'merge-base --all without commit-graph' '
+ rm -f .git/objects/info/commit-graph &&
+ git rev-parse B >expected &&
+ git merge-base --all A X >actual &&
+ test_cmp expected actual
+'
+
+test_done
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index c2e091aad1..4b771b4c58 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -294,6 +294,31 @@ test_expect_success 'get_merge_bases_many:infinity-both-sides' '
test_all_modes get_merge_bases_many
'
+test_expect_success 'setup mixed finite/INFINITY topology' '
+ # Create a commit outside all saved commit-graph files so it always
+ # has INFINITY generation, while its parent (ps-X) is in the graph
+ # with a finite generation. Use the ps-* orphan topology so we do
+ # not pollute the grid-based rev-list tests.
+ git checkout ps-X &&
+ test_env GIT_TEST_COMMIT_GRAPH= test_commit pm-INF
+'
+
+test_expect_success 'get_merge_bases_many:mixed-finite-infinity' '
+ # One tip (pm-INF) is outside the commit-graph with INFINITY
+ # generation; the other (ps-B) is in the graph with finite
+ # generation. The walk starts in the INFINITY region and crosses
+ # into the finite region where side-exhaustion can fire.
+ cat >input <<-\EOF &&
+ A:pm-INF
+ X:ps-B
+ EOF
+ {
+ echo "get_merge_bases_many(A,X):" &&
+ git rev-parse ps-X
+ } >expect &&
+ test_all_modes get_merge_bases_many
+'
+
test_expect_success 'reduce_heads' '
cat >input <<-\EOF &&
X:commit-1-10
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 4/8] commit-reach: add trace2 instrumentation to paint_down_to_common()
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add a step counter and trace2_data_intmax() call so that the number
of commits visited during the paint walk is observable via
GIT_TRACE2_EVENT. This provides a way to measure the impact of
future optimizations without relying on wall-clock benchmarks alone.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-reach.c | 5 ++++
t/t6600-test-reach.sh | 53 ++++++++++++++++++++++++++++++-------------
2 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index a9483759e0..f6a438550b 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -11,6 +11,7 @@
#include "tag.h"
#include "commit-reach.h"
#include "ewah/ewok.h"
+#include "trace2.h"
/* Remember to update object flag allocation in object.h */
#define PARENT1 (1u<<16)
@@ -112,6 +113,7 @@ static int paint_down_to_common(struct repository *r,
{ compare_commits_by_gen_then_commit_date }
};
int i;
+ int steps = 0;
timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
struct commit_list **tail = result;
@@ -135,6 +137,7 @@ static int paint_down_to_common(struct repository *r,
struct commit_list *parents;
int flags;
timestamp_t generation = commit_graph_generation(commit);
+ steps++;
if (min_generation && generation > last_gen)
BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
@@ -190,6 +193,8 @@ static int paint_down_to_common(struct repository *r,
}
clear_nonstale_queue(&queue);
+ trace2_data_intmax("paint_down_to_common", r,
+ "steps", steps);
commit_list_sort_by_date(result);
return 0;
}
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index 4b771b4c58..b3a31b80ac 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -118,24 +118,34 @@ test_expect_success 'setup' '
'
run_all_modes () {
- test_when_finished rm -rf .git/objects/info/commit-graph &&
- "$@" <input >actual &&
- test_cmp expect actual &&
- cp commit-graph-full .git/objects/info/commit-graph &&
- "$@" <input >actual &&
- test_cmp expect actual &&
- cp commit-graph-half .git/objects/info/commit-graph &&
- "$@" <input >actual &&
- test_cmp expect actual &&
- cp commit-graph-no-gdat .git/objects/info/commit-graph &&
- "$@" <input >actual &&
- test_cmp expect actual
+ graph=.git/objects/info/commit-graph &&
+ test_when_finished rm -rf "$graph" "${graph}s" &&
+ rm -f trace-mode-*.txt &&
+
+ for mode in none full half no-gdat
+ do
+ rm -rf "$graph" "${graph}s" &&
+ cp "commit-graph-${mode}" "$graph" 2>/dev/null ||
+ true &&
+ GIT_TRACE2_EVENT="$(pwd)/trace-mode-${mode}.txt" \
+ "$@" <input >actual &&
+ test_cmp expect actual || return 1
+ done
}
test_all_modes () {
run_all_modes test-tool reach "$@"
}
+test_paint_down_steps () {
+ for mode in none full half no-gdat
+ do
+ test_trace2_data paint_down_to_common steps "$1" \
+ <"trace-mode-${mode}.txt" || return 1
+ shift
+ done
+}
+
test_expect_success 'ref_newer:miss' '
cat >input <<-\EOF &&
A:commit-5-7
@@ -209,7 +219,8 @@ test_expect_success 'in_merge_bases_many:self' '
X:commit-6-8
EOF
echo "in_merge_bases_many(A,X):1" >expect &&
- test_all_modes in_merge_bases_many
+ test_all_modes in_merge_bases_many &&
+ test_paint_down_steps 45 2 25 3
'
test_expect_success 'is_descendant_of:hit' '
@@ -277,7 +288,8 @@ test_expect_success 'get_merge_bases_many:pending-stale' '
echo "get_merge_bases_many(A,X):" &&
git rev-parse ps-B
} >expect &&
- test_all_modes get_merge_bases_many
+ test_all_modes get_merge_bases_many &&
+ test_paint_down_steps 6 6 6 6
'
test_expect_success 'get_merge_bases_many:infinity-both-sides' '
@@ -291,7 +303,8 @@ test_expect_success 'get_merge_bases_many:infinity-both-sides' '
echo "get_merge_bases_many(A,X):" &&
git rev-parse pi-B
} >expect &&
- test_all_modes get_merge_bases_many
+ test_all_modes get_merge_bases_many &&
+ test_paint_down_steps 5 5 5 5
'
test_expect_success 'setup mixed finite/INFINITY topology' '
@@ -316,7 +329,15 @@ test_expect_success 'get_merge_bases_many:mixed-finite-infinity' '
echo "get_merge_bases_many(A,X):" &&
git rev-parse ps-X
} >expect &&
- test_all_modes get_merge_bases_many
+ test_all_modes get_merge_bases_many &&
+ test_paint_down_steps 3 3 3 3
+'
+
+test_expect_success 'merge-base --all commit-walk steps' '
+ >input &&
+ git rev-parse commit-9-1 >expect &&
+ run_all_modes git merge-base --all commit-9-9 commit-9-1 &&
+ test_paint_down_steps 81 80 81 81
'
test_expect_success 'reduce_heads' '
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 5/8] commit-reach: introduce struct paint_state with per-side counters
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add a paint_state struct for use by paint_down_to_common() that
wraps a prio_queue with per-side commit counters. Each non-stale
queued commit occupies exactly one counter bucket based on its
paint flags: PARENT1-only, PARENT2-only, or both sides (a pending
merge-base candidate).
The counters are maintained by paint_count_update() which adjusts
the appropriate bucket by a signed delta. An exhaustive switch on
the paint+stale bits documents all valid flag combinations in one
place.
Convert paint_down_to_common() to use paint_state. The loop now
drains the queue via paint_queue_get() which returns NULL when all
counters reach zero, replacing the old pointer-based termination
(max_nonstale). This is equivalent behavior -- both conditions
detect that no non-stale entries remain.
paint_queue_get() uses a "pop first" form: it dequeues a commit,
then checks the counters. This means the loop exits one iteration
earlier than the old code in some topologies (the popped stale
commit is never processed), so a few step counts drop by one.
The existing nonstale_queue is left in place for ahead_behind().
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
.../technical/paint-down-to-common.adoc | 9 +-
commit-reach.c | 94 ++++++++++++++++---
t/t6600-test-reach.sh | 4 +-
3 files changed, 85 insertions(+), 22 deletions(-)
diff --git a/Documentation/technical/paint-down-to-common.adoc b/Documentation/technical/paint-down-to-common.adoc
index a4dfcba038..ac3e2b39a5 100644
--- a/Documentation/technical/paint-down-to-common.adoc
+++ b/Documentation/technical/paint-down-to-common.adoc
@@ -88,15 +88,12 @@ re-enqueued is bounded by the number of flag transitions.
Termination
-----------
-The walk uses a `nonstale_queue` wrapper around `prio_queue` that
-tracks `max_nonstale`: the lowest-priority non-stale commit enqueued
-so far. Once that commit is dequeued, every remaining entry is known
-to be STALE and the loop terminates. Specifically, the main loop
+The walk tracks the number of commits of each type in the queue
+(PARENT1-only, PARENT2-only, pending merge-base). The main loop
ends when one of the following conditions holds:
1. The queue is empty.
- 2. `max_nonstale` has been dequeued, meaning the queue only contains
- STALE entries.
+ 2. The queue contains only stale entries.
3. Generation cutoff: the dequeued commit's generation is below
a caller-supplied `min_generation` threshold.
4. Single result: the caller only needs one merge base, one has
diff --git a/commit-reach.c b/commit-reach.c
index f6a438550b..9ae306f60c 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -97,6 +97,75 @@ static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
return commit;
}
+/*
+ * Priority queue with per-side commit counters for paint_down_to_common().
+ * Each non-stale queued commit occupies exactly one bucket: PARENT1-only,
+ * PARENT2-only, or both (a pending merge-base candidate).
+ */
+struct paint_state {
+ struct prio_queue queue;
+ size_t parent1_count;
+ size_t parent2_count;
+ size_t mb_candidate_count;
+};
+
+static void paint_count_update(struct paint_state *state,
+ unsigned flags, int delta)
+{
+ switch (flags & (PARENT1 | PARENT2 | STALE)) {
+ case PARENT1:
+ state->parent1_count += delta;
+ break;
+
+ case PARENT2:
+ state->parent2_count += delta;
+ break;
+
+ case PARENT1 | PARENT2:
+ state->mb_candidate_count += delta;
+ break;
+
+ case PARENT1 | PARENT2 | STALE:
+ break;
+
+ default:
+ BUG("unexpected paint state");
+ }
+}
+
+static void paint_queue_put(struct paint_state *state,
+ struct commit *c, unsigned add_flags)
+{
+ unsigned old_flags = c->object.flags;
+ c->object.flags |= add_flags;
+
+ if (old_flags & ENQUEUED) {
+ paint_count_update(state, old_flags, -1);
+ paint_count_update(state, c->object.flags, 1);
+ } else {
+ c->object.flags |= ENQUEUED;
+ prio_queue_put(&state->queue, c);
+ paint_count_update(state, c->object.flags, 1);
+ }
+}
+
+static struct commit *paint_queue_get(struct paint_state *state)
+{
+ struct commit *commit = prio_queue_get(&state->queue);
+
+ if (!commit)
+ return NULL;
+
+ commit->object.flags &= ~ENQUEUED;
+
+ if (!state->parent1_count && !state->parent2_count &&
+ !state->mb_candidate_count)
+ return NULL;
+
+ paint_count_update(state, commit->object.flags, -1);
+ return commit;
+}
+
/*
* See Documentation/technical/paint-down-to-common.adoc
*
@@ -109,31 +178,29 @@ static int paint_down_to_common(struct repository *r,
enum merge_base_flags mb_flags,
struct commit_list **result)
{
- struct nonstale_queue queue = {
- { compare_commits_by_gen_then_commit_date }
+ struct paint_state state = {
+ .queue = { compare_commits_by_gen_then_commit_date }
};
+ struct commit *commit;
int i;
int steps = 0;
timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
struct commit_list **tail = result;
if (!min_generation && !corrected_commit_dates_enabled(r))
- queue.pq.compare = compare_commits_by_commit_date;
+ state.queue.compare = compare_commits_by_commit_date;
one->object.flags |= PARENT1;
if (!n) {
commit_list_append(one, result);
return 0;
}
- nonstale_queue_put_dedup(&queue, one);
+ paint_queue_put(&state, one, 0);
- for (i = 0; i < n; i++) {
- twos[i]->object.flags |= PARENT2;
- nonstale_queue_put_dedup(&queue, twos[i]);
- }
+ for (i = 0; i < n; i++)
+ paint_queue_put(&state, twos[i], PARENT2);
- while (queue.max_nonstale) {
- struct commit *commit = nonstale_queue_get_dedup(&queue);
+ while ((commit = paint_queue_get(&state))) {
struct commit_list *parents;
int flags;
timestamp_t generation = commit_graph_generation(commit);
@@ -172,7 +239,7 @@ static int paint_down_to_common(struct repository *r,
if ((p->object.flags & flags) == flags)
continue;
if (repo_parse_commit(r, p)) {
- clear_nonstale_queue(&queue);
+ clear_prio_queue(&state.queue);
commit_list_free(*result);
*result = NULL;
/*
@@ -187,12 +254,11 @@ static int paint_down_to_common(struct repository *r,
return error(_("could not parse commit %s"),
oid_to_hex(&p->object.oid));
}
- p->object.flags |= flags;
- nonstale_queue_put_dedup(&queue, p);
+ paint_queue_put(&state, p, flags);
}
}
- clear_nonstale_queue(&queue);
+ clear_prio_queue(&state.queue);
trace2_data_intmax("paint_down_to_common", r,
"steps", steps);
commit_list_sort_by_date(result);
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index b3a31b80ac..51f3d70492 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -289,7 +289,7 @@ test_expect_success 'get_merge_bases_many:pending-stale' '
git rev-parse ps-B
} >expect &&
test_all_modes get_merge_bases_many &&
- test_paint_down_steps 6 6 6 6
+ test_paint_down_steps 5 5 5 5
'
test_expect_success 'get_merge_bases_many:infinity-both-sides' '
@@ -304,7 +304,7 @@ test_expect_success 'get_merge_bases_many:infinity-both-sides' '
git rev-parse pi-B
} >expect &&
test_all_modes get_merge_bases_many &&
- test_paint_down_steps 5 5 5 5
+ test_paint_down_steps 5 4 5 5
'
test_expect_success 'setup mixed finite/INFINITY topology' '
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 6/8] commit-reach: remove unused nonstale_queue dedup wrappers
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
nonstale_queue_put_dedup() and nonstale_queue_get_dedup() became
unused after the previous commit. The core nonstale_queue functions
remain in use by ahead_behind().
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-reach.c | 18 ------------------
1 file changed, 18 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 9ae306f60c..176ffd68d0 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -79,24 +79,6 @@ static void clear_nonstale_queue(struct nonstale_queue *queue)
queue->max_nonstale = NULL;
}
-static void nonstale_queue_put_dedup(struct nonstale_queue *queue,
- struct commit *c)
-{
- if (c->object.flags & ENQUEUED)
- return;
- c->object.flags |= ENQUEUED;
- nonstale_queue_put(queue, c);
-}
-
-static struct commit *nonstale_queue_get_dedup(struct nonstale_queue *queue)
-{
- struct commit *commit = nonstale_queue_get(queue);
-
- if (commit)
- commit->object.flags &= ~ENQUEUED;
- return commit;
-}
-
/*
* Priority queue with per-side commit counters for paint_down_to_common().
* Each non-stale queued commit occupies exactly one bucket: PARENT1-only,
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 7/8] commit-reach: terminate merge-base walk when one paint side is exhausted
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add an early termination check to paint_down_to_common() using the
per-side counters introduced earlier. Once the walk enters the
finite-generation region, terminate early when one side's exclusive
count drops to zero -- no new merge-base can form without both paint
sides meeting.
The check also waits for pending_merge_bases to reach zero, ensuring
all merge-base candidates have been dequeued and recorded before
exiting.
The INFINITY gate ensures correctness: commits without a commit-graph
entry have GENERATION_NUMBER_INFINITY and are ordered by commit date,
which is not topologically reliable. The optimization only fires
once the walk enters the finite-generation region where ordering
guarantees hold.
Step counts measured with trace2 on git.git with commit-graph:
merge-base --all v2.0.0 v2.55.0-rc1:
before: 72264 steps after: 44589 steps
merge-base --all v2.55.0-rc1 v2.55.0-rc1~5:
before: 110 steps after: 7 steps
Helped-by: Derrick Stolee <stolee@gmail.com>
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
.../technical/paint-down-to-common.adoc | 17 +++++++++++++++++
commit-reach.c | 17 ++++++++++++++---
t/t6600-test-reach.sh | 4 ++--
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/Documentation/technical/paint-down-to-common.adoc b/Documentation/technical/paint-down-to-common.adoc
index ac3e2b39a5..15adac7885 100644
--- a/Documentation/technical/paint-down-to-common.adoc
+++ b/Documentation/technical/paint-down-to-common.adoc
@@ -99,6 +99,9 @@ ends when one of the following conditions holds:
4. Single result: the caller only needs one merge base, one has
been found, and the walk has entered the finite-generation
region.
+ 5. Side exhaustion: no pure PARENT1 or pure PARENT2 commits
+ remain in the queue, no pending merge-base candidates exist,
+ and the walk has entered the finite-generation region.
Stale entry condition
~~~~~~~~~~~~~~~~~~~~~
@@ -109,6 +112,20 @@ existing candidates by proving one is an ancestor of another, but
`remove_redundant()` handles that as a post-processing step, so it
is safe to exit early.
+Side-exhaustion condition
+~~~~~~~~~~~~~~~~~~~~~~~~~
+A new merge-base requires commits from both sides to meet. When one
+side's exclusive counter reaches zero and there are no pending
+merge-base candidates, no future traversal step can produce a new
+candidate.
+
+This optimization only activates in the finite-generation region
+where topological ordering holds. In that region, children are
+always visited before parents, so paint flags are final at visit
+time and an exhausted side cannot reappear. In the INFINITY region,
+commit-date ordering can violate this guarantee, so the check is
+skipped.
+
Generation cutoff
~~~~~~~~~~~~~~~~~
Some callers (notably `remove_redundant()`) supply a `min_generation`
diff --git a/commit-reach.c b/commit-reach.c
index 176ffd68d0..e174b219c6 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -131,6 +131,10 @@ static void paint_queue_put(struct paint_state *state,
}
}
+/*
+ * Dequeue the next commit for the paint walk, or return NULL when
+ * no more merge bases can be discovered.
+ */
static struct commit *paint_queue_get(struct paint_state *state)
{
struct commit *commit = prio_queue_get(&state->queue);
@@ -140,9 +144,16 @@ static struct commit *paint_queue_get(struct paint_state *state)
commit->object.flags &= ~ENQUEUED;
- if (!state->parent1_count && !state->parent2_count &&
- !state->mb_candidate_count)
- return NULL;
+ if (!state->mb_candidate_count) {
+ /* only stale entries remain */
+ if (!state->parent1_count && !state->parent2_count)
+ return NULL;
+
+ /* one side is exhausted */
+ if ((!state->parent1_count || !state->parent2_count) &&
+ commit_graph_generation(commit) < GENERATION_NUMBER_INFINITY)
+ return NULL;
+ }
paint_count_update(state, commit->object.flags, -1);
return commit;
diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh
index 51f3d70492..6365007560 100755
--- a/t/t6600-test-reach.sh
+++ b/t/t6600-test-reach.sh
@@ -220,7 +220,7 @@ test_expect_success 'in_merge_bases_many:self' '
EOF
echo "in_merge_bases_many(A,X):1" >expect &&
test_all_modes in_merge_bases_many &&
- test_paint_down_steps 45 2 25 3
+ test_paint_down_steps 45 1 25 1
'
test_expect_success 'is_descendant_of:hit' '
@@ -337,7 +337,7 @@ test_expect_success 'merge-base --all commit-walk steps' '
>input &&
git rev-parse commit-9-1 >expect &&
run_all_modes git merge-base --all commit-9-9 commit-9-1 &&
- test_paint_down_steps 81 80 81 81
+ test_paint_down_steps 81 9 57 10
'
test_expect_success 'reduce_heads' '
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 8/8] commit-reach: move min_generation check into paint_queue_get()
From: Kristofer Karlsson via GitGitGadget @ 2026-06-28 12:25 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Consolidate the min_generation termination condition into
paint_queue_get(), alongside the existing stale-entry and
side-exhaustion checks.
Move last_gen into struct paint_state so that
commit_graph_generation() is called exactly once per dequeued commit
and the result is shared across all termination checks and the
monotonicity BUG assertion.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-reach.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index e174b219c6..5c5c54d66e 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -89,6 +89,8 @@ struct paint_state {
size_t parent1_count;
size_t parent2_count;
size_t mb_candidate_count;
+ timestamp_t min_generation;
+ timestamp_t last_gen;
};
static void paint_count_update(struct paint_state *state,
@@ -138,11 +140,23 @@ static void paint_queue_put(struct paint_state *state,
static struct commit *paint_queue_get(struct paint_state *state)
{
struct commit *commit = prio_queue_get(&state->queue);
+ timestamp_t generation;
if (!commit)
return NULL;
commit->object.flags &= ~ENQUEUED;
+ generation = commit_graph_generation(commit);
+
+ if (state->min_generation && generation > state->last_gen)
+ BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
+ generation, state->last_gen,
+ oid_to_hex(&commit->object.oid));
+ state->last_gen = generation;
+
+ /* generation cutoff */
+ if (generation < state->min_generation)
+ return NULL;
if (!state->mb_candidate_count) {
/* only stale entries remain */
@@ -151,7 +165,7 @@ static struct commit *paint_queue_get(struct paint_state *state)
/* one side is exhausted */
if ((!state->parent1_count || !state->parent2_count) &&
- commit_graph_generation(commit) < GENERATION_NUMBER_INFINITY)
+ generation < GENERATION_NUMBER_INFINITY)
return NULL;
}
@@ -177,9 +191,10 @@ static int paint_down_to_common(struct repository *r,
struct commit *commit;
int i;
int steps = 0;
- timestamp_t last_gen = GENERATION_NUMBER_INFINITY;
struct commit_list **tail = result;
+ state.min_generation = min_generation;
+ state.last_gen = GENERATION_NUMBER_INFINITY;
if (!min_generation && !corrected_commit_dates_enabled(r))
state.queue.compare = compare_commits_by_commit_date;
@@ -196,18 +211,8 @@ static int paint_down_to_common(struct repository *r,
while ((commit = paint_queue_get(&state))) {
struct commit_list *parents;
int flags;
- timestamp_t generation = commit_graph_generation(commit);
steps++;
- if (min_generation && generation > last_gen)
- BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
- generation, last_gen,
- oid_to_hex(&commit->object.oid));
- last_gen = generation;
-
- if (generation < min_generation)
- break;
-
flags = commit->object.flags & (PARENT1 | PARENT2 | STALE);
if (flags == (PARENT1 | PARENT2)) {
if (!(commit->object.flags & RESULT)) {
@@ -219,7 +224,7 @@ static int paint_down_to_common(struct repository *r,
* descendant of this one.
*/
if (!(mb_flags & MERGE_BASE_FIND_ALL) &&
- generation < GENERATION_NUMBER_INFINITY)
+ state.last_gen < GENERATION_NUMBER_INFINITY)
break;
}
/* Mark parents of a found merge stale */
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v4 1/1] environment: move excludes_file into repo_config_values
From: Tian Yuchen @ 2026-06-28 12:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, cirnovskyv, szeder.dev, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <xmqqbjcv2h3j.fsf@gitster.g>
On 6/28/26 16:40, Junio C Hamano wrote:
> Tian Yuchen <cat@malon.dev> writes:
>
>>> Wouldn't we rather want to try to be more strict and say
>>>
>>> if (!repo || !repo->initialized)
>>> BUG("repo must be an initialied repository");
>>>
>>> here? Aren't all the callers of this function supposed to be
>>> dealing with an already initialized repository?
>>
>> That makes sense, but from my point of view...
>>
>> 'repo_config_values()' already has a check for 'repo->initialized'. If
>> we're absolutely certain that the 'repo' is initialized, wouldn't it be
>> better to simply remove all the checks inside the getter and leave the
>> judgment to 'repo_config_values()'?
>
> Yes, that was what I was getting at ;-).
A lot of CI tests are failing, but that just goes to show that the
"bugs" are being properly identified, doesn’t it?
It means there are a lot of "invalid" calls in the tests (if the way we
define a 'valid' call, i.e. repo must be initialized, is correct)... It
seems that code like 'if (repo != the_repository) return' or something
similar is inevitably going to end up somewhere, even though, as you
said, it’s "sweeping problems under the rug."
I’m not sure how to proceed from here either..
Regards, yuchen
^ permalink raw reply
* Re: [PATCH v4 8/8] commit-reach: move min_generation check into paint_queue_get()
From: Derrick Stolee @ 2026-06-28 15:15 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget, git
Cc: Elijah Newren, Kristofer Karlsson
In-Reply-To: <8dd15d44e6a60fc39bbf6d894628507e839f9248.1782649547.git.gitgitgadget@gmail.com>
On 6/28/26 8:25 AM, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
...> @@ -138,11 +140,23 @@ static void paint_queue_put(struct paint_state *state,
> static struct commit *paint_queue_get(struct paint_state *state)
> {
> struct commit *commit = prio_queue_get(&state->queue);
> + timestamp_t generation;
>
> if (!commit)
> return NULL;
>
> commit->object.flags &= ~ENQUEUED;
> + generation = commit_graph_generation(commit);
> +
> + if (state->min_generation && generation > state->last_gen)
> + BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
> + generation, state->last_gen,
> + oid_to_hex(&commit->object.oid));
> + state->last_gen = generation;
> +
> + /* generation cutoff */
> + if (generation < state->min_generation)
> + return NULL;
...
> - if (min_generation && generation > last_gen)
> - BUG("bad generation skip %"PRItime" > %"PRItime" at %s",
> - generation, last_gen,
> - oid_to_hex(&commit->object.oid));
> - last_gen = generation;
> -
> - if (generation < min_generation)
> - break;
I'm just stopping in to say that this looks like a clean code move
in this version, without mutating this chunk in the previous patch.
LGTM.
-Stolee
^ permalink raw reply
* Re: [PATCH v4 0/8] commit-reach: terminate merge-base walk when one side is exhausted
From: Derrick Stolee @ 2026-06-28 15:16 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget, git
Cc: Elijah Newren, Kristofer Karlsson
In-Reply-To: <pull.2149.v4.git.1782649547.gitgitgadget@gmail.com>
On 6/28/26 8:25 AM, Kristofer Karlsson via GitGitGadget wrote:
> commit-reach: terminate merge-base walk when one paint side is exhausted
>
> Optimize paint_down_to_common() for merge-base queries that hit large
> one-sided histories.
>
> When the walk from one side reaches a commit with a very low generation
> number that the other side never paints, the walk is forced to drain most of
> the graph. A common trigger is a repository import that grafts a separate
> history with its own root, but any merge that introduces a low-generation
> commit never painted by the other side has the same effect.
> Changes since v3:
>
> * Fixed BUG assertion that was accidentally made unconditional in v3:
> restored the min_generation guard so it only fires when generation-based
> ordering is active.
>
> * Moved generation cutoff and single-result termination conditions into the
> documentation in patch 1/8, since they describe existing behavior.
>
> * Renamed paint_state counter fields for clarity: p1_count ->
> parent1_count, p2_count -> parent2_count, pending_merge_bases ->
> mb_candidate_count. Changed counter types from int to size_t. (Suggested
> by Rene Scharfe.)
I reviewed the v3 discussion, the range-diff, and reread patch 8. I think
that this version is good to go.
Thanks for your hard work!
-Stolee
^ permalink raw reply
* Re: 2.54.0: fyi: endless loop at 100% CPU
From: Steffen Nurpmeso @ 2026-06-28 16:37 UTC (permalink / raw)
To: Michael Montalbo; +Cc: git, Steffen Nurpmeso
In-Reply-To: <CAC2Qwm+v2pRp30TYJpy8Wxzb7gbX+nzybZ_3A99cHb-xjjpCnQ@mail.gmail.com>
Michael Montalbo wrote in
<CAC2Qwm+v2pRp30TYJpy8Wxzb7gbX+nzybZ_3A99cHb-xjjpCnQ@mail.gmail.com>:
|On Sat, Jun 27, 2026 at 1:16 PM Steffen Nurpmeso <steffen@sdaoden.eu> \
|wrote:
|>
|> Thanks for these pointers, i did not know about such configuration
|> variables. I will set them like you show.
|
|No problem! Just to clarify, I'm not sure you should actually use those
|configuration values verbatim. I was more pointing in the direction of
|potentially relevant options for debugging / working around the issue.
We'll see. But if there was some kind of "with love from canada"
misconfiguration -- i have seen quite a bit of those, and
permanent sub-second page reload was one of those effects, in
a browser though .. and git has a little road 'till it gets to
that stage (i hope) -- then maybe these settings .. Or i have to
tweak.
Restartable "fetch" is likely not on that roadmap of git -- that
would (have) be(en) so cool. (But for years i now have
a WireGuard VPN and go through that, which has improved my TCP
connectivity / stability massively. But it is still a thriller to
go for some rate-limited fetch of large size ..)
Oh. Maybe i see.
$ git ls-remote https://gitlab.xiph.org/xiph/opus.git
fatal: unable to access 'https://gitlab.xiph.org/xiph/opus.git/': Operation too slow. Less than 1000 bytes/sec transferred the last 10 seconds
$ git ls-remote https://gitlab.xiph.org/xiph/opus.git
fatal: unable to access 'https://gitlab.xiph.org/xiph/opus.git/': Operation too slow. Less than 500 bytes/sec transferred the last 10 seconds
$ git ls-remote https://gitlab.xiph.org/xiph/opus.git
fatal: unable to access 'https://gitlab.xiph.org/xiph/opus.git/': Operation too slow. Less than 500 bytes/sec transferred the last 21 seconds
It succeeds with
lowSpeedLimit = 500
lowSpeedTime = 33
But at least it does not busy loop:
steffen 2960 2959 0 0.0 2738 2016 S+ 00:00:00 18:26 pts/4 /usr/lib/git-core/git remote-https https://gitlab.xiph.org/xiph/opus.git https://gitlab.xiph.org/xiph/opus.git
steffen 2961 2960 0 0.0 41213 11316 S+ 00:00:00 18:26 pts/4 /usr/lib/git-core/git-remote-https https://gitlab.xiph.org/xiph/opus.git https://gitlab.xiph.org/xiph/opus.git
Unfortunately no info where and why it busy looped. If it would
be non-blocking I/O and .. in this area, one could understand
a bit. Some ticks-per-sec limit (without X progress) is not
configurable? I am not really keen to create a rlimit wrapper
for git, or whatever. (I have limiting cgroups, but still.)
A nice Sunday,
Ciao and greetings from Germany,
--steffen
|
|Der Kragenbaer, The moon bear,
|der holt sich munter he cheerfully and one by one
|einen nach dem anderen runter wa.ks himself off
|(By Robert Gernhardt)
^ permalink raw reply
* Re: [PATCH 3/3] t5551: pack refs after creating many tags
From: Junio C Hamano @ 2026-06-28 21:25 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Montalbo, Patrick Steinhardt, git
In-Reply-To: <20260628080710.GC107826@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So let's follow that recommendation and pack the refs ourselves.
> Unfortunately, this does not seem to produce an improvement to the
> run-time of the test script! That's because after producing this state,
> we perform only a few fetches of it. And packing the refs costs at least
> as much as serving a ref advertisement (both have to iterate the refs,
> but packing additionally must write .lock files as we pack).
Testing a pathological set-up with too many loose refs may have
extra value, as long as we are also testing the recommended set-up,
so ideally we should have both ;-) but if we have to pick only one
and drop the other, we probably should be testing the packed case.
> I'm iffy on whether this one is worth it.
I am ambivalent, too, about this change for the purpose of the
"yeek, apache times out while enumerating refs" issue. But see
above ;-)
^ permalink raw reply
* Re: [PATCH 2/3] t5551: put many-tags case into its own repo
From: Junio C Hamano @ 2026-06-28 21:44 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Montalbo, Patrick Steinhardt, git
In-Reply-To: <20260628080345.GB107826@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index e236e526f0..cd851f24b8 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -397,15 +397,16 @@ create_tags () {
> }
>
> test_expect_success 'create 2,000 tags in the repo' '
> + git init "$HTTPD_DOCUMENT_ROOT_PATH/many-tags.git" &&
> (
> - cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
> + cd "$HTTPD_DOCUMENT_ROOT_PATH/many-tags.git" &&
> create_tags 1 2000
> )
> '
While all the other repositories used in this tests are bare
repositories, this new one is a non-bare repository.
It shouldn't make any difference, but since I noticed it...
^ permalink raw reply
* Re: [PATCH] http: accept https:// proxies again
From: Junio C Hamano @ 2026-06-28 22:27 UTC (permalink / raw)
To: Johannes Schindelin via GitGitGadget; +Cc: git, Aliwoto, Johannes Schindelin
In-Reply-To: <xmqqjyrj2qsp.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> From this function nothing returns an error anymore, and looking at
>> the preimage of 663d7abe (http: reject unsupported proxy URL
>> schemes, 2026-05-05) that is the source of the bug, the original did
>> not do anything when the corresponding code did not find and set any
>> proxy settings, either.
>>
>> So perhaps it is a better fix to make it just a function that
>> returns void with early returns?
>
> Nah, I was being stupid. Disregard the above.
>
> The whole point of 663d7abe was that we wanted to reject what we did
> not recognise, and we cannot do so without returning "good/bad" from
> that function. The bug was that we did recognise https:// but still
> returned -1 because of the bug, which the patch in the thread fixed.
And as an important bugfix, this patch of course has been
fast-tracked. I'll make sure we have it in 'master' before Git 2.55
gets tagged.
Thanks.
^ permalink raw reply
* Re: [PATCH GSoC v14 09/13] serve: advertise object-info feature
From: Karthik Nayak @ 2026-06-28 22:55 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, chandrapratap3519, chriscool, eric.peijian, gitster,
jltobler, peff, toon, Calvin Wan, Jonathan Tan
In-Reply-To: <CAN5EUNTrdNArd5SX9df6x9bOhRfzE4c7dLOuNu7ONUdn4TLsUA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3367 bytes --]
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> El sáb, 27 jun 2026 a las 0:23, Karthik Nayak
> (<karthik.188@gmail.com>) escribió:
>>
>> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>>
>> > From: Calvin Wan <calvinwan@google.com>
>> >
>> > In order for a client to know what object-info components a server can
>> > provide, advertise supported object-info features. This will allow a
>> > client to decide whether to query the server for object-info or fetch
>> > as a fallback.
>> >
>> > Helped-by: Jonathan Tan <jonathantanmy@google.com>
>> > Helped-by: Christian Couder <chriscool@tuxfamily.org>
>> > Signed-off-by: Calvin Wan <calvinwan@google.com>
>> > Signed-off-by: Eric Ju <eric.peijian@gmail.com>
>> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
>> > ---
>> > serve.c | 5 ++++-
>> > 1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/serve.c b/serve.c
>> > index 49a6e39b1d..2b07d922b3 100644
>> > --- a/serve.c
>> > +++ b/serve.c
>> > @@ -89,7 +89,7 @@ static void session_id_receive(struct repository *r UNUSED,
>> > trace2_data_string("transfer", NULL, "client-sid", client_sid);
>> > }
>> >
>> > -static int object_info_advertise(struct repository *r, struct strbuf *value UNUSED)
>> > +static int object_info_advertise(struct repository *r, struct strbuf *value)
>> > {
>> > if (advertise_object_info == -1 &&
>> > repo_config_get_bool(r, "transfer.advertiseobjectinfo",
>> > @@ -97,6 +97,9 @@ static int object_info_advertise(struct repository *r, struct strbuf *value UNUS
>> > /* disabled by default */
>> > advertise_object_info = 0;
>> > }
>> > + /* Currently only size is supported */
>> > + if (value && advertise_object_info)
>> > + strbuf_addstr(value, "size");
>>
>> So is the plan that further options will be added here to value? If so,
>> whats the format we will follow?
>
> Hi!
> The current documented format is at `gitprotocol-v2.adoc`, however I
> think it could be improved. I have a more complete version in the
> not-yet-sent %(objecttype) support series, but since the question
> comes up here, I will update the format documentation in this series
> for size only:
>
Ah nice, I didn't see that.
> oid <oid>
> Indicates to the server an object which the client wants to obtain
> - information for.
> + information for. They must be full object IDs.
>
> - info = PKT-LINE(attrs) LF)
> + info = PKT-LINE(attrs LF)
> *PKT-LINE(obj-info LF)
>
> attrs = attr | attrs SP attrs
>
> + obj-size = 1*DIGIT
> +
> attr = "size"
>
> - obj-info = obj-id SP obj-size
> + obj-info = obj-id SP [obj-size]
> +
> +If the server does not recognize the object id, the response will be
> +`obj-id SP` regardless of the number of attributes requested.
>
> About the names `size` and future ones `type` they are arbitrarily
> chosen, so for example: `delta:base` could be `delta`. They are
> appended to the buffer so in case of adding `type`, it would look
> like:
>
> strbuf_addstr(value, "size type");
>
> What do you think?
>
I didn't know this part was already documented, so its all good :)
> Thanks for the review,
> Pablo.
>
>>
>> > return advertise_object_info;
>> > }
>> >
>> >
>> > --
>> > 2.54.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox