* [PATCH v2] config.mak.uname: avoid macOS linker warning on Xcode 16.3+
From: Harald Nordgren via GitGitGadget @ 2026-05-29 14:32 UTC (permalink / raw)
To: git; +Cc: Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2313.git.git.1779901919956.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Building on macOS with Xcode 16.3 or newer emits:
ld: warning: reducing alignment of section __DATA,__common
from 0x8000 to 0x4000 because it exceeds segment maximum
alignment
Pass -fno-common when "ld -v" reports ld-1167 or newer, so tentative
definitions of large arrays go into BSS instead of __DATA,__common.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
pkt-line: initialize packet_buffer to avoid macOS linker warning
* Check MacOS ld version instead
(https://en.wikipedia.org/wiki/Xcode#Xcode_15.0_-_16.x_(since_visionOS_support)_2)
Parsing output of
❯ ld -v
@(#)PROGRAM:ld PROJECT:ld-1267
BUILD 18:30:29 Apr 22 2026
configured to support archs: armv6 armv7 armv7s arm64 arm64e arm64_32 i386 x86_64 x86_64h armv6m armv7k armv7m armv7em armv8m.main armv8.1m.main
will use ld-classic for: armv6 armv7 armv7s i386 armv6m armv7k armv7m armv7em
LTO support using: LLVM version 21.0.0 (static support for 30, runtime is 30)
TAPI support using: Apple TAPI version 21.0.0 (tapi-2100.0.2.6)
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2313%2FHaraldNordgren%2Fpkt-line-init-buffer-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2313/HaraldNordgren/pkt-line-init-buffer-v2
Pull-Request: https://github.com/git/git/pull/2313
Range-diff vs v1:
1: 1c1c66d85b < -: ---------- pkt-line: initialize packet_buffer to avoid macOS linker warning
-: ---------- > 1: 0e660a346e config.mak.uname: avoid macOS linker warning on Xcode 16.3+
config.mak.uname | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/config.mak.uname b/config.mak.uname
index ce5e7de779..d4d55cb324 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -163,6 +163,12 @@ ifeq ($(uname_S),Darwin)
NEEDS_GOOD_LIBICONV = UnfortunatelyYes
endif
+ # Silence Xcode 16.3+ linker warning about __DATA,__common alignment.
+ LD_MAJOR_VERSION = $(shell ld -v 2>&1 | sed -n 's/.*PROJECT:ld-\([0-9]*\).*/\1/p')
+ ifeq ($(shell test "$(LD_MAJOR_VERSION)" -ge 1167 && echo 1),1)
+ BASIC_CFLAGS += -fno-common
+ endif
+
# The builtin FSMonitor on MacOS builds upon Simple-IPC. Both require
# Unix domain sockets and PThreads.
ifndef NO_PTHREADS
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
--
gitgitgadget
^ permalink raw reply related
* Re: [BUG] internal date format does not accept small unix timestamps
From: Kristoffer Haugsbakk @ 2026-05-29 12:50 UTC (permalink / raw)
To: Luna Schwalbe, git
In-Reply-To: <fffe0ea9-baea-47cc-b354-5be4fff08983@luna.gl>
On Fri, May 29, 2026, at 13:51, Luna Schwalbe wrote:
> While trying to create some test commits, I noticed the following issue:
>
> GIT_AUTHOR_DATE and GIT_COMMITTER_DATE should accept the "Git internal
> format" (displayed by git log with --date=raw), but this fails for small
> unix timestamps. A quick binary search indicates that it happens when
> the unix timestamp is below 100000000 (9 digits).
>
> So for example, GIT_AUTHOR_DATE='99999999 +0000' fails with "fatal:
> invalid date format", while GIT_AUTHOR_DATE='100000000 +0000' works as
> expected.
> It seems to be unaffected by the choice of timezone offset.
> Padding the timestamp with zeroes also does not change the behavior.
>
> The --date option does accept all the values, but interprets them
> wrongly and gives bogus results (most of them time it seems to act as if
> no date option was given, using the current system time, but with some
> inputs I've also observed things like "current date&time but set the
> year to 2000").
>
> I tested everything with git built from commit
> 2f8565e1d14d2de4cfbc9da0132131bf0d0dc087.
Apparently you need `@` in front for small Unix Epoch values. `@0 +0000`
^ permalink raw reply
* Re: [PATCH 0/5] git son: add command to create independent child repositories
From: Claus Schneider @ 2026-05-29 12:35 UTC (permalink / raw)
To: Ben Knoble; +Cc: Evan Haque via GitGitGadget, git, Evan Haque
In-Reply-To: <7377E3A2-C866-4E3D-85FC-BC6E10CBF8FC@gmail.com>
Hi ..
I would motivate to fix in git-submodules in stead. I updated the
logic around the ignored setting of a submodule, so it is truly
ignored - both in git status(already) and `git add`, which now
explicitly requires `--force`. It now gives the ability to configure
your submodule as 'loosely' by tracking branches without the friction
of `git add` is staging it all the time and you get conflicts in
PR/integrations. You can use git submodule status to list the sha1 of
the submodule for release etc.
https://github.com/gitgitgadget/git/pull/1987
alias "git-add: Skip submodules with ignore=all unless --force and
explicit path used by bicschneider · Pull Request #1987 ·
gitgitgadget/git"
Please try this update and then describe what is ( still ) missing as
i am looking into submodules in general and will try to "fix" the
friction points of developers.
Best regards
Claus Schneider
On Tue, May 26, 2026 at 11:29 PM Ben Knoble <ben.knoble@gmail.com> wrote:
>
>
> > Le 26 mai 2026 à 13:08, Evan Haque via GitGitGadget <gitgitgadget@gmail.com> a écrit :
> >
> >
> > Motivation
> > ==========
> >
> > When spinning off a new project that is related to an existing repository,
> > there is no built-in way to create a child repository that maintains a link
> > back to its parent without the tight coupling of submodules. Submodules pin
> > the child to a specific commit and require the parent to track the child in
> > its index, which is too heavyweight when the child is meant to be fully
> > independent.
> >
> > The typical workflow today is manual: git init, git remote add, update
> > .gitignore — three steps that are easy to forget or get wrong. git son
> > automates this and establishes a lightweight convention for the parent-child
> > relationship: a remote named parent in the child, and nothing in the parent
> > except an ignore rule.
>
> I don’t really understand the motivation, but if your goal is to create another repo with the current one as a remote, how does something like
>
> git clone . child
>
> help you? (I’m pretty sure you can even set the remote name to « parent » if you wish.)
>
> You also didn’t mention worktrees or subtrees, which might be useful for you.
^ permalink raw reply
* Re: [PATCH v3 0/3] line-log: integrate -L with the standard log output pipeline
From: Ben Knoble @ 2026-05-29 12:04 UTC (permalink / raw)
To: Michael Montalbo via GitGitGadget; +Cc: git, Michael Montalbo
In-Reply-To: <pull.2094.v3.git.1780001267.gitgitgadget@gmail.com>
> Le 28 mai 2026 à 16:47, Michael Montalbo via GitGitGadget <gitgitgadget@gmail.com> a écrit :
>
> Since its introduction, git log -L has short-circuited from
> log_tree_commit() into its own output function, bypassing log_tree_diff()
> and log_tree_diff_flush(). This skips no_free save/restore,
> always_show_header, diff_free() cleanup, and means that pickaxe (-S, -G,
> --find-object) and --diff-filter cannot suppress commits whose pairs are all
> filtered out, because show_log() runs before diffcore_std().
>
> This series restructures the flow so that -L goes through the same
> log_tree_diff() -> log_tree_diff_flush() path as normal single-parent and
> merge diffs, then uses that to enable several non-patch diff formats.
>
> Patch 1: revision: move -L setup before output_format-to-diff derivation
>
> Preparatory reorder in setup_revisions(). The -L block sets a default
> DIFF_FORMAT_PATCH when no format is requested; move it before the derivation
> of revs->diff from output_format so the default is visible to that check. No
> behavior change on its own.
>
> Patch 2: line-log: integrate -L output with the standard log-tree pipeline
>
> Rename line_log_print() to line_log_queue_pairs(), stripping it down to only
> queue pre-computed filepairs. log_tree_diff_flush() handles show_log(),
> diffcore_std(), and diff_flush(). This fixes pickaxe and --diff-filter
> suppression, and aligns the commit/diff separator with the rest of log
> output. Rejects --full-diff, which is not yet supported when filepairs are
> pre-computed.
>
> Patch 3: line-log: allow non-patch diff formats with -L
>
> Expand the allowlist to accept --raw, --name-only, --name-status, and
> --summary. These only read filepair metadata already set by the line-log
> machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat)
> remain blocked because they call compute_diffstat() on full blob content and
> would show whole-file statistics rather than range-scoped ones.
>
> Changes since v2:
>
> * Switch "! test_grep" to "test_grep !" in tests.
Thanks ! I did not read the tests carefully for semantic value, but the rationale and overall code looks good to me as discussed previously.
The range-diff here looks good, too.
> Michael Montalbo (3):
> revision: move -L setup before output_format-to-diff derivation
> line-log: integrate -L output with the standard log-tree pipeline
> line-log: allow non-patch diff formats with -L
>
> Documentation/line-range-options.adoc | 10 +-
> line-log.c | 30 ++----
> line-log.h | 2 +-
> log-tree.c | 10 +-
> revision.c | 24 +++--
> t/t4211-line-log.sh | 100 +++++++++++++++---
> t/t4211/sha1/expect.parallel-change-f-to-main | 1 -
> .../sha256/expect.parallel-change-f-to-main | 1 -
> 8 files changed, 121 insertions(+), 57 deletions(-)
>
>
> base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v3
> Pull-Request: https://github.com/gitgitgadget/git/pull/2094
>
> Range-diff vs v2:
>
> 1: 9633eb62c6 = 1: 9633eb62c6 revision: move -L setup before output_format-to-diff derivation
> 2: 7acfc5376e = 2: 7acfc5376e line-log: integrate -L output with the standard log-tree pipeline
> 3: 10a3d8dde2 ! 3: ae0b7f3ca8 line-log: allow non-patch diff formats with -L
> @@ t/t4211-line-log.sh: test_expect_success '-p shows the default patch output' '
> +test_expect_success '--raw shows mode, oid, status and path' '
> + git log -L1,24:b.c --raw --format= >actual &&
> + test_grep "^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual &&
> -+ ! test_grep "^diff --git" actual &&
> -+ ! test_grep "^@@" actual
> ++ test_grep ! "^diff --git" actual &&
> ++ test_grep ! "^@@" actual
> +'
> +
> +test_expect_success '--name-only shows path' '
> + git log -L1,24:b.c --name-only --format= >actual &&
> + test_grep "^b.c$" actual &&
> -+ ! test_grep "^diff --git" actual &&
> -+ ! test_grep "^@@" actual
> ++ test_grep ! "^diff --git" actual &&
> ++ test_grep ! "^@@" actual
> +'
> +
> +test_expect_success '--name-status shows status and path' '
> + git log -L1,24:b.c --name-status --format= >actual &&
> + test_grep "^M b.c$" actual &&
> -+ ! test_grep "^diff --git" actual &&
> -+ ! test_grep "^@@" actual
> ++ test_grep ! "^diff --git" actual &&
> ++ test_grep ! "^@@" actual
> +'
> +
> +test_expect_success '--stat is not yet supported with -L' '
>
> --
> gitgitgadget
^ permalink raw reply
* [BUG] internal date format does not accept small unix timestamps
From: Luna Schwalbe @ 2026-05-29 11:51 UTC (permalink / raw)
To: git
While trying to create some test commits, I noticed the following issue:
GIT_AUTHOR_DATE and GIT_COMMITTER_DATE should accept the "Git internal
format" (displayed by git log with --date=raw), but this fails for small
unix timestamps. A quick binary search indicates that it happens when
the unix timestamp is below 100000000 (9 digits).
So for example, GIT_AUTHOR_DATE='99999999 +0000' fails with "fatal:
invalid date format", while GIT_AUTHOR_DATE='100000000 +0000' works as
expected.
It seems to be unaffected by the choice of timezone offset.
Padding the timestamp with zeroes also does not change the behavior.
The --date option does accept all the values, but interprets them
wrongly and gives bogus results (most of them time it seems to act as if
no date option was given, using the current system time, but with some
inputs I've also observed things like "current date&time but set the
year to 2000").
I tested everything with git built from commit
2f8565e1d14d2de4cfbc9da0132131bf0d0dc087.
Luna
^ permalink raw reply
* Re: [PATCH] commit-reach: stop sorting in paint_down_to_common()
From: Jeff King @ 2026-05-29 8:43 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <450163b0-82c8-4b57-baab-a269efe430aa@web.de>
On Wed, May 27, 2026 at 05:52:17PM +0200, René Scharfe wrote:
> None of the three callers of paint_down_to_common() care about the order
> of its result list: merge_bases_many() sorts it again after removing
> stale items, remove_redundant_no_gen() and repo_in_merge_bases_many()
> throw the list away without even looking at it. So drop the unnecessary
> commit_list_sort_by_date() call.
Seems like an easy win. If some of the callers do not even look at the
result, could we avoid building it at all in those cases (e.g., by
passing in a NULL result pointer)?
I guess there is not much to be gained, though. The result is a list of
merge bases, so it should usually be rather small. The benefit in your
patch is probably not performance, but just reducing the size of the
code.
-Peff
^ permalink raw reply
* Re: [PATCH v2 0/2] commit: remove deprecated functions
From: Jeff King @ 2026-05-29 8:37 UTC (permalink / raw)
To: kristofferhaugsbakk
Cc: Junio C Hamano, Kristoffer Haugsbakk, Patrick Steinhardt, git
In-Reply-To: <V2_CV_commit.h_remove_deprecated.732@msgid.xyz>
On Thu, May 28, 2026 at 09:00:09AM +0200, kristofferhaugsbakk@fastmail.com wrote:
> Topic summary: Remove deprecated comments that were slated for removal
> after Git 2.53.0.
This looks obviously correct to me, but the whole topic made me wonder:
was it worth retaining the old names and deprecating them, versus just
removing them back then?
Topics in flight would have needed an update then, but they did
eventually anyway. So it feels like the total amount of work done is
larger, compared to just fixing them as the topics were merged. Either
way the compiler tells us, and the adjustments themselves are small.
Not a huge deal either way, but just pondering for future such
situations.
-Peff
^ permalink raw reply
* Re: [PATCH v2 0/8] pack-bitmap-write: speed up bitmap generation
From: Jeff King @ 2026-05-29 8:34 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <cover.1779911733.git.me@ttaylorr.com>
On Wed, May 27, 2026 at 03:55:44PM -0400, Taylor Blau wrote:
> Here is a reroll of my series to improve the performance of reachability
> bitmap generation, focusing on very large repositories and the penalty
> to generate pseudo-merge reachability bitmaps.
>
> The series is largely unchanged since last time. Notable changes in this
> round include:
>
> - minor refactoring in the pair of patches which consolidate the
> `find_object_pos()` success path and introduce the object position
> cache during bitmap fills, and
>
> - dropping a stale paragraph from the final patch's message, which
> described follow-up commits that are no longer part of this series.
Thanks, this version looks good to me.
-Peff
^ permalink raw reply
* Re: [PATCH 8/8] pack-bitmap: build pseudo-merge bitmaps after regular bitmaps
From: Jeff King @ 2026-05-29 8:33 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <ahdE+Je5YK9JoE7B@nand.local>
On Wed, May 27, 2026 at 03:24:40PM -0400, Taylor Blau wrote:
> Below are some numbers that give you a sense of how the runtime scales
> with the number of pseudo-merges. I'm relying exclusively on "stable"
> pseudo-merges here since they have more predictable bucketing behavior,
> though note that there isn't an exact way to dial in the number of these
> so-called "stable" pseudo-merge groups. We can only control their *size*
> (in terms of number of parents), so I ran the harness which produced the
> above code with powers of 10 between [10^3, 10^6].
>
> Results are as follows:
>
> +------------+-------+----------+
> | stableSize | count | time (s) |
> +------------+-------+----------+
> | 1000000 | 1 | 34.963 |
> | 100000 | 3 | 36.954 |
> | 10000 | 26 | 221.963 |
> | 1000 | 252 | 2779.373 |
> +------------+-------+----------+
>
> Which scales roughly like O(x^1.165) (the best fit function I could find
> was t(n) = 25.18 + 4.386 * n^1.165, where 'n' is the number of
> pseudo-merges, and t(n) is the time it took to generate them).
>
> So it does grow faster than linearly, but it's not too bad. The jump
> from 26 to 252 pseudo-merges is pretty significant, though, but having
> that many pseudo-merges is probably not something that we would want to
> do in practice.
OK, that matches my intuition. 1.165 is close enough that I can squint
and call it linear. ;) I agree that probably you'd want to target a
dozen or fewer pseudo-merges. In the long run we might be able to
auto-tune this a bit, or at the very least document the tradeoffs and
expectations. But think that can only come after getting some more
real-world experience. For all we know the feature might not help at all
in the real world, since any grouping strategy will be highly dependent
on heuristics about how refs are updated, what queries look like, and so
on.
Not that I'm not optimistic, just that I think it is too soon to try
writing this stuff up.
-Peff
^ permalink raw reply
* Re: [PATCH 6/8] pack-bitmap: sort bitmaps before XORing
From: Jeff King @ 2026-05-29 8:26 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <ahciIDuESxNa9Fzn@nand.local>
On Wed, May 27, 2026 at 12:56:00PM -0400, Taylor Blau wrote:
> > If you have some spare CPU cycles to burn, I would be interested in a
> > comparison of the bitmap size of your test repo using v2.30.0, v2.31.1,
> > and this patch.
>
> I started running this experiment, but I don't think I actually have
> enough CPU cycles to let it finish ;-). Pre-v2.31 bitmap generation is
> *really* slow[^1], and after multiple hours (forcing the same selection
> of bitmaps by back-porting and adjusting 'test-tool bitmap') I couldn't
> seem to make any meaningful progress.
Oof. I forgot just how slow it could be. Thanks for trying.
> I'm sure that you could get some plausible numbers out of benchmarking
> this on a smaller repository. In case you're interested, here's the
> patch I wrote on top of v2.30.0:
I tried it myself on linux.git, which is the biggest repo I usually have
on hand. But it seemed to generate the same size back then, and now, and
after your xor-sorting patch. I'm not sure what's different between that
and your super-big test case. Maybe just the number of bitmaps? Maybe
graph structure causing weird order of selection?
I grabbed chromium.git (63GB!) and tried that, too. Its bitmap size
shrinks a tiny bit with this patch (143MB to 140MB). So some
improvement, but not enough of an effect for me to slog through a v2.30
bitmap build just to see if things changed back then.
Regardless of whether the issue was introduced there, or was always
lurking, I think the sort order introduced by this patch is the right
thing to do.
> > Certainly good numbers. The obvious follow-up question is: how does the
> > reading side fare? I'd expect it to be a little better, if only because
> > there are fewer bytes to consider when XOR-ing. But if there's some
> > hidden assumption we're missing, then it could get wildly worse. It
> > would be good to confirm that that didn't happen. ;)
>
> It doesn't make a huge difference. Prior to this patch, the timings on
> my test repository for 'git rev-list --count --all --objects
> --use-bitmap-index' go from:
OK, good. I wasn't expecting great things, but just wanted to double
check that bad things did not appear.
-Peff
^ permalink raw reply
* [PATCH] docs: fix typos and grammar
From: Weijie Yuan @ 2026-05-29 8:17 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Kreimer, git
In-Reply-To: <ahHi0r7DZYfQ1Xqy@wy@wyuan.org>
Fix several spelling mistakes, subject-verb agreement issues, and
duplicated words.
Signed-off-by: Weijie Yuan <wy@wyuan.org>
---
Sorry for my previous meaningless email, now I realize it's better to
come up with a real patch, insteading of talks.
I hope this does not duplicate the previous patch, though I've check
twice.
summary:
* the ones that has
-> have
* duplicated words
* ambigious -> ambiguous
ps: my last email has an invalid Message-ID, sorry for b4 users.
Documentation/fetch-options.adoc | 2 +-
combine-diff.c | 2 +-
contrib/subtree/t/t7900-subtree.sh | 2 +-
csum-file.h | 2 +-
delta-islands.c | 2 +-
diffcore-pickaxe.c | 2 +-
odb.h | 2 +-
parse-options.c | 2 +-
rerere.c | 2 +-
t/t4203-mailmap.sh | 2 +-
t/t9100-git-svn-basic.sh | 2 +-
t/test-lib-github-workflow-markup.sh | 2 +-
t/test-lib-junit.sh | 2 +-
tree-walk.h | 2 +-
14 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/Documentation/fetch-options.adoc b/Documentation/fetch-options.adoc
index 8074004377..035f780e58 100644
--- a/Documentation/fetch-options.adoc
+++ b/Documentation/fetch-options.adoc
@@ -1,6 +1,6 @@
`--all`::
`--no-all`::
- Fetch all remotes, except for the ones that has the
+ Fetch all remotes, except for the ones that have the
`remote.<name>.skipFetchAll` configuration variable set.
This overrides the configuration variable `fetch.all`.
diff --git a/combine-diff.c b/combine-diff.c
index b799862068..720768ce41 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -666,7 +666,7 @@ static int make_hunks(struct sline *sline, unsigned long cnt,
* (-) line, which records from what parents the line
* was removed; this line does not appear in the result.
* then check the set of parents the result has difference
- * from, from all lines. If there are lines that has
+ * from, from all lines. If there are lines that have
* different set of parents that the result has differences
* from, that means we have more than two versions.
*
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 18d2b56448..4194687cfb 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -75,7 +75,7 @@ test_create_pre2_32_repo () {
#
# Create a simple subtree on a new branch named ORPHAN in REPO.
# The subtree is then merged into the current branch of REPO,
-# under PREFIX. The generated subtree has has one commit
+# under PREFIX. The generated subtree has one commit
# with subject and tag FILENAME with a single file "FILENAME.t"
#
# When this method returns:
diff --git a/csum-file.h b/csum-file.h
index a9b390d336..a270738a7a 100644
--- a/csum-file.h
+++ b/csum-file.h
@@ -52,7 +52,7 @@ struct hashfd_options {
*/
struct progress *progress;
- /* The length of the buffer that shall be used read read data. */
+ /* The length of the buffer that shall be used to read data. */
size_t buffer_len;
};
diff --git a/delta-islands.c b/delta-islands.c
index f4d2468790..e71a7e1c05 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -527,7 +527,7 @@ void free_island_marks(void)
kh_destroy_oid_map(island_marks);
}
- /* detect use-after-free with a an address which is never valid: */
+ /* detect use-after-free with an address which is never valid: */
island_marks = (void *)-1;
}
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index a52d569911..b0915be86f 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -203,7 +203,7 @@ static void pickaxe(struct diff_queue_struct *q, struct diff_options *o,
for (i = 0; i < q->nr; i++)
diff_free_filepair(q->queue[i]);
} else {
- /* Showing only the filepairs that has the needle */
+ /* Showing only the filepairs that have the needle */
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
if (pickaxe_match(p, o, regexp, kws, fn))
diff --git a/odb.h b/odb.h
index 73553ed5a7..0030467a52 100644
--- a/odb.h
+++ b/odb.h
@@ -40,7 +40,7 @@ struct object_database {
struct repository *repo;
/*
- * State of current current object database transaction. Only one
+ * State of current object database transaction. Only one
* transaction may be pending at a time. Is NULL when no transaction is
* configured.
*/
diff --git a/parse-options.c b/parse-options.c
index a676da86f5..f4647e0099 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1149,7 +1149,7 @@ enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
(ctx->flags & PARSE_OPT_KEEP_UNKNOWN_OPT)) {
/*
* Found an unknown option given to a command with
- * subcommands that has a default operation mode:
+ * subcommands that have a default operation mode:
* we treat this option and all remaining args as
* arguments meant to that default operation mode.
* So we are done parsing.
diff --git a/rerere.c b/rerere.c
index 0296700f9f..28a740b771 100644
--- a/rerere.c
+++ b/rerere.c
@@ -548,7 +548,7 @@ static int check_one_conflict(struct index_state *istate, int i, int *type)
/*
* Scan the index and find paths that have conflicts that rerere can
- * handle, i.e. the ones that has both stages #2 and #3.
+ * handle, i.e. the ones that have both stages #2 and #3.
*
* NEEDSWORK: we do not record or replay a previous "resolve by
* deletion" for a delete-modify conflict, as that is inherently risky
diff --git a/t/t4203-mailmap.sh b/t/t4203-mailmap.sh
index 74b7ddccb2..03f6df9d24 100755
--- a/t/t4203-mailmap.sh
+++ b/t/t4203-mailmap.sh
@@ -180,7 +180,7 @@ test_expect_success 'mailmap.file set' '
git shortlog HEAD >actual &&
test_cmp expect actual &&
- # The internal_mailmap/.mailmap file is an a subdirectory, but
+ # The internal_mailmap/.mailmap file is in a subdirectory, but
# as shown here it can also be outside the repository
test_when_finished "rm -rf sub-repo" &&
git clone . sub-repo &&
diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index af28b01fef..1ab98b9c37 100755
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -232,7 +232,7 @@ test_expect_success POSIXPERM,SYMLINKS "$name" '
test_cmp expected.$(test_oid algo) a
'
-test_expect_success 'exit if remote refs are ambigious' '
+test_expect_success 'exit if remote refs are ambiguous' '
git config --add svn-remote.svn.fetch \
bar:refs/remotes/git-svn &&
test_must_fail git svn migrate
diff --git a/t/test-lib-github-workflow-markup.sh b/t/test-lib-github-workflow-markup.sh
index 33405c90d7..fa29a62aa3 100644
--- a/t/test-lib-github-workflow-markup.sh
+++ b/t/test-lib-github-workflow-markup.sh
@@ -18,7 +18,7 @@
#
# The idea is for `test-lib.sh` to source this file when run in GitHub
# workflows; these functions will then override (empty) functions
-# that are are called at the appropriate times during the test runs.
+# that are called at the appropriate times during the test runs.
test_skip_test_preamble=t
diff --git a/t/test-lib-junit.sh b/t/test-lib-junit.sh
index 76cbbd3299..f4994dd9d3 100644
--- a/t/test-lib-junit.sh
+++ b/t/test-lib-junit.sh
@@ -19,7 +19,7 @@
#
# The idea is for `test-lib.sh` to source this file when the user asks
# for JUnit XML; these functions will then override (empty) functions
-# that are are called at the appropriate times during the test runs.
+# that are called at the appropriate times during the test runs.
start_test_output () {
junit_xml_dir="$TEST_OUTPUT_DIRECTORY/out"
diff --git a/tree-walk.h b/tree-walk.h
index 29a55328bd..9646c47ac5 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -177,7 +177,7 @@ struct traverse_info {
/**
* Walk trees starting with "tree_oid" to find the entry for "name", and
- * return the the object name and the mode of the found entry via the
+ * return the object name and the mode of the found entry via the
* "oid" and "mode" parameters. Return 0 if the entry is found, and -1
* otherwise.
*/
--
2.54.0
^ permalink raw reply related
* Re: [PATCH] t3070: skip ls-files tests with backslash patterns on Windows
From: Kristofer Karlsson @ 2026-05-29 8:04 UTC (permalink / raw)
To: Junio C Hamano
Cc: Kristofer Karlsson via GitGitGadget, git, Johannes Schindelin
In-Reply-To: <xmqqecivjn7k.fsf@gitster.g>
On Thu, 28 May 2026 at 22:26, Junio C Hamano <gitster@pobox.com> wrote:
> Two questions.
>
> * Has this been broken on Windows since October, or has something
> external change on Windows recently? I do not know. Anybody
> knows?
>
> * Is this change a workaround that sweeps ugly breakage under the
> rug, or is backslash inherently unusable as an excape character
> when handling paths on Windows (which I am afraid would make
> wildmatch fairly useless there)?
>
I am fairly new to the git ecosystem as a developer (not as a user),
so I am not sure how long this has been broken. The backslash patterns
in the ls-files test path predate 8a6d158a - patterns like 'foo\*'
and '[\-_]' have been there since de8bada2bf (2018) - so it may
have been failing for a while before anyone noticed.
My thinking was that it would be good in general if the CI results
were green and did not include false positives for errors that we
know cannot work on this platform. The risk is that people stop
looking into CI failures in detail because they start to assume it
is the same old backslash problem.
That said, there is also a risk that the real underlying issue does
not get fixed. I am hoping it is sufficient that the BSLASHPSPEC
prereq and the case *\\* filter make it obvious to anyone reading
the test what we are skipping over and why.
> Will queue. Thanks.
Thanks! It felt a bit heavyweight to add noise to the list for trivial CI test
changes but I suppose the process is the same even if it does not
affect the production code.
^ permalink raw reply
* Re: [PATCH 3/8] pack-bitmap: reuse stored selected bitmaps
From: Jeff King @ 2026-05-29 6:00 UTC (permalink / raw)
To: Taylor Blau; +Cc: git, Junio C Hamano, Elijah Newren, Derrick Stolee
In-Reply-To: <ahcCX+xAKFOL8HcW@nand.local>
On Wed, May 27, 2026 at 10:40:31AM -0400, Taylor Blau wrote:
> > > Teach `fill_bitmap_commit()` to notice that case. For non-root commits in
> > > the walk, look for a stored selected bitmap and OR it into the bitmap
> > > being built. If one exists, skip the commit, its tree, and its parents.
> >
> > I feel like this _shouldn't_ be necessary, because the idea of the
> > current writing code is to go from the roots up, following inverted
> > parent pointers, and passing the bitmap up as we go. So whenever we
> > visit a commit we should in theory have all of the ancestor's bits set
> > in that bitmap. But I remember that the simple-and-stupid approach ended
> > up being too memory hungry, so we pick some focal points in the graph
> > and then fill them independently.
>
> It's sharing within the non-first parent history that is killing us
> here. I think what you said is true in a completely linear repository
> with no merges. But since we only pass commit masks from commits to
> their first parents, we don't reuse any already-generated bitmaps for
> common points in history not shared between commits' first parents.
Ah, that makes sense. I had forgotten exactly how the maximal-commit
selection worked, and what we compromised versus the original naive
"build from the bottom up" strategy.
> Yeah, these were for my own curiosity as much as anything. I had written
> them as a temporary measure in order to write the "[...] there are 1,261
> commits selected for bitmap coverage, and 1,382 maximal commits induced
> [...]" portion of the commit message above.
>
> Once I had written it, I found the result useful enough to keep around.
Makes sense. It might help us (or even some very clueful user) debug or
fine-tune parameters down the road.
-Peff
^ permalink raw reply
* Re: [PATCH v2] http: fix memory leak in fetch_and_setup_pack_index()
From: Jeff King @ 2026-05-29 5:40 UTC (permalink / raw)
To: LorenzoPegorari; +Cc: git, Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox
In-Reply-To: <20260529053659.GC1099450@coredump.intra.peff.net>
On Fri, May 29, 2026 at 01:36:59AM -0400, Jeff King wrote:
> But it _could_ be done as a preparatory patch. And the rationale for
> doing that on its own I think is roughly:
>
> 1. It is mostly doing nothing, because 63aca3f7f1 registered it as a
> tempfile, so it will be cleaned up at process end anyway (whether
> we succeed in fetching it or not).
>
> 2. It is maybe a little harmful, because we are going to unlink() it
> now, and then later the tempfile code will try to unlink() it again
> (so a simultaneous fetch could have created the same file).
BTW, for (2) I wondered about going in the opposite direction. If we
actually passed the tempfile back up, like in the patch below, then we
could use delete_tempfile() to do the unlink (and remove it from the
tempfile list).
And then your patch would want to similarly delete_tempfile() in its
error path.
But I don't think it really buys us much. _If_ we were going to keep
passing the tempfile struct up the call stack on success, then we could
store it and call delete_tempfile() as soon as we had ran index-pack on
it. But that's even more surgery, for again little gain (we delete our
tempfiles a little earlier, rather than at process end).
So I'm inclined to go in the direction that shortens the code. ;)
-Peff
---
diff --git a/http.c b/http.c
index ea9b16861b..e83a3857b3 100644
--- a/http.c
+++ b/http.c
@@ -2546,9 +2546,10 @@ int http_fetch_ref(const char *base, struct ref *ref)
}
/* Helpers for fetching packs */
-static char *fetch_pack_index(unsigned char *hash, const char *base_url)
+static struct tempfile *fetch_pack_index(unsigned char *hash, const char *base_url)
{
char *url, *tmp;
+ struct tempfile *ret;
struct strbuf buf = STRBUF_INIT;
if (http_is_verbose)
@@ -2575,23 +2576,24 @@ static char *fetch_pack_index(unsigned char *hash, const char *base_url)
tmp = xstrfmt("%s/tmp_pack_%s.idx",
repo_get_object_directory(the_repository),
hash_to_hex(hash));
- register_tempfile(tmp);
+ ret = register_tempfile(tmp);
+ free(tmp);
- if (http_get_file(url, tmp, NULL) != HTTP_OK) {
+ if (http_get_file(url, ret->filename.buf, NULL) != HTTP_OK) {
error("Unable to get pack index %s", url);
- FREE_AND_NULL(tmp);
+ delete_tempfile(&ret);
}
free(url);
- return tmp;
+ return ret;
}
static int fetch_and_setup_pack_index(struct packfile_list *packs,
unsigned char *sha1,
const char *base_url)
{
struct packed_git *new_pack, *p;
- char *tmp_idx = NULL;
+ struct tempfile *tmp_idx;
int ret;
/*
@@ -2607,11 +2609,9 @@ static int fetch_and_setup_pack_index(struct packfile_list *packs,
if (!tmp_idx)
return -1;
- new_pack = parse_pack_index(the_repository, sha1, tmp_idx);
+ new_pack = parse_pack_index(the_repository, sha1, tmp_idx->filename.buf);
if (!new_pack) {
- unlink(tmp_idx);
- free(tmp_idx);
-
+ delete_tempfile(&tmp_idx);
return -1; /* parse_pack_index() already issued error message */
}
^ permalink raw reply related
* Re: [PATCH v2] http: fix memory leak in fetch_and_setup_pack_index()
From: Jeff King @ 2026-05-29 5:36 UTC (permalink / raw)
To: LorenzoPegorari; +Cc: git, Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox
In-Reply-To: <ahjUmMCKxREamQE-@lorenzo-VM>
On Fri, May 29, 2026 at 01:49:44AM +0200, LorenzoPegorari wrote:
> Inside the function `fetch_and_setup_pack_index()`, when the pack
> obtained using `parse_pack_index()` fails to be verified by
> `verify_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, when the verification
> fails.
>
> Also, do some more cleanup by removing the useless call to the function
> `unlink()`. This is not necessary anymore since 63aca3f7f1 (dumb-http:
> store downloaded pack idx as tempfile, 2024-10-25), when
> `fetch_pack_index()` started registering its return value (in this case
> `tmp_idx`) as a tempfile to be deleted at process exit.
I think the patch as-is is OK. But when I see this kind of "also, do
this..." in a commit message it is a good time to consider whether that
should happen in a separate patch.
Here it does not make sense to remove the unlink() afterwards; you'd
wonder why it was not present in the cleanup added by your patch.
But it _could_ be done as a preparatory patch. And the rationale for
doing that on its own I think is roughly:
1. It is mostly doing nothing, because 63aca3f7f1 registered it as a
tempfile, so it will be cleaned up at process end anyway (whether
we succeed in fetching it or not).
2. It is maybe a little harmful, because we are going to unlink() it
now, and then later the tempfile code will try to unlink() it again
(so a simultaneous fetch could have created the same file).
For something this small, though, I am OK just lumping it together.
There are diminishing returns from polishing it further.
-Peff
^ permalink raw reply
* Re: [PATCH] http: fix memory leak in fetch_and_setup_pack_index()
From: Jeff King @ 2026-05-29 5:32 UTC (permalink / raw)
To: Lorenzo Pegorari
Cc: git, Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox
In-Reply-To: <aheY6bLM2gxtMDdr@lorenzo-VM>
On Thu, May 28, 2026 at 03:22:49AM +0200, Lorenzo Pegorari wrote:
> > 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.
>
> The `unlink()` indeed is weird. Pointing me to the commit 63aca3f7f1
> really helped me understand how the code changed and the current
> situation. Thanks a lot for that.
>
> I've tried testing as thoroughly as possible whether removing the
> `unlink()` function call wouldn't change the expected behavior.
> *I think* that it can be removed safely, but I'm not 100% sure yet.
I think the only behavior difference in removing the unlink() is whether
we immediately delete the downloaded packfile on error, or if we wait
until process exit. From the perspective of somebody calling git-fetch,
the outcome is roughly the same (when the process ends, the file is
gone). It would only differ if the system crashed before the process
ended.
-Peff
^ permalink raw reply
* Re: git hook question
From: Jeff King @ 2026-05-29 5:21 UTC (permalink / raw)
To: Wesley Schwengle; +Cc: Git maillinglist
In-Reply-To: <cc9fda14-d8e8-4982-9a3d-9aa816c0b90c@opperschaap.net>
On Fri, May 29, 2026 at 01:01:34AM -0400, Wesley Schwengle wrote:
> I understand the why, normally pre-push gets `<local-ref> SP
> <local-object-name> SP <remote-ref> SP <remote-object-name> LF'. This has a
> similar feel, albeit a different syntax. The difference feels like a minor
> bug, but not one I'm worried about at this moment: you would expect it to
> get the same arguments/parameters as the regular pre-push hook. But I
> digress.
I think the "git hook" command is mostly intended for scripting, and the
caller is expected to understand the context and provide the appropriate
arguments. The hook command itself doesn't know about what a "pre-push"
hook should look like.
So not a bug, but definitely a gotcha that could perhaps be better
explained in the documentation.
> My actual question is: Is there a way to tell the hook "Don't give me
> arguments, just run the plain command that is defined". I looked in `man 1
> git-hook', but I was unable to find something that looks like it.
I don't think so; the command is expected to handle (or ignore) the
arguments as appropriate. You could obviously write a wrapper script to
handle that, but since hook commands are run with a shell you can inline
it, like:
git config hook.npm-test.command 'npm run test #'
Git will paste together the shell command:
npm run test # "$@"
which then treats everything after the "#" as a comment. The more
general form of this trick is to use a shell function, like:
f() { your_cmd_here; }; f
which would do what you want, but also let you access the arguments
however you like. For example:
f() { some_cmd "$1"; another_cmd "$2"; }; f
would let you consider the arguments independently.
But for your purposes, using "#" to ignore them is simpler to write.
-Peff
^ permalink raw reply
* Re: [PATCH] doc: add missing --message long option to merge docs
From: Junio C Hamano @ 2026-05-29 5:02 UTC (permalink / raw)
To: Brandon Dong via GitGitGadget; +Cc: git, Brandon Dong
In-Reply-To: <pull.2315.git.git.1780019726297.gitgitgadget@gmail.com>
"Brandon Dong via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Brandon <brandondong96@gmail.com>
As you identify yourself as "Brandon Dong" on the Signed-off-by line
below, please match this in-body From: line with it.
> Include mention of --message flag in merge docs to match what is
> accepted (builtin/merge.c) and to make it consistent with the git
> commit docs.
>
> Signed-off-by: Brandon Dong <brandondong96@gmail.com>
> ---
> doc: add missing --message long option to merge docs
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2315%2Fbrandondong%2Fmerge_message_docs-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2315/brandondong/merge_message_docs-v1
> Pull-Request: https://github.com/git/git/pull/2315
>
> Documentation/git-merge.adoc | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/git-merge.adoc b/Documentation/git-merge.adoc
> index a055384ad6..6581f4c69c 100644
> --- a/Documentation/git-merge.adoc
> +++ b/Documentation/git-merge.adoc
> @@ -68,6 +68,7 @@ OPTIONS
> include::merge-options.adoc[]
>
> `-m <msg>`::
> +`--message=<msg>`::
> Set the commit message to be used for the merge commit (in
> case one is created).
> +
Hmph. This is still not consistent with "git merge -h" output has,
which seems to accept --[no-]message as well.
It is not exactly your fault, but there are a few options other than
this one that support optional [no-] and they are not documented as
such, even though they appear in "git merge -h". "git merge -m foo
--no-message other" behaves as if "GIT_EDITOR=: git merge other" was
run, it seems.
^ permalink raw reply
* git hook question
From: Wesley Schwengle @ 2026-05-29 5:01 UTC (permalink / raw)
To: Git maillinglist
Hello,
I added the following to my gitconfig:
hook.npm-test.event=pre-push
hook.npm-test.command=npm run test
This works well when I run `git hook run pre-push' but when using `git
push' this breaks a little because it adds the remote and the location,
as seen via `GIT_TRACE=1 git push origin':
00:46:53.714453 run-command.c:673 trace: run_command: 'npm run
test' origin git@gitlab.com:waterkip/mything.git
00:46:53.714458 run-command.c:765 trace: start_command: /bin/sh -c
'npm run test "$@"' 'npm run test' origin
git@gitlab.com:waterkip/mything.git
I understand the why, normally pre-push gets `<local-ref> SP
<local-object-name> SP <remote-ref> SP <remote-object-name> LF'. This
has a similar feel, albeit a different syntax. The difference feels like
a minor bug, but not one I'm worried about at this moment: you would
expect it to get the same arguments/parameters as the regular pre-push
hook. But I digress.
My actual question is: Is there a way to tell the hook "Don't give me
arguments, just run the plain command that is defined". I looked in `man
1 git-hook', but I was unable to find something that looks like it.
Cheers,
Wesley
--
Wesley Schwengle
^ permalink raw reply
* [PATCH] doc: add missing --message long option to merge docs
From: Brandon Dong via GitGitGadget @ 2026-05-29 1:55 UTC (permalink / raw)
To: git; +Cc: Brandon Dong, Brandon
From: Brandon <brandondong96@gmail.com>
Include mention of --message flag in merge docs to match what is
accepted (builtin/merge.c) and to make it consistent with the git
commit docs.
Signed-off-by: Brandon Dong <brandondong96@gmail.com>
---
doc: add missing --message long option to merge docs
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2315%2Fbrandondong%2Fmerge_message_docs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2315/brandondong/merge_message_docs-v1
Pull-Request: https://github.com/git/git/pull/2315
Documentation/git-merge.adoc | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/git-merge.adoc b/Documentation/git-merge.adoc
index a055384ad6..6581f4c69c 100644
--- a/Documentation/git-merge.adoc
+++ b/Documentation/git-merge.adoc
@@ -68,6 +68,7 @@ OPTIONS
include::merge-options.adoc[]
`-m <msg>`::
+`--message=<msg>`::
Set the commit message to be used for the merge commit (in
case one is created).
+
base-commit: c69baaf57ba26cf117c2b6793802877f19738b0d
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH v2 3/4] diff: add long-running diff process via diff.<driver>.process
From: Michael Montalbo @ 2026-05-29 0:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Montalbo via GitGitGadget, git
In-Reply-To: <xmqqjysqnbxu.fsf@gitster.g>
On Mon, May 25, 2026 at 7:26 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Zero hunks with status=success means the tool considers the
> > files equivalent. Git skips diff output for that file.
>
> Is "zero hunk" a common word or some random string you invented? If
> the latter, which is I am assuming it to be, you should define what
> it means at/before the first use. Here in the proposed log message,
> and ...
>
> >
> > Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
> > ---
> > Documentation/config/diff.adoc | 8 +
> > Documentation/gitattributes.adoc | 40 ++++
> > Makefile | 1 +
> > diff-process.c | 206 +++++++++++++++++++
> > diff-process.h | 28 +++
> > diff.c | 23 +++
> > t/.gitattributes | 1 +
> > t/t4080-diff-process.sh | 338 +++++++++++++++++++++++++++++++
> > 8 files changed, 645 insertions(+)
> > create mode 100644 diff-process.c
> > create mode 100644 diff-process.h
> > create mode 100755 t/t4080-diff-process.sh
> >
> > diff --git a/Documentation/config/diff.adoc b/Documentation/config/diff.adoc
> > index 1135a62a0a..4ab5f60df6 100644
> > --- a/Documentation/config/diff.adoc
> > +++ b/Documentation/config/diff.adoc
> > @@ -218,6 +218,14 @@ endif::git-diff[]
> > Set this option to `true` to make the diff driver cache the text
> > conversion outputs. See linkgit:gitattributes[5] for details.
> >
> > +`diff.<driver>.process`::
> > + The command to run as a long-running diff process.
> > + The tool communicates via the pkt-line protocol and returns
> > + hunks that are fed into Git's diff and blame pipelines.
> > + If the tool returns zero hunks, the file is treated as
> > + unchanged for both diff output and blame attribution.
> > + See linkgit:gitattributes[5] for details.
>
> ... also here.
>
> I do not know if you mean "the tool returns no hunks" (there is no
> "hunk <old_start> <old_count> <new_start> <new_count>" line passed
> from the tool over the protocol) or "the tool returns zero-hunk"
> (there is a special "zero-hunk" message to signal this particular
> condition sent over the protocol), and this description does not
> quite help disambiguating between the two.
>
> If the former, then avoid "zero hunks" as it sounds like a noun with
> special meaning. Yes, we can say "tool returns one hunk", "tool
> returns 31 hunks", etc., so "tool returns zero hunks" may logically
> be correct, but "when the tool returns no hunks with status=success"
> is much less confusing, I think.
Yes, "zero hunks" was my own invention and I see why it's confusing. Will
update the messaging to use "no hunks" instead and do a broader sweep of
the documentation to clarify the protocol and expected tool behavior.
^ permalink raw reply
* Re: [PATCH v2 3/4] diff: add long-running diff process via diff.<driver>.process
From: Michael Montalbo @ 2026-05-29 0:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Montalbo via GitGitGadget, git
In-Reply-To: <xmqqpl2jlyr3.fsf@gitster.g>
On Mon, May 25, 2026 at 6:56 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +struct diff_subprocess {
> > + struct subprocess_entry subprocess;
> > + unsigned int supported_capabilities;
> > +};
> > +
> > +static int subprocess_map_initialized;
> > +static struct hashmap subprocess_map;
>
> Can we avoid introducing new global variables like these? Would
> "struct userdiff_driver" or "struct diff_options" be a good place to
> hang this hashmap, perhaps?
>
Will clean this up.
> > +static int send_file_content(int fd, const char *buf, long size)
> > +{
> > + int ret;
> > +
> > + if (size > 0)
> > + ret = write_packetized_from_buf_no_flush(buf, size, fd);
> > + else
> > + ret = 0;
>
> Shouldn't "size == -24" be flagged as an invalid input?
>
Will fix and do a broader audit of input validation and bounds checking.
> > + if (ret)
> > + return ret;
> > + return packet_flush_gently(fd);
> > +}
>
> > +static int parse_hunk_line(const char *line, struct xdl_hunk *hunk)
> > +{
> > +...
> > +}
>
> This gives a silent error diagnosis, which is good for a lower level
> helper.
>
> > +int diff_process_get_hunks(struct userdiff_driver *drv,
> > + const char *path,
> > + const char *old_buf, long old_size,
> > + const char *new_buf, long new_size,
> > + struct xdl_hunk **hunks_out,
> > + size_t *nr_hunks_out)
> > +{
> > + struct diff_subprocess *backend;
> > + struct child_process *process;
> > + int fd_in, fd_out;
> > + struct strbuf status = STRBUF_INIT;
> > + struct xdl_hunk *hunks = NULL;
> > + struct xdl_hunk hunk;
> > + size_t nr_hunks = 0, alloc_hunks = 0;
> > + int len;
> > + char *line;
> > +
> > + if (!drv || !drv->process)
> > + return -1;
>
> A driver that does not define process is not an error; it is
> perfectly normal in the current world order where nobody has such an
> external process and even fi this patch lands, external processes
> are optional. So here "return -1" does not mean an error, and
> silent return is perfectly fine.
>
> > + backend = find_or_start_process(drv->process);
> > + if (!backend)
> > + return -1;
>
> This is probably an error; the user specified drv->process, we
> either tried to find or start the process and failed. Isn't it an
> event that deserves to be reported in an error message?
>
> > + if (!(backend->supported_capabilities & CAP_HUNKS))
> > + return -1;
>
> Backend started, but the "hunks" feature is not supported. Perhaps
> in a year or two, this external process protocol may have become so
> popular that it gained more capabilities, possibly making get_hunks
> obsolete. We may be looking at such an external process that uses
> other capabilities but not this one. This is not an error, so
> silent return is perfectly fine.
>
> > + process = subprocess_get_child_process(&backend->subprocess);
> > + fd_in = process->in;
> > + fd_out = process->out;
> > +
> > + /* Send request */
> > + if (packet_write_fmt_gently(fd_in, "command=hunks\n") ||
> > + packet_write_fmt_gently(fd_in, "pathname=%s\n", path) ||
> > + packet_flush_gently(fd_in))
> > + goto error;
> > +
> > + /* Send old file content */
> > + if (send_file_content(fd_in, old_buf, old_size))
> > + goto error;
> > +
> > + /* Send new file content */
> > + if (send_file_content(fd_in, new_buf, new_size))
> > + goto error;
> > +
> > + /* Read hunks until flush packet */
> > + while ((len = packet_read_line_gently(fd_out, NULL, &line)) >= 0 &&
> > + line) {
> > + if (parse_hunk_line(line, &hunk) < 0)
> > + goto error;
> > + ALLOC_GROW(hunks, nr_hunks + 1, alloc_hunks);
> > + hunks[nr_hunks++] = hunk;
> > + }
> > + if (len < 0)
> > + goto error;
> > +
> > + /* Read status */
> > + if (subprocess_read_status(fd_out, &status))
> > + goto error;
> > +
> > + if (strcmp(status.buf, "success")) {
> > + if (!strcmp(status.buf, "abort"))
> > + backend->supported_capabilities &= ~CAP_HUNKS;
> > + goto error;
> > + }
> > +
> > + *hunks_out = hunks;
> > + *nr_hunks_out = nr_hunks;
> > + strbuf_release(&status);
> > + return 0;
> > +
> > +error:
>
> All exceptions that lead here look like events that should be
> reported to the end-user.
>
Agreed on all points. I will restructure things so errors are flagged when
appropriate (i.e., user specified a process but one was not found / couldn't
start and exceptions) and non-errors are treated as they should be.
> > + free(hunks);
> > + strbuf_release(&status);
> > + return -1;
> > +}
>
> > +/*
> > + * Query a diff process for hunks describing the changes
> > + * between old_buf and new_buf.
> > + *
> > + * The backend is a long-running subprocess configured via
> > + * diff.<driver>.process. It receives file content via
> > + * pkt-line and returns hunks with 1-based line numbers.
> > + *
> > + * On success, sets *hunks_out and *nr_hunks_out to a newly allocated
> > + * array (caller must free) and returns 0.
> > + *
> > + * On failure, returns -1. The caller should fall back to the
> > + * builtin diff algorithm.
> > + */
>
> I do not agree with this. If it is a failure, the user should fix
> the external process (or disable). It shouldn't be hidden behind a
> fallback. As I left comments, in this round of implementation,
> there are conditions that returns -1 for soemthing that is not an
> error (i.e., not configured, or process not supporting the
> particular capability) *and* in those cases the caller should fall
> back as if nothing happened. But some error cases, the caller
> should't hide them.
Will address in a follow-up.
Thank you for the feedback!
^ permalink raw reply
* [PATCH v2] http: fix memory leak in fetch_and_setup_pack_index()
From: LorenzoPegorari @ 2026-05-28 23:49 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Junio C Hamano, Patrick Steinhardt, fox, Jeff King
In-Reply-To: <agx5tblaCZNsYEBq@lorenzo-VM>
Inside the function `fetch_and_setup_pack_index()`, when the pack
obtained using `parse_pack_index()` fails to be verified by
`verify_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, when the verification
fails.
Also, do some more cleanup by removing the useless call to the function
`unlink()`. This is not necessary anymore since 63aca3f7f1 (dumb-http:
store downloaded pack idx as tempfile, 2024-10-25), when
`fetch_pack_index()` started registering its return value (in this case
`tmp_idx`) as a tempfile to be deleted at process exit.
Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
---
http.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/http.c b/http.c
index 67c9c6fc60..99da4d7529 100644
--- a/http.c
+++ b/http.c
@@ -2538,18 +2538,18 @@ static int fetch_and_setup_pack_index(struct packfile_list *packs,
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 */
}
ret = verify_pack_index(new_pack);
- if (!ret)
- close_pack_index(new_pack);
+
+ close_pack_index(new_pack);
free(tmp_idx);
- if (ret)
+ if (ret) {
+ free(new_pack);
return -1;
+ }
packfile_list_prepend(packs, new_pack);
return 0;
--
2.54.0.129.g2dffd77b94.dirty
^ permalink raw reply related
* [PATCH] describe: fix --exclude, --match with --contains and --all
From: Jacob Keller @ 2026-05-28 23:29 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller, Tuomas Ahola
From: Jacob Keller <jacob.keller@gmail.com>
git describe --contains acts as a wrapper around git name-rev. When
operating with --contains and --all, the --match and --exclude patterns
are not properly forwarded to name-rev as --exclude and --refs options.
This results in the command silently discarding match and exclude
requests from the user when operating in --all mode.
We could check and die() if the user provides --contains, --all, and
--match/--exclude. However, its also straight forward to just pass the
filters down to git name-rev.
Notice that the documentation for --match and --exclude mention the
--all mode. It explains that they operate on refs with the prefix
refs/tags, and additionally refs/heads and refs/remotes when using
--all.
Fix the describe logic to pass the patterns down with the appropriate
prefixes when --all is provided. This fixes the support to match the
documented behavior.
Add tests to check that this works as expected.
Reported-by: Tuomas Ahola <taahol@utu.fi>
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
I was looking into reviving the patch that just added a simple die() and
realized that its actually pretty straight forward to just fix the support
instead. I'm open to either route, if we think this support isn't
necessary... I'm not sure if there are any gotchas or other issues with how
I implemented this.
builtin/describe.c | 18 +++++++++++++++---
t/t6120-describe.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 1c47d7c0b7c3..faaf44cec573 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -712,13 +712,25 @@ int cmd_describe(int argc,
NULL);
if (always)
strvec_push(&args, "--always");
- if (!all) {
+ if (!all)
strvec_push(&args, "--tags");
+
+ for_each_string_list_item(item, &patterns)
+ strvec_pushf(&args, "--refs=refs/tags/%s", item->string);
+ for_each_string_list_item(item, &exclude_patterns)
+ strvec_pushf(&args, "--exclude=refs/tags/%s", item->string);
+
+ if (all) {
for_each_string_list_item(item, &patterns)
- strvec_pushf(&args, "--refs=refs/tags/%s", item->string);
+ strvec_pushf(&args, "--refs=refs/heads/%s", item->string);
for_each_string_list_item(item, &exclude_patterns)
- strvec_pushf(&args, "--exclude=refs/tags/%s", item->string);
+ strvec_pushf(&args, "--exclude=refs/heads/%s", item->string);
+ for_each_string_list_item(item, &patterns)
+ strvec_pushf(&args, "--refs=refs/remotes/%s", item->string);
+ for_each_string_list_item(item, &exclude_patterns)
+ strvec_pushf(&args, "--exclude=refs/remotes/%s", item->string);
}
+
if (argc)
strvec_pushv(&args, argv);
else
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 8ee3d2c37d02..f46e628d6a1a 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -359,6 +359,35 @@ test_expect_success 'describe --contains and --no-match' '
test_cmp expect actual
'
+test_expect_success 'describe --contains --all --match' '
+ echo "tags/A^0" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ test_must_fail git describe --contains --all --match="B" $tagged_commit >actual &&
+ git describe --contains --all --match="A" $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'describe --contains --all --match branch' '
+ echo "branch_A" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ git describe --contains --all --match="branch*" $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'describe --contains --all --match and --exclude' '
+ echo "branch_C~1" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ git describe --contains --all --match="branch*" --exclude="branch_A" $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'describe --contains --all --exclude' '
+ echo "branch_A" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ git describe --contains --all --exclude="A" --exclude="c" --exclude="test*" $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'setup and absorb a submodule' '
test_create_repo sub1 &&
test_commit -C sub1 initial &&
--
2.54.0.633.g0ded84c31b89
^ permalink raw reply related
* [BUG] broken behaviour when running cd in a hook in a secondary worktree
From: Baptiste Jean-Louis @ 2026-05-28 21:03 UTC (permalink / raw)
To: git
Hello,
Thank you for your great work
here is a bug I encountered
What did you do before the bug happened? (Steps to reproduce your issue)
#!/bin/bash
mkdir topfolder
cd topfolder
mkdir main-worktree
cd main-worktree
git init
mkdir dir
touch file-a dir/file-b
git add .
git commit -m "Initial commit":
cat >.git/hooks/cd-bug << EOF
#!/bin/bash
echo -e "\n\nrunning cd-bug hook"
# Redirect output to stderr.
exec 1>&2
echo "<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<"
git describe
echo pwd : $(pwd)
echo pwd_var : $PWD
echo git_prefix: $GIT_PREFIX
echo git_dir: $GIT_DIR
echo git_work_tree: $GIT_WORK_TREE
git status
echo "========================================================"
cd dir
echo pwd : $(pwd)
echo pwd_var : $PWD
echo git_prefix: $GIT_PREFIX
echo git_dir: $GIT_DIR
echo git_work_tree: $GIT_WORK_TREE
git status
echo ">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>"
echo -e "cd-bug hook done\n\n"
EOF
chmod u+x .git/hooks/cd-bug
# behave as expected
git hook run cd-bug
git branch branch_b
git worktree add ../second-worktree branch_b
cd ../second-worktree
# broken behaviour here
git hook run cd-bug
# end of script
What did you expect to happen? (Expected behavior)
On branch branch_b
nothing to commit, working tree clean
What happened instead? (Actual behavior)
On branch branch_b
Changes not staged for commit:
(use "git add/rm <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
deleted: dir/file-b
deleted: file-a
Untracked files:
(use "git add <file>..." to include in what will be committed)
file-b
What's different between what you expected and what actually happened?
-> Running `git status` after `cd` in a pre-commit hook lists all
repo's tracked file as deleted.
However when running the hook from the main-worktree, I have no issue.
In my final use case, I'm doing something that goes like below
cd subfolder
`git stash --keep-index`
`./update-generated-files` which updates some files in subfolder
and subfolder/*/
`git stash pop`
[System Info]
git version:
git version 2.51.1.windows.1
cpu: x86_64
built from commit: 1454f0a9c4a3a22fb3fd7cc33f76f88cd65ced41
sizeof-long: 4
sizeof-size_t: 8
shell-path: D:/git-sdk-64-build-installers/usr/bin/sh
feature: fsmonitor--daemon
zlib: 1.3.1
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
uname: Windows 10.0 22631
compiler info: gnuc: 15.2
libc info: no libc information available
$SHELL (typically, interactive shell): /bin/bash
[System Info 2]
git version:
git version 2.47.3
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
zlib: 1.3.1
uname: Linux 6.12.74+deb13+1-amd64 #1 SMP PREEMPT_DYNAMIC Debian
6.12.74-2 (2026-03-08) x86_64
compiler info: gnuc: 14.2
libc info: glibc: 2.41
$SHELL (typically, interactive shell): /bin/bash
[System Info 3]
Ubuntu 20.04LTS
git version : 2.25.1
[Enabled Hooks]
yes , see reproduction script
Best Regards,
Baptiste JEAN-LOUIS
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox