* Re: [PATCH v3 4/8] promisor-remote: add 'local_name' to 'struct promisor_info'
From: Junio C Hamano @ 2026-05-20 0:12 UTC (permalink / raw)
To: Christian Couder
Cc: git, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Toon Claes, Christian Couder
In-Reply-To: <20260519153808.494105-5-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> struct promisor_info {
> - const char *name;
> + const char *name; /* name the server advertised */
> + const char *local_name; /* name used locally (may be auto-generated) */
OK.
> @@ -449,6 +450,7 @@ struct promisor_info {
> static void promisor_info_free(struct promisor_info *p)
> {
> free((char *)p->name);
> + free((char *)p->local_name);
> free((char *)p->url);
> free((char *)p->filter);
> free((char *)p->token);
Having to cast away constness is irritating, but to the users of the
struct it may be safer to mark the members const so that they do not
touch them, perhaps. It is not a new problem with this patch but is
inherited from the existing code, so let's not worry too much about
it.
> +static const char *promisor_info_internal_name(struct promisor_info *p)
> +{
> + return p->local_name ? p->local_name : p->name;
> +}
Hmph.
> @@ -829,7 +836,7 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised,
> {
> struct promisor_info *p;
> struct string_list_item *item;
> - const char *remote_name = advertised->name;
> + const char *remote_name = promisor_info_internal_name(advertised);
Is this really a "remote_name", though? As ...
> @@ -937,7 +944,8 @@ static void filter_promisor_remote(struct repository *repo,
> /* Apply accepted remotes to the stable repo state */
> for_each_string_list_item(item, accepted_remotes) {
> struct promisor_info *info = item->util;
> - struct promisor_remote *r = repo_promisor_remote_find(repo, info->name);
> + const char *local = promisor_info_internal_name(info);
... this name "local" is "the name the thing is locally known to
us", promisor_info_local_name() might be a better name? I dunno.
I jsut found it odd that the return value of the same function is
stored in variables named "remote" and "local" at the same time ;-)
^ permalink raw reply
* Re: [PATCH v3 1/2] builtin/maintenance: fix locking with "--detach"
From: Taylor Blau @ 2026-05-20 0:10 UTC (permalink / raw)
To: Junio C Hamano
Cc: Patrick Steinhardt, git, Jean-Christophe Manciot,
Mikael Magnusson, Jeff King, Derrick Stolee
In-Reply-To: <xmqqy0hnipy4.fsf@gitster.g>
On Wed, May 13, 2026 at 07:06:27PM +0900, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
>
> > Note that this is a broader fix, as we now always reassign tempfiles
> > when daemonizing. This is a natural consequence of the semantics of
> > `daemonize()` though, as it essentially promises to continue running the
> > current process in the background.
>
> Exactly. I do agree that it is the right wy to look at it. The
> process that daemonise creates and leaves in the background is
> logically the process that continues to execute the service the
> process the user started, and unless the original process explicitly
> says "we are done serving this thing" and cleans up tempfile or
> lockfile it needed to serve that thing, it is natural to make the
> surviving process to take over the responsibility.
Yeah, this is how I had been thinking about it as well.
Thanks, Patrick, for making the change. I think that this series is in a
good spot, though I'd like to hear from Peff who had some comments on
the second patch from the previous round.
Once this is merged, I would suggest that we consider tagging a v2.54.1
with this in it, as the failure mode is pretty significant for users who
have concurrent maintenance processes running.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 7/9] notes: support an external command to display notes
From: Junio C Hamano @ 2026-05-20 0:03 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: git, Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk
In-Reply-To: <9619077369f1a567bd505b1de1e4f672a5cd1950.1779207350.git.siddh.raman.pant@oracle.com>
Siddh Raman Pant <siddh.raman.pant@oracle.com> writes:
> This problem excaberates on scale.
>
> One solution to this is a realtime fetch or faster updation via
> external means, but unfortunately we lose the coherence in the
> display of information, and the user would end up reinventing
> git log.
>
> So let's add support for an external command to display the notes.
It is unclear how we would arrive at "So let's" from the previous
paragraph. It is not limited to notes but multiple people updating
the same thing racing against each other happens all the time in the
main part of the history, no? Isn't a better solution for such
racing situation usually based on a better merge support, I have to
wonder?
> We split the addition of documentation and tests from this commit for
> easier review. The new help text added in Documentation/ in the next
> commit should make the usage clear.
It is unclear why a large body of code that is not documented or
whose uses are not illustrated by examples found in the test scripts
is easier to review, though.
^ permalink raw reply
* Re: [PATCH v4 00/13] pack-objects: integrate --path-walk and some --filter options
From: Taylor Blau @ 2026-05-19 23:53 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <pull.2101.v4.git.1778707135.gitgitgadget@gmail.com>
On Wed, May 13, 2026 at 09:18:42PM +0000, Derrick Stolee via GitGitGadget wrote:
> UPDATES IN V4
> =============
>
> Thanks, Taylor for the careful review.
>
> * Several typos are fixed.
> * The performance test is corrected for issues around piping Git commands
> and made more robust to the existence of submodules.
> * BIG: The tree:0 patch is significantly updated in this version. Taylor
> correctly smelled a problem with the new logic to emit the /tagged-trees
> object set, and that signaled that those trees were previously never
> emitted. I update the test to demonstrate that changing the data shape
> (including tagged trees that are otherwise-unreachable) doesn't change
> the test behavior, signaling a bug. The behavior change details all the
> complexities of visiting only directly-requested trees under a tree:0
> filter and recursing on all trees in other cases.
Thanks for the new round; I gave this a lighter pass since I had
reviewed v3 in detail and the range-diff here looks good. I focused in
on a few patches in particular, and left a couple of minor comments.
My main reservation is that the "path starts with a '/' slash character
when directly requested" behavior feels brittle to me, and I am not sure
if there is a cleaner way to express that.
I'm curious what your thoughts are there. I think barring that things
are near-complete here, though I did note one issue with the t/perf
changes (that is my fault for having a bad suggestion on the earlier
round).
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v4 03/13] t/perf: add pack-objects filter and path-walk benchmark
From: Taylor Blau @ 2026-05-19 23:51 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <fb8a0f9c43d4e41712839a93c4db6a294a7b5285.1778707135.git.gitgitgadget@gmail.com>
On Wed, May 13, 2026 at 09:18:45PM +0000, Derrick Stolee via GitGitGadget wrote:
> + >depth2-dirs &&
> + while read tdir
> + do
> + git ls-tree -d --name-only "HEAD:$tdir" 2>/dev/null || return 1
> + done <top-dirs >depth2-dirs.raw &&
> + sed "s|^|$tdir/|" <depth2-dirs.raw >depth2-dirs &&
Ugh, I think that this was a bad suggestion on my part, since $tdir
should be empty at this point.
Could we use --format here like so?
while read tdir
do
git ls-tree -d --format="$tdir/%(path)" "HEAD:$tdir" || return 1
done 2>/dev/null
I guess that breaks if $tdir contains a formatting atom, so perhaps we
should keep the spirit of the original (but using an intermediary file
instead of piping the output of Git to another command).
Sorry about that, I'm not sure why I thought that was a good idea when I
wrote it :-<.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH 3/9] wrapper: add sleep_nanosec
From: Junio C Hamano @ 2026-05-19 23:50 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: git, Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk
In-Reply-To: <6a8c2093643a385641ef0b2cde33839dc98d8678.1779207350.git.siddh.raman.pant@oracle.com>
Siddh Raman Pant <siddh.raman.pant@oracle.com> writes:
> Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> ---
The space above the signed-off-by line should be utilized to explain
why we want this change. For the purpose of this series, why do we
want to sleep at nanosecond precision?
> wrapper.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> wrapper.h | 1 +
> 2 files changed, 50 insertions(+)
>
> diff --git a/wrapper.c b/wrapper.c
> index 16f5a63fbb61..1349255f1eb4 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -10,6 +10,7 @@
> #include "gettext.h"
> #include "strbuf.h"
> #include "trace2.h"
> +#include <time.h>
>
> #ifdef HAVE_RTLGENRANDOM
> /* This is required to get access to RtlGenRandom. */
> @@ -708,6 +709,54 @@ void sleep_millisec(int millisec)
> poll(NULL, 0, millisec);
> }
>
> +#ifdef GIT_WINDOWS_NATIVE
> +/* No nanosleep() on Windows, so fall-back to using sleep_millisec(). */
> +int sleep_nanosec(uint64_t nanosec)
> +{
> + uint64_t ns_in_1ms = 1000000ULL; /* 1 ms = 10^6 ns */
> +
> + uint64_t millisec = nanosec / ns_in_1ms;
> + if (nanosec % ns_in_1ms)
> + millisec++;
> +
> + /* Chunked sleep if we can't represent in integer. */
> + while (millisec > INT_MAX) {
> + sleep_millisec(INT_MAX);
> + millisec -= INT_MAX;
> + }
> +
> + sleep_millisec((int)millisec);
> +
> + return 0;
> +}
> +#else
> +/* Not Windows, so use the more exact nanosleep(). */
> +int sleep_nanosec(uint64_t nanosec)
> +{
> + int ret;
> + struct timespec duration, remaining;
> +
> + /* Construct the duration by dividing the given total (1s = 10^9ns). */
> + duration.tv_sec = nanosec / 1000000000ULL;
> + duration.tv_nsec = nanosec % 1000000000ULL;
> +
> + while(1) {
> + ret = nanosleep(&duration, &remaining);
> +
> + /* Continue sleeping if interrupted. */
> + if (ret == -1 && errno == EINTR) {
> + duration = remaining;
> + continue;
> + }
> +
> + /* Either success or an error. */
> + break;
> + }
> +
> + return ret;
> +}
> +#endif /* GIT_WINDOWS_NATIVE */
> +
> int xgethostname(char *buf, size_t len)
> {
> /*
> diff --git a/wrapper.h b/wrapper.h
> index 15ac3bab6e97..c39992893a81 100644
> --- a/wrapper.h
> +++ b/wrapper.h
> @@ -130,6 +130,7 @@ int warn_on_fopen_errors(const char *path);
> int open_nofollow(const char *path, int flags);
>
> void sleep_millisec(int millisec);
> +int sleep_nanosec(uint64_t nanosec);
>
> enum {
> /*
^ permalink raw reply
* Re: [PATCH 1/9] Documentation/git-range-diff: add missing notes options in synopsis
From: Junio C Hamano @ 2026-05-19 23:47 UTC (permalink / raw)
To: Siddh Raman Pant
Cc: git, Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk
In-Reply-To: <290fe06d81e956253d3a06fc1e16848e0b86b603.1779207350.git.siddh.raman.pant@oracle.com>
Siddh Raman Pant <siddh.raman.pant@oracle.com> writes:
> Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
> ---
> Documentation/git-range-diff.adoc | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
This has nothing to do with "external notes" topic, no?
>
> diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
> index 880557084533..5cc5e2ed5673 100644
> --- a/Documentation/git-range-diff.adoc
> +++ b/Documentation/git-range-diff.adoc
> @@ -11,7 +11,7 @@ SYNOPSIS
> git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
> [--no-dual-color] [--creation-factor=<factor>]
> [--left-only | --right-only] [--diff-merges=<format>]
> - [--remerge-diff]
> + [--remerge-diff] [--no-notes | --notes[=<ref>]]
> ( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
> [[--] <path>...]
^ permalink raw reply
* Re: [PATCH v4 04/13] path-walk: always emit directly-requested objects
From: Taylor Blau @ 2026-05-19 23:22 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget
Cc: git, christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, newren, peff, ps,
Derrick Stolee
In-Reply-To: <e77c8a6bbc22da3428751f81ff5ee79aa5364237.1778707135.git.gitgitgadget@gmail.com>
On Wed, May 13, 2026 at 09:18:46PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/path-walk.c b/path-walk.c
> index 6e426af433..05bfc1c114 100644
> --- a/path-walk.c
> +++ b/path-walk.c
> @@ -248,6 +248,17 @@ static int add_tree_entries(struct path_walk_context *ctx,
> return 0;
> }
>
> +/*
> + * Paths starting with '/' (e.g., "/tags", "/tagged-blobs") hold objects that
> + * were directly requested by 'pending' objects rather than discovered during
> + * tree traversal.
> + */
> +static int path_is_for_direct_objects(const char *path)
> +{
> + ASSERT(path);
> + return path[0] == '/';
> +}
> +
Hmm, I still find this a little brittle. I think that 'path' here is
doing a number of jobs: it serves as a strmap key, it's visible to the
caller, and now also a "direct object" marker.
Could we instead store this explicitly on the type_and_oid_list, e.g. a
"direct" flag? I'm not sure whether that type has the right scope for
this information. If not, I wonder if there is another way to store this
information, since I worry that future callers may not know about this
convention and end up changing the result of the path-walk depending on
how they name their paths.
Thanks,
Taylor
^ permalink raw reply
* Re: What's cooking in git.git (May 2026, #04)
From: Justin Tobler @ 2026-05-19 22:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqv7clbizy.fsf@gitster.g>
On 26/05/18 10:32AM, Junio C Hamano wrote:
> * jt/odb-transaction-write (2026-05-14) 7 commits
> - odb/transaction: make `write_object_stream()` pluggable
> - object-file: generalize packfile writes to use odb_write_stream
> - object-file: avoid fd seekback by checking object size upfront
> - object-file: remove flags from transaction packfile writes
> - odb: update `struct odb_write_stream` read() callback
> - odb/transaction: use pluggable `begin_transaction()`
> - odb: split `struct odb_transaction` into separate header
> (this branch is used by ps/odb-in-memory.)
>
> ODB transaction interface is being reworked to explicitly handle
> object writes.
>
> Will merge to 'next'?
> source: <20260514183740.1505171-1-jltobler@gmail.com>
I think this series should be ready to go now. The last version
submitted fixed the leak reported by Peff.
Thanks,
-Justin
^ permalink raw reply
* Re: [PATCH] revision: use priority queue in limit_list()
From: Jeff King @ 2026-05-19 21:56 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: Derrick Stolee, Junio C Hamano,
Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <CAL71e4O6UcnqmxDgqyGqvgvfruSzeoz6Wj5muXiwEp_8y2wAcg@mail.gmail.com>
On Tue, May 19, 2026 at 11:33:19AM +0200, Kristofer Karlsson wrote:
> I took a look at your branch. Our approaches differ mainly in
> how broadly the prio_queue replaces the linked list. Here's a summary
> of the tradeoffs as I see them:
>
> Your approach: replace commits entirely with struct prio_queue.
> Every access site is converted, and boundary cases (bisect,
> topo-sort, simplify_merges) convert queue->list->queue when they need
> list-based APIs.
>
> My approach: keep the linked list for setup and add a separate
> commit_queue for the walk phase. External callers that read the
> list between prepare_revision_walk() and the walk are unchanged.
> The conversion happens once when the walk begins.
Yeah, I think that is an accurate summary. What I worry about with your
approach is any code that looks at or modifies the commit list during
the traversal. It has to know whether to use the queue or the list.
> On the walk side, my second and third commits refactor
> get_revision_1() to use a vtable ("walk_ops") that selects the right
> pop/expand strategy once and caches it:
>
> struct revision_walk_ops {
> void (*init)(struct rev_info *);
> struct commit *(*next)(struct rev_info *);
> int (*expand)(struct rev_info *, struct commit *);
> };
>
> static struct revision_walk_ops streaming_ops =
> { rev_info_commit_list_to_queue, next_streaming, expand_streaming };
> static struct revision_walk_ops limited_ops =
> { NULL, next_commit_list, NULL };
> /* ...reflog_ops, topo_ops, no_walk_ops... */
I looked at the patch you linked for this. I'm undecided on whether this
makes things simpler (because the if/else-cascade is in one spot) or
more confusing (because now the details are all hidden behind a layer of
abstraction).
> I benchmarked both approaches against a 2.4M-commit squash-merge-
> heavy monorepo (best of 3 runs each, commit-graph present):
>
> Benchmark mainline kk jk
> rev-list HEAD (streaming, full DAG) 21.8s 6.9s 6.9s
> --ancestry-path ~100K (limited) 21.8s 4.8s 5.0s
> rev-list --count HEAD~10000..HEAD 17.7s 3.7s 3.8s
> log --oneline -1000 0.1s 0.1s 0.1s
>
> Both give ~3-5x speedups over mainline. The streaming walk is
> identical. On limited walks kk is ~4% faster, which I think comes
> from avoiding the queue rebuild at the end of limit_list() -- jk's
> commit_list_to_queue() drains the result list back into the queue,
> while kk leaves the result as a linked list (which the limited walk
> then just pops from directly).
It would be easy-ish to further convert limit_list() to store newlist as
a queue, and then transfer ownership of its fields into revs->commits
(i.e., a struct assignment).
One possible complication is that we do pass "newlist" into a few
sub-functions, like cherry_pick_list(). Looking at that function, it
iterates over the list, but it's not clear to me if the order matters.
Certainly not in the first loop, but later we do some flag assignments.
I _think_ they're all independent, but I'm not sure.
Obviously we can iterate over the prio_queue in date order with a series
of get() calls, but that is roughly equivalent to building a list (and
we have to rebuild the queue after, too). Of course that is already
happening in limit_to_ancestry(), which builds the reverse-order list.
So I dunno. Moving to the dual-structure state feels messy and
error-prone to me, but it does perhaps let us move a little more
incrementally.
-Peff
^ permalink raw reply
* Re: [PATCH] git-jump: pick a mode automatically when invoked without arguments
From: Jeff King @ 2026-05-19 21:22 UTC (permalink / raw)
To: Greg Hurrell; +Cc: Erik Cervin Edin, git
In-Reply-To: <8f4b75d8-f875-434a-8fc5-06a708cbc53f@app.fastmail.com>
On Tue, May 19, 2026 at 11:03:44AM +0200, Greg Hurrell wrote:
> On Thu, May 14, 2026, at 5:40 PM, Erik Cervin Edin wrote:
> > On 26/05/08 09:07AM, Greg Hurrell via GitGitGadget wrote:
> > > -usage: git jump [--stdout] <mode> [<args>]
> > > +usage: git jump [--stdout] [<mode>] [<args>]
> >
> > The usage message makes <mode> optional but doesn't explain what
> > happens when you omit it. Seems worth documenting the auto-detect behavior
> > there too.
> >
> > If we're going to teach git-jump how to be more clever about where to jump,
> > does it also make sense to bake `git jump ws` into this?
> >
> > Also, if this is going to grow into a proper auto-detect heuristic, it
> > might be cleaner as a first-class mode rather than logic spliced into the
> > argument parser. Something like:
> >
> > mode_auto() {
> > if test -n "$(git ls-files -u)"; then
> > mode_merge "$@"
> > elif ! git diff --quiet; then
> > mode_diff "$@"
> > elif ! git diff --cached --check >/dev/null 2>&1; then
> > mode_ws --cached "$@"
> > else
> > return 0
> > fi
> > }
> >
> > That way `git jump auto` works explicitly, bare `git jump` defaults
> > to it (just `set -- auto` when $# -lt 1), and the usage text can
> > document the heuristic. It also keeps the detection and dispatch in
> > one place in case someone wants to tweak the priority later.
>
> All of those suggestions sound reasonable to me. Jeff, do you agree?
> If so, I can update the patch.
Yeah, I agree that having an explicit "auto" mode (and then just
defaulting to it) makes perfect sense.
I don't really have an opinion on adding "ws" in here. Despite being the
person who added the whitespace mode in the first place, I can't
remember ever using it in the last 10 years. ;) But the cost is fairly
low to support it, so we might as well.
-Peff
^ permalink raw reply
* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
From: Johannes Sixt @ 2026-05-19 21:15 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <ffbbd733-2af9-4ff5-9354-cb2f333927a8@gmail.com>
Am 19.05.26 um 20:45 schrieb Mark Levedahl:
> What I have now is
>
> if (enabled gitdir discovery) {
> discover gitdir
> maybe an error occurs and gitdir remains {}
> }
>
> if (enabled pick && gitdir eq {}) {
> unset GIT_DIR .. (just to be friendly, could throw an error instead...)
> pick
> discover gitdir to VALIDATE pick gave us a good thing
> }
>
> if no gitdir {
> error No Repository
> }
>
> then on to worktree discovery (which also validates what pick returns as pick may not have
> done so).
Sounds good.
-- Hannes
^ permalink raw reply
* Re: [PATCH 1/5] doc: convert git-bisect to synopsis style
From: Jean-Noël AVILA @ 2026-05-19 21:03 UTC (permalink / raw)
To: Jean-Noël Avila via GitGitGadget, Junio C Hamano; +Cc: git
In-Reply-To: <87tss5wjpp.fsf@gitster.g>
On Monday, 18 May 2026 04:10:58 CEST Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> >> +[synopsis]
> >>
> >> ------------------------------------------------
> >> $ git bisect reset <commit>
> >> ------------------------------------------------
> >
> > and
> >
> >> +[synopsis]
> >>
> >> ------------------------------------------------
> >> git bisect old [<rev>]
> >> ------------------------------------------------
> >
> > were a bit surprising and confusing. They are not exactly command
> > syntax definitions (which is the SYNOPSIS section is about), but
> > examples of usage. The one with '$' command line prompt feels
> > particularly confusing, as the prompt is not something that the
> > end-user gives, unlike what we write in the synopsis section.
> >
> > Other than that, this is quite exciting.
>
> Well, my local test with asciidoctor did not barf, but it seems that
> the documentation pipeline run in GitHub Actions CI is unhappy.
>
> https://github.com/git/git/actions/runs/26008649802/job/
76444895183#step:4:4846
>
> I do not know what the differences among the three environments
> (counting your development environment---only one of which fails)
> are offhand.
Thank you for pointing out that the test fails with Asciidoctor. On my debian
testing, both asciidoc.py and asciidoctor pass. I can try and revert to
paragraph styling instead of block styling.
^ permalink raw reply
* Re: [PATCH 1/5] doc: convert git-bisect to synopsis style
From: Jean-Noël AVILA @ 2026-05-19 20:57 UTC (permalink / raw)
To: Jean-Noël Avila via GitGitGadget, Junio C Hamano; +Cc: git
In-Reply-To: <xmqq4ik5d0le.fsf@gitster.g>
On Monday, 18 May 2026 02:26:37 CEST Junio C Hamano wrote:
> "Jean-Noël Avila via GitGitGadget" <gitgitgadget@gmail.com> writes:
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> >
> > Convert Documentation/git-bisect.adoc to the modern synopsis style.
> >
> > - Replace [verse] with [synopsis] in the SYNOPSIS block
>
> This was expected.
>
> > - Remove single quotes around command names in the synopsis
> > - Use backticks for inline commands, options, refs, and special values
> > - Apply [synopsis] attribute to in-body command-form code blocks
>
> This is very much unexpected. I think everybody thought [synopsis]
> was invented to be used for the SYNOPSIS section at the beginning of
> each manual page, and ...
In fact, the synopsis style was already applied before outside the SYNOPSIS
sections in diff-generate-patch.adoc when describing the output of the -p
option.
This formatting reaches beyond the synopsis, but the rationale is simple. Each
time a listing contains some <placeholder>, what is actually described is a
model, not an actual output. The <placeholder> needs a special formatting to
convey its special meaning.
That may mean that the naming of "synopsis style" may not be adequate.
>
> > SYNOPSIS
> > --------
> >
> > -[verse]
> > -'git bisect' start [--term-(bad|new)=<term-new> --term-(good|old)=<term-
old>]
> > - [--no-checkout] [--first-parent] [<bad> [<good>...]]
[--] [<pathspec>...]
> > ...
> > -'git bisect' help
> > +[synopsis]
> > +git bisect start [--term-(bad|new)=<term-new> --term-(good|old)=<term-
old>]
> > + [--no-checkout] [--first-parent] [<bad> [<good>...]] [--]
[<pathspec>...]
> > ...
> > +git bisect help
>
> ... a change like this is very much expected and understandable, but
>
> new appearances of [synonsis] in places like:
> > +[synopsis]
> >
> > ------------------------------------------------
> > $ git bisect reset <commit>
> > ------------------------------------------------
>
> and
>
> > +[synopsis]
> >
> > ------------------------------------------------
> > git bisect old [<rev>]
> > ------------------------------------------------
>
> were a bit surprising and confusing. They are not exactly command
> syntax definitions (which is the SYNOPSIS section is about), but
> examples of usage.
Are they? the "[<rev>]" block is typical of synopsis syntax, not something you
would actually type in.
> The one with '$' command line prompt feels
> particularly confusing, as the prompt is not something that the
> end-user gives, unlike what we write in the synopsis section.
>
The '$' is an error to me. Will fix.
> Other than that, this is quite exciting.
^ permalink raw reply
* Re: What's cooking in git.git (May 2026, #04)
From: Jeff King @ 2026-05-19 19:19 UTC (permalink / raw)
To: Taylor Blau; +Cc: Junio C Hamano, git
In-Reply-To: <agyPJa3E2lPI9K/G@nand.local>
On Tue, May 19, 2026 at 12:26:13PM -0400, Taylor Blau wrote:
> > * tb/incremental-midx-part-3.3 (2026-04-29) 16 commits
> [...]
> Apologies, I didn't realize you were waiting on these until seeing this
> WC report. I sent an extremely tiny reroll
>
> https://lore.kernel.org/git/cover.1779206239.git.me@ttaylorr.com/
>
> that addresses the two outstanding comments you linked. They are very
> minor changes, and queueing either version of the series would be
> equally fine IMHO.
I peeked at the v4 range diff, and it looks good to me.
-Peff
^ permalink raw reply
* Re: [PATCH] http: fix memory leak in fetch_and_setup_pack_index()
From: Jeff King @ 2026-05-19 19:17 UTC (permalink / raw)
To: LorenzoPegorari; +Cc: git, Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox
In-Reply-To: <agx5tblaCZNsYEBq@lorenzo-VM>
On Tue, May 19, 2026 at 04:54:45PM +0200, LorenzoPegorari wrote:
> Inside the function `fetch_and_setup_pack_index()`, when the pack
> obtained using `fetch_pack_index()` fails to be verified by
> `parse_pack_index()`, the function returns without closing and freeing
> said pack.
>
> Fix this by calling `close_pack_index()` to munmap the index file for
> the leaking pack (which might have been mmapped by `fetch_pack_index()`
> or `verify_pack_index()`), and then free it.
OK, I agree we are leaking here, but after reading the patch I'm left
with a few questions.
> ret = verify_pack_index(new_pack);
> - if (!ret)
> - close_pack_index(new_pack);
> +
> + close_pack_index(new_pack);
This part was a little confusing at first, because it looked like we are
already closing the index. But we were doing so on _success_, not on
failure. Which is a little funny since the point is to be able to read
from it later, but OK.
At any rate, that is an existing oddity, and I agree that closing it
before freeing the struct is obviously the right thing to do.
> free(tmp_idx);
> - if (ret)
> + if (ret) {
> + free(new_pack);
> return -1;
> + }
And here we free the actual struct. Good.
But this existing free(tmp_idx) is what puzzles me. We do not need the
filename anymore regardless of success or failure, so freeing it makes
sense. But earlier in the function we have:
new_pack = parse_pack_index(the_repository, sha1, tmp_idx);
if (!new_pack) {
unlink(tmp_idx);
free(tmp_idx);
return -1; /* parse_pack_index() already issued error message */
}
So on parse failure we actually unlink it, but not on verification
failure. Which seems like it would leave cruft after the process ends.
And I suspect we probably we did prior to 63aca3f7f1 (dumb-http: store
downloaded pack idx as tempfile, 2024-10-25), when we started
registering it as a tempfile to be deleted at process exit.
So I _think_ we could get away with dropping the existing unlink() call
and just let it get cleaned up at process exit. But if we are going to
keep it, do we want to also unlink() in this error path? At which point
it might make more sense to have an "out" label to consolidate all of
this cleanup.
If we are going to unlink() here it may also make sense to just return
the tempfile struct from fetch_pack_index(), and then we can call
delete_tempfile() on it. See the in-code comment in 63aca3f7f1 which
mentions this hackery.
So I dunno. I think your patch is doing the right thing as-is, but it
may be worth taking a moment to clean this up a bit further.
-Peff
^ permalink raw reply
* Re: [PATCH v1 10/11] git-gui: improve worktree discovery
From: Mark Levedahl @ 2026-05-19 19:00 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <5081fcc5-19b5-49aa-a33c-2c13aba7edb1@kdbg.org>
On 5/19/26 4:16 AM, Johannes Sixt wrote:
> Am 16.05.26 um 17:28 schrieb Mark Levedahl:
>> On 5/16/26 4:16 AM, Johannes Sixt wrote:
>>> Am 14.05.26 um 16:33 schrieb Mark Levedahl:
>>>> + if {[is_gitvars_error $err]} {
>>>> + exit 1
>>>> + }
>>>> + set _gitworktree {}
>>>> + set _prefix {}
>>>> + if {[is_enabled bare]} {
>>>> + cd $_gitdir
>>> Why change the directory here? If we run `git gui browser master dir` we
>>> do not want to change the directory in an uncontrolled manner. The
>>> argument parser will want to check for the existence of files, and then
>>> we do not want to operate from a random directory.
>>>
>>> Also, I think that the check must be for [is_bare] and not [is_enabled
>>> bare].
>> [is_enabled_bare] is correct. This code handles the case:
>> - neither the startup directory nor GIT_WORK_TREE are useable worktrees, so [is_bare]
>> is currently true.
>> - the command given is browser or blame so a worktree is not needed. We can proceed.
> But in the case where the command is browser or blame, the argument
> parser must later check for the existence of files, provided that a
> worktree is present. But this conditional would change directory to
> somewhere that is not a worktree at all even though a worktree is
> available. So, I am still convinced that [is_bare] is correct.
I did change this. but...
I have reworked the blame/browser parser so it fully matches git blame parsing for the
single rev + path (or path rev) cases, all now do the same thing with or without a
worktree as they all work from git history, so having a worktree becomes almost moot (I
found issues with how git-gui handles --, it is dead wrong in some cases if the intent is
to match git-blame). What doesn't change is what blame displays based upon uncommitted or
staged changes in the worktree (if it does anything, but comments from 2007 suggest it
does, I haven't tested). I've done nothing to change that, only finding the args to pass
in to browser / blame. So, if a worktree exists, blame will still use the info there as it
does now.
Mark
^ permalink raw reply
* Re: [PATCH v1 11/11] git-gui: add gui and pick as explicit subcommands
From: Mark Levedahl @ 2026-05-19 18:45 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <3b16fbc6-074b-410d-861e-6f77794b02a0@kdbg.org>
On 5/19/26 4:21 AM, Johannes Sixt wrote:
> Am 16.05.26 um 17:42
>
>
> I think I would be happier with the structure
>
> if not subcommand pick
> discover gitdir
> if error
> set subcommand pick
>
> if subcommand pick
> pick_repo
> set subcommand gui
>
> because this clarifies that pick_repo must erase all current traces of
> GIT_DIR and GIT_WORK_TREE from the envionment and must complete with a
> valid setup.
>
> With the structure in the proposed patch
>
> if subcommand pick
> pick_repo
> set subcommand gui
>
> discover gitdir
> if error
> pick_repo
>
> we still need the same operation of pick_repo, but after it runs due to
> a pick command, we go into "discover gitdir" mode in an already modified
> environment, something that does not happen if pick_repo runs due to the
> error in the gitdir discovery.
>
> -- Hannes
>
What I have now is
if (enabled gitdir discovery) {
discover gitdir
maybe an error occurs and gitdir remains {}
}
if (enabled pick && gitdir eq {}) {
unset GIT_DIR .. (just to be friendly, could throw an error instead...)
pick
discover gitdir to VALIDATE pick gave us a good thing
}
if no gitdir {
error No Repository
}
then on to worktree discovery (which also validates what pick returns as pick may not have
done so).
So, we can independently enable normal discovery or pick, and with both enabled pick can
be used to recover from an error in normal discovery. Either way, there is only one block
of code running pick, it is not a separate proc invoked multiple places. I hope this
scratches your itch.
Still scrubbing things, should send out in a day or two.
Mark
^ permalink raw reply
* Re: [PATCH v6 0/8] fetch: rework negotiation tip options
From: Matthew John Cheetham @ 2026-05-19 16:48 UTC (permalink / raw)
To: Derrick Stolee via GitGitGadget, git; +Cc: gitster, ps, Derrick Stolee
In-Reply-To: <pull.2085.v6.git.1779207896.gitgitgadget@gmail.com>
On 2026-05-19 17:24, Derrick Stolee via GitGitGadget wrote:
> Updates in v6
> =============
>
> Corrected reviewed-by annotations in commit messages.
>
> Thanks, -Stolee
>
v6 look good to me!
Thanks,
Matthew
^ permalink raw reply
* [PATCH 9/9] t: add tests for external notes command
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
t/helper/meson.build | 1 +
t/helper/test-external-notes | 64 +++
t/helper/test-notes-external-config-reset.c | 20 +
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/lib-notes.sh | 19 +
t/t3206-range-diff.sh | 68 +++
t/t3301-notes.sh | 437 ++++++++++++++++++++
t/t6120-describe.sh | 17 +
9 files changed, 628 insertions(+)
create mode 100755 t/helper/test-external-notes
create mode 100644 t/helper/test-notes-external-config-reset.c
create mode 100644 t/lib-notes.sh
diff --git a/t/helper/meson.build b/t/helper/meson.build
index 675e64c0101b..739614b90e78 100644
--- a/t/helper/meson.build
+++ b/t/helper/meson.build
@@ -35,6 +35,7 @@ test_tool_sources = [
'test-mergesort.c',
'test-mktemp.c',
'test-name-hash.c',
+ 'test-notes-external-config-reset.c',
'test-online-cpus.c',
'test-pack-deltas.c',
'test-pack-mtimes.c',
diff --git a/t/helper/test-external-notes b/t/helper/test-external-notes
new file mode 100755
index 000000000000..5e9dde3977ab
--- /dev/null
+++ b/t/helper/test-external-notes
@@ -0,0 +1,64 @@
+#!/bin/sh
+
+prefix=${TEST_EXTERNAL_NOTES_PREFIX:-external-notes}
+response=${TEST_EXTERNAL_NOTES_RESPONSE:-ok}
+line_ending=${TEST_EXTERNAL_NOTES_LINE_ENDING:-lf}
+exit_after_response=${TEST_EXTERNAL_NOTES_EXIT_AFTER_RESPONSE:-}
+exit_delay=${TEST_EXTERNAL_NOTES_EXIT_DELAY:-}
+delay=${TEST_EXTERNAL_NOTES_DELAY:-}
+char_delay=${TEST_EXTERNAL_NOTES_CHAR_DELAY:-}
+ignore_term=${TEST_EXTERNAL_NOTES_IGNORE_TERM:-}
+
+newline='\n'
+case "$line_ending" in
+crlf)
+ newline='\r\n'
+ ;;
+none)
+ newline=
+ ;;
+esac
+
+echo start >>"$prefix-starts"
+
+test "$ignore_term" = true && trap '' TERM
+
+emit_output() {
+ if test -n "$char_delay"
+ then
+ LC_ALL=C
+ payload=$(printf "$@"; printf .)
+ payload=${payload%.}
+
+ while test -n "$payload"
+ do
+ char=${payload%"${payload#?}"}
+ printf '%s' "$char" || return 1
+ payload=${payload#?}
+ sleep "$char_delay" || return 1
+ done
+ else
+ printf "$@"
+ fi
+}
+
+while IFS= read -r commit; do
+ if test "${TEST_EXTERNAL_NOTES_BODY+x}" = x
+ then
+ note=$TEST_EXTERNAL_NOTES_BODY
+ else
+ note=$commit
+ fi
+ printf "%s\n" "$commit" >>"$prefix-requests"
+ test -z "$delay" || sleep "$delay"
+ if test "$response" = missing
+ then
+ emit_output "%s missing%b" "$commit" "$newline"
+ else
+ emit_output "%s ok %d%b%s%b" \
+ "$commit" "${#note}" "$newline" "$note" "$newline"
+ fi
+ test "$exit_after_response" = true && break
+done
+
+test -z "$exit_delay" || sleep "$exit_delay"
diff --git a/t/helper/test-notes-external-config-reset.c b/t/helper/test-notes-external-config-reset.c
new file mode 100644
index 000000000000..1c6b26e3b49a
--- /dev/null
+++ b/t/helper/test-notes-external-config-reset.c
@@ -0,0 +1,20 @@
+#include "test-tool.h"
+#include "notes-external.h"
+
+int cmd__notes_external_config_reset(int argc, const char **argv UNUSED)
+{
+ if (argc != 1)
+ die("usage: test-tool notes-external-config-reset");
+
+ set_external_notes_command("helper");
+ set_external_notes_command_name("label");
+ set_external_notes_command_timeout_ms(250);
+ set_external_notes_for_grep(1);
+ reset_external_notes_command();
+
+ printf("configured=%d\n", external_notes_command_configured());
+ printf("name=%s\n", external_notes_command_name());
+ printf("timeout_ms=%d\n", external_notes_command_timeout_ms());
+ printf("grep=%d\n", external_notes_for_grep_enabled());
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index a7abc618b388..31bb2d1dca47 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -45,6 +45,7 @@ static struct test_cmd cmds[] = {
{ "mergesort", cmd__mergesort },
{ "mktemp", cmd__mktemp },
{ "name-hash", cmd__name_hash },
+ { "notes-external-config-reset", cmd__notes_external_config_reset },
{ "online-cpus", cmd__online_cpus },
{ "pack-deltas", cmd__pack_deltas },
{ "pack-mtimes", cmd__pack_mtimes },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 7f150fa1eb9a..ff25f0a29cf2 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -38,6 +38,7 @@ int cmd__match_trees(int argc, const char **argv);
int cmd__mergesort(int argc, const char **argv);
int cmd__mktemp(int argc, const char **argv);
int cmd__name_hash(int argc, const char **argv);
+int cmd__notes_external_config_reset(int argc, const char **argv);
int cmd__online_cpus(int argc, const char **argv);
int cmd__pack_deltas(int argc, const char **argv);
int cmd__pack_mtimes(int argc, const char **argv);
diff --git a/t/lib-notes.sh b/t/lib-notes.sh
new file mode 100644
index 000000000000..07422540d58f
--- /dev/null
+++ b/t/lib-notes.sh
@@ -0,0 +1,19 @@
+# Helpers for scripts testing notes behavior.
+
+# notes.externalCommand is run through a shell, so quote the path.
+external_notes_command=$(
+ printf "%s\n" "$TEST_DIRECTORY/helper/test-external-notes" |
+ sed "s/'/'\\\\''/g; s/^/'/; s/$/'/"
+)
+
+# The helper above is a shell script. Few Windows CI tests (3 out of 10
+# in matrix) are spending more than the production default timeout just
+# starting the shell and exchanging the first response, so tests that
+# are not about timeout behavior fail. So let us opt into a wider 1s
+# deadline for Windows instead of 100ms.
+external_notes_command_timeout_config=
+if test_have_prereq MINGW
+then
+ _timeout_config="notes.externalCommandTimeoutMs=1000"
+ external_notes_command_timeout_config="-c $_timeout_config"
+fi
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 1e812df806bb..96adeb9bc4fe 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-notes.sh
# Note that because of the range-diff's heuristics, test_commit does more
# harm than good. We need some real history.
@@ -690,6 +691,37 @@ test_expect_success 'range-diff with --notes=custom does not show default notes'
grep "## Notes (custom) ##" actual
'
+test_expect_success 'range-diff with --external-notes' '
+ topic_oid=$(git rev-parse topic) &&
+ unmodified_oid=$(git rev-parse unmodified) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ range-diff --no-color --external-notes \
+ main..topic main..unmodified >actual &&
+ test_grep "## Notes (external) ##" actual &&
+ test_grep "^ - $topic_oid$" actual &&
+ test_grep "^ + $unmodified_oid$" actual &&
+ ! grep "## Notes ##" actual
+'
+
+test_expect_success 'range-diff with disabled external notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
+ git notes add -m "topic note" topic &&
+ git notes add -m "unmodified note" unmodified &&
+ TEST_EXTERNAL_NOTES_PREFIX=range-diff-external-notes \
+ git -c notes.externalCommand="$external_notes_command" \
+ range-diff --no-color --external-notes --no-external-notes \
+ main..topic main..unmodified >actual &&
+ cat >expect <<-EOF &&
+ 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/
+ 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/
+ 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/
+ 4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/
+ EOF
+ test_cmp expect actual &&
+ test_path_is_missing range-diff-external-notes-starts
+'
+
test_expect_success 'format-patch --range-diff does not compare notes by default' '
test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
@@ -780,6 +812,42 @@ test_expect_success 'format-patch --range-diff with --notes' '
test_cmp expect actual
'
+test_expect_success 'format-patch --range-diff with --external-notes' '
+ topic_oid=$(git rev-parse topic) &&
+ unmodified_oid=$(git rev-parse unmodified) &&
+ test_when_finished "rm -f 000?-*" &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ format-patch --external-notes --cover-letter --range-diff=$prev \
+ main..unmodified >actual &&
+ test_line_count = 5 actual &&
+ test_grep "^Range-diff:$" 0000-* &&
+ test_grep "## Notes (external) ##" 0000-* &&
+ test_grep "^ - $topic_oid$" 0000-* &&
+ test_grep "^ + $unmodified_oid$" 0000-* &&
+ ! grep "## Notes ##" 0000-*
+'
+
+test_expect_success 'format-patch --range-diff with disabled external notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
+ git notes add -m "topic note" topic &&
+ git notes add -m "unmodified note" unmodified &&
+ test_when_finished "rm -f 000?-*" &&
+ TEST_EXTERNAL_NOTES_PREFIX=range-diff-external-notes \
+ git -c notes.externalCommand="$external_notes_command" \
+ format-patch --external-notes --no-external-notes \
+ --cover-letter --range-diff=$prev main..unmodified >actual &&
+ test_line_count = 5 actual &&
+ test_grep "^Range-diff:$" 0000-* &&
+ grep "= 1: .* s/5/A" 0000-* &&
+ grep "= 2: .* s/4/A" 0000-* &&
+ grep "= 3: .* s/11/B" 0000-* &&
+ grep "= 4: .* s/12/B" 0000-* &&
+ ! grep "Notes" 0000-* &&
+ ! grep "note" 0000-* &&
+ test_path_is_missing range-diff-external-notes-starts
+'
+
test_expect_success 'format-patch --range-diff with format.notes config' '
test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 27439010dfbc..5a162dff3917 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -6,6 +6,7 @@
test_description='Test commit notes'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-notes.sh
write_script fake_editor <<\EOF
echo "$MSG" >"$1"
@@ -16,6 +17,11 @@ export GIT_EDITOR
indent=" "
+run_with_limited_time () (
+ { set +x; } 2>/dev/null
+ "$PERL_PATH" -e 'alarm shift; exec @ARGV' -- "$@"
+)
+
test_expect_success 'cannot annotate non-existing HEAD' '
test_must_fail env MSG=3 git notes add
'
@@ -909,6 +915,437 @@ test_expect_success 'displayed notes are used for grep matching' '
test_must_be_empty actual
'
+test_expect_success 'notes.externalCommand shows external notes from protected config' '
+ commit=$(git rev-parse HEAD) &&
+ parent=$(git rev-parse HEAD^) &&
+ rm -f external-notes-starts external-notes-requests &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log -2 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ {
+ printf "%s\n" "$commit" &&
+ printf "%s\n" "$parent"
+ } >expect-requests &&
+ test_cmp expect-requests external-notes-requests &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ test_grep "^ $parent$" actual
+'
+
+test_expect_success PERL,EXECKEEPSPID 'notes.externalCommand terminates helper during exit cleanup' '
+ commit=$(git rev-parse HEAD) &&
+ test_env TEST_EXTERNAL_NOTES_EXIT_DELAY=10 \
+ run_with_limited_time 2 \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommandName labels external notes' '
+ commit=$(git rev-parse HEAD) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandName=commit-id log -1 >actual &&
+ test_grep "Notes (commit-id):" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommandName is rendered literally' '
+ commit=$(git rev-parse HEAD) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandName=refs/notes/commits \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (refs/notes/commits):$" actual &&
+ ! grep "^Notes:$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs rejects negative values' '
+ test_must_fail git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=-1 log -1 2>err &&
+ test_grep "notes.externalCommandTimeoutMs must be non-negative" err
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs times out delayed response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_DELAY=1 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=1 \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs applies to whole response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_BODY=x \
+ TEST_EXTERNAL_NOTES_CHAR_DELAY=0.02 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=50 \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success PERL,EXECKEEPSPID 'notes.externalCommandTimeoutMs terminates timed-out helper' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_DELAY=10 \
+ run_with_limited_time 2 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=1 \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success PERL,EXECKEEPSPID 'notes.externalCommandTimeoutMs force-kills timed-out helper' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_DELAY=10 \
+ TEST_EXTERNAL_NOTES_IGNORE_TERM=true \
+ run_with_limited_time 2 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=1 \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs=0 disables timeout' '
+ commit=$(git rev-parse HEAD) &&
+ test_env TEST_EXTERNAL_NOTES_DELAY=1 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=0 \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommand handles CRLF note bodies' '
+ body=$(printf "A\r\nB") &&
+ test_env TEST_EXTERNAL_NOTES_BODY="$body" \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ B$" actual
+'
+
+test_expect_success 'notes.externalCommand accepts CRLF missing response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_RESPONSE=missing \
+ TEST_EXTERNAL_NOTES_LINE_ENDING=crlf \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log -1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand rejects unterminated missing response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_RESPONSE=missing \
+ TEST_EXTERNAL_NOTES_LINE_ENDING=none \
+ TEST_EXTERNAL_NOTES_EXIT_AFTER_RESPONSE=true \
+ git -c notes.externalCommand="$external_notes_command" \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success PERL,EXECKEEPSPID 'notes.externalCommand rejects unterminated live response without deadlock' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_RESPONSE=missing \
+ TEST_EXTERNAL_NOTES_LINE_ENDING=none \
+ run_with_limited_time 2 \
+ git -c notes.externalCommand="$external_notes_command" \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success 'notes.externalCommand accepts CRLF protocol lines' '
+ commit=$(git rev-parse HEAD) &&
+ test_env TEST_EXTERNAL_NOTES_LINE_ENDING=crlf \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommand missing response shows no external notes' '
+ write_script external-notes-missing <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s missing\n" "$commit"
+ done
+ EOF
+ git log -1 >expect &&
+ git -c notes.externalCommand=./external-notes-missing log -1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand empty note shows no external notes' '
+ write_script external-notes-empty <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s ok 0\n\n" "$commit"
+ done
+ EOF
+ git log -1 >expect &&
+ git -c notes.externalCommand=./external-notes-empty log -1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand rejects invalid note lengths' '
+ write_script external-notes-invalid-length <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s ok %s\n" "$commit" "$1"
+ done
+ EOF
+ git log -2 >expect &&
+ for bad_length in -1 +1 1x x
+ do
+ git -c notes.externalCommand="./external-notes-invalid-length $bad_length" \
+ log -2 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err || return 1
+ done
+'
+
+test_expect_success 'notes.externalCommand is suppressed by --no-notes' '
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" log --no-notes -1 >actual &&
+ test_path_is_missing external-notes-starts &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommand is suppressed by --no-external-notes' '
+ rm -f external-notes-starts &&
+ git log -1 >expect &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --no-external-notes -1 >actual &&
+ test_cmp expect actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommand combines with explicit notes ref' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --notes=other --external-notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (other):" actual &&
+ test_grep "^ other note$" actual &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success '--show-notes=ref remains additive after --external-notes' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes --show-notes=other -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "^Notes:$" actual &&
+ test_grep "^ order test$" actual &&
+ test_grep "^Notes (other):$" actual &&
+ test_grep "^ other note$" actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommand can be enabled without default notes refs' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual &&
+ ! grep "^ other note$" actual
+'
+
+test_expect_success 'notes.externalCommand combines with default notes refs' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes --notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes:" actual &&
+ test_grep "^ order test$" actual &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ other note$" actual
+'
+
+test_expect_success 'notes.externalCommand obeys last --external-notes option' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git log --no-notes -1 >expect &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --external-notes --no-external-notes -1 >actual &&
+ test_cmp expect actual &&
+ test_path_is_missing external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --notes=other --no-external-notes --external-notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (other):" actual &&
+ test_grep "^ other note$" actual &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success 'notes.externalCommand honors raw notes formatting' '
+ commit=$(git rev-parse HEAD) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ show -s --format=%N >actual &&
+ test_grep "^$commit$" actual &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'format-patch --external-notes includes external notes only' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ format-patch --external-notes -1 --stdout >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "^Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success 'notes.externalCommand is not used for grep matching' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --grep="$commit" >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommandForGrep includes external notes in grep matching' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandForGrep=true \
+ log --grep="$commit" -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommandForGrep does not search hidden notes' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandForGrep=true \
+ log --oneline --grep="$commit" -1 >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommandForGrep honors --no-external-notes' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandForGrep=true \
+ log --no-external-notes --grep="$commit" -1 >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommandForGrep combines with explicit notes ref' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandForGrep=true \
+ log --notes=other --external-notes --grep="$commit" -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (external):" actual &&
+ test_grep "Notes (other):" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success 'notes.externalCommandForGrep is ignored from local config' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ test_config notes.externalCommandForGrep true &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --grep="$commit" >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommand is not used with explicit notes ref' '
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" log --notes=other -1 >actual &&
+ test_path_is_missing external-notes-starts &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommand is ignored from local config' '
+ rm -f external-notes-starts &&
+ test_config notes.externalCommand "$external_notes_command" &&
+ git log -1 >actual &&
+ test_path_is_missing external-notes-starts &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommandName is ignored from local config' '
+ test_config notes.externalCommandName local &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log -1 >actual &&
+ test_grep "Notes (external):" actual &&
+ ! grep "Notes (local):" actual
+'
+
+test_expect_success 'reset_external_notes_command clears cached helper config' '
+ test-tool notes-external-config-reset >actual &&
+ cat >expect <<-\EOF &&
+ configured=0
+ name=external
+ timeout_ms=100
+ grep=0
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand warning is shown once' '
+ write_script external-notes-fail <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s-mismatch missing\n" "$commit"
+ done
+ EOF
+ git -c notes.externalCommand=./external-notes-fail log -2 >actual 2>err &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
test_config core.notesRef refs/notes/other &&
echo "Note on a tree" >expect &&
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 8ee3d2c37d02..abbdb42dc9f7 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -15,6 +15,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-notes.sh
check_describe () {
indir= &&
@@ -867,6 +868,22 @@ test_expect_success 'format-rev with %N (note)' '
test_cmp expect actual
'
+test_expect_success 'format-rev with %N uses external notes' '
+ commit=$(git -C repo-format rev-parse HEAD) &&
+ rm -f repo-format/format-rev-external-notes-starts \
+ repo-format/format-rev-external-notes-requests &&
+ printf "%s\n" "$commit" >input &&
+ printf "%s\n\n" "$commit" >expect &&
+ TEST_EXTERNAL_NOTES_PREFIX=format-rev-external-notes \
+ git -C repo-format -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ format-rev --stdin-mode=text --format="tformat:%N" \
+ <input >actual &&
+ test_line_count = 1 repo-format/format-rev-external-notes-starts &&
+ test_cmp input repo-format/format-rev-external-notes-requests &&
+ test_cmp expect actual
+'
+
test_expect_success 'format-rev --notes<ref> (custom notes ref)' '
# One custom notes ref
test_when_finished "git -C repo-format notes remove" &&
--
2.53.0
^ permalink raw reply related
* [PATCH 8/9] Documentation: document external notes command options
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
Documentation/config/notes.adoc | 57 ++++++++++++++++++++++++++
Documentation/git-format-patch.adoc | 11 ++++-
Documentation/git-range-diff.adoc | 6 +++
Documentation/pretty-options.adoc | 9 ++++
contrib/completion/git-completion.bash | 4 +-
5 files changed, 84 insertions(+), 3 deletions(-)
diff --git a/Documentation/config/notes.adoc b/Documentation/config/notes.adoc
index b7e536496f51..b3ef3fa52950 100644
--- a/Documentation/config/notes.adoc
+++ b/Documentation/config/notes.adoc
@@ -34,6 +34,63 @@ The effective value of `core.notesRef` (possibly overridden by
`GIT_NOTES_REF`) is also implicitly added to the list of refs to be
displayed.
+`notes.externalCommand`::
+ Command to invoke as a long-lived helper when showing commit messages
+ with the `git log` family of commands. Git sends one commit object ID
+ per request on the command's standard input:
++
+------------
+<hex-commit-id>
+------------
++
+For each request, the command must respond on its standard output with either
+`<hex-commit-id> missing` followed by a newline, or `<hex-commit-id> ok <n>`
+followed by a newline and exactly `<n>` bytes of UTF-8 note text followed by a
+newline. The command must respond to each request as it is received; Git does
+not send all commit object IDs before reading responses. Empty note text is not
+displayed. If Git cannot start or communicate with the command, or the command
+sends an invalid response, Git warns once and disables it for the rest of the
+command. External notes are only used while formatting output by default; see
+`notes.externalCommandForGrep` to include them when matching commits.
++
+This setting is only respected in protected configuration (see
+linkgit:git-config[1]). This prevents untrusted repositories from running
+arbitrary commands when notes are displayed.
++
+This setting does not take effect when:
++
+--
+* the value is empty;
+* `--no-notes` is given;
+* `--no-external-notes` is given; or
+* `--notes=<ref>` is given by itself without `--external-notes` or `--notes`.
+--
+
+`notes.externalCommandName`::
+ Name to use in the `Notes (<name>):` header for notes returned by
+ `notes.externalCommand`. Defaults to `external`. This setting is only
+ respected in protected configuration.
+
+`notes.externalCommandTimeoutMs`::
+ Number of milliseconds to wait when reading each response from
+ `notes.externalCommand`. Defaults to `100`. If the command does not
+ produce the expected response in time, Git warns once and disables it
+ for the rest of the command. A value of `0` disables timeout handling,
+ so reads can block until the command writes output or exits. This
+ setting is only respected in protected configuration.
+
+`notes.externalCommandForGrep`::
+ Boolean indicating whether notes returned by `notes.externalCommand`
+ are included when matching commits with `--grep`, wherever notes would
+ normally participate in grep matching. Defaults to false. This does
+ not make hidden notes searchable in formats such as `--oneline` or
+ `--pretty=%s`; use `--notes` or `--external-notes` if those formats
+ should search notes too. When enabled, revision traversal may invoke
+ the external command for many commits that are not ultimately
+ displayed, which can be expensive for slow commands. The note output
+ can also change which commits match. This setting is only respected in
+ protected configuration.
+
`notes.rewrite.<command>`::
When rewriting commits with _<command>_ (currently `amend` or
`rebase`), if this variable is `false`, git will not copy
diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
index 566238245028..472b37e5237a 100644
--- a/Documentation/git-format-patch.adoc
+++ b/Documentation/git-format-patch.adoc
@@ -26,7 +26,7 @@ SYNOPSIS
[--[no-]cover-letter] [--quiet]
[--commit-list-format=<format-spec>]
[--[no-]encode-email-headers]
- [--no-notes | --notes[=<ref>]]
+ [--no-notes | --notes[=<ref>]] [--[no-]external-notes]
[--interdiff=<previous>]
[--range-diff=<previous> [--creation-factor=<percent>]]
[--filename-max-length=<n>]
@@ -395,6 +395,15 @@ configuration options in linkgit:git-notes[1] to use this workflow).
The default is `--no-notes`, unless the `format.notes` configuration is
set.
+--external-notes::
+--no-external-notes::
+ Invoke or do not invoke `notes.externalCommand` to obtain external
+ notes. Like `--notes=<ref>`, `--external-notes` names an explicit
+ note source and by itself does not include the default notes refs.
+ Use `--external-notes --notes` to include the default notes refs
+ too, or combine `--external-notes` with `--notes=<ref>` to include
+ external notes with specific notes refs.
+
--signature=<signature>::
--no-signature::
Add a signature to each message produced. Per RFC 3676 the signature
diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 5cc5e2ed5673..1de23f300517 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -12,6 +12,7 @@ git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
[--no-dual-color] [--creation-factor=<factor>]
[--left-only | --right-only] [--diff-merges=<format>]
[--remerge-diff] [--no-notes | --notes[=<ref>]]
+ [--[no-]external-notes]
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
[[--] <path>...]
@@ -101,6 +102,11 @@ diff.
This flag is passed to the `git log` program
(see linkgit:git-log[1]) that generates the patches.
+`--external-notes`::
+`--no-external-notes`::
+ This flag is passed to the `git log` program
+ (see linkgit:git-log[1]) that generates the patches.
+
`<range1> <range2>`::
Compare the commits specified by the two ranges, where
_<range1>_ is considered an older version of _<range2>_.
diff --git a/Documentation/pretty-options.adoc b/Documentation/pretty-options.adoc
index 658e462b2533..aad851c92cfd 100644
--- a/Documentation/pretty-options.adoc
+++ b/Documentation/pretty-options.adoc
@@ -93,6 +93,15 @@ being displayed. Examples: "`--notes=foo`" will show only notes from
"`--notes --notes=foo --no-notes --notes=bar`" will only show notes
from `refs/notes/bar`.
+`--external-notes`::
+`--no-external-notes`::
+ Invoke or do not invoke `notes.externalCommand` to obtain external
+ notes. Like `--notes=<ref>`, `--external-notes` names an explicit
+ note source and by itself does not include the default notes refs.
+ Use `--external-notes --notes` to include the default notes refs
+ too, or combine `--external-notes` with `--notes=<ref>` to include
+ external notes with specific notes refs.
+
`--show-notes-by-default`::
Show the default notes unless options for displaying specific
notes are given.
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbfb2..146444e65860 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2023,7 +2023,7 @@ _git_fetch ()
__git_format_patch_extra_options="
--full-index --not --all --no-prefix --src-prefix=
- --dst-prefix= --notes
+ --dst-prefix= --notes --external-notes --no-external-notes
"
_git_format_patch ()
@@ -2215,7 +2215,7 @@ __git_log_common_options="
__git_log_gitk_options="
--dense --sparse --full-history
--simplify-merges --simplify-by-decoration
- --left-right --notes --no-notes
+ --left-right --notes --no-notes --external-notes --no-external-notes
"
# Options that go well for log and shortlog (not gitk)
__git_log_shortlog_options="
--
2.53.0
^ permalink raw reply related
* [PATCH 3/9] wrapper: add sleep_nanosec
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
wrapper.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
wrapper.h | 1 +
2 files changed, 50 insertions(+)
diff --git a/wrapper.c b/wrapper.c
index 16f5a63fbb61..1349255f1eb4 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -10,6 +10,7 @@
#include "gettext.h"
#include "strbuf.h"
#include "trace2.h"
+#include <time.h>
#ifdef HAVE_RTLGENRANDOM
/* This is required to get access to RtlGenRandom. */
@@ -708,6 +709,54 @@ void sleep_millisec(int millisec)
poll(NULL, 0, millisec);
}
+#ifdef GIT_WINDOWS_NATIVE
+/* No nanosleep() on Windows, so fall-back to using sleep_millisec(). */
+int sleep_nanosec(uint64_t nanosec)
+{
+ uint64_t ns_in_1ms = 1000000ULL; /* 1 ms = 10^6 ns */
+
+ uint64_t millisec = nanosec / ns_in_1ms;
+ if (nanosec % ns_in_1ms)
+ millisec++;
+
+ /* Chunked sleep if we can't represent in integer. */
+ while (millisec > INT_MAX) {
+ sleep_millisec(INT_MAX);
+ millisec -= INT_MAX;
+ }
+
+ sleep_millisec((int)millisec);
+
+ return 0;
+}
+#else
+/* Not Windows, so use the more exact nanosleep(). */
+int sleep_nanosec(uint64_t nanosec)
+{
+ int ret;
+ struct timespec duration, remaining;
+
+ /* Construct the duration by dividing the given total (1s = 10^9ns). */
+ duration.tv_sec = nanosec / 1000000000ULL;
+ duration.tv_nsec = nanosec % 1000000000ULL;
+
+ while(1) {
+ ret = nanosleep(&duration, &remaining);
+
+ /* Continue sleeping if interrupted. */
+ if (ret == -1 && errno == EINTR) {
+ duration = remaining;
+ continue;
+ }
+
+ /* Either success or an error. */
+ break;
+ }
+
+ return ret;
+}
+#endif /* GIT_WINDOWS_NATIVE */
+
int xgethostname(char *buf, size_t len)
{
/*
diff --git a/wrapper.h b/wrapper.h
index 15ac3bab6e97..c39992893a81 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -130,6 +130,7 @@ int warn_on_fopen_errors(const char *path);
int open_nofollow(const char *path, int flags);
void sleep_millisec(int millisec);
+int sleep_nanosec(uint64_t nanosec);
enum {
/*
--
2.53.0
^ permalink raw reply related
* [PATCH 2/9] notes: convert raw arg in format_display_notes() to bool
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
It's used as a boolean flag, let's not use an int.
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
log-tree.c | 3 +--
notes.c | 6 +++---
notes.h | 2 +-
revision.c | 2 +-
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 7e048701d0c5..4503a42dde6b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -854,10 +854,9 @@ void show_log(struct rev_info *opt)
}
if (opt->show_notes) {
- int raw;
struct strbuf notebuf = STRBUF_INIT;
+ bool raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
- raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
format_display_notes(&commit->object.oid, ¬ebuf,
get_log_output_encoding(), raw);
ctx.notes_message = strbuf_detach(¬ebuf, NULL);
diff --git a/notes.c b/notes.c
index 8f315e2a00d2..201f1df3dc29 100644
--- a/notes.c
+++ b/notes.c
@@ -1273,11 +1273,11 @@ void free_notes(struct notes_tree *t)
* If the given notes_tree is NULL, the internal/default notes_tree will be
* used instead.
*
- * (raw != 0) gives the %N userformat; otherwise, the note message is given
+ * (raw == true) gives the %N userformat; otherwise, the note message is given
* for human consumption.
*/
static void format_note(struct notes_tree *t, const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, int raw)
+ struct strbuf *sb, const char *output_encoding, bool raw)
{
static const char utf8[] = "utf-8";
const struct object_id *oid;
@@ -1338,7 +1338,7 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
}
void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, int raw)
+ struct strbuf *sb, const char *output_encoding, bool raw)
{
int i;
assert(display_notes_trees);
diff --git a/notes.h b/notes.h
index 6dc6d7b26548..f6410b31e1c9 100644
--- a/notes.h
+++ b/notes.h
@@ -313,7 +313,7 @@ void load_display_notes(struct display_notes_opt *opt);
* You *must* call load_display_notes() before using this function.
*/
void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, int raw);
+ struct strbuf *sb, const char *output_encoding, bool raw);
/*
* Load the notes tree from each ref listed in 'refs'. The output is
diff --git a/revision.c b/revision.c
index 599b3a66c369..cd9fcefa0a88 100644
--- a/revision.c
+++ b/revision.c
@@ -4107,7 +4107,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
if (opt->show_notes) {
if (!buf.len)
strbuf_addstr(&buf, message);
- format_display_notes(&commit->object.oid, &buf, encoding, 1);
+ format_display_notes(&commit->object.oid, &buf, encoding, true);
}
/*
--
2.53.0
^ permalink raw reply related
* [PATCH 7/9] notes: support an external command to display notes
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
git notes is a very very helpful feature to show user-supplied
information about a commit alongside its message transparently.
For distributed teams working on large git repos (huge number of
branches/refs, files, etc.) and using the notes feature to mark
information on git commits, a TOCTOU race can happen due to very
large size of the repo and notes ref:
- Person A updates a note for commit X.
- Person A pushes the notes but it takes some time.
- Person B fetches notes and doesn't find the updated note.
- Person B can come to know of it only when he overwrites it
and encounters a push failure.
This problem excaberates on scale.
One solution to this is a realtime fetch or faster updation via
external means, but unfortunately we lose the coherence in the
display of information, and the user would end up reinventing
git log.
So let's add support for an external command to display the notes.
We split the addition of documentation and tests from this commit for
easier review. The new help text added in Documentation/ in the next
commit should make the usage clear.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
Makefile | 2 +
builtin/log.c | 17 ++-
builtin/name-rev.c | 9 +-
builtin/range-diff.c | 2 +
log-tree.c | 7 +-
meson.build | 1 +
notes-external.c | 330 +++++++++++++++++++++++++++++++++++++++++++
notes-external.h | 19 +++
notes.c | 242 ++++++++++++++++++++++++-------
notes.h | 32 ++++-
revision.c | 32 ++++-
11 files changed, 633 insertions(+), 60 deletions(-)
create mode 100644 notes-external.c
create mode 100644 notes-external.h
diff --git a/Makefile b/Makefile
index c739ae78d0ef..a919bdd75f01 100644
--- a/Makefile
+++ b/Makefile
@@ -838,6 +838,7 @@ TEST_BUILTINS_OBJS += test-match-trees.o
TEST_BUILTINS_OBJS += test-mergesort.o
TEST_BUILTINS_OBJS += test-mktemp.o
TEST_BUILTINS_OBJS += test-name-hash.o
+TEST_BUILTINS_OBJS += test-notes-external-config-reset.o
TEST_BUILTINS_OBJS += test-online-cpus.o
TEST_BUILTINS_OBJS += test-pack-deltas.o
TEST_BUILTINS_OBJS += test-pack-mtimes.o
@@ -1209,6 +1210,7 @@ LIB_OBJS += negotiator/default.o
LIB_OBJS += negotiator/noop.o
LIB_OBJS += negotiator/skipping.o
LIB_OBJS += notes-cache.o
+LIB_OBJS += notes-external.o
LIB_OBJS += notes-merge.o
LIB_OBJS += notes-utils.o
LIB_OBJS += notes.o
diff --git a/builtin/log.c b/builtin/log.c
index 8c0939dd42ad..bed4c1576f2d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1337,9 +1337,24 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev)
(rev->notes_opt.use_default_notes == -1 &&
!rev->notes_opt.extra_notes_refs.nr)) {
strvec_push(arg, "--notes");
- } else {
+ } else if (rev->notes_opt.extra_notes_refs.nr) {
for_each_string_list(&rev->notes_opt.extra_notes_refs, get_notes_refs, arg);
+ } else if (rev->notes_opt.use_external_notes <= 0) {
+ /*
+ * rev->show_notes can stay set after
+ * --external-notes --no-external-notes.
+ *
+ * Since range-diff's child log starts with
+ * --show-notes-by-default, explicitly suppress
+ * notes when no notes source remains.
+ */
+ strvec_push(arg, "--no-notes");
}
+
+ if (rev->notes_opt.use_external_notes > 0)
+ strvec_push(arg, "--external-notes");
+ else if (rev->notes_opt.use_external_notes == 0)
+ strvec_push(arg, "--no-external-notes");
}
static void generate_shortlog_cover_letter(struct shortlog *log,
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 60cbbfb4b7d1..95d5e076e24b 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -277,6 +277,7 @@ struct name_ref_data {
struct pretty_format {
struct pretty_print_context ctx;
struct userformat_want want;
+ bool show_external_notes;
};
enum command_type {
@@ -525,9 +526,9 @@ static const char *get_format_rev(const struct commit *c,
if (format_ctx->want.notes) {
struct strbuf notebuf = STRBUF_INIT;
- format_display_notes(&c->object.oid, ¬ebuf,
- get_log_output_encoding(),
- format_ctx->ctx.fmt == CMIT_FMT_USERFORMAT);
+ format_display_notes(c, ¬ebuf, get_log_output_encoding(),
+ format_ctx->ctx.fmt == CMIT_FMT_USERFORMAT,
+ format_ctx->show_external_notes);
format_ctx->ctx.notes_message = strbuf_detach(¬ebuf, NULL);
}
@@ -878,6 +879,8 @@ int cmd_format_rev(int argc,
enable_ref_display_notes(&format_notes_opt,
&ignore_show_notes,
n->string);
+ format_pp.show_external_notes =
+ display_notes_use_external(&format_notes_opt);
load_display_notes(&format_notes_opt);
}
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index e54c0f7fe156..41c27250404a 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -56,6 +56,8 @@ int cmd_range_diff(int argc,
OPT_PASSTHRU_ARGV(0, "notes", &log_arg,
N_("notes"), N_("passed to 'git log'"),
PARSE_OPT_OPTARG),
+ OPT_PASSTHRU_ARGV(0, "external-notes", &log_arg, NULL,
+ N_("passed to 'git log'"), PARSE_OPT_NOARG),
OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg,
N_("style"), N_("passed to 'git log'"), 0),
OPT_CALLBACK(0, "max-memory", &range_diff_opts.max_memory,
diff --git a/log-tree.c b/log-tree.c
index 4503a42dde6b..3289a085f66b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -856,9 +856,12 @@ void show_log(struct rev_info *opt)
if (opt->show_notes) {
struct strbuf notebuf = STRBUF_INIT;
bool raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
+ const struct display_notes_opt *notes_opt = &opt->notes_opt;
+
+ format_display_notes(commit, ¬ebuf,
+ get_log_output_encoding(), raw,
+ display_notes_use_external(notes_opt));
- format_display_notes(&commit->object.oid, ¬ebuf,
- get_log_output_encoding(), raw);
ctx.notes_message = strbuf_detach(¬ebuf, NULL);
}
diff --git a/meson.build b/meson.build
index de917bcf1146..21cdbc15aa18 100644
--- a/meson.build
+++ b/meson.build
@@ -397,6 +397,7 @@ libgit_sources = [
'notes-merge.c',
'notes-utils.c',
'notes.c',
+ 'notes-external.c',
'object-file-convert.c',
'object-file.c',
'object-name.c',
diff --git a/notes-external.c b/notes-external.c
new file mode 100644
index 000000000000..7d6b2c6060e0
--- /dev/null
+++ b/notes-external.c
@@ -0,0 +1,330 @@
+#include "git-compat-util.h"
+#include "gettext.h"
+#include "hex.h"
+#include "notes-external.h"
+#include "run-command.h"
+#include "sigchain.h"
+#include "strbuf.h"
+#include "trace.h"
+
+#define convert_ms_to_ns(ms) (uint64_t)(ms) * 1000000ULL
+#define convert_ns_to_ms(ns) (uint64_t)(ns) / 1000000ULL
+#define EXTERNAL_NOTES_TERMINATE_GRACE_NS 100000000ULL /* 100 ms = 10^8 ns */
+#define EXTERNAL_NOTES_READ_CHUNK_SIZE 16384 /* (16 * 1024) bytes */
+
+#ifdef GIT_WINDOWS_NATIVE
+#define EXTERNAL_NOTES_FORCE_KILL_SIGNAL SIGTERM
+#else
+#define EXTERNAL_NOTES_FORCE_KILL_SIGNAL SIGKILL
+#endif
+
+/* ------------------------------------------------------------------------- */
+
+/* Configuration helpers. */
+
+static char *external_notes_command;
+static char *external_notes_command_name_value;
+static uint64_t external_notes_read_timeout_ns = convert_ms_to_ns(100);
+static int external_notes_command_failed;
+static int external_notes_for_grep;
+static bool external_notes_started;
+
+void set_external_notes_command(const char *command)
+{
+ FREE_AND_NULL(external_notes_command);
+ if (command && *command)
+ external_notes_command = xstrdup(command);
+}
+
+void set_external_notes_command_name(const char *name)
+{
+ FREE_AND_NULL(external_notes_command_name_value);
+ if (name && *name)
+ external_notes_command_name_value = xstrdup(name);
+}
+
+void set_external_notes_command_timeout_ms(int timeout_ms)
+{
+ if (timeout_ms < 0)
+ BUG("negative notes.externalCommandTimeoutMs");
+
+ external_notes_read_timeout_ns = convert_ms_to_ns(timeout_ms);
+}
+
+void reset_external_notes_command(void)
+{
+ if (external_notes_started)
+ BUG("cannot reset external notes config while cmd is running");
+
+ FREE_AND_NULL(external_notes_command);
+ FREE_AND_NULL(external_notes_command_name_value);
+ external_notes_read_timeout_ns = convert_ms_to_ns(100);
+ external_notes_command_failed = 0;
+ external_notes_for_grep = 0;
+}
+
+int external_notes_command_configured(void)
+{
+ return external_notes_command && !external_notes_command_failed;
+}
+
+const char *external_notes_command_name(void)
+{
+ return external_notes_command_name_value ?
+ external_notes_command_name_value : "external";
+}
+
+int external_notes_command_timeout_ms(void)
+{
+ return (int)convert_ns_to_ms(external_notes_read_timeout_ns);
+}
+
+void set_external_notes_for_grep(int enabled)
+{
+ external_notes_for_grep = enabled;
+}
+
+int external_notes_for_grep_enabled(void)
+{
+ return external_notes_for_grep;
+}
+
+/* ------------------------------------------------------------------------- */
+
+/* Process management helpers. */
+
+static struct child_process external_notes_process = CHILD_PROCESS_INIT;
+static FILE *external_notes_in;
+static int external_notes_out_fd = -1;
+
+static void mute_routine(const char *msg UNUSED, va_list params UNUSED)
+{
+ /* do nothing */
+}
+
+static void close_external_notes_process_pipes(struct child_process *process)
+{
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ if (external_notes_in) {
+ fclose(external_notes_in);
+ external_notes_in = NULL;
+ } else {
+ close(process->in);
+ }
+
+ if (external_notes_out_fd >= 0) {
+ close(external_notes_out_fd);
+ external_notes_out_fd = -1;
+ } else {
+ close(process->out);
+ }
+
+ sigchain_pop(SIGPIPE);
+}
+
+/* We set this as callback later, so can't have void argument. */
+static void cleanup_external_notes_process(struct child_process *process)
+{
+ report_fn old_error = NULL;
+
+ /**
+ * The helper may still be sleeping with its pipes open, or may not
+ * exit promptly after EOF. Ask it to stop, then use a bounded wait
+ * that escalates if it ignores the signal.
+ */
+ kill(process->pid, SIGTERM);
+ old_error = get_error_routine();
+ set_error_routine(mute_routine);
+
+ close_external_notes_process_pipes(process);
+ finish_command_with_timeout(process, EXTERNAL_NOTES_TERMINATE_GRACE_NS,
+ EXTERNAL_NOTES_FORCE_KILL_SIGNAL);
+
+ if (old_error)
+ set_error_routine(old_error);
+
+ external_notes_started = false;
+}
+
+static void stop_external_notes_process(void)
+{
+ if (!external_notes_started)
+ return;
+
+ external_notes_process.clean_on_exit = 0;
+ cleanup_external_notes_process(&external_notes_process);
+ child_process_init(&external_notes_process);
+}
+
+static int fail_external_notes_command(void)
+{
+ if (!external_notes_command_failed)
+ warning(_("notes.externalCommand failed: %s"),
+ external_notes_command);
+
+ external_notes_command_failed = 1;
+ stop_external_notes_process();
+ return -1;
+}
+
+static int start_external_notes_command(void)
+{
+ struct child_process *cmd = &external_notes_process;
+
+ if (external_notes_started)
+ return 0;
+
+ if (!external_notes_command || external_notes_command_failed)
+ return -1;
+
+ child_process_init(cmd);
+ strvec_push(&cmd->args, external_notes_command);
+ cmd->use_shell = 1;
+ cmd->in = -1;
+ cmd->out = -1;
+ cmd->clean_on_exit = 1;
+ cmd->clean_on_exit_handler = cleanup_external_notes_process;
+ cmd->trace2_child_class = "notes-external";
+
+ if (start_command(cmd))
+ return fail_external_notes_command();
+
+ external_notes_in = xfdopen(cmd->in, "wb");
+ external_notes_out_fd = cmd->out;
+ external_notes_started = true;
+ return 0;
+}
+
+/* ------------------------------------------------------------------------- */
+
+/* Command parser. Essentially the main() function of this file. */
+int format_external_note(const struct object_id *object_oid,
+ struct strbuf *note_buf)
+{
+ struct strbuf status = STRBUF_INIT;
+ char commit_id_hex_str[GIT_MAX_HEXSZ + 1];
+ const char *arg;
+ char *end;
+ char ch;
+ unsigned long len;
+ uint64_t deadline_ns;
+ bool input_fail;
+ int ret = 0;
+
+ if (start_external_notes_command())
+ return -1;
+
+ /* Fetch the commit ID hex. */
+ oid_to_hex_r(commit_id_hex_str, object_oid);
+
+ /* Pass the input to the external command. */
+ sigchain_push(SIGPIPE, SIG_IGN);
+ input_fail = fprintf(external_notes_in, "%s\n", commit_id_hex_str) < 0
+ || fflush(external_notes_in) != 0;
+ sigchain_pop(SIGPIPE);
+
+ if (input_fail)
+ goto out_fail;
+
+ if (external_notes_read_timeout_ns == 0)
+ deadline_ns = 0;
+ else
+ deadline_ns = getnanotime() + external_notes_read_timeout_ns;
+
+ /**
+ * The output for each commit is either of the two:
+ * "{commit id} missing\n"
+ * "{commit id} ok {num_bytes}\n{str_of_num_bytes}\n"
+ *
+ * We can have "\r\n" instead of "\n" due to Windows.
+ */
+
+ /* Read the first line with its delimiter. */
+ if (strbuf_getwholeline_fd_deadline(&status, external_notes_out_fd,
+ '\n', deadline_ns) == EOF)
+ goto out_fail;
+
+ /* Reject EOF-terminated partial lines. */
+ if (!status.len || status.buf[status.len - 1] != '\n')
+ goto out_fail;
+
+ /**
+ * Strip LF and then optional CR so both LF and CRLF protocol lines
+ * are accepted.
+ */
+ strbuf_setlen(&status, status.len - 1);
+ strbuf_strip_suffix(&status, "\r");
+
+ /* Check if line starts with the commit ID. */
+ if (!skip_prefix(status.buf, commit_id_hex_str, &arg))
+ goto out_fail;
+
+ if (*arg++ != ' ') /* After commit ID there should be a space. */
+ goto out_fail;
+
+ if (strcmp(arg, "missing") == 0) /* No note available. */
+ goto out_success; /* Ending newline is already ensured. */
+
+ if (!skip_prefix(arg, "ok ", &arg)) /* Neither missing nor ok. */
+ goto out_fail;
+
+ /* We are in "ok" case. */
+
+ /* The next thing is length of the note. It must be unsigned digits. */
+ if (!isdigit(*arg))
+ goto out_fail;
+
+ /* Get the length of note. */
+ errno = 0;
+ len = strtoul(arg, &end, 10);
+ if (errno != 0 || *end != '\0' || end == arg)
+ goto out_fail;
+
+ /* Ending newline is already ensured. */
+
+ /* Read the trailing note in bounded-chunks. */
+ while (note_buf->len < len) {
+ ssize_t got;
+ size_t remaining = len - note_buf->len;
+ size_t want = remaining < EXTERNAL_NOTES_READ_CHUNK_SIZE ?
+ remaining : EXTERNAL_NOTES_READ_CHUNK_SIZE;
+
+ strbuf_grow(note_buf, want);
+
+ got = read_in_full_deadline(external_notes_out_fd,
+ note_buf->buf + note_buf->len,
+ want, deadline_ns);
+ if (got < 0 || (size_t)got != want)
+ goto out_fail;
+
+ strbuf_setlen(note_buf, note_buf->len + (size_t)got);
+ }
+
+ /* Ensure the ending newline (LF/CRLF) after the note. */
+ if (xread_deadline(external_notes_out_fd, &ch, 1, deadline_ns) != 1)
+ goto out_fail;
+
+ if (ch != '\n') { /* Not a LF. */
+ if (ch != '\r') /* Not a CRLF. */
+ goto out_fail;
+
+ /* We have '\r', let's read the next char. */
+ if (xread_deadline(external_notes_out_fd, &ch, 1,
+ deadline_ns) != 1)
+ goto out_fail;
+
+ if (ch != '\n') /* Not a CRLF. */
+ goto out_fail;
+ }
+
+ goto out_success;
+
+out_fail:
+ ret = fail_external_notes_command();
+out_success:
+ strbuf_release(&status);
+ return ret;
+}
+
+/* ------------------------------------------------------------------------- */
diff --git a/notes-external.h b/notes-external.h
new file mode 100644
index 000000000000..e49a50b09063
--- /dev/null
+++ b/notes-external.h
@@ -0,0 +1,19 @@
+#ifndef NOTES_EXTERNAL_H
+#define NOTES_EXTERNAL_H
+
+struct object_id;
+struct strbuf;
+
+void set_external_notes_command(const char *command);
+void set_external_notes_command_name(const char *name);
+void set_external_notes_command_timeout_ms(int timeout_ms);
+void set_external_notes_for_grep(int enabled);
+void reset_external_notes_command(void);
+int external_notes_command_configured(void);
+const char *external_notes_command_name(void);
+int external_notes_command_timeout_ms(void);
+int external_notes_for_grep_enabled(void);
+int format_external_note(const struct object_id *object_oid,
+ struct strbuf *out);
+
+#endif /* NOTES_EXTERNAL_H */
diff --git a/notes.c b/notes.c
index 201f1df3dc29..0ff8ba94afc5 100644
--- a/notes.c
+++ b/notes.c
@@ -3,9 +3,12 @@
#include "git-compat-util.h"
#include "config.h"
+#include "commit.h"
#include "environment.h"
+#include "gettext.h"
#include "hex.h"
#include "notes.h"
+#include "notes-external.h"
#include "object-file.h"
#include "object-name.h"
#include "odb.h"
@@ -983,18 +986,56 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,
free(globs_copy);
}
+struct notes_display_config_data {
+ int load_refs;
+ int load_command;
+};
+
static int notes_display_config(const char *k, const char *v,
- const struct config_context *ctx UNUSED,
+ const struct config_context *ctx,
void *cb)
{
- int *load_refs = cb;
+ struct notes_display_config_data *data = cb;
- if (*load_refs && !strcmp(k, "notes.displayref")) {
+ if (data->load_refs && !strcmp(k, "notes.displayref")) {
if (!v)
return config_error_nonbool(k);
string_list_add_refs_by_glob(&display_notes_refs, v);
}
+ if (data->load_command && !strcmp(k, "notes.externalcommand")) {
+ if (!v)
+ return config_error_nonbool(k);
+
+ set_external_notes_command(v);
+ }
+
+ if (data->load_command && !strcmp(k, "notes.externalcommandname")) {
+ if (!v)
+ return config_error_nonbool(k);
+
+ if (strchr(v, '\n') || strchr(v, '\r'))
+ return error(_("notes.externalCommandName must not contain a newline"));
+
+ set_external_notes_command_name(v);
+ }
+
+ if (data->load_command && !strcmp(k, "notes.externalcommandtimeoutms")) {
+ int timeout_ms;
+
+ if (!v)
+ return config_error_nonbool(k);
+
+ timeout_ms = git_config_int(k, v, ctx->kvi);
+ if (timeout_ms < 0)
+ return error(_("notes.externalCommandTimeoutMs must be non-negative"));
+
+ set_external_notes_command_timeout_ms(timeout_ms);
+ }
+
+ if (data->load_command && !strcmp(k, "notes.externalcommandforgrep"))
+ set_external_notes_for_grep(git_config_bool(k, v));
+
return 0;
}
@@ -1075,6 +1116,7 @@ void init_display_notes(struct display_notes_opt *opt)
{
memset(opt, 0, sizeof(*opt));
opt->use_default_notes = -1;
+ opt->use_external_notes = -1;
string_list_init_dup(&opt->extra_notes_refs);
}
@@ -1086,6 +1128,7 @@ void release_display_notes(struct display_notes_opt *opt)
void enable_default_display_notes(struct display_notes_opt *opt, int *show_notes)
{
opt->use_default_notes = 1;
+ opt->default_notes_suppressed_by_external = 0;
*show_notes = 1;
}
@@ -1102,31 +1145,85 @@ void enable_ref_display_notes(struct display_notes_opt *opt, int *show_notes,
void disable_display_notes(struct display_notes_opt *opt, int *show_notes)
{
opt->use_default_notes = -1;
+ opt->use_external_notes = -1;
+ opt->default_notes_suppressed_by_external = 0;
string_list_clear(&opt->extra_notes_refs, 0);
*show_notes = 0;
}
+/*
+ * Resolve the default-notes tri-state in one place. Callers must not test
+ * use_default_notes directly unless they specifically need the unresolved
+ * command-line state.
+ */
+static bool display_notes_use_default(const struct display_notes_opt *opt)
+{
+ /* Options aren't specified, default to true. */
+ if (!opt)
+ return true;
+
+ /* Explicitly enabled. */
+ if (opt->use_default_notes > 0)
+ return true;
+
+ /* Undefined and no explicit notes-ref specified, default to true. */
+ if (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)
+ return true;
+
+ return false;
+}
+
+/*
+ * Resolve the external-notes tri-state. The unset value follows the resolved
+ * default-notes decision, which means "git log" runs the helper by default
+ * but "git log --notes=<ref>" does not.
+ */
+bool display_notes_use_external(const struct display_notes_opt *opt)
+{
+ /* Options aren't specified, default to true. */
+ if (!opt)
+ return true;
+
+ /* Explicitly enabled. */
+ if (opt->use_external_notes > 0)
+ return true;
+
+ /* Undefined and to use default notes set, default to true. */
+ if (opt->use_external_notes < 0 && display_notes_use_default(opt))
+ return true;
+
+ return false;
+}
+
void load_display_notes(struct display_notes_opt *opt)
{
char *display_ref_env;
- int load_config_refs = 0;
+ struct notes_display_config_data config = { 0, 0 };
+ struct notes_display_config_data protected_config = { 0, 0 };
+ bool use_default_notes = display_notes_use_default(opt);
+ bool use_external_notes = display_notes_use_external(opt);
+
display_notes_refs.strdup_strings = 1;
+ reset_external_notes_command();
assert(!display_notes_trees);
- if (!opt || opt->use_default_notes > 0 ||
- (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)) {
+ if (use_default_notes) {
string_list_append_nodup(&display_notes_refs, default_notes_ref(the_repository));
display_ref_env = getenv(GIT_NOTES_DISPLAY_REF_ENVIRONMENT);
if (display_ref_env) {
string_list_add_refs_from_colon_sep(&display_notes_refs,
display_ref_env);
- load_config_refs = 0;
+ config.load_refs = 0;
} else
- load_config_refs = 1;
+ config.load_refs = 1;
}
- repo_config(the_repository, notes_display_config, &load_config_refs);
+ if (use_external_notes)
+ protected_config.load_command = 1;
+
+ repo_config(the_repository, notes_display_config, &config);
+ git_protected_config(notes_display_config, &protected_config);
if (opt) {
struct string_list_item *item;
@@ -1266,47 +1363,31 @@ void free_notes(struct notes_tree *t)
}
/*
- * Fill the given strbuf with the notes associated with the given object.
+ * Append one already-loaded note message to the given strbuf.
*
- * If the given notes_tree structure is not initialized, it will be auto-
- * initialized to the default value (see documentation for init_notes() above).
- * If the given notes_tree is NULL, the internal/default notes_tree will be
- * used instead.
+ * Notes read from refs and notes obtained from notes.externalCommand both use
+ * this helper so they share the same encoding, header, and indentation rules.
*
* (raw == true) gives the %N userformat; otherwise, the note message is given
* for human consumption.
*/
-static void format_note(struct notes_tree *t, const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, bool raw)
+static void format_note_data(const char *ref, const char *msg, size_t msglen,
+ struct strbuf *sb, const char *output_encoding,
+ bool raw, bool literal_ref)
{
static const char utf8[] = "utf-8";
- const struct object_id *oid;
- char *msg, *msg_p;
- unsigned long linelen, msglen;
- enum object_type type;
-
- if (!t)
- t = &default_notes_tree;
- if (!t->initialized)
- init_notes(t, NULL, NULL, 0);
-
- oid = get_note(t, object_oid);
- if (!oid)
- return;
-
- if (!(msg = odb_read_object(the_repository->objects, oid, &type, &msglen)) ||
- type != OBJ_BLOB) {
- free(msg);
- return;
- }
+ char *reencoded = NULL;
+ const char *msg_p, *msg_end;
+ /* Convert the note text from UTF-8 to the requested output encoding. */
if (output_encoding && *output_encoding &&
!is_encoding_utf8(output_encoding)) {
- char *reencoded = reencode_string(msg, output_encoding, utf8);
+ size_t reencoded_len;
+ reencoded = reencode_string_len(msg, msglen, output_encoding,
+ utf8, &reencoded_len);
if (reencoded) {
- free(msg);
msg = reencoded;
- msglen = strlen(msg);
+ msglen = reencoded_len;
}
}
@@ -1314,37 +1395,100 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
if (msglen && msg[msglen - 1] == '\n')
msglen--;
+ /* Raw mode is the %N userformat, so it omits the "Notes" header. */
if (!raw) {
- const char *ref = t->ref;
- if (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {
+ if (!ref)
strbuf_addstr(sb, "\nNotes:\n");
- } else {
- skip_prefix(ref, "refs/", &ref);
- skip_prefix(ref, "notes/", &ref);
+ else if (!literal_ref && !strcmp(ref, GIT_NOTES_DEFAULT_REF))
+ strbuf_addstr(sb, "\nNotes:\n");
+ else {
+ if (!literal_ref) {
+ skip_prefix(ref, "refs/", &ref);
+ skip_prefix(ref, "notes/", &ref);
+ }
strbuf_addf(sb, "\nNotes (%s):\n", ref);
}
}
- for (msg_p = msg; msg_p < msg + msglen; msg_p += linelen + 1) {
- linelen = strchrnul(msg_p, '\n') - msg_p;
+ msg_end = msg + msglen;
+ for (msg_p = msg; msg_p < msg_end; ) {
+ const char *eol = memchr(msg_p, '\n', msg_end - msg_p);
+ size_t linelen = eol ? eol - msg_p : msg_end - msg_p;
+ /* Human output indents note body lines under the header. */
if (!raw)
strbuf_addstr(sb, " ");
+
strbuf_add(sb, msg_p, linelen);
strbuf_addch(sb, '\n');
+
+ msg_p += linelen;
+ if (msg_p < msg_end)
+ msg_p++;
+ }
+
+ free(reencoded);
+}
+
+/*
+ * Fill the given strbuf with the notes associated with the given object.
+ *
+ * If the given notes_tree structure is not initialized, it will be auto-
+ * initialized to the default value (see documentation for init_notes() above).
+ * If the given notes_tree is NULL, the internal/default notes_tree will be
+ * used instead.
+ */
+static void format_note_from_tree(struct notes_tree *t,
+ const struct object_id *object_oid,
+ struct strbuf *sb,
+ const char *output_encoding, bool raw)
+{
+ const struct object_id *oid;
+ char *msg;
+ unsigned long msglen;
+ enum object_type type;
+
+ if (!t)
+ t = &default_notes_tree;
+ if (!t->initialized)
+ init_notes(t, NULL, NULL, 0);
+
+ oid = get_note(t, object_oid);
+ if (!oid)
+ return;
+
+ if (!(msg = odb_read_object(the_repository->objects, oid, &type, &msglen)) ||
+ type != OBJ_BLOB) {
+ free(msg);
+ return;
}
+ format_note_data(t->ref, msg, msglen, sb, output_encoding, raw, false);
+
free(msg);
}
-void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, bool raw)
+void format_display_notes(const struct commit *commit,
+ struct strbuf *sb, const char *output_encoding,
+ bool raw, bool show_external)
{
int i;
+ const struct object_id *commit_oid = &commit->object.oid;
+
assert(display_notes_trees);
for (i = 0; display_notes_trees[i]; i++)
- format_note(display_notes_trees[i], object_oid, sb,
- output_encoding, raw);
+ format_note_from_tree(display_notes_trees[i], commit_oid, sb,
+ output_encoding, raw);
+
+ if (show_external && external_notes_command_configured()) {
+ struct strbuf out = STRBUF_INIT;
+
+ if (format_external_note(commit_oid, &out) == 0 && out.len)
+ format_note_data(external_notes_command_name(),
+ out.buf, out.len, sb,
+ output_encoding, raw, true);
+ strbuf_release(&out);
+ }
}
int copy_note(struct notes_tree *t,
diff --git a/notes.h b/notes.h
index f6410b31e1c9..748af70e34af 100644
--- a/notes.h
+++ b/notes.h
@@ -6,6 +6,7 @@
struct object_id;
struct repository;
struct strbuf;
+struct commit;
/*
* Function type for combining two notes annotating the same object.
@@ -264,6 +265,20 @@ struct display_notes_opt {
*/
int use_default_notes;
+ /*
+ * Less than `0` is "unset", which means external notes are shown iff
+ * the default notes are shown. Otherwise, treat it like a boolean.
+ */
+ int use_external_notes;
+
+ /*
+ * Tracks the synthetic "default notes off" state introduced by
+ * `--external-notes`, so a later deprecated `--show-notes=<ref>`
+ * can still preserve its historical additive behavior without
+ * overriding an explicit `--no-standard-notes`.
+ */
+ int default_notes_suppressed_by_external;
+
/*
* A list of globs (in the same style as notes.displayRef) where
* notes should be loaded from.
@@ -304,16 +319,27 @@ void disable_display_notes(struct display_notes_opt *opt, int *show_notes);
void load_display_notes(struct display_notes_opt *opt);
/*
- * Append notes for the given 'object_sha1' from all trees set up by
+ * Return true if notes.externalCommand should be used for 'opt'.
+ *
+ * 'opt' may be NULL.
+ */
+bool display_notes_use_external(const struct display_notes_opt *opt);
+
+/*
+ * Append notes for the given commit from all trees set up by
* load_display_notes() to 'sb'.
*
* If 'raw' is false the note will be indented by 4 places and
* a 'Notes (refname):' header added.
*
+ * If 'show_external' is true then notes.externalCommand will be used to append
+ * the note from external source.
+ *
* You *must* call load_display_notes() before using this function.
*/
-void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, bool raw);
+void format_display_notes(const struct commit *commit,
+ struct strbuf *sb, const char *output_encoding,
+ bool raw, bool show_external);
/*
* Load the notes tree from each ref listed in 'refs'. The output is
diff --git a/revision.c b/revision.c
index cd9fcefa0a88..84d9af961988 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@
#include "environment.h"
#include "gettext.h"
#include "hex.h"
+#include "notes-external.h"
#include "object-name.h"
#include "object-file.h"
#include "odb.h"
@@ -2583,18 +2584,40 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
} else if (skip_prefix(arg, "--show-notes=", &optarg) ||
skip_prefix(arg, "--notes=", &optarg)) {
if (starts_with(arg, "--show-notes=") &&
- revs->notes_opt.use_default_notes < 0)
+ (revs->notes_opt.use_default_notes < 0 ||
+ revs->notes_opt.default_notes_suppressed_by_external)) {
revs->notes_opt.use_default_notes = 1;
+ revs->notes_opt.default_notes_suppressed_by_external = 0;
+ }
enable_ref_display_notes(&revs->notes_opt, &revs->show_notes, optarg);
revs->show_notes_given = 1;
} else if (!strcmp(arg, "--no-notes")) {
disable_display_notes(&revs->notes_opt, &revs->show_notes);
revs->show_notes_given = 1;
+ } else if (!strcmp(arg, "--external-notes")) {
+ revs->notes_opt.use_external_notes = 1;
+ revs->show_notes = 1;
+ revs->show_notes_given = 1;
+ /*
+ * `--external-notes` names a note source on its own. If the
+ * default notes ref is still undecided, settle it to "off" so
+ * this option does not also trigger the "no explicit notes
+ * refs" fallback. A later use of `--notes` or the deprecated
+ * `--show-notes=<ref>` can still turn the default ref on.
+ */
+ if (revs->notes_opt.use_default_notes < 0) {
+ revs->notes_opt.use_default_notes = 0;
+ revs->notes_opt.default_notes_suppressed_by_external = 1;
+ }
+ } else if (!strcmp(arg, "--no-external-notes")) {
+ revs->notes_opt.use_external_notes = 0;
} else if (!strcmp(arg, "--standard-notes")) {
revs->show_notes_given = 1;
revs->notes_opt.use_default_notes = 1;
+ revs->notes_opt.default_notes_suppressed_by_external = 0;
} else if (!strcmp(arg, "--no-standard-notes")) {
revs->notes_opt.use_default_notes = 0;
+ revs->notes_opt.default_notes_suppressed_by_external = 0;
} else if (!strcmp(arg, "--oneline")) {
revs->verbose_header = 1;
get_commit_format("oneline", revs);
@@ -4105,9 +4128,14 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
/* Append "fake" message parts as needed */
if (opt->show_notes) {
+ const struct display_notes_opt *notes_opt = &opt->notes_opt;
+
if (!buf.len)
strbuf_addstr(&buf, message);
- format_display_notes(&commit->object.oid, &buf, encoding, true);
+
+ format_display_notes(commit, &buf, encoding, true,
+ (display_notes_use_external(notes_opt)
+ && external_notes_for_grep_enabled()));
}
/*
--
2.53.0
^ permalink raw reply related
* [PATCH 6/9] t3301: cover generic displayed notes behavior
From: Siddh Raman Pant @ 2026-05-19 16:30 UTC (permalink / raw)
To: git
Cc: Calvin Wan, Patrick Steinhardt, Elijah Newren,
Kristoffer Haugsbakk, Junio C Hamano
In-Reply-To: <cover.1779207350.git.siddh.raman.pant@oracle.com>
Displayed notes already participate in common log behavior.
Add explicit coverage for raw notes formatting, --no-notes
suppression, explicit notes refs, and --grep matching before
teaching external notes to feed the same display path.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
t/t3301-notes.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index d6c50460d086..27439010dfbc 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -885,6 +885,30 @@ test_expect_success '--show-notes=ref accumulates' '
test_cmp expect-both-reversed actual
'
+test_expect_success 'displayed notes honor raw notes formatting' '
+ git show -s --format=%N >actual &&
+ test_grep "^order test$" actual &&
+ ! grep "Notes" actual
+'
+
+test_expect_success 'displayed notes are suppressed by --no-notes' '
+ git log --no-notes -1 >actual &&
+ test_cmp expect-not-other actual
+'
+
+test_expect_success 'explicit notes ref replaces default displayed notes' '
+ git log --notes=other -1 >actual &&
+ test_cmp expect-other actual
+'
+
+test_expect_success 'displayed notes are used for grep matching' '
+ commit=$(git rev-parse HEAD) &&
+ git log --grep="order test" -1 >actual &&
+ test_grep "^commit $commit$" actual &&
+ git log --no-notes --grep="order test" -1 >actual &&
+ test_must_be_empty actual
+'
+
test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
test_config core.notesRef refs/notes/other &&
echo "Note on a tree" >expect &&
--
2.53.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox