* Re: [PATCH 1/2] mingw: optionally use legacy (non-POSIX) delete semantics
From: Johannes Schindelin @ 2026-05-07 12:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Johannes Schindelin via GitGitGadget, git
In-Reply-To: <afipTWyj2zVYYqMz@pks.im>
Hi Patrick,
On Thu, 7 May 2026, Patrick Steinhardt wrote:
> On Tue, Apr 28, 2026 at 12:52:47PM +0000, Johannes Schindelin via GitGitGadget wrote:
> >
> > - if (DeleteFileW(wpathname))
> > + if (use_legacy_delete < 0)
> > + use_legacy_delete = !!getenv("GIT_TEST_LEGACY_DELETE");
>
> Should this use `git_env_bool()`?
Yes! Will fix in v2.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v2 00/10] pack-objects: integrate --path-walk and some --filter options
From: Derrick Stolee @ 2026-05-07 11:40 UTC (permalink / raw)
To: Taylor Blau
Cc: Derrick Stolee via GitGitGadget, git, christian.couder, gitster,
johannes.schindelin, johncai86, karthik.188, kristofferhaugsbakk,
newren, peff, ps
In-Reply-To: <afpWOGhWJrRdPtcy@nand.local>
On 5/5/26 4:42 PM, Taylor Blau wrote:
> On Tue, May 05, 2026 at 03:44:56PM -0400, Derrick Stolee wrote:
>> Thanks for this suggestion. I got pulled away from my investigation, so
>> wasn't to this point yet.
>
> No problem. One of those things that in the course of responding to your
> email, I had written enough to fix at least one of the tests. I hope I
> didn't step on any toes as a consequence.
>
>> I think the initial idea was that the feature was too niche to add it to
>> the CI builds right away. Your series is going to make it a lot more
>> important, so adding this to CI builds may be valuable.
>
> Sounds good to me. I imagine that this makes more sense to place as a
> preparatory patch in your series, but LMK if you would rather I place it
> in mine.
I've got some changes that make everything work, and they plug into
the right patches at the right time. I need to do a few more rounds
of tweaking, but I should have a new version of the series ready by
tomorrow with these fixes.
In summary, we've got a few things going on:
1. At the base of the topic, the backfill tests fail because they
assert that stderr is empty and the warning about combining
filters comes up. This goes away with the integration, but it
was causing the tests to fail patch-by-patch.
2. In partial clone situations, as you deduced, the direct object
requests were getting filtered out. This starts with the blob
requests with blob:none and then the tree requests are added
in with the tree:0 filter.
3. When getting to your bitmap patches, there are some subtleties
around the --path-walk option adding --boundary to the revision
walk but that changes the object set and breaks tests. I need
to think more about the solution here, but I'll include it as
a reply to my v3 cover letter when the rest of this series is
re-rolled.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
From: Patrick Steinhardt @ 2026-05-07 10:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Derrick Stolee,
Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King
In-Reply-To: <87se83efx1.fsf@gitster.g>
On Thu, May 07, 2026 at 06:18:34PM +0900, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> I thought we updated the codebase to avoid stripping away constness
> >> with strchr() and friends, but the error seems to be more like one
> >> hand in the system passing -Wc11-extensions to stick to older version
> >> of C and the other hand in the system that uses _Generic to implement
> >> the const/non-const variants of strchr() in the system header not
> >> knowing that the other tells C11 const-preserving strchr() should not
> >> be used?
> >
> > This was diagnosed (with a proposed fix) by Patrick over in
> > https://lore.kernel.org/git/20260505-b4-pks-ci-tolerate-glibc-generic-v1-1-5786386fe512@pks.im/.
>
> Indeed.
>
> > tl;dr It's not about `const`-ness at all, but about glibc using a C11
> > construct which clang's strict c99 checker now refuses, thanks to the
> > upgrade to Ubuntu 26.04 in the `ubuntu:rolling` runners.
>
> Yes, that is exactly what I meant by one hand knowing that it was
> told not to use c11 extensions while the other hand ignoring and
> always using c11 extensions in the header. I recall that in the
> past gnu library headers were a bit more careful to make the life
> more pleasant when we use (or decline to use) various features by
> using conditional compilation, but apparently not this case.
Yeah, it's a bit unfortunate indeed. I'd claim that this is a plain bug
though -- as mentioned in the commit message, I think what glibc should
have used is `_has_feature()` instead of `_has_extension()`, and if so,
I think the issue wouldn't exist.
But oh, well.
Patrick
^ permalink raw reply
* Re: [PATCH] config.mak.dev: suppress C11 extension warning for Clang on Linux
From: Pablo @ 2026-05-07 10:07 UTC (permalink / raw)
To: Shardul Natu via GitGitGadget; +Cc: git, Shnatu
In-Reply-To: <pull.2291.git.git.1778120192298.gitgitgadget@gmail.com>
El jue, 7 may 2026 a las 4:16, Shardul Natu via GitGitGadget
(<gitgitgadget@gmail.com>) escribió:
>
> From: Shnatu <snatu@google.com>
>
> When building Git with Clang on Linux with DEVELOPER=1, the build fails
> because Clang treats C11 features used in glibc headers as extensions
> and raises errors due to -std=gnu99, -pedantic, and -Werror.
Hi Shnatu!
This is already being discussed at:
https://lore.kernel.org/git/20260505-b4-pks-ci-tolerate-glibc-generic-v1-1-5786386fe512@pks.im/T/#u
You might want to check out that thread.
Hope this helps,
--
Pablo
>
> Specifically, glibc's string.h uses _Generic (a C11 feature) in macros
> like strchr. When these macros are expanded in Git's C files, Clang
> warns about them being C11 extensions.
>
> GCC does not exhibit this behavior because it suppresses pedantic
> warnings for macros defined in system headers.
>
> To fix this, add -Wno-c11-extensions to DEVELOPER_CFLAGS when using
> Clang, but restrict it to Linux (uname_S == Linux). This suppresses
> the warning for glibc headers while keeping the build strict on other
> platforms (like macOS) to catch accidental C11 usage in Git's own code.
>
> Signed-off-by: Shnatu <snatu@google.com>
> ---
> config.mak.dev: suppress C11 extension warning for Clang on Linux
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2291%2Fkiranani%2Fnext-2-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2291/kiranani/next-2-v1
> Pull-Request: https://github.com/git/git/pull/2291
>
> config.mak.dev | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/config.mak.dev b/config.mak.dev
> index c8dcf78779..f1dcf4329a 100644
> --- a/config.mak.dev
> +++ b/config.mak.dev
> @@ -87,6 +87,9 @@ endif
> # The bug was fixed in Apple clang 12.
> #
> ifneq ($(filter clang1,$(COMPILER_FEATURES)),) # if we are using clang
> +ifeq ($(uname_S),Linux)
> +DEVELOPER_CFLAGS += -Wno-c11-extensions
> +endif
> ifeq ($(uname_S),Darwin) # if we are on darwin
> ifeq ($(filter clang12,$(COMPILER_FEATURES)),) # if version < 12
> DEVELOPER_CFLAGS += -Wno-missing-braces
>
> base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2
> --
> gitgitgadget
>
^ permalink raw reply
* Re: git 2.54 fails to build for rhel 6 (2.53 builds fine)
From: Phillip Wood @ 2026-05-07 10:02 UTC (permalink / raw)
To: Азат Усманов,
brian m. carlson
Cc: git
In-Reply-To: <14616621778106505@de6a8060-5532-44d2-bc08-24865b6e9c4d>
On 06/05/2026 23:28, Азат Усманов wrote:
> any chance someone could show what commit causing the issue, since 2.53 builds ok. Ive looked through the changelog for 2.54 didn't see anythihg specific that migh cause this.
The compiler errors you are seeing are complaining that it cannot find a
function that was added in openssl 3.0 and as brain has already said it
looks like git is linking to an older version of openssl. So if you have
openssl version 3.5.6 installed the configure script is not finding it.
You need to tell the build process where to find the libraries you want
it to use, either by passing that information to configure, or by using
the makefile directly and setting the appropiate compiler flags.
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH v6] revision.c: implement --max-count-oldest
From: Junio C Hamano @ 2026-05-07 9:20 UTC (permalink / raw)
To: Mirko Faina
Cc: Johannes Sixt, Jeff King, Jean-Noël Avila,
Patrick Steinhardt, Tian Yuchen, Ben Knoble, Chris Torek, git
In-Reply-To: <afs2QVHerGLALFcl@exploit>
Mirko Faina <mroik@delayed.space> writes:
>> BTW, this makes me think whether this kind of limiting could be
>> triggered by a negative argument to --max-count.
>
> Would be a good idea if it weren't for the fact that --max-count < 0 has
> for a long time acted like no max count. I'd imagine many could be
> asssuming this behaviour in their scripts.
Many? I am not sure.
What do these script try to achieve by having "--max-count=-1"? It
would be to defeat --max-count=<n> coming from elsewhere, but where?
^ permalink raw reply
* Re: [PATCH v2 11/11] ci: run expensive tests on push builds to integration branches
From: Junio C Hamano @ 2026-05-07 9:18 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Derrick Stolee, Johannes Schindelin via GitGitGadget, git,
Torsten Bögershausen, Jeff King, Patrick Steinhardt
In-Reply-To: <e00dbf04-5866-008f-12e9-efdaacc3f2e0@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> I thought we updated the codebase to avoid stripping away constness
>> with strchr() and friends, but the error seems to be more like one
>> hand in the system passing -Wc11-extensions to stick to older version
>> of C and the other hand in the system that uses _Generic to implement
>> the const/non-const variants of strchr() in the system header not
>> knowing that the other tells C11 const-preserving strchr() should not
>> be used?
>
> This was diagnosed (with a proposed fix) by Patrick over in
> https://lore.kernel.org/git/20260505-b4-pks-ci-tolerate-glibc-generic-v1-1-5786386fe512@pks.im/.
Indeed.
> tl;dr It's not about `const`-ness at all, but about glibc using a C11
> construct which clang's strict c99 checker now refuses, thanks to the
> upgrade to Ubuntu 26.04 in the `ubuntu:rolling` runners.
Yes, that is exactly what I meant by one hand knowing that it was
told not to use c11 extensions while the other hand ignoring and
always using c11 extensions in the header. I recall that in the
past gnu library headers were a bit more careful to make the life
more pleasant when we use (or decline to use) various features by
using conditional compilation, but apparently not this case.
^ permalink raw reply
* Re: [PATCH] add -p: introduce 'w' command to view hunk with --word-diff
From: Pablo @ 2026-05-07 7:55 UTC (permalink / raw)
To: Javier Bassi
Cc: git, Junio C Hamano, Abraham Samuel Adekunle, Rene Scharfe,
Elijah Newren, Ruben Justo, Patrick Steinhardt
In-Reply-To: <20260506235459.529862-1-javierbassi@gmail.com>
El jue, 7 may 2026 a las 1:58, Javier Bassi (<javierbassi@gmail.com>) escribió:
>
> When using `git add --patch`, reviewing changes in long lines can be
> difficult with the default line-based diff. This is particularly
> noticeable in formats such as JSONP, CSV, LaTeX, Markdown, or other
> plain text where small inline edits can be hard to spot.
Very good explanation. I think it's a good idea.
>
> Added `w - print the current hunk with word-diff` during hunk selection
^^^^^^^
Nit: following Documentation/SubmittingPatches, try to use an
imperative mood to write what has been done.
> to re-display the current hunk using `--word-diff`. This provides a
> clearer inline view of changes without modifying the hunk or affecting
> how patches are applied or staged.
>
> Signed-off-by: Javier Bassi <javierbassi@gmail.com>
> ---
> Documentation/git-add.adoc | 1 +
> add-patch.c | 105 ++++++++++++++++++++++++++++++++++++-
> t/t3701-add-interactive.sh | 59 +++++++++++++--------
> 3 files changed, 142 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc
> index 941135dc63..d2ee1cf9a9 100644
> --- a/Documentation/git-add.adoc
> +++ b/Documentation/git-add.adoc
> @@ -351,6 +351,7 @@ patch::
> K - go to the previous hunk, roll over at the top
> s - split the current hunk into smaller hunks
> e - manually edit the current hunk
> + w - print the current hunk with word-diff
> p - print the current hunk
> P - print the current hunk using the pager
> ? - print help
> diff --git a/add-patch.c b/add-patch.c
> index f27edcbe8d..0364f4bc97 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -7,6 +7,7 @@
> #include "commit.h"
> #include "config.h"
> #include "diff.h"
> +#include "diffcore.h"
> #include "editor.h"
> #include "environment.h"
> #include "gettext.h"
> @@ -1508,6 +1509,105 @@ static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,
> strbuf_complete_line(out);
> }
>
> +static void trim_trailing_lf(struct strbuf *buf)
> +{
> + if (buf->len && buf->buf[buf->len - 1] == '\n')
> + strbuf_setlen(buf, buf->len - 1);
> +}
> +
> +static void add_word_diff_line(struct strbuf *old, struct strbuf *new,
> + const char *line, size_t len, char marker)
> +{
> + if (marker == '-' || marker == '+' || *line == ' ') {
> + line++;
> + len--;
> + }
Maybe a tiny comment here would help, to know why '*line' is being
checked here instead of 'marker'. They seem the same and one has to go
to marker declaration and see the comment at 'normalize_marker()'
/* Empty context lines may omit the leading ' ' */
> +
> + if (marker != '+')
> + strbuf_add(old, line, len);
> + if (marker != '-')
> + strbuf_add(new, line, len);
> +}
> +
> +static void build_word_diff_files(struct add_p_state *s, struct hunk *hunk,
> + struct strbuf *old, struct strbuf *new)
> +{
> + size_t i;
> + char last_marker = '\0';
> +
> + for (i = hunk->start; i < hunk->end; i = find_next_line(&s->plain, i)) {
> + size_t next = find_next_line(&s->plain, i);
> + char marker = normalize_marker(s->plain.buf + i);
> +
> + if (marker == '\\') {
> + if (last_marker != '+')
> + trim_trailing_lf(old);
> + if (last_marker != '-')
> + trim_trailing_lf(new);
> + continue;
> + }
Here we check about "\No newline at end of file", after this point I
believe that 'buf->buf[buf->len - 1] == '\n'' will always be true.
Same should be for 'buf->len' because "\No newline at end of file"
shouldn't come first and a '+' '-' line should have been added on a
previous iteration, but the check it's fine, just in case I'm wrong.
What I want to point out is, is the 'trim_trailing_lf' function
necessary? It's only called in the same place and it carries a check
that could be on the caller instead, leaving the function only with
'strbuf_setlen(buf, buf->len - 1);" making sense to inline it at this
point.
You could keep the buf->len check:
if (marker == '\\') {
if (last_marker != '+' && old->len)
strbuf_setlen(old, old->len - 1);
if (last_marker != '-' && new->len)
strbuf_setlen(new, new->len - 1);
continue;
}
> +
> + if (marker != ' ' && marker != '-' && marker != '+')
> + BUG("unhandled diff marker: '%c'", marker);
> +
> + add_word_diff_line(old, new, s->plain.buf + i, next - i,
> + marker);
> + last_marker = marker;
> + }
> +}
> +
> +static struct diff_filespec *word_diff_filespec(struct repository *r,
> + const char *name,
> + struct strbuf *buf)
> +{
> + struct diff_filespec *spec = alloc_filespec(name);
> + size_t size;
> +
> + fill_filespec(spec, null_oid(r->hash_algo), 0, 0100644);
> + spec->data = strbuf_detach(buf, &size);
> + spec->size = size;
> + spec->should_free = 1;
> + spec->is_stdin = 1;
> +
> + return spec;
> +}
> +
> +static void show_hunk_word_diff(struct add_p_state *s, struct hunk *hunk,
> + int colored)
> +{
> + struct hunk_header *header = &hunk->header;
> + struct strbuf old = STRBUF_INIT, new = STRBUF_INIT;
> + struct diff_options opts;
> + struct diff_queue_struct queue;
> +
> + if (!header->old_offset && !header->new_offset) {
Nit: a comment here would help to understand this early return
> + strbuf_reset(&s->buf);
> + render_hunk(s, hunk, 0, colored, &s->buf);
> + fputs(s->buf.buf, stdout);
> + return;
> + }
> +
> + build_word_diff_files(s, hunk, &old, &new);
> +
> + repo_diff_setup(s->r, &opts);
> + opts.output_format = DIFF_FORMAT_PATCH;
> + opts.use_color = colored ? s->cfg.use_color_diff : GIT_COLOR_NEVER;
> + opts.word_diff = DIFF_WORDS_PLAIN;
> + opts.context = header->old_count > header->new_count ?
> + header->old_count : header->new_count;
> + opts.flags.suppress_diff_headers = 1;
> + diff_setup_done(&opts);
> +
> + memcpy(&queue, &diff_queued_diff, sizeof(diff_queued_diff));
> + diff_queue_init(&diff_queued_diff);
> + diff_queue(&diff_queued_diff,
> + word_diff_filespec(s->r, "a", &old),
> + word_diff_filespec(s->r, "b", &new));
> + diffcore_std(&opts);
> + diff_flush(&opts);
> + memcpy(&diff_queued_diff, &queue, sizeof(diff_queued_diff));
> +}
> +
> #define DISPLAY_HUNKS_LINES 20
> static size_t display_hunks(struct add_p_state *s,
> struct file_diff *file_diff, size_t start_index)
> @@ -1540,6 +1640,7 @@ N_("j - go to the next undecided hunk, roll over at the bottom\n"
> "/ - search for a hunk matching the given regex\n"
> "s - split the current hunk into smaller hunks\n"
> "e - manually edit the current hunk\n"
> + "w - print the current hunk with word-diff\n"
> "p - print the current hunk\n"
> "P - print the current hunk using the pager\n"
> "> - go to the next file, roll over at the bottom\n"
> @@ -1731,7 +1832,7 @@ static size_t patch_update_file(struct add_p_state *s,
> permitted |= ALLOW_GOTO_PREVIOUS_FILE;
> strbuf_addstr(&s->buf, ",<");
> }
> - strbuf_addstr(&s->buf, ",p,P");
> + strbuf_addstr(&s->buf, ",w,p,P");
> }
> if (file_diff->deleted)
> prompt_mode_type = PROMPT_DELETION;
> @@ -1953,6 +2054,8 @@ static size_t patch_update_file(struct add_p_state *s,
> hunk->use = USE_HUNK;
> goto soft_increment;
> }
> + } else if (s->answer.buf[0] == 'w') {
> + show_hunk_word_diff(s, hunk, colored);
> } else if (ch == 'p') {
> rendered_hunk_index = -1;
> use_pager = (s->answer.buf[0] == 'P') ? 1 : 0;
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index 6e120a4001..e1ce98d62b 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -48,8 +48,8 @@ test_expect_success 'unknown command' '
> git add -N command &&
> git diff command >expect &&
> cat >>expect <<-EOF &&
> - (1/1) Stage addition [y,n,q,a,d,e,p,P,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
> - (1/1) Stage addition [y,n,q,a,d,e,p,P,?]?$SP
> + (1/1) Stage addition [y,n,q,a,d,e,w,p,P,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
> + (1/1) Stage addition [y,n,q,a,d,e,w,p,P,?]?$SP
> EOF
> git add -p -- command <command >actual 2>&1 &&
> test_cmp expect actual
> @@ -332,9 +332,9 @@ test_expect_success 'different prompts for mode change/deleted' '
> git -c core.filemode=true add -p >actual &&
> sed -n "s/^\(([0-9/]*) Stage .*?\).*/\1/p" actual >actual.filtered &&
> cat >expect <<-\EOF &&
> - (1/1) Stage deletion [y,n,q,a,d,p,P,?]?
> - (1/2) Stage mode change [y,n,q,a,d,k,K,j,J,g,/,p,P,?]?
> - (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]?
> + (1/1) Stage deletion [y,n,q,a,d,w,p,P,?]?
> + (1/2) Stage mode change [y,n,q,a,d,k,K,j,J,g,/,w,p,P,?]?
> + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]?
> EOF
> test_cmp expect actual.filtered
> '
> @@ -521,13 +521,13 @@ test_expect_success 'split hunk setup' '
> test_expect_success 'goto hunk 1 with "g 1"' '
> test_when_finished "git reset" &&
> tr _ " " >expect <<-EOF &&
> - (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? + 1: -1,2 +1,3 +15
> + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]? + 1: -1,2 +1,3 +15
> _ 2: -2,4 +3,8 +21
> go to which hunk? @@ -1,2 +1,3 @@
> _10
> +15
> _20
> - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
> + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
> EOF
> test_write_lines s y g 1 | git add -p >actual &&
> tail -n 7 <actual >actual.trimmed &&
> @@ -540,7 +540,7 @@ test_expect_success 'goto hunk 1 with "g1"' '
> _10
> +15
> _20
> - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
> + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
> EOF
> test_write_lines s y g1 | git add -p >actual &&
> tail -n 4 <actual >actual.trimmed &&
> @@ -550,11 +550,11 @@ test_expect_success 'goto hunk 1 with "g1"' '
> test_expect_success 'navigate to hunk via regex /pattern' '
> test_when_finished "git reset" &&
> tr _ " " >expect <<-EOF &&
> - (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@
> + (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]? @@ -1,2 +1,3 @@
> _10
> +15
> _20
> - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
> + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
> EOF
> test_write_lines s y /1,2 | git add -p >actual &&
> tail -n 5 <actual >actual.trimmed &&
> @@ -567,7 +567,7 @@ test_expect_success 'navigate to hunk via regex / pattern' '
> _10
> +15
> _20
> - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
> + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
> EOF
> test_write_lines s y / 1,2 | git add -p >actual &&
> tail -n 4 <actual >actual.trimmed &&
> @@ -579,27 +579,42 @@ test_expect_success 'print again the hunk' '
> tr _ " " >expect <<-EOF &&
> +15
> 20
> - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@
> + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? @@ -1,2 +1,3 @@
> 10
> +15
> 20
> - (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
> + (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
> EOF
> test_write_lines s y g 1 p | git add -p >actual &&
> tail -n 7 <actual >actual.trimmed &&
> test_cmp expect actual.trimmed
> '
>
> +test_expect_success 'print hunk with word-diff' '
> + test_when_finished "rm -rf word-diff-repo" &&
> + git init word-diff-repo &&
> + (
> + cd word-diff-repo &&
> + test_write_lines "alpha old beta" context >word-diff &&
> + git add word-diff &&
> + git commit -m word-diff &&
> + test_write_lines "alpha new beta" context >word-diff &&
> + test_write_lines w n | git add -p word-diff >actual &&
> + test_grep "alpha \\[-old-\\]{+new+} beta" actual &&
> + git diff --cached --exit-code
> + )
> +'
> +
> test_expect_success TTY 'print again the hunk (PAGER)' '
> test_when_finished "git reset" &&
> cat >expect <<-EOF &&
> <GREEN>+<RESET><GREEN>15<RESET>
> 20<RESET>
> - <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
> + <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
> PAGER 10<RESET>
> PAGER <GREEN>+<RESET><GREEN>15<RESET>
> PAGER 20<RESET>
> - <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
> + <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET>
> EOF
> test_write_lines s y g 1 P |
> (
> @@ -796,21 +811,21 @@ test_expect_success 'colors can be overridden' '
> <BLUE>+<RESET><BLUE>new<RESET>
> <CYAN> more-context<RESET>
> <BLUE>+<RESET><BLUE>another-one<RESET>
> - <YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,p,P,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
> + <YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,w,p,P,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
> <MAGENTA>@@ -1,3 +1,3 @@<RESET>
> <CYAN> context<RESET>
> <BOLD>-old<RESET>
> <BLUE>+<RESET><BLUE>new<RESET>
> <CYAN> more-context<RESET>
> - <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
> + <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
> <CYAN> more-context<RESET>
> <BLUE>+<RESET><BLUE>another-one<RESET>
> - <YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
> + <YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
> <CYAN> context<RESET>
> <BOLD>-old<RESET>
> <BLUE>+new<RESET>
> <CYAN> more-context<RESET>
> - <YELLOW>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
> + <YELLOW>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET>
> EOF
> test_cmp expect actual
> '
> @@ -1424,9 +1439,9 @@ test_expect_success 'invalid option s is rejected' '
> test_write_lines j s q | git add -p >out &&
> sed -ne "s/ @@.*//" -e "s/ \$//" -e "/^(/p" <out >actual &&
> cat >expect <<-EOF &&
> - (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,p,P,?]?
> - (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? Sorry, cannot split this hunk
> - (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?
> + (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,w,p,P,?]?
> + (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? Sorry, cannot split this hunk
> + (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?
> EOF
> test_cmp expect actual
> '
> --
> 2.54.0
>
>
I tried and found out that if a character is contiguous to a word diff
it will be carried as part of the diff. e.g.:
diff --git a/file b/file
index f33f47f..93dd39c 100644
--- a/file
+++ b/file
@@ -1 +1 @@
-foo, bar
+baz, bar
(1/1) Stage this hunk [y,n,q,a,d,e,w,p,P,?]? w
@@ -1 +1 @@
[-foo,-]{+baz,+} bar
the ',' remains at the same place but became part of the diff which adds noise.
Regards,
--
Pablo
^ permalink raw reply
* Re: [PATCH] parser: Fix multi-byte output in here-doc with quoted delimiter
From: Herbert Xu @ 2026-05-07 7:37 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Eric Sunshine, DASH Mailing List
In-Reply-To: <afwpyiK9mh23c-JV@gondor.apana.org.au>
On Thu, May 07, 2026 at 01:57:30PM +0800, Herbert Xu wrote:
> On Thu, Apr 02, 2026 at 08:51:18AM +0200, Patrick Steinhardt wrote:
> > When executing our test suite with Dash v0.5.13.2 one can observe
> > several test failures that all have the same symptoms: we have a quoted
> > heredoc that contains multibyte characters, but the final data does not
> > match what we actually wanted to write. One such example is in t0300,
> > where we see the diffs like the following:
> >
> > --- expect-stdout 2026-04-01 07:25:45.249919440 +0000
> > +++ stdout 2026-04-01 07:25:45.254919509 +0000
> > @@ -1,5 +1,5 @@
> > protocol=https
> > host=example.com
> > -path=perú.git
> > +path=perú.git
> > username=foo
> > password=bar
>
> Thanks for the report.
>
> This patch should fix the problem. Please let me know if there are
> any more oustanding issues.
Oops, I forgot to cc the mailing list. Sorry for the resend.
---8<---
For a here-document with a quoted delimiter, multi-byte characters
should be written out as is with no escaping. Fix this by checking
for syntax == SQSYNTAX (the only time readtoken1 gets called with
SQSYNTAX is for such a here-document) before calling getmbc in
readtoken1.
Reported-by: Patrick Steinhardt <ps@pks.im>
Fixes: b12f136cc704 ("builtin: Process multi-byte characters in read(1)")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/src/parser.c b/src/parser.c
index bea4148..412e876 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -998,9 +998,13 @@ static char *dollarsq_escape(char *out)
STATIC int
readtoken1(int firstc, char const *syntax, char *eofmark, int striptabs)
{
- struct synstack synbase = { .syntax = syntax };
+ struct synstack synbase = {
+ .dblquote = syntax == DQSYNTAX,
+ .syntax = syntax,
+ };
int chkeofmark = checkkwd & CHKEOFMARK;
struct synstack *synstack = &synbase;
+ bool sqheredoc = syntax == SQSYNTAX;
struct nodelist *bqlist = NULL;
int dollarsq = 0;
int c = firstc;
@@ -1009,9 +1013,6 @@ readtoken1(int firstc, char const *syntax, char *eofmark, int striptabs)
size_t len;
char *out;
- if (syntax == DQSYNTAX)
- synstack->dblquote = 1;
-
STARTSTACKSTR(out);
loop: { /* for each line, until end of word */
#if ATTY
@@ -1035,7 +1036,8 @@ readtoken1(int firstc, char const *syntax, char *eofmark, int striptabs)
out);
fieldsplitting = synstack->syntax == BASESYNTAX &&
!synstack->varnest ? 4 : 0;
- ml = getmbc(c, out, fieldsplitting);
+ ml = getmbc(c, out, fieldsplitting |
+ (sqheredoc ? 2 : 0));
if (ml == 1) {
if (out == stackblock())
return TBLANK;
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* [PATCH] parser: Fix multi-byte output in here-doc with quoted delimiter
From: Herbert Xu @ 2026-05-07 5:57 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Eric Sunshine
In-Reply-To: <20260402-pks-tests-with-dash-v2-1-cd7ab11dabc0@pks.im>
On Thu, Apr 02, 2026 at 08:51:18AM +0200, Patrick Steinhardt wrote:
> When executing our test suite with Dash v0.5.13.2 one can observe
> several test failures that all have the same symptoms: we have a quoted
> heredoc that contains multibyte characters, but the final data does not
> match what we actually wanted to write. One such example is in t0300,
> where we see the diffs like the following:
>
> --- expect-stdout 2026-04-01 07:25:45.249919440 +0000
> +++ stdout 2026-04-01 07:25:45.254919509 +0000
> @@ -1,5 +1,5 @@
> protocol=https
> host=example.com
> -path=perú.git
> +path=perú.git
> username=foo
> password=bar
Thanks for the report.
This patch should fix the problem. Please let me know if there are
any more oustanding issues.
---8<---
For a here-document with a quoted delimiter, multi-byte characters
should be written out as is with no escaping. Fix this by checking
for syntax == SQSYNTAX (the only time readtoken1 gets called with
SQSYNTAX is for such a here-document) before calling getmbc in
readtoken1.
Reported-by: Patrick Steinhardt <ps@pks.im>
Fixes: b12f136cc704 ("builtin: Process multi-byte characters in read(1)")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/src/parser.c b/src/parser.c
index bea4148..412e876 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -998,9 +998,13 @@ static char *dollarsq_escape(char *out)
STATIC int
readtoken1(int firstc, char const *syntax, char *eofmark, int striptabs)
{
- struct synstack synbase = { .syntax = syntax };
+ struct synstack synbase = {
+ .dblquote = syntax == DQSYNTAX,
+ .syntax = syntax,
+ };
int chkeofmark = checkkwd & CHKEOFMARK;
struct synstack *synstack = &synbase;
+ bool sqheredoc = syntax == SQSYNTAX;
struct nodelist *bqlist = NULL;
int dollarsq = 0;
int c = firstc;
@@ -1009,9 +1013,6 @@ readtoken1(int firstc, char const *syntax, char *eofmark, int striptabs)
size_t len;
char *out;
- if (syntax == DQSYNTAX)
- synstack->dblquote = 1;
-
STARTSTACKSTR(out);
loop: { /* for each line, until end of word */
#if ATTY
@@ -1035,7 +1036,8 @@ readtoken1(int firstc, char const *syntax, char *eofmark, int striptabs)
out);
fieldsplitting = synstack->syntax == BASESYNTAX &&
!synstack->varnest ? 4 : 0;
- ml = getmbc(c, out, fieldsplitting);
+ ml = getmbc(c, out, fieldsplitting |
+ (sqheredoc ? 2 : 0));
if (ml == 1) {
if (out == stackblock())
return TBLANK;
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH v3 1/1] git-gui: handle missing worktree and separated gitdir
From: Mark Levedahl @ 2026-05-07 5:09 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Shroom Moo
In-Reply-To: <7a963eec-80d2-4605-8cb1-52fb7bc9cf8e@gmail.com>
On 5/6/26 10:05 AM, Mark Levedahl wrote:
>
> On 5/6/26 8:57 AM, Johannes Sixt wrote:
>> Am 06.05.26 um 13:27 schrieb Mark Levedahl:
>>> A git repository (gitdir) can have config.bare true | false | not set
>>> git rev-parse --is-bare-repository tells you that whatever gitdir is discovered from the
>>> current directory has core.bare==true. This happens whether the call is from inside the
>>> gitdir, or in the parent dir of a gitdir named '.git', or in a directory containing a
>>> symlink or a gitfile link to the gitdir. This call never tells you what directory you are
>>> actually in.
>> OK. But how does "find out which directory we are in" come into play
>> here? If we find a bare repository, we do not need a worktree. If we are
>> in a non-bare repository, we can find the worktree with `rev-parse
>> --show-toplevel`.
The influence of config variable core.bare in a git repository breaks the design I
contemplated using rev-parse --is-inside-work-tree. core.bare is a strange tri-state item,
usually only set if the repository is explicitly bare. With core.bare undefined, rev-parse
can give different answers inside and outside a gitdir. For instance
rev-parse --is-bare-repository gives true inside a repository, and false outside,
--is-inside-work-tree is also influenced by core.bare, so results cannot be interpreted as
I initially thought.
Anyway, after some experiments, the minimal sufficient tests I find for blame|browser and
gui|citool are different:
- blame|browser need a readable gitdir, and do not need a worktree. The current directory
can be inside the gitdir, core.bare is irrelevant.
a sufficient startup test:
git rev-parse --git-dir returns the name of a gitdir, rather than throwing an error.
core.bare and being inside/outside of the gitdir do affect error / no error.
(the return value may be a relative path, so can depend upon current
directory)
only GIT_DIR should be exported. GIT_WORK_TREE should not be in env.
- gui|citool, require a gitdir, a worktree outside the gitdir, and core.bare is not true
a sufficient and minimal startup test:
git rev-parse --show-toplevel returns the worktree root, rather than throwing an
error.
core.bare = true in the gitdir will cause error in the above.
being inside the gitdir causes error in the above.
But, starting in a .git directory supporting a worktree is allowed by the fallback
logic
git rev-parse --git-dir returns '.', (current dir is the root of the gitdir),
rather than throwing an error.
the current directory is named ".git", meaning the parent dir is this gitdir's
worktree if core.bare is not true
git -C .. rev-parse --show-toplevel returns the worktree root: (will throw an
error if core.bare is true)
the current directory should be changed to the worktree root.
GIT_DIR and GIT_WORK_TREE should be exported to env.
Mark
^ permalink raw reply
* [PATCH] config.mak.dev: suppress C11 extension warning for Clang on Linux
From: Shardul Natu via GitGitGadget @ 2026-05-07 2:16 UTC (permalink / raw)
To: git; +Cc: Shnatu
From: Shnatu <snatu@google.com>
When building Git with Clang on Linux with DEVELOPER=1, the build fails
because Clang treats C11 features used in glibc headers as extensions
and raises errors due to -std=gnu99, -pedantic, and -Werror.
Specifically, glibc's string.h uses _Generic (a C11 feature) in macros
like strchr. When these macros are expanded in Git's C files, Clang
warns about them being C11 extensions.
GCC does not exhibit this behavior because it suppresses pedantic
warnings for macros defined in system headers.
To fix this, add -Wno-c11-extensions to DEVELOPER_CFLAGS when using
Clang, but restrict it to Linux (uname_S == Linux). This suppresses
the warning for glibc headers while keeping the build strict on other
platforms (like macOS) to catch accidental C11 usage in Git's own code.
Signed-off-by: Shnatu <snatu@google.com>
---
config.mak.dev: suppress C11 extension warning for Clang on Linux
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2291%2Fkiranani%2Fnext-2-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2291/kiranani/next-2-v1
Pull-Request: https://github.com/git/git/pull/2291
config.mak.dev | 3 +++
1 file changed, 3 insertions(+)
diff --git a/config.mak.dev b/config.mak.dev
index c8dcf78779..f1dcf4329a 100644
--- a/config.mak.dev
+++ b/config.mak.dev
@@ -87,6 +87,9 @@ endif
# The bug was fixed in Apple clang 12.
#
ifneq ($(filter clang1,$(COMPILER_FEATURES)),) # if we are using clang
+ifeq ($(uname_S),Linux)
+DEVELOPER_CFLAGS += -Wno-c11-extensions
+endif
ifeq ($(uname_S),Darwin) # if we are on darwin
ifeq ($(filter clang12,$(COMPILER_FEATURES)),) # if version < 12
DEVELOPER_CFLAGS += -Wno-missing-braces
base-commit: 4f69b47b940100b02630f745a52f9d9850f122b2
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH] Makefile: link osxkeychain helper against Rust
From: Shnatu @ 2026-05-07 0:39 UTC (permalink / raw)
To: kristofferhaugsbakk; +Cc: git, gitgitgadget, snatu
In-Reply-To: <a9e71e67-a853-4cfb-beb8-69b98ead720e@app.fastmail.com>
I have remove the Google specific paragraph, in addition to updating the description
^ permalink raw reply
* [PATCH] add -p: introduce 'w' command to view hunk with --word-diff
From: Javier Bassi @ 2026-05-06 23:54 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Abraham Samuel Adekunle, Rene Scharfe,
Elijah Newren, Ruben Justo, Patrick Steinhardt, Javier Bassi
When using `git add --patch`, reviewing changes in long lines can be
difficult with the default line-based diff. This is particularly
noticeable in formats such as JSONP, CSV, LaTeX, Markdown, or other
plain text where small inline edits can be hard to spot.
Added `w - print the current hunk with word-diff` during hunk selection
to re-display the current hunk using `--word-diff`. This provides a
clearer inline view of changes without modifying the hunk or affecting
how patches are applied or staged.
Signed-off-by: Javier Bassi <javierbassi@gmail.com>
---
Documentation/git-add.adoc | 1 +
add-patch.c | 105 ++++++++++++++++++++++++++++++++++++-
t/t3701-add-interactive.sh | 59 +++++++++++++--------
3 files changed, 142 insertions(+), 23 deletions(-)
diff --git a/Documentation/git-add.adoc b/Documentation/git-add.adoc
index 941135dc63..d2ee1cf9a9 100644
--- a/Documentation/git-add.adoc
+++ b/Documentation/git-add.adoc
@@ -351,6 +351,7 @@ patch::
K - go to the previous hunk, roll over at the top
s - split the current hunk into smaller hunks
e - manually edit the current hunk
+ w - print the current hunk with word-diff
p - print the current hunk
P - print the current hunk using the pager
? - print help
diff --git a/add-patch.c b/add-patch.c
index f27edcbe8d..0364f4bc97 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -7,6 +7,7 @@
#include "commit.h"
#include "config.h"
#include "diff.h"
+#include "diffcore.h"
#include "editor.h"
#include "environment.h"
#include "gettext.h"
@@ -1508,6 +1509,105 @@ static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,
strbuf_complete_line(out);
}
+static void trim_trailing_lf(struct strbuf *buf)
+{
+ if (buf->len && buf->buf[buf->len - 1] == '\n')
+ strbuf_setlen(buf, buf->len - 1);
+}
+
+static void add_word_diff_line(struct strbuf *old, struct strbuf *new,
+ const char *line, size_t len, char marker)
+{
+ if (marker == '-' || marker == '+' || *line == ' ') {
+ line++;
+ len--;
+ }
+
+ if (marker != '+')
+ strbuf_add(old, line, len);
+ if (marker != '-')
+ strbuf_add(new, line, len);
+}
+
+static void build_word_diff_files(struct add_p_state *s, struct hunk *hunk,
+ struct strbuf *old, struct strbuf *new)
+{
+ size_t i;
+ char last_marker = '\0';
+
+ for (i = hunk->start; i < hunk->end; i = find_next_line(&s->plain, i)) {
+ size_t next = find_next_line(&s->plain, i);
+ char marker = normalize_marker(s->plain.buf + i);
+
+ if (marker == '\\') {
+ if (last_marker != '+')
+ trim_trailing_lf(old);
+ if (last_marker != '-')
+ trim_trailing_lf(new);
+ continue;
+ }
+
+ if (marker != ' ' && marker != '-' && marker != '+')
+ BUG("unhandled diff marker: '%c'", marker);
+
+ add_word_diff_line(old, new, s->plain.buf + i, next - i,
+ marker);
+ last_marker = marker;
+ }
+}
+
+static struct diff_filespec *word_diff_filespec(struct repository *r,
+ const char *name,
+ struct strbuf *buf)
+{
+ struct diff_filespec *spec = alloc_filespec(name);
+ size_t size;
+
+ fill_filespec(spec, null_oid(r->hash_algo), 0, 0100644);
+ spec->data = strbuf_detach(buf, &size);
+ spec->size = size;
+ spec->should_free = 1;
+ spec->is_stdin = 1;
+
+ return spec;
+}
+
+static void show_hunk_word_diff(struct add_p_state *s, struct hunk *hunk,
+ int colored)
+{
+ struct hunk_header *header = &hunk->header;
+ struct strbuf old = STRBUF_INIT, new = STRBUF_INIT;
+ struct diff_options opts;
+ struct diff_queue_struct queue;
+
+ if (!header->old_offset && !header->new_offset) {
+ strbuf_reset(&s->buf);
+ render_hunk(s, hunk, 0, colored, &s->buf);
+ fputs(s->buf.buf, stdout);
+ return;
+ }
+
+ build_word_diff_files(s, hunk, &old, &new);
+
+ repo_diff_setup(s->r, &opts);
+ opts.output_format = DIFF_FORMAT_PATCH;
+ opts.use_color = colored ? s->cfg.use_color_diff : GIT_COLOR_NEVER;
+ opts.word_diff = DIFF_WORDS_PLAIN;
+ opts.context = header->old_count > header->new_count ?
+ header->old_count : header->new_count;
+ opts.flags.suppress_diff_headers = 1;
+ diff_setup_done(&opts);
+
+ memcpy(&queue, &diff_queued_diff, sizeof(diff_queued_diff));
+ diff_queue_init(&diff_queued_diff);
+ diff_queue(&diff_queued_diff,
+ word_diff_filespec(s->r, "a", &old),
+ word_diff_filespec(s->r, "b", &new));
+ diffcore_std(&opts);
+ diff_flush(&opts);
+ memcpy(&diff_queued_diff, &queue, sizeof(diff_queued_diff));
+}
+
#define DISPLAY_HUNKS_LINES 20
static size_t display_hunks(struct add_p_state *s,
struct file_diff *file_diff, size_t start_index)
@@ -1540,6 +1640,7 @@ N_("j - go to the next undecided hunk, roll over at the bottom\n"
"/ - search for a hunk matching the given regex\n"
"s - split the current hunk into smaller hunks\n"
"e - manually edit the current hunk\n"
+ "w - print the current hunk with word-diff\n"
"p - print the current hunk\n"
"P - print the current hunk using the pager\n"
"> - go to the next file, roll over at the bottom\n"
@@ -1731,7 +1832,7 @@ static size_t patch_update_file(struct add_p_state *s,
permitted |= ALLOW_GOTO_PREVIOUS_FILE;
strbuf_addstr(&s->buf, ",<");
}
- strbuf_addstr(&s->buf, ",p,P");
+ strbuf_addstr(&s->buf, ",w,p,P");
}
if (file_diff->deleted)
prompt_mode_type = PROMPT_DELETION;
@@ -1953,6 +2054,8 @@ static size_t patch_update_file(struct add_p_state *s,
hunk->use = USE_HUNK;
goto soft_increment;
}
+ } else if (s->answer.buf[0] == 'w') {
+ show_hunk_word_diff(s, hunk, colored);
} else if (ch == 'p') {
rendered_hunk_index = -1;
use_pager = (s->answer.buf[0] == 'P') ? 1 : 0;
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 6e120a4001..e1ce98d62b 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -48,8 +48,8 @@ test_expect_success 'unknown command' '
git add -N command &&
git diff command >expect &&
cat >>expect <<-EOF &&
- (1/1) Stage addition [y,n,q,a,d,e,p,P,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
- (1/1) Stage addition [y,n,q,a,d,e,p,P,?]?$SP
+ (1/1) Stage addition [y,n,q,a,d,e,w,p,P,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help)
+ (1/1) Stage addition [y,n,q,a,d,e,w,p,P,?]?$SP
EOF
git add -p -- command <command >actual 2>&1 &&
test_cmp expect actual
@@ -332,9 +332,9 @@ test_expect_success 'different prompts for mode change/deleted' '
git -c core.filemode=true add -p >actual &&
sed -n "s/^\(([0-9/]*) Stage .*?\).*/\1/p" actual >actual.filtered &&
cat >expect <<-\EOF &&
- (1/1) Stage deletion [y,n,q,a,d,p,P,?]?
- (1/2) Stage mode change [y,n,q,a,d,k,K,j,J,g,/,p,P,?]?
- (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]?
+ (1/1) Stage deletion [y,n,q,a,d,w,p,P,?]?
+ (1/2) Stage mode change [y,n,q,a,d,k,K,j,J,g,/,w,p,P,?]?
+ (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]?
EOF
test_cmp expect actual.filtered
'
@@ -521,13 +521,13 @@ test_expect_success 'split hunk setup' '
test_expect_success 'goto hunk 1 with "g 1"' '
test_when_finished "git reset" &&
tr _ " " >expect <<-EOF &&
- (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? + 1: -1,2 +1,3 +15
+ (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]? + 1: -1,2 +1,3 +15
_ 2: -2,4 +3,8 +21
go to which hunk? @@ -1,2 +1,3 @@
_10
+15
_20
- (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
+ (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
EOF
test_write_lines s y g 1 | git add -p >actual &&
tail -n 7 <actual >actual.trimmed &&
@@ -540,7 +540,7 @@ test_expect_success 'goto hunk 1 with "g1"' '
_10
+15
_20
- (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
+ (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
EOF
test_write_lines s y g1 | git add -p >actual &&
tail -n 4 <actual >actual.trimmed &&
@@ -550,11 +550,11 @@ test_expect_success 'goto hunk 1 with "g1"' '
test_expect_success 'navigate to hunk via regex /pattern' '
test_when_finished "git reset" &&
tr _ " " >expect <<-EOF &&
- (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@
+ (2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]? @@ -1,2 +1,3 @@
_10
+15
_20
- (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
+ (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
EOF
test_write_lines s y /1,2 | git add -p >actual &&
tail -n 5 <actual >actual.trimmed &&
@@ -567,7 +567,7 @@ test_expect_success 'navigate to hunk via regex / pattern' '
_10
+15
_20
- (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
+ (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
EOF
test_write_lines s y / 1,2 | git add -p >actual &&
tail -n 4 <actual >actual.trimmed &&
@@ -579,27 +579,42 @@ test_expect_success 'print again the hunk' '
tr _ " " >expect <<-EOF &&
+15
20
- (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? @@ -1,2 +1,3 @@
+ (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? @@ -1,2 +1,3 @@
10
+15
20
- (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?_
+ (1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?_
EOF
test_write_lines s y g 1 p | git add -p >actual &&
tail -n 7 <actual >actual.trimmed &&
test_cmp expect actual.trimmed
'
+test_expect_success 'print hunk with word-diff' '
+ test_when_finished "rm -rf word-diff-repo" &&
+ git init word-diff-repo &&
+ (
+ cd word-diff-repo &&
+ test_write_lines "alpha old beta" context >word-diff &&
+ git add word-diff &&
+ git commit -m word-diff &&
+ test_write_lines "alpha new beta" context >word-diff &&
+ test_write_lines w n | git add -p word-diff >actual &&
+ test_grep "alpha \\[-old-\\]{+new+} beta" actual &&
+ git diff --cached --exit-code
+ )
+'
+
test_expect_success TTY 'print again the hunk (PAGER)' '
test_when_finished "git reset" &&
cat >expect <<-EOF &&
<GREEN>+<RESET><GREEN>15<RESET>
20<RESET>
- <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
+ <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET>PAGER <CYAN>@@ -1,2 +1,3 @@<RESET>
PAGER 10<RESET>
PAGER <GREEN>+<RESET><GREEN>15<RESET>
PAGER 20<RESET>
- <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
+ <BOLD;BLUE>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET>
EOF
test_write_lines s y g 1 P |
(
@@ -796,21 +811,21 @@ test_expect_success 'colors can be overridden' '
<BLUE>+<RESET><BLUE>new<RESET>
<CYAN> more-context<RESET>
<BLUE>+<RESET><BLUE>another-one<RESET>
- <YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,p,P,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
+ <YELLOW>(1/1) Stage this hunk [y,n,q,a,d,s,e,w,p,P,?]? <RESET><BOLD>Split into 2 hunks.<RESET>
<MAGENTA>@@ -1,3 +1,3 @@<RESET>
<CYAN> context<RESET>
<BOLD>-old<RESET>
<BLUE>+<RESET><BLUE>new<RESET>
<CYAN> more-context<RESET>
- <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
+ <YELLOW>(1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET><MAGENTA>@@ -3 +3,2 @@<RESET>
<CYAN> more-context<RESET>
<BLUE>+<RESET><BLUE>another-one<RESET>
- <YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,p,P,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
+ <YELLOW>(2/2) Stage this hunk [y,n,q,a,d,K,J,g,/,e,w,p,P,?]? <RESET><MAGENTA>@@ -1,3 +1,3 @@<RESET>
<CYAN> context<RESET>
<BOLD>-old<RESET>
<BLUE>+new<RESET>
<CYAN> more-context<RESET>
- <YELLOW>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? <RESET>
+ <YELLOW>(1/2) Stage this hunk (was: y) [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? <RESET>
EOF
test_cmp expect actual
'
@@ -1424,9 +1439,9 @@ test_expect_success 'invalid option s is rejected' '
test_write_lines j s q | git add -p >out &&
sed -ne "s/ @@.*//" -e "s/ \$//" -e "/^(/p" <out >actual &&
cat >expect <<-EOF &&
- (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,p,P,?]?
- (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]? Sorry, cannot split this hunk
- (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,p,P,?]?
+ (1/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,s,e,w,p,P,?]?
+ (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]? Sorry, cannot split this hunk
+ (2/2) Stage this hunk [y,n,q,a,d,k,K,j,J,g,/,e,w,p,P,?]?
EOF
test_cmp expect actual
'
--
2.54.0
^ permalink raw reply related
* Re: git 2.54 fails to build for rhel 6 (2.53 builds fine)
From: Ben Knoble @ 2026-05-06 23:21 UTC (permalink / raw)
To: usmanov; +Cc: brian m. carlson, git
In-Reply-To: <14616621778106505@de6a8060-5532-44d2-bc08-24865b6e9c4d>
> Le 6 mai 2026 à 18:35, Азат Усманов <usmanov@ieml.ru> a écrit :
>
> any chance someone could show what commit causing the issue, since 2.53 builds ok. Ive looked through the changelog for 2.54 didn't see anythihg specific that migh cause this.
Since you are presumably able to produce the build failure, you could use git bisect to find the cause of your issue.
^ permalink raw reply
* [PATCH/RFC 4/5] test-tool: add a "historian" subcommand for building merge fixtures
From: Johannes Schindelin via GitGitGadget @ 2026-05-06 22:43 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2106.git.1778107405.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
The merge-replay tests added in a follow-up commit need a way to set
up specific topologies with full control over blob contents, parent
order, and per-side trees. Sequencing plumbing commands or driving
plain `git fast-import` from shell quickly becomes unreadable for
the kinds of scenarios that exercise non-trivial merge resolution
(textual conflicts, semantic edits outside the conflict region,
intentional limitations such as new content on one side).
Add a small `test-tool historian` subcommand that reads a tight,
shell-quoted, one-line-per-object DSL and feeds an equivalent stream
to a `git fast-import` child process. Each blob and commit is given
a logical name; the helper allocates fast-import marks on first use
and emits a lightweight tag for every commit so tests can refer to
the resulting object via `refs/tags/<name>`.
The DSL has just two directives:
blob NAME LINE...
commit NAME BRANCH SUBJECT [from=NAME] [merge=NAME]... [PATH=BLOB]...
A blob's content is the listed lines joined with `\n` (and a final
`\n`); a commit's tree is exactly the listed PATH=BLOB pairs (the
helper emits a `deleteall` so nothing leaks in from the implicit
parent). Token splitting is delegated to `split_cmdline()` so quoted
arguments work as in shell. Marks for parent references and file
contents go through the same `strintmap`-backed name resolver, which
keeps the helper itself trivially small: blob writing, tree
construction, commit creation and merge-base computation are all
handled by `git fast-import`.
Note that the DSL reserves the names `from` and `merge` (with a
trailing `=`) for parent specification; a tree path called `from` or
`merge` cannot be expressed via this helper. That is acceptable here
because every input is a tightly controlled test fixture and the
filenames are chosen by the test author.
The helper trusts its caller: malformed input results in a
fast-import error rather than a friendly diagnostic.
Wire the new subcommand into the Makefile and meson build, register
it in `t/helper/test-tool.{c,h}`.
Assisted-by: Claude Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 1 +
t/helper/meson.build | 1 +
t/helper/test-historian.c | 189 ++++++++++++++++++++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
5 files changed, 193 insertions(+)
create mode 100644 t/helper/test-historian.c
diff --git a/Makefile b/Makefile
index cedc234173..b38678b484 100644
--- a/Makefile
+++ b/Makefile
@@ -832,6 +832,7 @@ TEST_BUILTINS_OBJS += test-hash-speed.o
TEST_BUILTINS_OBJS += test-hash.o
TEST_BUILTINS_OBJS += test-hashmap.o
TEST_BUILTINS_OBJS += test-hexdump.o
+TEST_BUILTINS_OBJS += test-historian.o
TEST_BUILTINS_OBJS += test-json-writer.o
TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
TEST_BUILTINS_OBJS += test-match-trees.o
diff --git a/t/helper/meson.build b/t/helper/meson.build
index 675e64c010..704edd1e1f 100644
--- a/t/helper/meson.build
+++ b/t/helper/meson.build
@@ -29,6 +29,7 @@ test_tool_sources = [
'test-hash.c',
'test-hashmap.c',
'test-hexdump.c',
+ 'test-historian.c',
'test-json-writer.c',
'test-lazy-init-name-hash.c',
'test-match-trees.c',
diff --git a/t/helper/test-historian.c b/t/helper/test-historian.c
new file mode 100644
index 0000000000..2250d420c0
--- /dev/null
+++ b/t/helper/test-historian.c
@@ -0,0 +1,189 @@
+/*
+ * Build a small history out of a tiny declarative input. Used by tests
+ * that need specific merge topologies without long sequences of
+ * plumbing commands or fragile shell helpers.
+ *
+ * The historian reads stdin line by line and emits an equivalent
+ * stream to a `git fast-import` child process. It also allocates marks
+ * for named objects so tests can refer to commits and blobs by name.
+ *
+ * Input directives (one per line, shell-style quoting):
+ *
+ * blob NAME LINE1 LINE2 ...
+ * Each LINE becomes a content line in the blob; lines are
+ * joined with '\n' and the blob ends with a final '\n'. With
+ * no LINEs, the blob is empty.
+ *
+ * commit NAME BRANCH SUBJECT [from=PARENT] [merge=PARENT]... [PATH=BLOB]...
+ * Creates a commit on refs/heads/BRANCH using the listed
+ * file=blob mappings as the entire tree (no inheritance from
+ * parents). Up to one `from=` and any number of `merge=`
+ * parents may be given. `from=` defaults to the current branch
+ * tip; if BRANCH has no tip yet, the commit becomes a root.
+ *
+ * Each `commit NAME` directive also creates a lightweight tag
+ * `refs/tags/NAME` so tests can `git rev-parse NAME`.
+ *
+ * This helper trusts its caller; malformed input results in fast-import
+ * errors. That is fine because test scripts feed it tightly controlled
+ * input.
+ */
+
+#define USE_THE_REPOSITORY_VARIABLE
+
+#include "test-tool.h"
+#include "git-compat-util.h"
+#include "alias.h"
+#include "run-command.h"
+#include "setup.h"
+#include "strbuf.h"
+#include "strmap.h"
+#include "strvec.h"
+
+static int next_mark = 1;
+
+static int resolve_mark(struct strintmap *names, const char *name)
+{
+ int n = strintmap_get(names, name);
+ if (!n) {
+ n = next_mark++;
+ strintmap_set(names, name, n);
+ }
+ return n;
+}
+
+static void emit_data(FILE *out, const char *data, size_t len)
+{
+ fprintf(out, "data %"PRIuMAX"\n", (uintmax_t)len);
+ fwrite(data, 1, len, out);
+ fputc('\n', out);
+}
+
+static void emit_blob(FILE *out, struct strintmap *names,
+ int argc, const char **argv)
+{
+ struct strbuf content = STRBUF_INIT;
+ int n = resolve_mark(names, argv[1]);
+ int i;
+
+ for (i = 2; i < argc; i++) {
+ strbuf_addstr(&content, argv[i]);
+ strbuf_addch(&content, '\n');
+ }
+
+ fprintf(out, "blob\nmark :%d\n", n);
+ emit_data(out, content.buf, content.len);
+ strbuf_release(&content);
+}
+
+static void emit_tag(FILE *out, const char *name, int mark)
+{
+ fprintf(out, "reset refs/tags/%s\nfrom :%d\n\n", name, mark);
+}
+
+static void emit_commit(FILE *out, struct strintmap *names,
+ int argc, const char **argv, int seq)
+{
+ int n = resolve_mark(names, argv[1]);
+ const char *branch = argv[2];
+ const char *subject = argv[3];
+ const char *rest;
+ int i;
+
+ fprintf(out, "commit refs/heads/%s\nmark :%d\n", branch, n);
+ fprintf(out, "author A <a@e> %d +0000\n", 1700000000 + seq);
+ fprintf(out, "committer A <a@e> %d +0000\n", 1700000000 + seq);
+ emit_data(out, subject, strlen(subject));
+
+ /*
+ * fast-import requires `from` and `merge` to precede all file
+ * operations; emit them first regardless of argv ordering.
+ */
+ for (i = 4; i < argc; i++) {
+ if (skip_prefix(argv[i], "from=", &rest))
+ fprintf(out, "from :%d\n", resolve_mark(names, rest));
+ else if (skip_prefix(argv[i], "merge=", &rest))
+ fprintf(out, "merge :%d\n", resolve_mark(names, rest));
+ }
+
+ /*
+ * The PATH=BLOB list is the entire tree; wipe whatever the
+ * implicit parent contributed before re-applying it.
+ */
+ fprintf(out, "deleteall\n");
+ for (i = 4; i < argc; i++) {
+ const char *eq;
+ size_t key_len;
+ char *path;
+
+ if (skip_prefix(argv[i], "from=", &rest) ||
+ skip_prefix(argv[i], "merge=", &rest))
+ continue;
+ eq = strchr(argv[i], '=');
+ if (!eq)
+ die("bad commit spec '%s'", argv[i]);
+ key_len = eq - argv[i];
+ path = xmemdupz(argv[i], key_len);
+ fprintf(out, "M 100644 :%d %s\n",
+ resolve_mark(names, eq + 1), path);
+ free(path);
+ }
+
+ fputc('\n', out);
+ emit_tag(out, argv[1], n);
+}
+
+int cmd__historian(int argc, const char **argv UNUSED)
+{
+ struct child_process fi = CHILD_PROCESS_INIT;
+ struct strintmap names = STRINTMAP_INIT;
+ struct strbuf line = STRBUF_INIT;
+ int seq = 0;
+ int ret = 0;
+ FILE *fi_in;
+
+ if (argc != 1)
+ die("usage: test-tool historian <input");
+
+ setup_git_directory();
+
+ strvec_pushl(&fi.args, "fast-import", "--quiet", "--force", NULL);
+ fi.git_cmd = 1;
+ fi.in = -1;
+ fi.no_stdout = 1;
+ if (start_command(&fi))
+ die("failed to start git fast-import");
+ fi_in = xfdopen(fi.in, "w");
+
+ while (strbuf_getline_lf(&line, stdin) != EOF) {
+ const char **a = NULL;
+ int n;
+
+ strbuf_trim(&line);
+ if (!line.len || line.buf[0] == '#')
+ continue;
+
+ n = split_cmdline(line.buf, &a);
+ if (n < 0)
+ die("split_cmdline failed: %s",
+ split_cmdline_strerror(n));
+
+ if (n >= 2 && !strcmp(a[0], "blob"))
+ emit_blob(fi_in, &names, n, a);
+ else if (n >= 4 && !strcmp(a[0], "commit"))
+ emit_commit(fi_in, &names, n, a, seq++);
+ else
+ die("unknown directive: %s", a[0]);
+
+ free(a);
+ }
+
+ if (fclose(fi_in))
+ die_errno("close fast-import stdin");
+ if (finish_command(&fi))
+ ret = 1;
+
+ strbuf_release(&line);
+ strintmap_clear(&names);
+ return ret;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index a7abc618b3..28bde98ce1 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {
{ "hashmap", cmd__hashmap },
{ "hash-speed", cmd__hash_speed },
{ "hexdump", cmd__hexdump },
+ { "historian", cmd__historian },
{ "json-writer", cmd__json_writer },
{ "lazy-init-name-hash", cmd__lazy_init_name_hash },
{ "match-trees", cmd__match_trees },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 7f150fa1eb..78cec8594a 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -32,6 +32,7 @@ int cmd__getcwd(int argc, const char **argv);
int cmd__hashmap(int argc, const char **argv);
int cmd__hash_speed(int argc, const char **argv);
int cmd__hexdump(int argc, const char **argv);
+int cmd__historian(int argc, const char **argv);
int cmd__json_writer(int argc, const char **argv);
int cmd__lazy_init_name_hash(int argc, const char **argv);
int cmd__match_trees(int argc, const char **argv);
--
gitgitgadget
^ permalink raw reply related
* [PATCH/RFC 5/5] t3454: cover merge-replay scenarios with the historian helper
From: Johannes Schindelin via GitGitGadget @ 2026-05-06 22:43 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2106.git.1778107405.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Add a dedicated test script for `git history reword` (and
`git replay` via the same code path) across 2-parent merges, using
the `test-tool historian` fixture builder so each scenario reads as
a small declarative recipe rather than a sequence of plumbing
commands.
The script exercises the cases that motivated the merge-replay
work:
* a clean merge where each side touches unrelated files;
* a non-trivial merge where the same line was changed on both
sides and the user resolved by hand (textual manual resolution
must be preserved through the replay);
* a non-trivial merge where the user also touched a line outside
any conflict region (a "semantic" edit must also be preserved
through the replay);
* an octopus merge in the rewrite path, which is rejected;
* a function rename across the merge with a brand-new caller
introduced by the rewritten parents. The pre-existing caller
that the user manually renamed in the original merge must keep
its rename, and the brand-new caller must _not_ be rewritten
(calvin/hobbes naming chosen for legibility). This second part
is the documented limitation: the replay propagates the textual
diffs the user actually made, it does not extrapolate
symbol-level intent. Symbol-aware refactoring is out of scope,
just as it is for plain rebase.
The fixture builder lets each scenario sit in roughly a dozen lines
of historian directives plus the assertions, which keeps the test
file readable when more scenarios are added later.
Assisted-by: Claude Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/meson.build | 1 +
t/t3454-history-merges.sh | 308 ++++++++++++++++++++++++++++++++++++++
2 files changed, 309 insertions(+)
create mode 100755 t/t3454-history-merges.sh
diff --git a/t/meson.build b/t/meson.build
index 7528e5cda5..25b0119d43 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -397,6 +397,7 @@ integration_tests = [
't3450-history.sh',
't3451-history-reword.sh',
't3452-history-split.sh',
+ 't3454-history-merges.sh',
't3500-cherry.sh',
't3501-revert-cherry-pick.sh',
't3502-cherry-pick-merge.sh',
diff --git a/t/t3454-history-merges.sh b/t/t3454-history-merges.sh
new file mode 100755
index 0000000000..2eb3c947eb
--- /dev/null
+++ b/t/t3454-history-merges.sh
@@ -0,0 +1,308 @@
+#!/bin/sh
+
+test_description='git history reword across merge commits
+
+Exercises the merge-replay path in `git history reword` using the
+`test-tool historian` test fixture builder so each scenario is
+described in a small declarative input rather than a sprawling
+sequence of plumbing commands. The interesting cases are:
+
+ * a clean merge with each side touching unrelated files;
+ * a non-trivial merge whose conflicting line was resolved by hand
+ (textually) and whose resolution must be preserved through the
+ replay;
+ * a non-trivial merge with a manual *semantic* edit (an additional
+ change outside the conflict region) that must also be preserved.
+'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+# Replace the commit's message via a fake editor and run reword.
+reword_to () {
+ new_msg="$1"
+ target="$2"
+ write_script fake-editor.sh <<-EOF &&
+ echo "$new_msg" >"\$1"
+ EOF
+ test_set_editor "$(pwd)/fake-editor.sh" &&
+ git history reword "$target" &&
+ rm fake-editor.sh
+}
+
+build_clean_merge () {
+ test-tool historian <<-\EOF
+ # Setup:
+ # A (a) --- C (a, h) ----+--- M (a, g, h)
+ # \ /
+ # +-- B (a, g) ------+
+ #
+ # Topic touches `g` only; main touches `h` only. The auto-merge
+ # at M is clean.
+ blob a "shared content"
+ blob g guarded
+ blob h host
+ commit A main "A" a=a
+ commit B topic "B (introduces g)" from=A a=a g=g
+ commit C main "C (introduces h)" a=a h=h
+ commit M main "Merge topic" merge=B a=a g=g h=h
+ EOF
+}
+
+test_expect_success 'clean merge: both sides touch unrelated files' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ build_clean_merge &&
+
+ reword_to "AA" A &&
+
+ # The merge is still a 2-parent merge with the same subject
+ # and tree (clean replay leaves content unchanged).
+ test_cmp_rev HEAD^{tree} M^{tree} &&
+
+ echo "Merge topic" >expect-subject &&
+ git log -1 --format=%s HEAD >subject &&
+ test_cmp expect-subject subject &&
+
+ git rev-list --merges HEAD~..HEAD >merges &&
+ test_line_count = 1 merges
+ )
+'
+
+build_textual_resolution () {
+ test-tool historian <<-\EOF
+ # Both sides change the same line of `a`; the user resolved with
+ # their own combined text, recorded directly as the merge tree.
+ blob a_v1 line1 line2 line3
+ blob a_main line1 line2-main line3
+ blob a_topic line1 line2-topic line3
+ blob a_resolution line1 line2-merged-by-hand line3
+ commit A main "A" a=a_v1
+ commit B topic "B (line2 on topic)" from=A a=a_topic
+ commit C main "C (line2 on main)" a=a_main
+ commit M main "Merge topic" merge=B a=a_resolution
+ EOF
+}
+
+test_expect_success 'non-trivial merge: textual manual resolution is preserved' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ build_textual_resolution &&
+
+ reword_to "AA" A &&
+
+ git show HEAD:a >after &&
+ test_write_lines line1 line2-merged-by-hand line3 >expect &&
+ test_cmp expect after
+ )
+'
+
+build_semantic_edit () {
+ test-tool historian <<-\EOF
+ # Topic and main conflict on line2 of `a`. The user's resolution
+ # at M not only picks combined text on line2 but ALSO touches
+ # line5 (a "semantic" edit outside any conflict region) -- this
+ # kind of edit is invisible to a naive pick-one-side strategy and
+ # must be preserved by replay.
+ blob a_v1 line1 line2 line3 line4 line5
+ blob a_main line1 line2-main line3 line4 line5
+ blob a_topic line1 line2-topic line3 line4 line5
+ blob a_resolution line1 line2-merged line3 line4 line5-touched
+ commit A main "A" a=a_v1
+ commit B topic "B (line2 on topic)" from=A a=a_topic
+ commit C main "C (line2 on main)" a=a_main
+ commit M main "Merge topic" merge=B a=a_resolution
+ EOF
+}
+
+test_expect_success 'non-trivial merge: semantic edit outside conflict region is preserved' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ build_semantic_edit &&
+
+ reword_to "AA" A &&
+
+ git show HEAD:a >after &&
+ test_write_lines line1 line2-merged line3 line4 line5-touched \
+ >expect &&
+ test_cmp expect after
+ )
+'
+
+build_octopus () {
+ test-tool historian <<-\EOF
+ blob a "x"
+ commit A main "A" a=a
+ commit B b1 "B" from=A a=a
+ commit C b2 "C" from=A a=a
+ commit D b3 "D" from=A a=a
+ commit O main "octopus" merge=B merge=C merge=D a=a
+ EOF
+}
+
+test_expect_success 'octopus merge in the rewrite path is rejected' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ build_octopus &&
+
+ test_must_fail git -c core.editor=true history reword \
+ --dry-run A 2>err &&
+ test_grep "octopus" err
+ )
+'
+
+build_with_boundary_other_than_onto () {
+ test-tool historian <<-\EOF
+ # Setup an "evil merge" topology where the rewrite range crosses
+ # a 2-parent merge whose first parent sits outside that range:
+ #
+ # side -- O (a=v0)
+ # \
+ # M (parent1=O, parent2=R, a=v0, s=top)
+ # /
+ # A (a=v0) -- R (a=v0) -- T (a=v0, s=top)
+ # |
+ # reword target
+ #
+ # The walk for `history reword A` excludes A and its ancestors,
+ # so O sits outside the rewrite range and is not the boundary
+ # either. Replaying M correctly requires that first parent to
+ # remain at O (preserve, not replant).
+ blob v0 line1 line2 line3
+ blob top "marker"
+ commit X side "X" v0=v0
+ commit O side "O" v0=v0
+ commit A main "A" from=X v0=v0
+ commit R main "R" v0=v0
+ commit M main "Merge side into main" from=O merge=R v0=v0 s=top
+ commit T main "T" v0=v0 s=top
+ EOF
+}
+
+# A descendant merge whose first parent sits outside the rewrite
+# range is a topology that any reasonable replay of merges has to
+# handle correctly: the first parent must be preserved verbatim,
+# while the in-range second parent is rewritten. Without that, the
+# replayed merge would silently graft itself onto a different
+# ancestry than the author chose, which is far worse than a loud
+# failure.
+test_expect_success 'merge whose first parent sits outside the rewrite range keeps that parent' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ build_with_boundary_other_than_onto &&
+
+ reword_to "AA" A &&
+
+ # The replayed M (now HEAD~) is still a 2-parent merge.
+ # Its first parent is the original O (preserved, outside
+ # the rewrite range), its second parent is the rewritten
+ # R. T was rebased on top of M, so HEAD = T.
+ git rev-list --parents -1 HEAD~ >parents &&
+ new_p1=$(awk "{print \$2}" parents) &&
+ new_p2=$(awk "{print \$3}" parents) &&
+
+ # First parent is preserved verbatim.
+ test_cmp_rev O $new_p1 &&
+
+ # Second parent is the rewritten R: a fresh commit whose
+ # subject is still "R" but whose OID differs from the
+ # original (because its parent A is now reworded).
+ echo R >expect &&
+ git log -1 --format=%s $new_p2 >actual &&
+ test_cmp expect actual &&
+ ! test_cmp_rev R $new_p2 &&
+
+ # T was rebased on top of the new M, and its tree still
+ # contains the s=top marker introduced in the original M.
+ echo "marker" >expect &&
+ git show HEAD:s >actual &&
+ test_cmp expect actual
+ )
+'
+
+build_function_rename () {
+ test-tool historian <<-\EOF
+ # Topic renames harry() -> hermione() (defs.h plus caller1). main
+ # adds caller2 calling harry(); the original merge M manually
+ # renames caller2 to hermione(). The "newer" base on a side branch
+ # contains caller2 AND a brand-new caller3 calling harry();
+ # replaying onto `newer` therefore introduces caller3 into the
+ # merged tree.
+ blob defs_harry "void harry(void);"
+ blob defs_hermione "void hermione(void);"
+ blob harry_call "harry();"
+ blob hermione_call "hermione();"
+ commit A main "A" defs.h=defs_harry caller1=harry_call
+ commit B topic "B (rename)" from=A defs.h=defs_hermione caller1=hermione_call
+ commit C main "C (caller2 calls harry)" defs.h=defs_harry caller1=harry_call caller2=harry_call
+ commit M main "Merge topic" merge=B defs.h=defs_hermione caller1=hermione_call caller2=hermione_call
+ commit NEW newer "newer base with caller3" from=A defs.h=defs_harry caller1=harry_call caller2=harry_call caller3=harry_call
+ EOF
+}
+
+# This case checks two things at once. First, the manual semantic
+# edit in M (renaming caller2) must be preserved when we replay onto
+# a different base; that is the case `git history` and `git replay`
+# need to handle correctly, even though nothing in the conflict
+# markers tells us about it. Second, a file that only enters the
+# tree via the rewritten parents (caller3, present on the `newer`
+# base) is _not_ renamed by the replay. The replay propagates the
+# textual diffs the user actually made in M; it does _not_ infer
+# the user's symbol-level intent ("rename every caller of harry").
+# This is a known and intentional limitation. Symbol-aware
+# refactoring is out of scope here, just as it is for plain rebase.
+test_expect_success 'preserves manual rename of pre-existing caller; does not extrapolate to new files' '
+ test_when_finished "rm -rf repo" &&
+ git init repo &&
+ (
+ cd repo &&
+ build_function_rename &&
+
+ # Replay (C, B, M) onto the newer base. A `main..M` style
+ # range across two unrelated branches is awkward; spin up a
+ # temp branch and use --advance.
+ git branch tmp main &&
+ git replay --ref-action=print --onto NEW A..tmp >result &&
+ new_tip=$(cut -f 3 -d " " result) &&
+
+ # defs.h and caller1 came from B (clean cherry-pick of the
+ # rename commit) and must reflect the rename.
+ echo "void hermione(void);" >expect &&
+ git show $new_tip:defs.h >actual &&
+ test_cmp expect actual &&
+
+ echo "hermione();" >expect &&
+ git show $new_tip:caller1 >actual &&
+ test_cmp expect actual &&
+
+ # caller2 existed in the original M; its manual rename to
+ # hermione() is the semantic edit the replay must preserve.
+ echo "hermione();" >expect &&
+ git show $new_tip:caller2 >actual &&
+ test_cmp expect actual &&
+
+ # caller3 only exists on the newer base, so it was brought
+ # in by N (the auto-merge of the rewritten parents). The
+ # replay has no way to know the user intended to rename
+ # every caller; caller3 keeps harry(). The resulting tree
+ # is therefore _not_ symbol-correct and needs a follow-up
+ # edit. This is the documented limitation.
+ echo "harry();" >expect &&
+ git show $new_tip:caller3 >actual &&
+ test_cmp expect actual
+ )
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH/RFC 3/5] history.adoc: describe merge-replay support and its limits
From: Johannes Schindelin via GitGitGadget @ 2026-05-06 22:43 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2106.git.1778107405.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Replace the blanket "does not (yet) work with histories that contain
merges" caveat now that 2-parent merges are supported via the R/O/N
algorithm. Spell out what works (the user's manual conflict
resolution and any semantic edits inside the merge are preserved
through the replay), what is intentionally out of scope (octopus
merges; symbol-level extrapolation when rewriting parents pulls in
genuinely new content), and what still requires interactive rebase
(merges that would actually conflict on replay).
Assisted-by: Claude Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Documentation/git-history.adoc | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc
index 24dc907033..56328a7c59 100644
--- a/Documentation/git-history.adoc
+++ b/Documentation/git-history.adoc
@@ -40,13 +40,26 @@ at once.
LIMITATIONS
-----------
-This command does not (yet) work with histories that contain merges. You
-should use linkgit:git-rebase[1] with the `--rebase-merges` flag instead.
-
-Furthermore, the command does not support operations that can result in merge
-conflicts. This limitation is by design as history rewrites are not intended to
-be stateful operations. The limitation can be lifted once (if) Git learns about
-first-class conflicts.
+This command supports two-parent merge commits in the rewrite path:
+the auto-remerged tree of the original parents, the merge commit
+itself, and the auto-merged tree of the rewritten parents are
+combined so that the user's manual conflict resolution (textual or
+semantic) is preserved through the replay. Octopus merges (more than
+two parents) are not supported and are rejected with an error.
+
+The replay propagates the textual diffs the user actually made in
+the merge commit. It does _not_ extrapolate symbol-level intent: if
+rewriting the parents pulls in genuinely new content (for example, a
+new caller of a function that the merge renamed), that new content
+is _not_ rewritten by the replay and may need a follow-up edit.
+Symbol-aware refactoring is out of scope here, just as it is for
+plain rebase.
+
+The command does not support operations that can result in merge
+conflicts on the replayed merge itself. This limitation is by design
+as history rewrites are not intended to be stateful operations. Use
+linkgit:git-rebase[1] with the `--rebase-merges` flag when the
+rewrite is expected to require interactive conflict resolution.
COMMANDS
--------
--
gitgitgadget
^ permalink raw reply related
* [PATCH/RFC 2/5] replay: short-circuit merge replay when parent and base trees are unchanged
From: Johannes Schindelin via GitGitGadget @ 2026-05-06 22:43 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2106.git.1778107405.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
For the common `git history reword` case the rewrite changes only
commit messages, so every commit on the line being replayed has the
same tree as before. When such a rewrite reaches a 2-parent merge
whose rewritten parents AND merge bases all carry the same trees as
the originals, the inner auto-merge of the rewritten parents (N) is
tree-equal to the inner auto-merge of the original parents (R), and
the outer 3-way merge with R as the merge base, the original merge
tree as side 1 and N as side 2 yields the original tree as result.
Detect this in `pick_merge_commit()` before doing any merge work and
write the new merge commit directly with the original tree and the
rewritten parents. This saves two recursive merges and one
non-recursive merge per merge commit on the rewrite path, which
dominates the cost of `git history reword` across histories with
many merges.
The merge-base trees must be checked too, in order. Tree-same
parents over a tree-different base could still produce a different
auto-merge (a conflict region that did not exist before, or vice
versa), and the original resolution would be inappropriate to apply.
To avoid recomputing the merge bases when the fast path does not
apply, both pairs are computed up front and the slow path that
follows reuses them.
Assisted-by: Claude Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
replay.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 58 insertions(+), 9 deletions(-)
diff --git a/replay.c b/replay.c
index 3dbce095f9..5dfdef1447 100644
--- a/replay.c
+++ b/replay.c
@@ -396,6 +396,64 @@ static struct commit *pick_merge_commit(struct repository *repo,
replayed_par1 = mapped_commit(replayed_commits, parent1, parent1);
replayed_par2 = mapped_commit(replayed_commits, parent2, parent2);
+ /*
+ * Compute both pairs of merge bases up front. The fast path below
+ * needs them for the tree-equality check, and the slow path that
+ * follows reuses them to avoid recomputing.
+ */
+ if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < 0 ||
+ repo_get_merge_bases(repo, replayed_par1, replayed_par2,
+ &replayed_bases) < 0) {
+ result->clean = -1;
+ goto out;
+ }
+
+ /*
+ * Fast path: when both rewritten parents carry the same trees as
+ * the originals AND every merge base does too (in order), the
+ * auto-merges R and N would be tree-equal (their inputs match
+ * content-wise), so the outer 3-way merge trivially yields the
+ * original merge's tree. Skip the inner merges and write the new
+ * merge commit directly.
+ *
+ * This is the common case for `git history reword`, which only
+ * changes commit messages and so leaves every tree on the line
+ * being replayed unchanged. The merge-base trees must be checked
+ * too: tree-same parents over a tree-different base could still
+ * produce a different auto-merge (a conflict region that did not
+ * exist before, or vice versa), and the original resolution would
+ * be inappropriate.
+ */
+ if (oideq(&repo_get_commit_tree(repo, parent1)->object.oid,
+ &repo_get_commit_tree(repo, replayed_par1)->object.oid) &&
+ oideq(&repo_get_commit_tree(repo, parent2)->object.oid,
+ &repo_get_commit_tree(repo, replayed_par2)->object.oid)) {
+ struct commit_list *bo, *bn;
+ int bases_match = 1;
+
+ for (bo = parent_bases, bn = replayed_bases;
+ bo && bn;
+ bo = bo->next, bn = bn->next) {
+ if (!oideq(&repo_get_commit_tree(repo, bo->item)->object.oid,
+ &repo_get_commit_tree(repo, bn->item)->object.oid)) {
+ bases_match = 0;
+ break;
+ }
+ }
+ if (bo || bn)
+ bases_match = 0;
+
+ if (bases_match) {
+ pickme_tree = repo_get_commit_tree(repo, pickme);
+ parents = NULL;
+ commit_list_insert(replayed_par2, &parents);
+ commit_list_insert(replayed_par1, &parents);
+ picked = create_commit(repo, pickme_tree, pickme,
+ parents, REPLAY_MODE_PICK);
+ goto out;
+ }
+ }
+
/*
* R: auto-remerge of the original parents.
*
@@ -408,10 +466,6 @@ static struct commit *pick_merge_commit(struct repository *repo,
remerge_opt.show_rename_progress = 0;
remerge_opt.branch1 = "ours";
remerge_opt.branch2 = "theirs";
- if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < 0) {
- result->clean = -1;
- goto out;
- }
merge_incore_recursive(&remerge_opt, parent_bases,
parent1, parent2, &remerge_res);
parent_bases = NULL; /* consumed by merge_incore_recursive */
@@ -425,11 +479,6 @@ static struct commit *pick_merge_commit(struct repository *repo,
new_merge_opt.show_rename_progress = 0;
new_merge_opt.branch1 = "ours";
new_merge_opt.branch2 = "theirs";
- if (repo_get_merge_bases(repo, replayed_par1, replayed_par2,
- &replayed_bases) < 0) {
- result->clean = -1;
- goto out;
- }
merge_incore_recursive(&new_merge_opt, replayed_bases,
replayed_par1, replayed_par2, &new_merge_res);
replayed_bases = NULL; /* consumed by merge_incore_recursive */
--
gitgitgadget
^ permalink raw reply related
* [PATCH/RFC 1/5] replay: support replaying 2-parent merges
From: Johannes Schindelin via GitGitGadget @ 2026-05-06 22:43 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Johannes Schindelin,
Johannes Schindelin
In-Reply-To: <pull.2106.git.1778107405.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
`git history` (introduced in v2.54) and the underlying `git replay`
infrastructure both refused to walk past any commit with more than
one parent, dying with "replaying merge commits is not supported
yet!". For real history-rewriting work this is a showstopper: the
natural fallback `git rebase --rebase-merges` is interactive and
stops to ask for re-resolution even when no re-resolution is needed.
Elijah Newren spelled out a way to lift this limitation in his
replay-design-notes [1] and prototyped it in a 2022
work-in-progress sketch [2]. The idea is that a merge commit M on
parents (P1, P2) records both an automatic merge of those parents
AND any manual layer the author put on top of that automatic merge
(textual conflict resolution and any semantic edit outside conflict
markers). Replaying M onto rewritten parents (P1', P2') must
preserve that manual layer, but the rewritten parents change the
automatic merge, so a simple cherry-pick is wrong: the manual layer
would be re-introduced on top of stale auto-merge text.
What works instead is a three-way merge of three trees the existing
infrastructure already knows how to compute. Let R be the recursive
auto-merge of (P1, P2), O be M's actual tree and N be the recursive
auto-merge of (P1', P2'). Then `git diff R O` is morally
`git show --remerge-diff M`: it captures exactly what the author
added on top of the automatic merge. A non-recursive 3-way merge
with R as the merge base, O as side 1 and N as side 2 layers that
manual contribution onto the freshly auto-merged rewritten parents
(N) and produces the replayed tree.
Implement `pick_merge_commit()` along those lines and dispatch to it
from `replay_revisions()` when the commit being replayed has exactly
two parents. Two specific points (learned the hard way) keep
non-trivial cases working where the WIP sketch [2] bailed out.
First, R and N use identical `merge_options.branch1` and `branch2`
labels ("ours"/"theirs"). When the original parents conflicted on a
region of a file, both R and N produce textually identical conflict
markers; the outer non-recursive merge then sees N == R in that
region and the user's manual resolution from O wins cleanly. Without
this, the conflict-marker text would differ between R and N (because
the inner merges would label the conflicts differently), and the
outer merge would itself be unclean even when the user did supply a
clean resolution. Second, an unclean inner merge
(`result.clean == 0`) is _not_ fatal: the tree merge-ort produces in
that case still has well-defined contents (with conflict markers in
the conflicted files) and is a valid input to the outer
non-recursive merge. Only a real error (`< 0`) propagates as
failure.
The replay propagates the textual diffs the user actually made in M;
it does _not_ extrapolate symbol-level intent. If rewriting the
parents pulls in genuinely new content (for example, a brand-new
caller of a function that the merge renamed), that new content stays
as the rewritten parents have it. Symbol-aware refactoring is out of
scope here, just as it is for plain rebase.
Octopus merges (more than two parents) and revert-of-merge are not
supported and are surfaced as explicit errors at the dispatch point.
The "split" sub-command of `git history` continues to refuse when
the targeted commit is itself a merge: split semantics do not apply
to merges. The pre-walk gate in `builtin/history.c` that previously
rejected any merge in the rewrite path now only rejects octopus
merges; rename it accordingly.
A small refactor in `create_commit()` makes the merge case possible:
the helper now takes a `struct commit_list *parents` rather than a
single parent pointer and takes ownership of the list. The single
existing caller in `pick_regular_commit()` builds and passes a
one-element list; the new `pick_merge_commit()` builds a two-element
list, with the order of the `from` and `merge` parents preserved.
Update the negative expectations in t3451, t3452 and t3650 that were
asserting the now-retired "not supported yet" message, replacing
them with positive coverage where it fits. Octopus rejection and
revert-of-merge rejection are covered by new positive tests in
t3650. A dedicated test script with merge-replay scenarios driven by
a new test-tool fixture builder will follow in a subsequent commit.
[1] https://github.com/newren/git/blob/replay/replay-design-notes.txt
[2] https://github.com/newren/git/commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45
Helped-by: Elijah Newren <newren@gmail.com>
Assisted-by: Claude Opus 4.7
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/history.c | 16 ++-
replay.c | 209 ++++++++++++++++++++++++++++++++++++--
t/t3451-history-reword.sh | 21 ++--
t/t3452-history-split.sh | 6 +-
t/t3650-replay-basics.sh | 46 ++++++++-
5 files changed, 269 insertions(+), 29 deletions(-)
diff --git a/builtin/history.c b/builtin/history.c
index 9526938085..00097b2226 100644
--- a/builtin/history.c
+++ b/builtin/history.c
@@ -195,15 +195,15 @@ static int parse_ref_action(const struct option *opt, const char *value, int uns
return 0;
}
-static int revwalk_contains_merges(struct repository *repo,
- const struct strvec *revwalk_args)
+static int revwalk_contains_octopus_merges(struct repository *repo,
+ const struct strvec *revwalk_args)
{
struct strvec args = STRVEC_INIT;
struct rev_info revs;
int ret;
strvec_pushv(&args, revwalk_args->v);
- strvec_push(&args, "--min-parents=2");
+ strvec_push(&args, "--min-parents=3");
repo_init_revisions(repo, &revs, NULL);
@@ -217,7 +217,7 @@ static int revwalk_contains_merges(struct repository *repo,
}
if (get_revision(&revs)) {
- ret = error(_("replaying merge commits is not supported yet!"));
+ ret = error(_("replaying octopus merges is not supported"));
goto out;
}
@@ -289,7 +289,7 @@ static int setup_revwalk(struct repository *repo,
strvec_push(&args, "HEAD");
}
- ret = revwalk_contains_merges(repo, &args);
+ ret = revwalk_contains_octopus_merges(repo, &args);
if (ret < 0)
goto out;
@@ -482,6 +482,9 @@ static int cmd_history_reword(int argc,
if (ret < 0) {
ret = error(_("failed replaying descendants"));
goto out;
+ } else if (ret) {
+ ret = error(_("conflict during replay; some descendants were not rewritten"));
+ goto out;
}
ret = 0;
@@ -721,6 +724,9 @@ static int cmd_history_split(int argc,
if (ret < 0) {
ret = error(_("failed replaying descendants"));
goto out;
+ } else if (ret) {
+ ret = error(_("conflict during replay; some descendants were not rewritten"));
+ goto out;
}
ret = 0;
diff --git a/replay.c b/replay.c
index f96f1f6551..3dbce095f9 100644
--- a/replay.c
+++ b/replay.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "git-compat-util.h"
+#include "commit-reach.h"
#include "environment.h"
#include "hex.h"
#include "merge-ort.h"
@@ -77,15 +78,21 @@ static void generate_revert_message(struct strbuf *msg,
repo_unuse_commit_buffer(repo, commit, message);
}
+/*
+ * Build a new commit with the given tree and parent list, copying author,
+ * extra headers and (for pick mode) the commit message from `based_on`.
+ *
+ * Takes ownership of `parents`: it will be freed before returning, even on
+ * error. Parent order is preserved as supplied by the caller.
+ */
static struct commit *create_commit(struct repository *repo,
struct tree *tree,
struct commit *based_on,
- struct commit *parent,
+ struct commit_list *parents,
enum replay_mode mode)
{
struct object_id ret;
struct object *obj = NULL;
- struct commit_list *parents = NULL;
char *author = NULL;
char *sign_commit = NULL; /* FIXME: cli users might want to sign again */
struct commit_extra_header *extra = NULL;
@@ -96,7 +103,6 @@ static struct commit *create_commit(struct repository *repo,
const char *orig_message = NULL;
const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL };
- commit_list_insert(parent, &parents);
extra = read_commit_extra_headers(based_on, exclude_gpgsig);
if (mode == REPLAY_MODE_REVERT) {
generate_revert_message(&msg, based_on, repo);
@@ -273,6 +279,7 @@ static struct commit *pick_regular_commit(struct repository *repo,
{
struct commit *base, *replayed_base;
struct tree *pickme_tree, *base_tree, *replayed_base_tree;
+ struct commit_list *parents = NULL;
if (pickme->parents) {
base = pickme->parents->item;
@@ -327,7 +334,143 @@ static struct commit *pick_regular_commit(struct repository *repo,
if (oideq(&replayed_base_tree->object.oid, &result->tree->object.oid) &&
!oideq(&pickme_tree->object.oid, &base_tree->object.oid))
return replayed_base;
- return create_commit(repo, result->tree, pickme, replayed_base, mode);
+ commit_list_insert(replayed_base, &parents);
+ return create_commit(repo, result->tree, pickme, parents, mode);
+}
+
+/*
+ * Replay a 2-parent merge commit by composing three calls into merge-ort:
+ *
+ * R = recursive merge of pickme's two original parents (auto-remerge of
+ * the original merge, accepting any conflicts)
+ * N = recursive merge of the (possibly rewritten) parents
+ * O = pickme's tree (the user's actual merge, including any manual
+ * resolutions)
+ *
+ * The picked tree comes from a non-recursive merge using R as the base,
+ * O as side1 and N as side2. `git diff R O` is morally `git show
+ * --remerge-diff $oldmerge`, so this layers the user's original manual
+ * resolution on top of the freshly auto-merged rewritten parents (see
+ * `replay-design-notes.txt` on the `replay` branch of newren/git).
+ *
+ * If the outer 3-way merge is unclean, propagate the conflict status to
+ * the caller via `result->clean = 0` and return NULL. The two inner
+ * merges (R and N) being unclean is _not_ fatal: the conflict-markered
+ * trees they produce are valid inputs to the outer merge, and using
+ * identical labels for both inner merges keeps the marker text
+ * byte-equal between R and N so the user's resolution recorded in O
+ * collapses the conflict cleanly there. Octopus merges (more than two
+ * parents) and revert-of-merge are rejected by the caller before this
+ * function is invoked.
+ */
+static struct commit *pick_merge_commit(struct repository *repo,
+ struct commit *pickme,
+ kh_oid_map_t *replayed_commits,
+ struct merge_options *merge_opt,
+ struct merge_result *result)
+{
+ struct commit *parent1, *parent2;
+ struct commit *replayed_par1, *replayed_par2;
+ struct tree *pickme_tree;
+ struct merge_options remerge_opt = { 0 };
+ struct merge_options new_merge_opt = { 0 };
+ struct merge_result remerge_res = { 0 };
+ struct merge_result new_merge_res = { 0 };
+ struct commit_list *parent_bases = NULL;
+ struct commit_list *replayed_bases = NULL;
+ struct commit_list *parents;
+ struct commit *picked = NULL;
+ char *ancestor_name = NULL;
+
+ parent1 = pickme->parents->item;
+ parent2 = pickme->parents->next->item;
+
+ /*
+ * Map the merge's parents to their replayed counterparts. With the
+ * boundary commits pre-seeded into `replayed_commits`, every parent
+ * either has an explicit mapping (rewritten or boundary -> onto) or
+ * sits outside the rewrite range entirely; the latter must stay at
+ * the original parent commit, so use `parent` itself as the fallback
+ * for both sides.
+ */
+ replayed_par1 = mapped_commit(replayed_commits, parent1, parent1);
+ replayed_par2 = mapped_commit(replayed_commits, parent2, parent2);
+
+ /*
+ * R: auto-remerge of the original parents.
+ *
+ * Use the same branch labels for the inner merges that compute R
+ * and N so conflict markers (if any) are textually identical
+ * between the two; the outer non-recursive merge can then collapse
+ * the manual resolution from O against them.
+ */
+ init_basic_merge_options(&remerge_opt, repo);
+ remerge_opt.show_rename_progress = 0;
+ remerge_opt.branch1 = "ours";
+ remerge_opt.branch2 = "theirs";
+ if (repo_get_merge_bases(repo, parent1, parent2, &parent_bases) < 0) {
+ result->clean = -1;
+ goto out;
+ }
+ merge_incore_recursive(&remerge_opt, parent_bases,
+ parent1, parent2, &remerge_res);
+ parent_bases = NULL; /* consumed by merge_incore_recursive */
+ if (remerge_res.clean < 0) {
+ result->clean = remerge_res.clean;
+ goto out;
+ }
+
+ /* N: fresh merge of the (possibly rewritten) parents. */
+ init_basic_merge_options(&new_merge_opt, repo);
+ new_merge_opt.show_rename_progress = 0;
+ new_merge_opt.branch1 = "ours";
+ new_merge_opt.branch2 = "theirs";
+ if (repo_get_merge_bases(repo, replayed_par1, replayed_par2,
+ &replayed_bases) < 0) {
+ result->clean = -1;
+ goto out;
+ }
+ merge_incore_recursive(&new_merge_opt, replayed_bases,
+ replayed_par1, replayed_par2, &new_merge_res);
+ replayed_bases = NULL; /* consumed by merge_incore_recursive */
+ if (new_merge_res.clean < 0) {
+ result->clean = new_merge_res.clean;
+ goto out;
+ }
+
+ /*
+ * Outer non-recursive merge: base=R, side1=O (pickme), side2=N.
+ */
+ pickme_tree = repo_get_commit_tree(repo, pickme);
+ ancestor_name = xstrfmt("auto-remerge of %s",
+ oid_to_hex(&pickme->object.oid));
+ merge_opt->ancestor = ancestor_name;
+ merge_opt->branch1 = short_commit_name(repo, pickme);
+ merge_opt->branch2 = "merge of replayed parents";
+ merge_incore_nonrecursive(merge_opt,
+ remerge_res.tree,
+ pickme_tree,
+ new_merge_res.tree,
+ result);
+ merge_opt->ancestor = NULL;
+ merge_opt->branch1 = NULL;
+ merge_opt->branch2 = NULL;
+ if (!result->clean)
+ goto out;
+
+ parents = NULL;
+ commit_list_insert(replayed_par2, &parents);
+ commit_list_insert(replayed_par1, &parents);
+ picked = create_commit(repo, result->tree, pickme, parents,
+ REPLAY_MODE_PICK);
+
+out:
+ free(ancestor_name);
+ free_commit_list(parent_bases);
+ free_commit_list(replayed_bases);
+ merge_finalize(&remerge_opt, &remerge_res);
+ merge_finalize(&new_merge_opt, &new_merge_res);
+ return picked;
}
void replay_result_release(struct replay_result *result)
@@ -407,17 +550,63 @@ int replay_revisions(struct rev_info *revs,
merge_opt.show_rename_progress = 0;
last_commit = onto;
replayed_commits = kh_init_oid_map();
+
+ /*
+ * Seed the rewritten-commit map with each negative-side ("BOTTOM")
+ * cmdline entry pointing at `onto`. This matters for merge replay:
+ * a 2-parent merge whose first parent is the boundary (e.g. the
+ * commit being reworded) must replay onto the rewritten boundary,
+ * yet pick_merge_commit uses a self fallback so the second parent
+ * (a side branch outside the rewrite range) is preserved as-is.
+ * Pre-seeding the boundary disambiguates the two: in the map ->
+ * rewritten, missing -> kept as-is.
+ *
+ * Only do this for the pick path; revert mode chains reverts
+ * through last_commit and a pre-seeded boundary would short-circuit
+ * that chain.
+ */
+ if (mode == REPLAY_MODE_PICK) {
+ for (size_t i = 0; i < revs->cmdline.nr; i++) {
+ struct rev_cmdline_entry *e = &revs->cmdline.rev[i];
+ struct commit *boundary;
+ khint_t pos;
+ int hr;
+
+ if (!(e->flags & BOTTOM))
+ continue;
+ boundary = lookup_commit_reference_gently(revs->repo,
+ &e->item->oid, 1);
+ if (!boundary)
+ continue;
+ pos = kh_put_oid_map(replayed_commits,
+ boundary->object.oid, &hr);
+ if (hr != 0)
+ kh_value(replayed_commits, pos) = onto;
+ }
+ }
+
while ((commit = get_revision(revs))) {
const struct name_decoration *decoration;
khint_t pos;
int hr;
- if (commit->parents && commit->parents->next)
- die(_("replaying merge commits is not supported yet!"));
-
- last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
- mode == REPLAY_MODE_REVERT ? last_commit : onto,
- &merge_opt, &result, mode);
+ if (commit->parents && commit->parents->next) {
+ if (commit->parents->next->next) {
+ ret = error(_("replaying octopus merges is not supported"));
+ goto out;
+ }
+ if (mode == REPLAY_MODE_REVERT) {
+ ret = error(_("reverting merge commits is not supported"));
+ goto out;
+ }
+ last_commit = pick_merge_commit(revs->repo, commit,
+ replayed_commits,
+ &merge_opt, &result);
+ } else {
+ last_commit = pick_regular_commit(revs->repo, commit, replayed_commits,
+ mode == REPLAY_MODE_REVERT ? last_commit : onto,
+ &merge_opt, &result, mode);
+ }
if (!last_commit)
break;
diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh
index de7b357685..d103f866a2 100755
--- a/t/t3451-history-reword.sh
+++ b/t/t3451-history-reword.sh
@@ -201,12 +201,21 @@ test_expect_success 'can reword a merge commit' '
git switch - &&
git merge theirs &&
- # It is not possible to replay merge commits embedded in the
- # history (yet).
- test_must_fail git -c core.editor=false history reword HEAD~ 2>err &&
- test_grep "replaying merge commits is not supported yet" err &&
+ # Reword a non-merge commit whose descendants include the
+ # merge: replay carries the merge through.
+ reword_with_message HEAD~ <<-EOF &&
+ ours reworded
+ EOF
+ expect_graph <<-EOF &&
+ * Merge tag ${SQ}theirs${SQ}
+ |\\
+ | * theirs
+ * | ours reworded
+ |/
+ * base
+ EOF
- # But it is possible to reword a merge commit directly.
+ # And reword a merge commit directly.
reword_with_message HEAD <<-EOF &&
Reworded merge commit
EOF
@@ -214,7 +223,7 @@ test_expect_success 'can reword a merge commit' '
* Reworded merge commit
|\
| * theirs
- * | ours
+ * | ours reworded
|/
* base
EOF
diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh
index 8ed0cebb50..ad6309f98b 100755
--- a/t/t3452-history-split.sh
+++ b/t/t3452-history-split.sh
@@ -36,7 +36,7 @@ expect_tree_entries () {
test_cmp expect actual
}
-test_expect_success 'refuses to work with merge commits' '
+test_expect_success 'refuses to split a merge commit' '
test_when_finished "rm -rf repo" &&
git init repo &&
(
@@ -49,9 +49,7 @@ test_expect_success 'refuses to work with merge commits' '
git switch - &&
git merge theirs &&
test_must_fail git history split HEAD 2>err &&
- test_grep "cannot split up merge commit" err &&
- test_must_fail git history split HEAD~ 2>err &&
- test_grep "replaying merge commits is not supported yet" err
+ test_grep "cannot split up merge commit" err
)
'
diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh
index 3353bc4a4d..368b1b0f9a 100755
--- a/t/t3650-replay-basics.sh
+++ b/t/t3650-replay-basics.sh
@@ -103,10 +103,48 @@ test_expect_success 'cannot advance target ... ordering would be ill-defined' '
test_cmp expect actual
'
-test_expect_success 'replaying merge commits is not supported yet' '
- echo "fatal: replaying merge commits is not supported yet!" >expect &&
- test_must_fail git replay --advance=main main..topic-with-merge 2>actual &&
- test_cmp expect actual
+test_expect_success 'using replay to rebase a 2-parent merge' '
+ # main..topic-with-merge contains a 2-parent merge (P) introduced
+ # via test_merge. Use --ref-action=print so this test does not
+ # mutate state for subsequent tests in this file.
+ git replay --ref-action=print --onto main main..topic-with-merge >result &&
+ test_line_count = 1 result &&
+
+ new_tip=$(cut -f 3 -d " " result) &&
+
+ # Result is still a 2-parent merge.
+ git cat-file -p $new_tip >cat &&
+ grep -c "^parent " cat >count &&
+ echo 2 >expect &&
+ test_cmp expect count &&
+
+ # Merge subject is preserved.
+ echo P >expect &&
+ git log -1 --format=%s $new_tip >actual &&
+ test_cmp expect actual &&
+
+ # The replayed merge sits on top of main: walking back via the
+ # first-parent chain reaches main.
+ git merge-base --is-ancestor main $new_tip
+'
+
+test_expect_success 'replaying an octopus merge is rejected' '
+ # Build an octopus side-branch so the rest of the test state stays
+ # untouched.
+ test_when_finished "git update-ref -d refs/heads/octopus-tip" &&
+ octopus_tip=$(git commit-tree -p topic4 -p topic1 -p topic3 \
+ -m "octopus" $(git rev-parse topic4^{tree})) &&
+ git update-ref refs/heads/octopus-tip "$octopus_tip" &&
+
+ test_must_fail git replay --ref-action=print --onto main \
+ topic4..octopus-tip 2>actual &&
+ test_grep "octopus merges" actual
+'
+
+test_expect_success 'reverting a merge commit is rejected' '
+ test_must_fail git replay --ref-action=print --revert=topic-with-merge \
+ topic4..topic-with-merge 2>actual &&
+ test_grep "reverting merge commits" actual
'
test_expect_success 'using replay to rebase two branches, one on top of other' '
--
gitgitgadget
^ permalink raw reply related
* [PATCH/RFC 0/5] replay: support replaying 2-parent merges
From: Johannes Schindelin via GitGitGadget @ 2026-05-06 22:43 UTC (permalink / raw)
To: git; +Cc: Elijah Newren, Patrick Steinhardt, Johannes Schindelin
git history, the new history-rewriting builtin in v2.54, dies on any merge
in the rewrite path with replaying merge commits is not supported yet!. That
makes it not very useful for the workflows I actually have, where almost
every interesting branch contains at least one merge of a feature topic. The
natural fallback, git rebase --rebase-merges, is interactive and stops to
ask for re-resolution even when no re-resolution is needed.
This series lifts that limitation for the common 2-parent case. The
algorithm itself is not new: Elijah Newren wrote it down in his replay
design notes
[https://github.com/newren/git/blob/replay/replay-design-notes.txt] and
prototyped it in a 2022 work-in-progress sketch
[https://github.com/newren/git/commit/4c45e8955ef9bf7d01fd15d9106b3bdb8ea91b45].
What is new is wiring it into the replay_revisions() API that backs both git
replay and git history, plus three specific tweaks that make the trickier
cases work where the WIP sketch bailed out: identical conflict-marker labels
for the inner remerges of the original and the rewritten parents (so their
conflict-markered trees compare equal in the regions the user did not
touch), tolerating result.clean == 0 from those inner merges (their
well-defined conflict-markered trees are valid inputs to the outer 3-way
merge), and self-fallback for both merge parents combined with mapping the
rev-range boundary commits to the onto commit.
Octopus merges and revert-of-merge are surfaced as explicit errors at the
dispatch point. The split sub-command of git history continues to refuse
when its target is a merge: split semantics simply do not apply there. The
xdiff special mode for matching conflict-marker hunks across inner remerges,
the XDL_MERGE_FAVOR_BASE variant, and the modify/delete and binary-file
specials that the design notes flag as future work all remain future work.
While I was at it, git history reword had a pre-existing silent-success bug:
a positive return from replay_revisions() (which means "conflict, no updates
queued") was treated as success. Obviously this should never occur, as a
reword simply does not change any of the file contents, but bugs do happen.
The merge-replay work is complex enough to make that class of bugs more
likely, therefore I introduce error messages for those instances.
Johannes Schindelin (5):
replay: support replaying 2-parent merges
replay: short-circuit merge replay when parent and base trees are
unchanged
history.adoc: describe merge-replay support and its limits
test-tool: add a "historian" subcommand for building merge fixtures
t3454: cover merge-replay scenarios with the historian helper
Documentation/git-history.adoc | 27 ++-
Makefile | 1 +
builtin/history.c | 16 +-
replay.c | 258 +++++++++++++++++++++++++--
t/helper/meson.build | 1 +
t/helper/test-historian.c | 189 ++++++++++++++++++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/meson.build | 1 +
t/t3451-history-reword.sh | 21 ++-
t/t3452-history-split.sh | 6 +-
t/t3454-history-merges.sh | 308 +++++++++++++++++++++++++++++++++
t/t3650-replay-basics.sh | 46 ++++-
13 files changed, 840 insertions(+), 36 deletions(-)
create mode 100644 t/helper/test-historian.c
create mode 100755 t/t3454-history-merges.sh
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2106%2Fdscho%2Fsupport-merge-commits-in-git-history-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2106/dscho/support-merge-commits-in-git-history-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2106
--
gitgitgadget
^ permalink raw reply
* Re:git 2.54 fails to build for rhel 6 (2.53 builds fine)
From: Азат Усманов @ 2026-05-06 22:28 UTC (permalink / raw)
To: brian m. carlson; +Cc: git
In-Reply-To: <affprsBwpM7L47j6@fruit.crustytoothpaste.net>
any chance someone could show what commit causing the issue, since 2.53 builds ok. Ive looked through the changelog for 2.54 didn't see anythihg specific that migh cause this.
^ permalink raw reply
* [PATCH 1/1] shallow: fix relative deepen on non-shallow repositories
From: Samo Pogačnik @ 2026-05-06 21:56 UTC (permalink / raw)
To: git; +Cc: l.s.r, owen, Samo Pogačnik, Junio C Hamano
In-Reply-To: <e8257951-4ea7-40ba-8043-f4f2a080b70b@web.de>
The previous patch "shallow: handling fetch relative-deepen"
introduced a bug where using --deepen=<n> on a non-shallow
repository incorrectly treated the value as an absolute depth,
resulting in a shallow fetch and truncated history.
This patch prevents any modification when a relative deepen is
requested on a non-shallow repository.
A test is added to ensure that history is not changed when
--deepen is used on a non-shallow repository.
Reported-by: Owen Stephens <owen@owenstephens.co.uk>
Signed-off-by: Samo Pogačnik <samo_pogacnik@t-2.net>
---
shallow.c | 6 +++++-
t/t5537-fetch-shallow.sh | 10 ++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/shallow.c b/shallow.c
index a8ad92e303..610ff3d13b 100644
--- a/shallow.c
+++ b/shallow.c
@@ -245,7 +245,11 @@ struct commit_list *get_shallow_commits(struct object_array *heads,
int depth, int shallow_flag, int not_shallow_flag)
{
if (shallows && deepen_relative) {
- depth += get_shallows_depth(heads, shallows);
+ int cur_shallow_depth = get_shallows_depth(heads, shallows);
+ if (cur_shallow_depth)
+ depth += cur_shallow_depth;
+ else
+ return NULL;
}
return get_shallows_or_depth(heads, NULL, NULL,
depth, shallow_flag, not_shallow_flag);
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
index 6588ce6226..9982dd2aa6 100755
--- a/t/t5537-fetch-shallow.sh
+++ b/t/t5537-fetch-shallow.sh
@@ -251,6 +251,16 @@ test_expect_success '.git/shallow is edited by repack' '
origin "+refs/heads/*:refs/remotes/origin/*"
'
+test_expect_success 'fetch --deepen does not truncate' '
+ git clone --no-local .git full-clone &&
+ git -C full-clone rev-parse --is-shallow-repository >expect &&
+ git -C full-clone log --oneline >>expect &&
+ git -C full-clone fetch --deepen=1 &&
+ git -C full-clone rev-parse --is-shallow-repository >actual &&
+ git -C full-clone log --oneline >>actual &&
+ test_cmp expect actual
+'
+
. "$TEST_DIRECTORY"/lib-httpd.sh
start_httpd
--
2.43.0
^ permalink raw reply related
* [PATCH v6 3/3] git-gui: handle GIT_DIR and GIT_WORK_TREE early
From: Shroom Moo @ 2026-05-06 20:27 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Mark Levedahl, Shroom Moo
In-Reply-To: <20260506202751.3294-1-egg_mushroomcow@foxmail.com>
Users expect these two invocations to be equivalent:
GIT_WORK_TREE=/some/path GIT_DIR=/some/path/.git git gui
git -C /some/path gui
Currently, the environment variable variant often brings up the
repository picker or ignores the requested worktree because
GIT_WORK_TREE is processed too late. Moreover, after determining
the working tree, git-gui unconditionally exports GIT_WORK_TREE.
When no worktree is found (e.g., in a bare repository with a
read-only subcommand like blame), an empty value is exported, which
confuses commands like `git branch --show-current`.
Fix both issues:
- Introduce resolve_initial_environment, which is called before any
repository detection. When both GIT_DIR and GIT_WORK_TREE are
set, it changes to the specified worktree, verifies that it is a
valid worktree, and adopts the given GIT_DIR. In this case, the
subsequent automatic detection is skipped. After successful
validation, GIT_WORK_TREE is unset so that later steps do not
inherit it.
- At the end of startup, only export GIT_DIR. The current directory
is already at the top of the worktree (when a worktree exists), so
Git can discover the worktree automatically. Not exporting an
empty GIT_WORK_TREE fixes `git gui blame` in bare repositories.
When only GIT_DIR is set without GIT_WORK_TREE, the existing logic
remains unchanged for backward compatibility. Setting only
GIT_WORK_TREE is not a valid Git use-case and is silently ignored.
Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
git-gui/git-gui.sh | 84 ++++++++++++++++++++++++++++++++++++----------
1 file changed, 66 insertions(+), 18 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 1191e6654c..ca651dc576 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1122,25 +1122,69 @@ unset argv0dir
##
## repository setup
+proc resolve_initial_environment {} {
+ global _gitdir env
+
+ # Only act if both GIT_DIR and GIT_WORK_TREE are set
+ if {[info exists env(GIT_DIR)] && [info exists env(GIT_WORK_TREE)]} {
+ # Validate GIT_DIR by resolving its absolute path
+ if {[catch {set _abs [git rev-parse --absolute-git-dir]} err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Invalid GIT_DIR:"] "\n\n$err"]
+ exit 1
+ }
+
+ # Change current directory to GIT_WORK_TREE
+ if {[catch {cd $env(GIT_WORK_TREE)} err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Cannot change to GIT_WORK_TREE:"] "\n\n$err"]
+ exit 1
+ }
+
+ # Verify that GIT_WORK_TREE is a valid Git worktree
+ if {[catch {git rev-parse --show-toplevel} err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "GIT_WORK_TREE is not a valid worktree:"] "\n\n$err"]
+ exit 1
+ }
+
+ # Use repository path specified by environment variables
+ set _gitdir $_abs
+ set ::_prefix {}
+
+ # Unset GIT_WORK_TREE to prevent it from being inherited by child processes
+ unset env(GIT_WORK_TREE)
+
+ return 1
+ }
+
+ # Other cases (only GIT_DIR, only GIT_WORK_TREE, or neither):
+ # Do nothing here and let the existing logic handle it later
+ return 0
+}
+set force_gitdir [resolve_initial_environment]
+
set picked 0
-if {[catch {
- set _gitdir $env(GIT_DIR)
- set _prefix {}
- }]
- && [catch {
- # beware that from the .git dir this sets _gitdir to .
- # and _prefix to the empty string; this is handled by
- # the startup safety checks below
- set _gitdir [git rev-parse --git-dir]
- set _prefix [git rev-parse --show-prefix]
- } err]} {
- load_config 1
- apply_config
- choose_repository::pick
- if {![file isdirectory $_gitdir]} {
- exit 1
+if {!$force_gitdir} {
+ if {[catch {
+ set _gitdir $env(GIT_DIR)
+ set _prefix {}
+ }]
+ && [catch {
+ # beware that from the .git dir this sets _gitdir to .
+ # and _prefix to the empty string; this is handled by
+ # the startup safety checks below
+ set _gitdir [git rev-parse --git-dir]
+ set _prefix [git rev-parse --show-prefix]
+ } err]} {
+ load_config 1
+ apply_config
+ choose_repository::pick
+ if {![file isdirectory $_gitdir]} {
+ exit 1
+ }
+ set picked 1
}
- set picked 1
}
if {![file isdirectory $_gitdir]} {
@@ -1228,7 +1272,11 @@ if {[lindex $_reponame end] eq {.git}} {
# Export the final paths
set env(GIT_DIR) $_gitdir
-set env(GIT_WORK_TREE) $_gitworktree
+# Do not export `GIT_WORK_TREE`. The current directory is already
+# the top-level of the working tree (if it exists), and Git will
+# automatically identify the working tree. For bare repositories,
+# `_gitworktree` is empty, and exporting an empty value would cause
+# commands like `git branch --show-current` to fail
######################################################################
##
--
2.52.0.windows.1
^ permalink raw reply related
* [PATCH v6 1/3] git-gui: restructure repository startup
From: Shroom Moo @ 2026-05-06 20:27 UTC (permalink / raw)
To: git; +Cc: Johannes Sixt, Mark Levedahl, Shroom Moo
In-Reply-To: <20260506202751.3294-1-egg_mushroomcow@foxmail.com>
When git-gui is started inside a .git directory of a non-bare
repository, it should treat the parent directory as the worktree,
as it did before commit 2d92ab32fd (rev-parse: make --show-toplevel
without a worktree an error, 2019-11-19). However, a bare repository
or a separated gitdir without a worktree must be rejected early.
Protect the previously unguarded calls to `git rev-parse
--show-object-format` and `--show-toplevel`. Restructure the startup
sequence to:
- Check for a bare repository right after loading the config. If the
repository is bare and the current subcommand does not allow bare
repos (e.g. normal commit mode), show "Cannot use bare repository"
and exit.
- When `rev-parse --show-toplevel` fails and the repository is
non-bare, the gitdir path ends with ".git", and we are inside that
gitdir, use the parent directory as the worktree. This preserves
the ability to start git-gui from within a regular repository’s
.git directory, which was intentionally supported since 87cd09f43e56
(git-gui: work from the .git dir, 2010-01-23).
- Otherwise, show a descriptive error and exit.
- Wrap `rev-parse --show-object-format` in a catch to avoid a crash
when the repository configuration is broken (e.g. core.worktree
pointing to an invalid path).
Also removes the old `_prefix`‑based fallback that computed a relative
path to the worktree top from a subdirectory, and the unconditional
`[file dirname $_gitdir]` guess. Both are unnecessary now that
`rev‑parse --show‑toplevel` directly provides the absolute top‑level
path and we can `cd` to it. The guess is further unsafe in
multi‑worktree setups, where a gitdir may have more than one worktree.
The only remaining fallback is the explicit “.git directory” rule for
non‑bare repositories, which mirrors the historical behaviour.
This fixes the fatal Tcl error when the working tree is missing, while
keeping the .git startup feature and avoiding any automatic directory
switching that could be dangerous in multi‑worktree setups.
Helped-by: Johannes Sixt <j6t@kdbg.org>
Helped-by: Mark Levedahl <mlevedahl@gmail.com>
Signed-off-by: Shroom Moo <egg_mushroomcow@foxmail.com>
---
git-gui/git-gui.sh | 72 +++++++++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 23fe76e498..fbdc0b2a41 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1129,7 +1129,8 @@ if {[catch {
}]
&& [catch {
# beware that from the .git dir this sets _gitdir to .
- # and _prefix to the empty string
+ # and _prefix to the empty string; this is handled by
+ # the startup safety checks below
set _gitdir [git rev-parse --git-dir]
set _prefix [git rev-parse --show-prefix]
} err]} {
@@ -1142,8 +1143,20 @@ if {[catch {
set picked 1
}
+if {![file isdirectory $_gitdir]} {
+ catch {wm withdraw .}
+ error_popup [strcat
+ [mc "Git directory not found:"] "\n\n$_gitdir\n\n" \
+ [mc "Please ensure GIT_DIR points to a valid Git repository"]]
+ exit 1
+}
+
# Use object format as hash algorithm (either "sha1" or "sha256")
-set hashalgorithm [git rev-parse --show-object-format]
+if {[catch {set hashalgorithm [git rev-parse --show-object-format]} err]} {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Failed to determine hash algorithm:"] "\n\n$err"]
+ exit 1
+}
if {$hashalgorithm eq "sha1"} {
set hashlength 40
} elseif {$hashalgorithm eq "sha256"} {
@@ -1160,46 +1173,52 @@ if {$_gitdir eq "."} {
set _gitdir [pwd]
}
-if {![file isdirectory $_gitdir]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
- exit 1
-}
# _gitdir exists, so try loading the config
load_config 0
apply_config
-set _gitworktree [git rev-parse --show-toplevel]
-
-if {$_prefix ne {}} {
- if {$_gitworktree eq {}} {
- regsub -all {[^/]+/} $_prefix ../ cdup
- } else {
- set cdup $_gitworktree
- }
- if {[catch {cd $cdup} err]} {
+# Handle bare repository and determine working tree
+if {[is_bare]} {
+ # Bare repository: only allowed for certain subcommands
+ if {![is_enabled bare]} {
catch {wm withdraw .}
- error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
+ error_popup [strcat [mc "Cannot use bare repository:"] "\n\n" [file normalize $_gitdir]]
exit 1
}
- set _gitworktree [pwd]
- unset cdup
-} elseif {![is_enabled bare]} {
- if {[is_bare]} {
- catch {wm withdraw .}
- error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
- exit 1
+ # Allowed bare repo does not have a worktree
+ set _gitworktree {}
+} else {
+ # Non-bare repository: we must find a worktree
+ if {[catch {set _gitworktree [git rev-parse --show-toplevel]} err]} {
+ # The only acceptable failure is when we are inside
+ # the .git directory of a regular repository.
+ set inside_gitdir 0
+ catch {set inside_gitdir [git rev-parse --is-inside-git-dir]}
+ if {$inside_gitdir eq {true} && [file tail $_gitdir] eq {.git}} {
+ # Use the parent directory as worktree (historic behavior)
+ set _gitworktree [file normalize [file dirname $_gitdir]]
+ } else {
+ catch {wm withdraw .}
+ error_popup [strcat [mc "Cannot determine working tree:"] "\n\n$err"]
+ exit 1
+ }
}
+
if {$_gitworktree eq {}} {
- set _gitworktree [file dirname $_gitdir]
+ catch {wm withdraw .}
+ error_popup [mc "Cannot determine working tree (unexpected empty result)"]
+ exit 1
}
+
if {[catch {cd $_gitworktree} err]} {
catch {wm withdraw .}
- error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
+ error_popup [strcat [mc "Cannot move to working directory:"] "\n\n$err"]
exit 1
}
set _gitworktree [pwd]
}
+
+# Derive a human-readable repository name
set _reponame [file split [file normalize $_gitdir]]
if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end-1]
@@ -1207,6 +1226,7 @@ if {[lindex $_reponame end] eq {.git}} {
set _reponame [lindex $_reponame end]
}
+# Export the final paths
set env(GIT_DIR) $_gitdir
set env(GIT_WORK_TREE) $_gitworktree
--
2.52.0.windows.1
^ 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