* Re: [PATCH GSoC RFC v13 00/12] cat-file: add remote-object-info to batch-command
From: Chandra Pratap @ 2026-06-21 5:25 UTC (permalink / raw)
To: Pablo Sabater
Cc: gitster, peff, eric.peijian, chriscool, git, jltobler,
karthik.188, toon
In-Reply-To: <20260619-ps-eric-work-rebase-v13-0-3d4c7315d2f8@gmail.com>
On Fri, 19 Jun 2026 at 20:26, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> This path series is a continuation of Eric Ju's (eric.peijian@gmail.com) and
s/path/patch
> Calvin Wan's (calvinwan@google.com) patch series [1] and [2] respectively.
>
> Sometimes it is beneficial to retrieve information about an object without
> having to download it completely. The server logic for retrieving size has
> already been implemented and merged in "a2ba162cda (object-info: support for
> retrieving object info, 2021-04-20)"[3]. This patch series implement the client
> option for it.
>
> Eric's series adds the `remote-object-info` command to
> `cat-file --batch-command`. This command allows the client to make an
> object-info command request to a server that supports protocol v2.
>
> If the server uses protocol v2 but does not support the object-info capability,
> `cat-file --batch-command` will die.
>
> If a user attempts to use `remote-object-info` with protocol v1,
> `cat-file --batch-command` will die.
>
> Currently, only the size (%(objectsize)) is supported end to end in this
> implementation. The type (%(objecttype)) is known by the client's allow-list
> and request path but is not supported on the server side nor the response
> parsing. A follow up series will add full end-to-end support for %(objecttype).
>
> The default format for remote-object-info is set to %(objectname) %(objectsize).
> Once %(objecttype) is supported, the default format will be unified accordingly.
>
> If the batch command format includes unsupported fields such as %(objecttype),
> %(objectsize:disk), or %(deltabase), the command will return empty strings for
> each unsupported field.
>
> This series completes Eric's work mainly with the refactor of the validation
> of the placeholder with an allow-list that filters what the client asks with
> what the server is capable of provide following Jeff King's idea [4].
>
> I have a question for the design:
>
> 1. If the format includes unsupported fields such as %(objecttype) or
> %(deltabase) it currently returns an empty string for each unsupported
> field, this follows what for-each-ref does with known but inapplicable
> atoms. However future placeholders that will be implemented: %(rest),
> %(objectmode) can return empty strings. How should we differentiate
> "unsupported" vs "no data".
> Eric proposed to use a placeholder like "???" [5].
> Should a placeholder be used?
Maybe it's best to fail cleanly if the user requests an unsupported atom?
I don't really like the placeholder idea though. If a placeholder like "???"
is introduced, any script/test parsing the output must add explicit logic
to check for literal question marks, and that sounds flaky. Not to
mention some atom's response may legitimately contain "???".
> 2. _tangent/not related with this series_
> 'a2ba162cda' is designed to only work with full OIDs, which is
> inconsistent with local `info` that does support short OIDs and in
> case of being ambiguous returns a list of what possibly the user meant.
>
> Because V2 protocol is thought to be stateless supporting short OIDs
> could become more inconsistent with other remote commands that do not
> support short OIDs. Maybe a --pick-first option? That does accept
> short oids and picks the first match.
We might return the wrong object's info if we do this. With the server
giving us no information to verify whether the returned value _really_
corresponds to our intended object, I'd say this isn't the right choice.
> Alternatively, would sending a list of possible OIDs to the client so
> it can re-request with the correct one be ok?
As far as I know, disambiguation like this is treated purely as a local
UI convenience in Git, never a network-level operation.
`fetch` already requires users to input exact, full OIDs for their `want`
lines (obtained via a prior `ls-remote` or ref advertisement), and dies
if one isn't provided. Thus, I think erroring out is fine here.
^ permalink raw reply
* Re: git-diff in a worktree is an order of magnitude slower?
From: Junio C Hamano @ 2026-06-21 3:58 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Jeff King, Git
In-Reply-To: <xmqqa4sog1e9.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> I initially thought it was an embarrassing thinko, but after seeing
> how .skip_stat_unmatch is used as a 1-based counter (i.e., if the
> member says 42, it means it saw 41 paths that were stat-dirty but
> without actual content change), I do not think so.
>
> Now, it is a different matter if such a "dual" purpose "more than a
> simple boolean" counter is a good idea. Apparently it confused both
> of us in this case ;-).
FWIW the patch was done as part of this discussion thread:
https://lore.kernel.org/git/20070830063810.GD16312@mellanox.co.il/
^ permalink raw reply
* Re: [PATCH v6 2/3] revision: add peek functions for lookahead
From: Junio C Hamano @ 2026-06-21 1:48 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, krka, ayu.chandekar, chandrapratap3519, christian.couder,
jltobler, karthik.188, peff, phillip.wood, siddharthasthana31,
Kristofer Karlsson
In-Reply-To: <20260620-ps-pre-commit-indent-v6-2-cdc6d8fd5fbc@gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> +int revision_has_commits_after (struct rev_info *revs, int n)
> +{
> + struct topo_walk_info *info = revs->topo_walk_info;
> +
> + if (info) {
> + int visible = 0;
> + for (size_t i = 0; i < info->topo_queue.nr && visible < n; i++) {
> + struct commit *c = info->topo_queue.array[i].data;
> + if (get_commit_action(revs, c) == commit_show)
> + visible++;
> + }
> + return visible > n-1;
The loop needs to be rethought, perhaps with a better abstraction
than ".nr is the number of elements in the queue and we can walk
them over as a dense array", using prio_queue_for_each(), once this
topic meets the kk/prio-queue-get-put-fusion topic.
I see Kristofer is already on the CC: line. Depending on the
done-ness of the topic, we may want to include the topic in the
updated base for this topic to resolve semantic conflicts early, or
the other way around (i.e., let this topic graduate first and then
rebuild the prio-queue topic on top of the updated 'master').
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/2] completion: hide dotfiles for selected path completion
From: Junio C Hamano @ 2026-06-21 1:17 UTC (permalink / raw)
To: Zakariyah Ali via GitGitGadget; +Cc: git, Zakariyah Ali
In-Reply-To: <pull.2311.v3.git.git.1781978156.gitgitgadget@gmail.com>
"Zakariyah Ali via GitGitGadget" <gitgitgadget@gmail.com> writes:
> The completion helper for index paths uses git ls-files rather than shell
> filename completion. As a result, leading-dot paths such as a tracked
> .gitignore were offered even when the user had not started the path with ..
>
> Hide leading-dot path components for git rm, git mv, and git ls-files when
> completing an empty path component. Explicit dot completion is still
> preserved, so git rm . can still complete .gitignore.
>
> This removes the existing TODO expectations in t/t9902-completion.sh and
> adds coverage for explicit dot completion.
OK.
> Validation:
>
> * git diff --check -- contrib/completion/git-completion.bash
> t/t9902-completion.sh
> * bash -n contrib/completion/git-completion.bash
> * ./t9902-completion.sh
I am not sure what you wanted to say with these lines. If you did
the above to build confidence that your patch works, that would be
great. Or are you telling readers to do these things and when they
do not see any issues consider your patch perfect?
What is missing around here in this cover letter is a description of
how this iteration is different from the previous one. And ...
> Zakariyah Ali (2):
> completion: hide dotfiles for selected path completion
> completion: hide dotfiles by default for path completion
>
> contrib/completion/git-completion.bash | 53 +++++++++++++++-----------
> t/t9902-completion.sh | 19 ++++-----
> 2 files changed, 40 insertions(+), 32 deletions(-)
>
>
> base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v3
> Pull-Request: https://github.com/git/git/pull/2311
>
> Range-diff vs v2:
>
> 1: 056e239e06 = 1: 056e239e06 completion: hide dotfiles for selected path completion
> -: ---------- > 2: 7482ee4645 completion: hide dotfiles by default for path completion
... I find this range diff very troubling. If we look at patch 2,
it seems that it redoes some part of what is done in patch 1 saying
"oops that was wrong, so let's do it better this time". Such a
drunken-mans' walk that goes in one direction in an earlier step,
only to be corrected to move to a different course, is now how we
want a new topic to be presented.
The end result may be much easier to read, mostly thanks to updated
loop in the awk script, so if we really want to pretend this as two
patches for "small pieces are easier to digest" value, perhaps have
[PATCH 1/2] that updates the awk script (without doing anything
related to hide-dotfiles theme) to make it easier to read by not
having multiple "print pfx p" in it, and then build on top of that
improved base, have [PATCH 2/2] that adds the support to hide
dotfiles, perhaps?
Since the initial iteration was quite a while ago, I no longer
remember the details of the review I gave, but I recall having hard
time telling which callers of the complete-index-file helper hide
dotfiles from their output and which callers do not hide them, and
how the patch decided to choose which ones should and should not
hide. Has it been improved and if so how? That is something we
expect the cover letter to tell, too.
Thanks.
^ permalink raw reply
* Re: [PATCH] meson: wire up USE_NSEC build knob
From: Junio C Hamano @ 2026-06-21 1:01 UTC (permalink / raw)
To: D. Ben Knoble
Cc: git, brian m . carlson, Jeff King, Patrick Steinhardt,
Ramsay Jones
In-Reply-To: <c4c5ade901ff95b0f95939ea818870e4f3d59da1.1781971201.git.ben.knoble+github@gmail.com>
"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
> Autotools-style builds permit enabling USE_NSEC for cases where that's
> desired; the equivalent knob is missing from meson-based builds.
With or without autoconf, Makefile based build can use USE_NSEC. It
is a welcome addition to the other side of thw world. I do not know
if 'meson setup -Dnanosec=true' is a name that is easy to discover,
though.
Will queue. Thanks.
^ permalink raw reply
* Re: git-diff in a worktree is an order of magnitude slower?
From: Junio C Hamano @ 2026-06-21 0:53 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: Jeff King, Git
In-Reply-To: <CALnO6CAx91kbJ84d6Ef655UNG0y0rhyknBRh6Y+0o7Xn-uVytQ@mail.gmail.com>
"D. Ben Knoble" <ben.knoble@gmail.com> writes:
> But the refresh_index_quietly call is guarded by (effectively; the
> actual code uses rev.diffopt.skip_stat_unmatch)
>
> 1 < !!diff_auto_refresh_index
It is not quite that, is it? In aecbf914 (git-diff: resurrect the
traditional empty "diff --git" behaviour, 2007-08-31), it read more
like
if (1 < rev.diffopt.skip_stat_unmatch)
refresh_index_quietly();
where rev.diffopt.skip_stat_unmatch was initialized to 1 if
diff_auto_refresh_index (boolean) is set to true.
Now, cmd_diff() dispatches to various diff backends to compare two
sets (like "a tree object vs the index", "the index vs the working
tree files"), each of which ends with a call to diffcore_std() and
diffcore_flush() to conclude. In diffcore_std() there is a call
to diffcore_skip_stat_unmatch() ONLY when skip_stat_unmatch member
is set (we initialize it to 1 when auto-refrresh-index is enabled,
as you saw above). The function is used to squelch the paths that
remain in diff_queued_diff only because they were stat-dirty without
having an actual content change, and _counts_ how many such ghost
changes existed by incrementing the .skip_stat_unmatch counter.
> which dates to aecbf914c4 (git-diff: resurrect the traditional empty
> "diff --git" behaviour, 2007-08-31). On my system that comparison is
> false because the double-negation produces 1
> (diff_auto_refresh_index=1 or the result of git_config_bool).
Not quite. It was false because double-negation initializes the
member to 1, which causes a call to diffcore_skip_stat_unmatch()
be made, *and* the diffcore_skip_stat_unmatch() function did not
find any ghost changes, i.e., paths that were only stat-dirty hence
needed a call to refresh_index_quietly().
> So… has that conditional been quietly dead all this time? I can't
> imagine that's right, but…
I initially thought it was an embarrassing thinko, but after seeing
how .skip_stat_unmatch is used as a 1-based counter (i.e., if the
member says 42, it means it saw 41 paths that were stat-dirty but
without actual content change), I do not think so.
Now, it is a different matter if such a "dual" purpose "more than a
simple boolean" counter is a good idea. Apparently it confused both
of us in this case ;-).
Thanks.
^ permalink raw reply
* [PATCH v4] SubmittingPatches: address design critiques
From: Junio C Hamano @ 2026-06-20 23:43 UTC (permalink / raw)
To: git; +Cc: Michael Montalbo, Kristoffer Haugsbakk
In-Reply-To: <xmqqik7eld2g.fsf_-_@gitster.g>
Contributors sometimes fail to answer fundamental design or
viability comments from reviewers and submit subsequent rounds
without addressing them. When design decisions are resolved on the
mailing list, the final justification should be recorded in the
commit messages.
Instruct authors to be particularly mindful of critiques regarding
high-level design or viability, to defend their choices on the list,
and to accompany new iterations with clearer explanations in the cover
letter, responses, and revised commit messages. Also instruct them to
explicitly document the resolution of these concerns in the commit
message body to keep the historical record complete.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Hopefully this will be the last iteration.
Documentation/SubmittingPatches | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index f042bb5aaf..28b4f2f795 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -51,6 +51,22 @@ area.
respond to them with "Reply-All" on the mailing list, while taking
them into account while preparing an updated set of patches.
+
+Be particularly mindful of critiques regarding the high-level design
+or viability of your proposal (e.g., questioning if the feature is
+worth implementing, or if the chosen approach is appropriate). Defend
+your design decisions on the list first and work with reviewers and
+other members to improve the design before revising the implementation.
+This will avoid wasting effort on an implementation before its design is
+solid.
++
+Make sure that any new version explains and justifies those design
+decisions more clearly, in the cover letter and in the revised commit
+messages. Aim to make the reviewers say "it is now clear why we may
+want to do this with the updated version".
++
+Topics with unresolved fundamental design critiques will not be
+considered ready for merging.
++
It is often beneficial to allow some time for reviewers to provide
feedback before sending a new version, rather than sending an updated
series immediately after receiving a review. This helps collect broader
@@ -323,6 +339,10 @@ The body should provide a meaningful commit message, which:
. alternate solutions considered but discarded, if any.
+. records the resolution of design or viability concerns raised by the
+ community during the review, if any, ensuring the historical record
+ explains why the chosen approach was accepted over alternatives.
+
[[present-tense]]
The problem statement that describes the status quo is written in the
present tense. Write "The code does X when it is given input Y",
Interdiff against v2:
diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index a9789e5303..28b4f2f795 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -54,9 +54,10 @@ area.
Be particularly mindful of critiques regarding the high-level design
or viability of your proposal (e.g., questioning if the feature is
worth implementing, or if the chosen approach is appropriate). Defend
-your design decisions on the list first, work with reviewers and other
-members to improve the design before revising the implementation, to
-avoid wasting effort on an implementation before its design is solid.
+your design decisions on the list first and work with reviewers and
+other members to improve the design before revising the implementation.
+This will avoid wasting effort on an implementation before its design is
+solid.
+
Make sure that any new version explains and justifies those design
decisions more clearly, in the cover letter and in the revised commit
--
2.55.0-rc1-134-gb7e3543e07
^ permalink raw reply related
* [PATCH v2] gitk, git-gui: drop msgfmt --statistics output
From: Harald Nordgren via GitGitGadget @ 2026-06-20 22:46 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2339.git.git.1781459539.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
The catalog rules ran msgfmt with --statistics, whose output went to
stderr and so survived "make -s" (gitk also echoed "Generating
catalog").
The statistics are not needed, as in 2f12b31b746c (Makefile: don't
invoke msgfmt with --statistics, 2021-12-17), and the "Generating
catalog" line is not needed either. Remove them so a quiet build stays
quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
Silence po catalog output under "make -s"
The gitk and git-gui catalog rules sent msgfmt --statistics output (and
a "Generating catalog" line) to stderr, so it survived "make -s". Emit
it only when "-s" is absent, keeping a quiet build silent and a verbose
build unchanged.
Changes in v2:
* Reworked from conditionally silencing msgfmt output under make -s to
just removing --statistics outright, following 2f12b31b74 (Makefile:
don't invoke msgfmt with --statistics, 2021-12-17)
* Also drop gitk's Generating catalog echo, which is not needed either
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2339%2FHaraldNordgren%2Fsilence-catalog-output-under-make-s-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2339/HaraldNordgren/silence-catalog-output-under-make-s-v2
Pull-Request: https://github.com/git/git/pull/2339
Range-diff vs v1:
1: 18451d46e4 < -: ---------- gitk: silence catalog output under "make -s"
2: 5071c5106a ! 1: ee57c25009 git-gui: silence statistics under "make -s"
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- git-gui: silence statistics under "make -s"
+ gitk, git-gui: drop msgfmt --statistics output
- The catalog rule passed --statistics to msgfmt unconditionally, and its
- output went to stderr, so it survived "make -s".
+ The catalog rules ran msgfmt with --statistics, whose output went to
+ stderr and so survived "make -s" (gitk also echoed "Generating
+ catalog").
- Pass --statistics only when "-s" is absent, leaving a quiet build silent
- while default and V=1 builds are unchanged.
+ The statistics are not needed, as in 2f12b31b746c (Makefile: don't
+ invoke msgfmt with --statistics, 2021-12-17), and the "Generating
+ catalog" line is not needed either. Remove them so a quiet build stays
+ quiet.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## git-gui/Makefile ##
-@@ git-gui/Makefile: ifndef V
- REMOVE_F0 = dst=
- REMOVE_F1 = && echo ' ' REMOVE `basename "$$dst"` && $(RM_RF) "$$dst"
- endif
-+ MSGFMT_STATS = --statistics
- endif
-
- TCLTK_PATH ?= wish
@@ git-gui/Makefile: $(PO_TEMPLATE): $(SCRIPT_SH) $(ALL_LIBFILES)
update-po:: $(PO_TEMPLATE)
$(foreach p, $(ALL_POFILES), echo Updating $p ; msgmerge -U $p $(PO_TEMPLATE) ; )
$(ALL_MSGFILES): %.msg : %.po
- $(QUIET_MSGFMT0)$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
-+ $(QUIET_MSGFMT0)$(MSGFMT) $(MSGFMT_STATS) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
++ $(QUIET_MSGFMT0)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
$(QUIET_INDEX)$(SHELL_PATH) generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)
+
+ ## gitk-git/Makefile ##
+@@ gitk-git/Makefile: update-po:: $(PO_TEMPLATE)
+ echo; \
+ echo " git config filter.gettext-no-location.clean \"msgcat --no-location -\""
+ $(ALL_MSGFILES): %.msg : %.po
+- @echo Generating catalog $@
+- $(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
++ $(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
+
+ .PHONY: all install uninstall clean update-po
+ .PHONY: FORCE
git-gui/Makefile | 2 +-
gitk-git/Makefile | 3 +--
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/git-gui/Makefile b/git-gui/Makefile
index d33204e875..48d848a59d 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -155,7 +155,7 @@ $(PO_TEMPLATE): $(SCRIPT_SH) $(ALL_LIBFILES)
update-po:: $(PO_TEMPLATE)
$(foreach p, $(ALL_POFILES), echo Updating $p ; msgmerge -U $p $(PO_TEMPLATE) ; )
$(ALL_MSGFILES): %.msg : %.po
- $(QUIET_MSGFMT0)$(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
+ $(QUIET_MSGFMT0)$(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $< $(QUIET_MSGFMT1)
lib/tclIndex: $(ALL_LIBFILES) generate-tclindex.sh GIT-GUI-BUILD-OPTIONS
$(QUIET_INDEX)$(SHELL_PATH) generate-tclindex.sh . ./GIT-GUI-BUILD-OPTIONS $(ALL_LIBFILES)
diff --git a/gitk-git/Makefile b/gitk-git/Makefile
index 41116d8a14..0ae083c1ca 100644
--- a/gitk-git/Makefile
+++ b/gitk-git/Makefile
@@ -75,8 +75,7 @@ update-po:: $(PO_TEMPLATE)
echo; \
echo " git config filter.gettext-no-location.clean \"msgcat --no-location -\""
$(ALL_MSGFILES): %.msg : %.po
- @echo Generating catalog $@
- $(MSGFMT) --statistics --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
+ $(MSGFMT) --tcl -l $(basename $(notdir $<)) -d $(dir $@) $<
.PHONY: all install uninstall clean update-po
.PHONY: FORCE
base-commit: 8d96f09e9245ddf80c1981476fcbac8c4bb4125f
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v3] config.mak.uname: avoid macOS dup-library warning
From: D. Ben Knoble @ 2026-06-20 20:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Harald Nordgren via GitGitGadget, git, Harald Nordgren
In-Reply-To: <xmqqv7bei2tf.fsf@gitster.g>
On Fri, Jun 19, 2026 at 6:27 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Harald Nordgren <haraldnordgren@gmail.com>
> >
> > Building on macOS with Xcode 15 or newer emits:
> >
> > ld: warning: ignoring duplicate libraries: 'libgit.a',
> > 'target/release/libgitcore.a'
> >
> > Some link recipes list the same archive twice, which is harmless.
> > Quiet the warning instead.
> >
> > Pass -Wl,-no_warn_duplicate_libraries on Xcode 15 and newer, whose
> > linkers added both the warning and the suppression flag (ld64-907
> > and dyld-1009). Earlier linkers reject the flag, so gate on the
> > linker version. Broaden the existing -fno-common version probe to
> > also match the "ld64-NNN" and "dyld-NNN" forms Xcode 15 reports.
> >
> > Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> > ---
>
> Yeah, this looks like what I expected.
>
> A few things to note.
>
> * Can folks with different versions of Xcode (or is 15 sufficiently
> old that practically nobody is expected to have anything older?)
> test this patch?
>
> * We only patch Makefile here; can folks who use meson report how
> well your build goes?
>
> Thanks.
On one (old) machine I have available:
$ pkgutil --pkg-info=com.apple.pkg.CLTools_Executables
[trimmed]
version: 14.2.0.0.1.1668646533
On said machine, I don't get the duplicate warnings on a Meson build.
No issues with the patch when running make.
I think I have seen this on my other machine, which is much newer.
When I get around to trying it there, I'll report back as well.
^ permalink raw reply
* [PATCH v3 2/2] completion: hide dotfiles by default for path completion
From: Zakariyah Ali via GitGitGadget @ 2026-06-20 17:55 UTC (permalink / raw)
To: git; +Cc: Zakariyah Ali, Zakariyah Ali, Zakariyah Ali
In-Reply-To: <pull.2311.v3.git.git.1781978156.gitgitgadget@gmail.com>
From: Zakariyah Ali <zakariyahali100@gmail.com>
The previous implementation required callers to explicitly pass a
"hide-dotfiles" flag to __git_complete_index_file to avoid cluttering
completions with hidden files. This led to inconsistent behavior across
commands (e.g., `git add` and `git mv` behaved differently) and forced
callers to maintain repetitive logic.
As suggested by Junio C Hamano, this commit simplifies the logic:
1. __git_complete_index_file now unconditionally hides dotfiles when
no match pattern is provided.
2. The awk loop in __git_index_files is refactored to check the dotfile
condition in a single, obvious place after handling path dequoting,
removing the previous duplication.
3. Callers no longer need to pass "hide-dotfiles".
This provides a cleaner API and ensures a consistent, expected behavior
where dotfiles are hidden unless explicitly requested by typing a dot.
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
contrib/completion/git-completion.bash | 65 ++++++++++++--------------
t/t9902-completion.sh | 9 +++-
2 files changed, 38 insertions(+), 36 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e8f8fab125..b0b1b3c27a 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -638,20 +638,23 @@ __git_ls_files_helper ()
}
-# __git_index_files accepts 1 to 4 arguments:
+# __git_index_files accepts 1 to 3 arguments:
# 1: Options to pass to ls-files (required).
# 2: A directory path (optional).
# If provided, only files within the specified directory are listed.
# Sub directories are never recursed. Path must have a trailing
# slash.
# 3: List only paths matching this path component (optional).
-# 4: Hide paths whose first component starts with a dot if this is
-# "hide-dotfiles" and the third argument is empty (optional).
+#
+# If the third argument is empty, paths that begin with a dot (dotfiles)
+# are hidden. This matches user expectations where dotfiles are considered
+# hidden configuration files/directories and shouldn't clutter default
+# completions unless explicitly requested by typing a dot.
__git_index_files ()
{
- local root="$2" match="$3" hide_dotfiles="${4-}"
+ local root="$2" match="$3"
local hide_dotfiles_awk=0
- if [ "$hide_dotfiles" = "hide-dotfiles" ] && [ -z "$match" ]; then
+ if [ -z "$match" ]; then
hide_dotfiles_awk=1
fi
@@ -661,28 +664,22 @@ __git_index_files ()
}
END {
for (p in paths) {
- if (substr(p, 1, 1) != "\"") {
- # No special characters, easy!
- if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+ if (substr(p, 1, 1) == "\"") {
+ # The path is quoted.
+ p = dequote(p)
+ if (p == "")
continue
- print pfx p
- continue
- }
-
- # The path is quoted.
- p = dequote(p)
- if (p == "")
- continue
- # Even when a directory name itself does not contain
- # any special characters, it will still be quoted if
- # any of its (stripped) trailing path components do.
- # Because of this we may have seen the same directory
- # both quoted and unquoted.
- if (p in paths)
- # We have seen the same directory unquoted,
- # skip it.
- continue
+ # Even when a directory name itself does not contain
+ # any special characters, it will still be quoted if
+ # any of its (stripped) trailing path components do.
+ # Because of this we may have seen the same directory
+ # both quoted and unquoted.
+ if (p in paths)
+ # We have seen the same directory unquoted,
+ # skip it.
+ continue
+ }
if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
continue
@@ -731,15 +728,13 @@ __git_index_files ()
}'
}
-# __git_complete_index_file accepts 1 or 2 arguments:
-# 1: the options to pass to ls-file
-# 2: Hide paths whose first component starts with a dot if this is
-# "hide-dotfiles" and the current word is empty (optional).
+# __git_complete_index_file accepts 1 argument:
+# 1: the options to pass to ls-files
#
# The exception is --committable, which finds the files appropriate commit.
__git_complete_index_file ()
{
- local dequoted_word pfx="" cur_ hide_dotfiles="${2-}"
+ local dequoted_word pfx="" cur_
__git_dequote "$cur"
@@ -752,7 +747,7 @@ __git_complete_index_file ()
cur_="$dequoted_word"
esac
- __gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_" "$hide_dotfiles")"
+ __gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_")"
}
# Lists branches from the local repository.
@@ -2176,7 +2171,7 @@ _git_ls_files ()
# XXX ignore options like --modified and always suggest all cached
# files.
- __git_complete_index_file "--cached" hide-dotfiles
+ __git_complete_index_file "--cached"
}
_git_ls_remote ()
@@ -2409,9 +2404,9 @@ _git_mv ()
if [ $(__git_count_arguments "mv") -gt 0 ]; then
# We need to show both cached and untracked files (including
# empty directories) since this may not be the last argument.
- __git_complete_index_file "--cached --others --directory" hide-dotfiles
+ __git_complete_index_file "--cached --others --directory"
else
- __git_complete_index_file "--cached" hide-dotfiles
+ __git_complete_index_file "--cached"
fi
}
@@ -3231,7 +3226,7 @@ _git_rm ()
;;
esac
- __git_complete_index_file "--cached" hide-dotfiles
+ __git_complete_index_file "--cached"
}
_git_shortlog ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 02aaf71876..7a7594455c 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2360,6 +2360,7 @@ test_expect_success 'setup for path completion tests' '
"spaces in dir" \
árvíztűrő &&
touch simple-dir/simple-file \
+ simple-dir/.dotfile-in-dir \
"spaces in dir/spaces in file" \
"árvíztűrő/Сайн яваарай" &&
if test_have_prereq !MINGW &&
@@ -2380,6 +2381,11 @@ test_expect_success '__git_complete_index_file - simple' '
test_path_completion simple-dir/simple simple-dir/simple-file
'
+test_expect_success '__git_complete_index_file - dotfiles' '
+ test_path_completion "simple-dir/" "simple-dir/simple-file" &&
+ test_path_completion "simple-dir/." "simple-dir/.dotfile-in-dir"
+'
+
test_expect_success \
'__git_complete_index_file - escaped characters on cmdline' '
test_path_completion spac "spaces in dir" && # Bash will turn this
@@ -2789,7 +2795,8 @@ test_expect_success 'complete files' '
echo "out_sorted" >> .gitignore &&
git add .gitignore &&
- test_completion "git commit " ".gitignore" &&
+ test_completion "git commit " "" &&
+ test_completion "git commit ." ".gitignore" &&
git commit -m ignore &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 1/2] completion: hide dotfiles for selected path completion
From: Zakariyah Ali via GitGitGadget @ 2026-06-20 17:55 UTC (permalink / raw)
To: git; +Cc: Zakariyah Ali, Zakariyah Ali, Zakariyah Ali
In-Reply-To: <pull.2311.v3.git.git.1781978156.gitgitgadget@gmail.com>
From: Zakariyah Ali <zakariyahali100@gmail.com>
The completion helper for index paths uses git ls-files rather than
shell filename completion. As a result, leading-dot paths such as a
tracked .gitignore were offered even when the user had not started the
path with ".".
Hide leading-dot path components for git rm, git mv, and git ls-files
when completing an empty path component. Explicit dot completion is
still preserved, so git rm . can still complete .gitignore.
This matches standard shell filename completion behavior, where dotfiles
are hidden by default unless the user starts their input with a dot.
This also resolves four TODO comments in t/9902-completion.sh which
have been present since 2013 (commit ddf07bddef9a, "completion: add file
completion tests", 2013-04-27), expecting that .gitignore would not be
shown when completing on an empty path component.
Signed-off-by: Zakariyah Ali <zakariyahali100@gmail.com>
---
contrib/completion/git-completion.bash | 36 +++++++++++++++++---------
t/t9902-completion.sh | 10 ++-----
2 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbf..e8f8fab125 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -638,25 +638,33 @@ __git_ls_files_helper ()
}
-# __git_index_files accepts 1 or 2 arguments:
+# __git_index_files accepts 1 to 4 arguments:
# 1: Options to pass to ls-files (required).
# 2: A directory path (optional).
# If provided, only files within the specified directory are listed.
# Sub directories are never recursed. Path must have a trailing
# slash.
# 3: List only paths matching this path component (optional).
+# 4: Hide paths whose first component starts with a dot if this is
+# "hide-dotfiles" and the third argument is empty (optional).
__git_index_files ()
{
- local root="$2" match="$3"
+ local root="$2" match="$3" hide_dotfiles="${4-}"
+ local hide_dotfiles_awk=0
+ if [ "$hide_dotfiles" = "hide-dotfiles" ] && [ -z "$match" ]; then
+ hide_dotfiles_awk=1
+ fi
__git_ls_files_helper "$root" "$1" "${match:-?}" |
- awk -F / -v pfx="${2//\\/\\\\}" '{
+ awk -F / -v pfx="${2//\\/\\\\}" -v hide_dotfiles="$hide_dotfiles_awk" '{
paths[$1] = 1
}
END {
for (p in paths) {
if (substr(p, 1, 1) != "\"") {
# No special characters, easy!
+ if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+ continue
print pfx p
continue
}
@@ -675,8 +683,10 @@ __git_index_files ()
# We have seen the same directory unquoted,
# skip it.
continue
- else
- print pfx p
+
+ if (hide_dotfiles == 1 && substr(p, 1, 1) == ".")
+ continue
+ print pfx p
}
}
function dequote(p, bs_idx, out, esc, esc_idx, dec) {
@@ -721,13 +731,15 @@ __git_index_files ()
}'
}
-# __git_complete_index_file requires 1 argument:
+# __git_complete_index_file accepts 1 or 2 arguments:
# 1: the options to pass to ls-file
+# 2: Hide paths whose first component starts with a dot if this is
+# "hide-dotfiles" and the current word is empty (optional).
#
# The exception is --committable, which finds the files appropriate commit.
__git_complete_index_file ()
{
- local dequoted_word pfx="" cur_
+ local dequoted_word pfx="" cur_ hide_dotfiles="${2-}"
__git_dequote "$cur"
@@ -740,7 +752,7 @@ __git_complete_index_file ()
cur_="$dequoted_word"
esac
- __gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_")"
+ __gitcomp_file_direct "$(__git_index_files "$1" "$pfx" "$cur_" "$hide_dotfiles")"
}
# Lists branches from the local repository.
@@ -2164,7 +2176,7 @@ _git_ls_files ()
# XXX ignore options like --modified and always suggest all cached
# files.
- __git_complete_index_file "--cached"
+ __git_complete_index_file "--cached" hide-dotfiles
}
_git_ls_remote ()
@@ -2397,9 +2409,9 @@ _git_mv ()
if [ $(__git_count_arguments "mv") -gt 0 ]; then
# We need to show both cached and untracked files (including
# empty directories) since this may not be the last argument.
- __git_complete_index_file "--cached --others --directory"
+ __git_complete_index_file "--cached --others --directory" hide-dotfiles
else
- __git_complete_index_file "--cached"
+ __git_complete_index_file "--cached" hide-dotfiles
fi
}
@@ -3219,7 +3231,7 @@ _git_rm ()
;;
esac
- __git_complete_index_file "--cached"
+ __git_complete_index_file "--cached" hide-dotfiles
}
_git_shortlog ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 28f61f08fb..02aaf71876 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2811,17 +2811,15 @@ test_expect_success 'complete files' '
touch untracked &&
- : TODO .gitignore should not be here &&
test_completion "git rm " <<-\EOF &&
- .gitignore
modified
EOF
+ test_completion "git rm ." ".gitignore" &&
+
test_completion "git clean " "untracked" &&
- : TODO .gitignore should not be here &&
test_completion "git mv " <<-\EOF &&
- .gitignore
modified
EOF
@@ -2832,9 +2830,7 @@ test_expect_success 'complete files' '
mkdir untracked-dir &&
- : TODO .gitignore should not be here &&
test_completion "git mv modified " <<-\EOF &&
- .gitignore
dir
modified
untracked
@@ -2843,9 +2839,7 @@ test_expect_success 'complete files' '
test_completion "git commit " "modified" &&
- : TODO .gitignore should not be here &&
test_completion "git ls-files " <<-\EOF &&
- .gitignore
dir
modified
EOF
--
gitgitgadget
^ permalink raw reply related
* [PATCH v3 0/2] completion: hide dotfiles for selected path completion
From: Zakariyah Ali via GitGitGadget @ 2026-06-20 17:55 UTC (permalink / raw)
To: git; +Cc: Zakariyah Ali, Zakariyah Ali
In-Reply-To: <pull.2311.v2.git.git.1779808987825.gitgitgadget@gmail.com>
The completion helper for index paths uses git ls-files rather than shell
filename completion. As a result, leading-dot paths such as a tracked
.gitignore were offered even when the user had not started the path with ..
Hide leading-dot path components for git rm, git mv, and git ls-files when
completing an empty path component. Explicit dot completion is still
preserved, so git rm . can still complete .gitignore.
This removes the existing TODO expectations in t/t9902-completion.sh and
adds coverage for explicit dot completion.
Validation:
* git diff --check -- contrib/completion/git-completion.bash
t/t9902-completion.sh
* bash -n contrib/completion/git-completion.bash
* ./t9902-completion.sh
Zakariyah Ali (2):
completion: hide dotfiles for selected path completion
completion: hide dotfiles by default for path completion
contrib/completion/git-completion.bash | 53 +++++++++++++++-----------
t/t9902-completion.sh | 19 ++++-----
2 files changed, 40 insertions(+), 32 deletions(-)
base-commit: 9b7fa37559a1b95ee32e32858b0d038b4cf583e5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2311%2Falibaba0010%2Fcompletion-hide-dotfiles-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2311/alibaba0010/completion-hide-dotfiles-v3
Pull-Request: https://github.com/git/git/pull/2311
Range-diff vs v2:
1: 056e239e06 = 1: 056e239e06 completion: hide dotfiles for selected path completion
-: ---------- > 2: 7482ee4645 completion: hide dotfiles by default for path completion
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH 0/2] Silence po catalog output under "make -s"
From: Johannes Sixt @ 2026-06-20 15:46 UTC (permalink / raw)
To: Harald Nordgren; +Cc: git, Harald Nordgren via GitGitGadget
In-Reply-To: <pull.2339.git.git.1781459539.gitgitgadget@gmail.com>
Am 14.06.26 um 19:52 schrieb Harald Nordgren via GitGitGadget:
> The gitk and git-gui catalog rules sent msgfmt --statistics output (and a
> "Generating catalog" line) to stderr, so it survived "make -s". Emit it only
> when "-s" is absent, keeping a quiet build silent and a verbose build
> unchanged.
I think that the statistics output isn't needed. A data point is that
the Git repository doesn't use --statistics: 2f12b31b746c ("Makefile:
don't invoke msgfmt with --statistics", 2021-12-17). So, my suggestion
would be to just remove the option.
We don't need the "Generating catalog" message, either. Just remove it
as well.
-- Hannes
^ permalink raw reply
* Re: [GSoC Patch v6 1/4] path: introduce append_formatted_path() for shared path formatting
From: K Jayatheerth @ 2026-06-20 16:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, jltobler, lucasseikioshiro, phillip.wood, sandals,
kumarayushjha123, a3205153416, kristofferhaugsbakk
In-Reply-To: <xmqqbjd5guci.fsf@gitster.g>
Hi Junio,
> It often, even though not always, is a sign of a bad topic structure
> to have an insertion-only patch without any removal of existing
> code, that adds totally unused code.
>
> If the step is to "extract the core algorithm", shouldn't it be able
> to replace existing code already?
Giving the helper and converting its
first caller (`rev-parse`) in the same step proves the implementation
avoids leaving unused code lingering in the tree, even temporarily.
> We may want to add new features to this helper function near the end
> of the topic, but wouldn't it make sense for the topic to first
> consolidate various path formatting logic already present in the
> existing code into a single helper for ease of extending it (which
> means replacing open-coded logic in existing code paths with a call
> to the new helper, which would have a code that may look very
> similar to the original code that was replaced with a single call to
> the helper function), and then expose the helper for use by new
> callers, and finally further add new features that existing code
> paths wouldn't have needed but the new callers would want?
Consolidating the existing logic first ensures
we aren't introducing unnecessary complexity up front. I agree with
restructuring the topic this way.
> How else can we make sure this new implementation added by the first
> step in the series is (1) capable enough to reproduce what we
> already have in different parts of the system, (2) does not bring in
> what the current codebase does not need, and (3) bug-to-bug
> compatible with the existing code paths?
Introducing the helper and swapping out the `rev-parse`
implementation in the same step is the best way to prove bug-to-bug
compatibility and demonstrate its immediate utility.
For v7, I will squash patches 1 and 2 together so that the extraction
and the replacement happen simultaneously, guaranteeing that the new
`append_formatted_path()` perfectly mirrors the old behavior before
we introduce the new `path.*` callers.
Thanks for taking the time to explain the rationale!
- K Jayatheerth
^ permalink raw reply
* [PATCH] meson: wire up USE_NSEC build knob
From: D. Ben Knoble @ 2026-06-20 16:00 UTC (permalink / raw)
To: git
Cc: D. Ben Knoble, brian m . carlson, Junio C Hamano, Jeff King,
Patrick Steinhardt, Ramsay Jones
Autotools-style builds permit enabling USE_NSEC for cases where that's
desired; the equivalent knob is missing from meson-based builds.
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---
meson.build | 4 ++++
meson_options.txt | 2 ++
2 files changed, 6 insertions(+)
diff --git a/meson.build b/meson.build
index 3247697f74..85a11119c5 100644
--- a/meson.build
+++ b/meson.build
@@ -838,6 +838,10 @@ if help_format_opt != 'man'
libgit_c_args += '-DDEFAULT_HELP_FORMAT="' + help_format_opt + '"'
endif
+if get_option('nanosec')
+ libgit_c_args += '-DUSE_NSEC'
+endif
+
libgit_include_directories = [ '.' ]
libgit_dependencies = [ ]
diff --git a/meson_options.txt b/meson_options.txt
index d936ada098..1bc75278a8 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -21,6 +21,8 @@ option('runtime_prefix', type: 'boolean', value: false,
description: 'Resolve ancillary tooling and support files relative to the location of the runtime binary instead of hard-coding them into the binary.')
option('sane_tool_path', type: 'array', value: [],
description: 'An array of paths to pick up tools from in case the normal tools are broken or lacking.')
+option('nanosec', type: 'boolean', value: false,
+ description: 'Care about sub-second file mtimes and ctimes.')
# Build information compiled into Git and other parts like documentation.
option('build_date', type: 'string', value: '',
base-commit: 0c8ab3ebcc76981376809c8fe632d0fe18e93347
--
2.55.0.rc0.738.g0c8ab3ebcc.dirty
^ permalink raw reply related
* Re: git-diff in a worktree is an order of magnitude slower?
From: D. Ben Knoble @ 2026-06-20 15:57 UTC (permalink / raw)
To: Jeff King; +Cc: Git
In-Reply-To: <20260611085526.GL2191159@coredump.intra.peff.net>
Coming back to index refreshing…
On Thu, Jun 11, 2026 at 4:55 AM Jeff King <peff@peff.net> wrote:
>
> On Tue, Jun 09, 2026 at 01:15:11PM -0400, D. Ben Knoble wrote:
>
> > > Which implies that the entries are stat dirty. And indeed, if I run:
> > >
> > > git -C linux update-index --refresh
> > >
> > > now they both take ~20ms.
> >
> > Ah, TIL about --refresh. I suppose it could be nice if "git diff"
> > updated the index in this way, but that sounds like a band-aid. Maybe
> > creating a fresh worktree should do the equivalent to make sure it's
> > considered "fresh"?
>
> I think "git diff" _does_ refresh the index internally (that's what
> takes so long!). I thought we then wrote out the result, but maybe we
> don't notice that it needs an update for some reason?
>
> I'm pretty sure "git status" does something similar, though running it
> in a slow working tree _does_ seem to make things faster. Maybe it's
> more aggressive about doing the update.
Thanks for the status pointer:
- cmd_status calls refresh_index and repo_updated_index_if_able
- those same calls are wrapped in refresh_index_quietly in builtin/diff.c
But the refresh_index_quietly call is guarded by (effectively; the
actual code uses rev.diffopt.skip_stat_unmatch)
1 < !!diff_auto_refresh_index
which dates to aecbf914c4 (git-diff: resurrect the traditional empty
"diff --git" behaviour, 2007-08-31). On my system that comparison is
false because the double-negation produces 1
(diff_auto_refresh_index=1 or the result of git_config_bool). Or at
least, I don't see it get written to elsewhere (maybe that's supposed
to happen in diff.c:diffcore_skip_stat_unmatch in this case and isn't?
Idk. (Even dirtying the worktree as a hypothesis that only when a diff
is found does the counter get bumped doesn't seem to work.)
So… has that conditional been quietly dead all this time? I can't
imagine that's right, but…
> > > I'd have thought USE_NSEC was the default these days, but looks like it
> > > isn't? Try building with that and I'll bet it goes away entirely.
> >
> > Thanks, I'll take a look.
> >
> > I can see on my Macbook that at least Meson does automatically set
> > either USE_ST_TIMESPEC or NO_NSEC automatically, but has no option to
> > enabled USE_NSEC and try that. I can probably write that patch (which
> > I'll do to test), and I can send it along with the "worktree add
> > should refresh the index" if you think that's an appropriate thing to
> > do.
>
> I think NO_NSEC is about not looking at the nsec fields of stat structs
> (since they might not exist). But we don't actually use them for stat
> matching unless USE_NSEC is set.
>
> I guess the distinction goes back to c06ff4908b (Record ns-timestamps if
> possible, but do not use it without USE_NSEC, 2009-03-04), which details
> some reasons you might not want USE_NSEC. Feels like it ought to be a
> run-time config, though, and maybe even something that gets auto-probed
> by git-init.
>
> Definitely not an area I have looked at much, though, nor thought hard
> about. So there might be gotchas. :)
>
> -Peff
Looks like adding USE_NSEC to my build did make the issue go away (the
patch is short, and I'll send it anyway for folks to have the knob),
but that now seems like a band-aid to me based on my confusion above.
--
D. Ben Knoble
^ permalink raw reply
* Re: [RFH] Why do osx CI jobs so unreliable?
From: Michael Montalbo @ 2026-06-20 15:33 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Junio C Hamano
Patrick Steinhardt <ps@pks.im> writes:
> So I strongly suspect that it most be one of the t555* tests.
> [...]
> Maybe this is something that's specific to GitHub's environment...
I think you're right it's t5551/t5559. The runs Junio linked:
osx-clang cancelled 360min
osx-gcc cancelled 360min
osx-reftable success 35min
osx-meson success 61min
All four run the same t5551/t5559 under EXPENSIVE. The two that
finished differ in just two ways, which look like the levers:
osx-reftable generates the 100k-ref advertisement in ~24ms vs ~1.2s
for loose refs on macOS (so much less time mid-response), and
osx-meson runs tests at nproc while the prove jobs hardcode --jobs=10
on a 3-core runner (over recent master/next the prove jobs hang ~40%,
meson ~10%).
When it is wedged the whole chain sits at 0% CPU. upload-pack is
blocked in write() on the ls-refs advertisement, curl blocked in
select(). So it looks like an HTTP/2 flow-control stall on the
response side. The same stall resets itself after ~60-85s on my Linux
box and on a bare-metal Mac, but not on the GitHub runner; I haven't
pinned down why yet.
On the chance those two levers are the fix, a branch off master:
https://github.com/mmontalbo/git/tree/mm/macos-ci-hang-fix
- pack the refs in t5551's enormous-ref-negotiation test (doesn't
change what it checks on the wire, just avoids re-reading 100k loose
files to advertise them, like reftable already does)
- use the core count for $JOBS on the GitHub macOS path, matching the
GitLab branch in the same ci/lib.sh and what meson does
I ran the two macOS jobs under EXPENSIVE about eight times with these
and they all finished in ~30-44min instead of hanging. Happy to send
out a patch if it's helpful.
^ permalink raw reply
* Re: [PATCH] environment: use 'repo->initialized' for repo_protect_hfs() and repo_protect_ntfs()
From: Junio C Hamano @ 2026-06-20 15:18 UTC (permalink / raw)
To: Tian Yuchen; +Cc: git, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260620140957.667820-1-cat@malon.dev>
Tian Yuchen <cat@malon.dev> writes:
> To match how we refrain from calling repo_config_values() on an
> uninitialized instance of a repository object in other two topics
> that deal with ignore_case and trust_executable_bit, check the
> repo->initialized bit instead of the repo->gitdir member.
OK.
> Base commit: 43192e7977f5f05138abcdb3212a3f87ab513bef
This line does not belong here. Besides, you do not build directly
on top of 'next', ever.
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
> Signed-off-by: Tian Yuchen <cat@malon.dev>
> ---
> environment.c | 4 ++--
> environment.h | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
I'll queue the change directly on top of ty/move-protect-hfs-ntfs
topic, which will be merged to 'next'.
Thanks.
> diff --git a/environment.c b/environment.c
> index 6ee11e9fc8..8f0c1c4f25 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -130,14 +130,14 @@ int is_bare_repository(struct repository *repo)
>
> int repo_protect_ntfs(struct repository *repo)
> {
> - return repo->gitdir ?
> + return (repo && repo->initialized) ?
> repo_config_values(repo)->protect_ntfs :
> PROTECT_NTFS_DEFAULT;
> }
>
> int repo_protect_hfs(struct repository *repo)
> {
> - return repo->gitdir ?
> + return (repo && repo->initialized) ?
> repo_config_values(repo)->protect_hfs :
> PROTECT_HFS_DEFAULT;
> }
> diff --git a/environment.h b/environment.h
> index d188955f5b..8aaedcfea3 100644
> --- a/environment.h
> +++ b/environment.h
> @@ -137,8 +137,8 @@ int git_default_core_config(const char *var, const char *value,
>
> /*
> * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
> - * They check `repo->gitdir` to prevent calling repo_config_values()
> - * before the configuration is loaded or in bare environments.
> + * They check `repo->initialized` to prevent calling `repo_config_values()`
> + * before the repository setup is fully complete or in non-git environments.
> */
> int repo_protect_hfs(struct repository *repo);
> int repo_protect_ntfs(struct repository *repo);
^ permalink raw reply
* Re: [PATCH v6 2/3] revision: add peek functions for lookahead
From: Junio C Hamano @ 2026-06-20 15:15 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, SZEDER Gábor, Michael Montalbo, krka, ayu.chandekar,
chandrapratap3519, christian.couder, jltobler, karthik.188, peff,
phillip.wood, siddharthasthana31, Kristofer Karlsson
In-Reply-To: <xmqqzf0pfefp.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> I _think_ this is OK, as "--graph" sets .rewrite_parents bit (as
> well as .topo_order bit) on, which makes want_ancestry() to return
> true. Which in turn means even if -L is in effect, we will not call
> line_log_process_ranges_arbitrary_commit() that is the only source
> of side effect in this function. Somebody needs to sanity check
> this, but we may want to leave an in-code comment to warn future
> developers not to call get_commit_action() on random commits outside
> of the normal history traversal under what condition (namely, -L
> without rewrite_parents).
>
> Even better, perhaps add
>
> if (revs->line_level_traverse && !want_ancestry(revs))
> BUG("do not call this");
>
> at the beginning of revision_has_commits_after() function, and
> describe why in the header file comment for this function below?
Having said all that, in the longer term we might be better off if
we fix the line-log code so that get_commit_action() becomes a pure
function again.
It might be a very simple change to move the "if we are doing -L and
!want_ancestry(), call the line_log_process_ranges_arbitrary_commit()"
to simplify_commit() before it calls get_commit_action(), but I
haven't thought things through.
[jc: Michael Cc'ed as there are a few topics on line-log recently
from him; SZEDER Cc'ed as his 3cb9d2b6 (line-log: more responsive,
incremental 'git log -L', 2020-05-11) introduced this side effect
there.]
In any case, this is a remote tangent that does not affect how we
want to proceed with this series ;-).
^ permalink raw reply
* Re: [PATCH v6 2/3] revision: add peek functions for lookahead
From: Junio C Hamano @ 2026-06-20 14:56 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, krka, ayu.chandekar, chandrapratap3519, christian.couder,
jltobler, karthik.188, peff, phillip.wood, siddharthasthana31,
Kristofer Karlsson
In-Reply-To: <20260620-ps-pre-commit-indent-v6-2-cdc6d8fd5fbc@gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> The graph code in a subsequent commit needs to be able to look ahead in
> order to set indentation-related flags.
>
> Using revs->commits is brittle and the data structure that holds the
> pending commits might change in the future.
>
> Add two functions that abstract this for the graph.
>
> Helped-by: Kristofer Karlsson <stoansen@gmail.com>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> revision.c | 38 ++++++++++++++++++++++++++++++++++++++
> revision.h | 10 ++++++++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/revision.c b/revision.c
> index e91d7e1f11..a472a28853 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -3708,6 +3708,44 @@ static unsigned int count_explore_walked;
> static unsigned int count_indegree_walked;
> static unsigned int count_topo_walked;
>
> +struct commit *revision_peek_next_commit (struct rev_info *revs)
> +{
> + struct topo_walk_info *info = revs->topo_walk_info;
> +
> + if (info)
> + return prio_queue_peek(&info->topo_queue);
> + if (revs->commits)
> + return revs->commits->item;
> +
> + return NULL;
> +}
OK. "If we are doing topo_walk, topo_queue is the priority queue to
peek into, otherwise revs->commits list is being used" is a bit too
intimate implementation detail I am not comfortable to depend on,
but as long as it is contained inside revision.c it should be OK.
Lose the space between the function name and its (parameter list)
from this and the next function.
> +int revision_has_commits_after (struct rev_info *revs, int n)
> +{
> + struct topo_walk_info *info = revs->topo_walk_info;
> +
> + if (info) {
> + int visible = 0;
> + for (size_t i = 0; i < info->topo_queue.nr && visible < n; i++) {
> + struct commit *c = info->topo_queue.array[i].data;
> + if (get_commit_action(revs, c) == commit_show)
> + visible++;
> + }
> + return visible > n-1;
> + }
> + if (revs->commits) {
> + struct commit_list *cl;
> + int visible = 0;
> + for (cl = revs->commits; cl && visible < n; cl = cl->next) {
> + if (get_commit_action(revs, cl->item) == commit_show)
> + visible++;
> + }
> + return visible > n-1;
> + }
> +
> + return 0;
> +}
Regarding the use of get_commit_action() here, I wondered if this is
safe, because usually get_commit_action() is called only once per
commit during history traversal from simplify_commit(), but this
patch adds calls to it for all of the remaining commits being
processed without consuming them (so get_commit_action() will be
called on these commits again later as the history traversal
progresses).
If get_commit_action() a pure function without any side effects,
this may be safe, but line-log has something with side effect in the
function.
I _think_ this is OK, as "--graph" sets .rewrite_parents bit (as
well as .topo_order bit) on, which makes want_ancestry() to return
true. Which in turn means even if -L is in effect, we will not call
line_log_process_ranges_arbitrary_commit() that is the only source
of side effect in this function. Somebody needs to sanity check
this, but we may want to leave an in-code comment to warn future
developers not to call get_commit_action() on random commits outside
of the normal history traversal under what condition (namely, -L
without rewrite_parents).
Even better, perhaps add
if (revs->line_level_traverse && !want_ancestry(revs))
BUG("do not call this");
at the beginning of revision_has_commits_after() function, and
describe why in the header file comment for this function below?
> diff --git a/revision.h b/revision.h
> index 00c392be37..a10c6b0940 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -572,4 +572,14 @@ int rewrite_parents(struct rev_info *revs,
> */
> struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
>
> +/*
> + * Peek into revision's next commit without consuming it.
> + */
> +struct commit *revision_peek_next_commit(struct rev_info *revs);
> +
> +/*
> + * Check if there are n more commits to be shown yet.
> + */
> +int revision_has_commits_after(struct rev_info *revs, int n);
> +
> #endif
^ permalink raw reply
* Re: [GSoC Patch v6 1/4] path: introduce append_formatted_path() for shared path formatting
From: Junio C Hamano @ 2026-06-20 14:27 UTC (permalink / raw)
To: K Jayatheerth
Cc: git, jltobler, lucasseikioshiro, phillip.wood, sandals,
kumarayushjha123, a3205153416, kristofferhaugsbakk
In-Reply-To: <20260620031644.353772-2-jayatheerthkulkarni2005@gmail.com>
K Jayatheerth <jayatheerthkulkarni2005@gmail.com> writes:
> The path-formatting logic in builtin/rev-parse.c is tightly coupled
> to that command and writes directly to stdout, making it impossible
> for other builtins to reuse.
>
> Extract the core algorithm into append_formatted_path() in path.c
> and expose a path_format enum in path.h so that any builtin can
> format paths consistently without duplicating logic.
>
> Mentored-by: Justin Tobler <jltobler@gmail.com>
> Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
> path.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> path.h | 30 +++++++++++++++++++++++++
> 2 files changed, 99 insertions(+)
It often, even though not always, is a sign of a bad topic structure
to have an insertion-only patch without any removal of existing
code, that adds totally unused code.
If the step is to "extract the core algorithm", shouldn't it be able
to replace existing code already?
We may want to add new features to this helper function near the end
of the topic, but wouldn't it make sense for the topic to first
consolidate various path formatting logic already present in the
existing code into a single helper for ease of extending it (which
means replacing open-coded logic in existing code paths with a call
to the new helper, which would have a code that may look very
similar to the original code that was replaced with a single call to
the helper function), and then expose the helper for use by new
callers, and finally further add new features that existing code
paths wouldn't have needed but the new callers would want?
How else can we make sure this new implementation added by the first
step in the series is (1) capable enough to reproduce what we
already have in different parts of the system, (2) does not bring in
what the current codebase does not need, and (3) bug-to-bug
compatible with the existing code paths?
> diff --git a/path.c b/path.c
> index d7e17bf174..6d8e892ada 100644
> --- a/path.c
> +++ b/path.c
> @@ -1579,6 +1579,75 @@ char *xdg_cache_home(const char *filename)
> return NULL;
> }
>
> +void append_formatted_path(struct strbuf *dest, const char *path,
> + const char *prefix, enum path_format format)
> +{
> + switch (format) {
> + case PATH_FORMAT_UNMODIFIED:
> + strbuf_addstr(dest, path);
> + break;
> +
> + case PATH_FORMAT_RELATIVE: {
> + struct strbuf relative_buf = STRBUF_INIT;
> + struct strbuf real_path = STRBUF_INIT;
> + struct strbuf real_prefix = STRBUF_INIT;
> + char *cwd = NULL;
> +
> + /*
> + * We don't ever produce a relative path if prefix is NULL,
> + * so set the prefix to the current directory so that we can
> + * produce a relative path whenever possible.
> + */
> + if (!prefix)
> + prefix = cwd = xgetcwd();
> +
> + if (!is_absolute_path(path)) {
> + strbuf_realpath_forgiving(&real_path, path, 1);
> + path = real_path.buf;
> + }
> + if (!is_absolute_path(prefix)) {
> + strbuf_realpath_forgiving(&real_prefix, prefix, 1);
> + prefix = real_prefix.buf;
> + }
> +
> + strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
> +
> + strbuf_release(&relative_buf);
> + strbuf_release(&real_path);
> + strbuf_release(&real_prefix);
> + free(cwd);
> + break;
> + }
> +
> + case PATH_FORMAT_RELATIVE_IF_SHARED: {
> + struct strbuf relative_buf = STRBUF_INIT;
> +
> + /*
> + * If we're using RELATIVE_IF_SHARED mode, then we want an
> + * absolute path unless the two share a common prefix, so don't
> + * default the prefix to the current working directory. Doing so
> + * would cause a relative path to always be produced if possible.
> + */
> + strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
> + strbuf_release(&relative_buf);
> + break;
> + }
> +
> + case PATH_FORMAT_CANONICAL: {
> + struct strbuf canonical_buf = STRBUF_INIT;
> +
> + strbuf_realpath_forgiving(&canonical_buf, path, 1);
> + strbuf_addbuf(dest, &canonical_buf);
> +
> + strbuf_release(&canonical_buf);
> + break;
> + }
> +
> + default:
> + BUG("unknown path_format value %d", format);
> + }
> +}
> +
> REPO_GIT_PATH_FUNC(squash_msg, "SQUASH_MSG")
> REPO_GIT_PATH_FUNC(merge_msg, "MERGE_MSG")
> REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR")
> diff --git a/path.h b/path.h
> index 4c2958a903..4d982a2c8e 100644
> --- a/path.h
> +++ b/path.h
> @@ -262,6 +262,36 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
> int safe_create_file_with_leading_directories(struct repository *repo,
> const char *path);
>
> +/**
> + * The formatting strategy to apply when writing a path into a buffer.
> + */
> +enum path_format {
> + /* Output the path exactly as-is without any modifications. */
> + PATH_FORMAT_UNMODIFIED,
> +
> + /* Output a path relative to the provided directory prefix. */
> + PATH_FORMAT_RELATIVE,
> +
> + /* Output a relative path only if the path shares a root with the prefix. */
> + PATH_FORMAT_RELATIVE_IF_SHARED,
> +
> + /* Output a fully resolved, absolute canonical path. */
> + PATH_FORMAT_CANONICAL
> +};
> +
> +/**
> + * Format a path according to the specified formatting strategy and append
> + * the result to the given strbuf.
> + *
> + * `dest` : The string buffer to append the formatted path to.
> + * `path` : The path string that needs to be formatted.
> + * `prefix` : The directory prefix to calculate relative offsets against.
> + * Pass NULL to default to the current working directory where applicable.
> + * `format` : The formatting behavior rule to execute.
> + */
> +void append_formatted_path(struct strbuf *dest, const char *path,
> + const char *prefix, enum path_format format);
> +
> # ifdef USE_THE_REPOSITORY_VARIABLE
> # include "strbuf.h"
> # include "repository.h"
^ permalink raw reply
* [PATCH] environment: use 'repo->initialized' for repo_protect_hfs() and repo_protect_ntfs()
From: Tian Yuchen @ 2026-06-20 14:09 UTC (permalink / raw)
To: git; +Cc: Tian Yuchen, Christian Couder, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <xmqqo6h6jvuk.fsf@gitster.g>
To match how we refrain from calling repo_config_values() on an
uninitialized instance of a repository object in other two topics
that deal with ignore_case and trust_executable_bit, check the
repo->initialized bit instead of the repo->gitdir member.
Base commit: 43192e7977f5f05138abcdb3212a3f87ab513bef
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
environment.c | 4 ++--
environment.h | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/environment.c b/environment.c
index 6ee11e9fc8..8f0c1c4f25 100644
--- a/environment.c
+++ b/environment.c
@@ -130,14 +130,14 @@ int is_bare_repository(struct repository *repo)
int repo_protect_ntfs(struct repository *repo)
{
- return repo->gitdir ?
+ return (repo && repo->initialized) ?
repo_config_values(repo)->protect_ntfs :
PROTECT_NTFS_DEFAULT;
}
int repo_protect_hfs(struct repository *repo)
{
- return repo->gitdir ?
+ return (repo && repo->initialized) ?
repo_config_values(repo)->protect_hfs :
PROTECT_HFS_DEFAULT;
}
diff --git a/environment.h b/environment.h
index d188955f5b..8aaedcfea3 100644
--- a/environment.h
+++ b/environment.h
@@ -137,8 +137,8 @@ int git_default_core_config(const char *var, const char *value,
/*
* Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
- * They check `repo->gitdir` to prevent calling repo_config_values()
- * before the configuration is loaded or in bare environments.
+ * They check `repo->initialized` to prevent calling `repo_config_values()`
+ * before the repository setup is fully complete or in non-git environments.
*/
int repo_protect_hfs(struct repository *repo);
int repo_protect_ntfs(struct repository *repo);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3] SubmittingPatches: address design critiques
From: Junio C Hamano @ 2026-06-20 14:09 UTC (permalink / raw)
To: Michael Montalbo; +Cc: code, git
In-Reply-To: <CAC2Qwm+WcGkd9pAV5=JL1hfCDRisGQRFmdfOsMTrMWyx7aa65A@mail.gmail.com>
Michael Montalbo <mmontalbo@gmail.com> writes:
> Slight reflow suggestions:
>
> Defend your design decisions on the list first; work with reviewers and
> other members to improve the design before revising the implementation.
> This will avoid wasting effort on an implementation before its design is
> solid.
Yeah, making the last part a separate sentence does make it much
easier to follow. Will use.
Thanks.
^ permalink raw reply
* Re: [PATCH v4 0/1] environment: move protect_hfs and protect_ntfs into repo_config_values
From: Tian Yuchen @ 2026-06-20 13:38 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Christian Couder, Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <xmqqjyrujvco.fsf@gitster.g>
On 6/20/26 01:25, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> diff --git c/environment.h w/environment.h
>> index fdd9775900..b1ae4a70de 100644
>> --- c/environment.h
>> +++ w/environment.h
>> @@ -127,8 +127,8 @@ int git_default_core_config(const char *var, const char *value,
>>
>> /*
>> * Getters for the `protect_hfs` and `protect_ntfs` fields of `struct repo_config_values`.
>> - * They check `repo->gitdir` to prevent calling repo_config_values()
>> - * before the configuration is loaded or in bare environments.
>> + * They check `repo->initialized` to prevent calling `repo_config_values()`
>> + * before the repository setup is fully complete or in non-git environments.
>> */
>> int repo_protect_hfs(struct repository *repo);
>> int repo_protect_ntfs(struct repository *repo);
>
> Another thing we should remember (but should *NOT* do while these
> topics are still in flight) to do is to consolidate these comments
> into one. The hfs and htfs getters are covered by the same single
> comment, but ignorecase and trustexecutable bit getters have their
> own comments, only because they came in different topics. We should
> conslidate them into a single comment block once all of these have
> landed in 'master', which may happen soon after 2.55 final gets
> tagged.
>
I understand. I’ll send the hfs/ntfs fix patch right away, and will
clean up the comments once all of the associated patches (there will be
more in the next few weeks) have been merged into master. ;)
Thanks, yuchen
^ permalink raw reply
* [PATCH/RFC 6/6] Documentation/technical: add paint-down-to-common doc
From: Kristofer Karlsson via GitGitGadget @ 2026-06-20 10:36 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Kristofer Karlsson
In-Reply-To: <pull.2149.git.1781951820.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.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
Documentation/Makefile | 1 +
Documentation/technical/meson.build | 1 +
.../technical/paint-down-to-common.adoc | 130 ++++++++++++++++++
3 files changed, 132 insertions(+)
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..e677cce84d
--- /dev/null
+++ b/Documentation/technical/paint-down-to-common.adoc
@@ -0,0 +1,130 @@
+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
+-----------
+
+Termination happens when we can prove that no extra progress is
+possible. We are done with the main loop when one of the following
+conditions holds:
+
+ 1. The queue is empty.
+ 2. The queue only contains STALE entries.
+ 3. Side-exhaustion: the walk has reached the finite region and one
+ of the sides is fully exhausted.
+
+The loop waits for all pending merge-base candidates to be popped
+and recorded before any early exit fires, so no separate drain phase
+is needed after termination.
+
+Stale entry condition
+~~~~~~~~~~~~~~~~~~~~~
+If all entries are stale we cannot find any new merge bases since
+that requires at least one enqueued side node meeting the other side.
+However, we could still invalidate merge bases (if there are more
+than one). This is unnecessary since `remove_redundant()` will clean
+that up as a post-process step.
+
+Side-exhaustion
+~~~~~~~~~~~~~~~
+A commit is *exclusive* to one side if it carries that side's paint
+but not the other (e.g. PARENT1 without PARENT2).
+
+If we have reached the finite region of the graph, no future
+traversal step can add paint to an already-visited commit. Thus if
+there are no exclusive PARENT2 commits in the queue, no additional
+PARENT2 paint can be introduced into the walk. Even if exclusive
+PARENT1 commits remain, no new merge-base candidates can be
+discovered. The same holds symmetrically for PARENT1.
+
+This invariant is only valid in the finite region of the graph.
+
+Related documentation
+---------------------
+
+ - `Documentation/technical/commit-graph.adoc` -- generation numbers
+ and the reachability closure property.
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox