public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
* git-am applies commit message diffs
@ 2026-02-06  7:43 Matthias Beyer
  2026-02-06  8:04 ` Jacob Keller
                   ` (3 more replies)
  0 siblings, 4 replies; 65+ messages in thread
From: Matthias Beyer @ 2026-02-06  7:43 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

Hi,

I am not sure whether this was already reported, searching the lore did
not yield anything for me, but I might have overlooked it...

This was just posted on mastodon[0]:

    PSA: Did you know that it’s **unsafe** to put code diffs into your commit messages?

    Like https://
    github.com/i3/i3/pull/6564 for example

    Such diffs will be applied by patch(1) (also git-am(1)) as part of the code change!

    This is how a sleep(1) made it into i3 4.25-2 in Debian unstable.

TL;DR: If you put a diff in the commit message, that diff will be
applied by git-am.

This looks clearly like unintended and might be an attack-vector, right?

Best,
Matthias

[0]: https://mas.to/@zekjur/116022397626943871

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  7:43 git-am applies commit message diffs Matthias Beyer
@ 2026-02-06  8:04 ` Jacob Keller
  2026-02-06  8:18   ` Matthias Beyer
  2026-02-06  8:59   ` git-am applies commit message diffs Florian Weimer
  2026-02-06  8:43 ` Kristoffer Haugsbakk
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 65+ messages in thread
From: Jacob Keller @ 2026-02-06  8:04 UTC (permalink / raw)
  To: Matthias Beyer; +Cc: git

On Thu, Feb 5, 2026 at 11:50 PM Matthias Beyer <mail@beyermatthias.de> wrote:
>
> Hi,
>
> I am not sure whether this was already reported, searching the lore did
> not yield anything for me, but I might have overlooked it...
>
> This was just posted on mastodon[0]:
>
>     PSA: Did you know that it’s **unsafe** to put code diffs into your commit messages?
>
>     Like https://
>     github.com/i3/i3/pull/6564 for example
>
>     Such diffs will be applied by patch(1) (also git-am(1)) as part of the code change!
>
>     This is how a sleep(1) made it into i3 4.25-2 in Debian unstable.
>
> TL;DR: If you put a diff in the commit message, that diff will be
> applied by git-am.
>
> This looks clearly like unintended and might be an attack-vector, right?
>

It is certainly surprising. I am not certain I would consider it an
attack-vector since you should definitely be reading the commit
messages before applying, but I could see the fact that its
unintentional is a problem.

I'm surprised patch would apply since it would likely fail due to
other non-patch formatted text, no? I suspect this is something that
could be handled by using the scissors marker  "-- >8 --" in the patch
description to indicate the diff is not part of the patch, or perhaps
the splitting of the email should somehow indicate this, for example
when formatting a patch with a diff in it.

I checked by formatting a patch from my own commit message with an
embedded diff, and there is nothing in place to prevent that diff
section from being applied. In practice, I think the advice is "don't
put diffs in your commit message" or "indent the diff text so that it
won't  be parsed as a diff hunk by patch or am."

It seems like a good idea to me to improve the format patch output and
the git am patch splitting to somehow try and detect the end of a
valid commit message and not treat it as a patch content, but I am
really uncertain how to go about doing so safely without risking
backwards compatibility (modifying format-patch to insert a marker
that properly denotes end of commit would cause issues with older
versions of git, so we need to use some marker that a well formatted
patch already does.

> Best,
> Matthias
>
> [0]: https://mas.to/@zekjur/116022397626943871

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  8:04 ` Jacob Keller
@ 2026-02-06  8:18   ` Matthias Beyer
  2026-02-06  9:03     ` Jeff King
  2026-02-06  8:59   ` git-am applies commit message diffs Florian Weimer
  1 sibling, 1 reply; 65+ messages in thread
From: Matthias Beyer @ 2026-02-06  8:18 UTC (permalink / raw)
  To: Jacob Keller; +Cc: git, pyokagan, peff

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]

Hi,

CCing some git-am contributors, hope that's alright for you!

On Fri, Feb 06, 2026 at 12:04:54AM -0800, Jacob Keller wrote:
> On Thu, Feb 5, 2026 at 11:50 PM Matthias Beyer <mail@beyermatthias.de> wrote:
> >
> > Hi,
> >
> > I am not sure whether this was already reported, searching the lore did
> > not yield anything for me, but I might have overlooked it...
> >
> > This was just posted on mastodon[0]:
> >
> >     PSA: Did you know that it’s **unsafe** to put code diffs into your commit messages?
> >
> >     Like https://
> >     github.com/i3/i3/pull/6564 for example
> >
> >     Such diffs will be applied by patch(1) (also git-am(1)) as part of the code change!
> >
> >     This is how a sleep(1) made it into i3 4.25-2 in Debian unstable.
> >
> > TL;DR: If you put a diff in the commit message, that diff will be
> > applied by git-am.
> >
> > This looks clearly like unintended and might be an attack-vector, right?
> >
> 
> It is certainly surprising. I am not certain I would consider it an
> attack-vector since you should definitely be reading the commit
> messages before applying, but I could see the fact that its
> unintentional is a problem.
> [...]
>
> > [0]: https://mas.to/@zekjur/116022397626943871

As per the issue linked in that toot I quoted above, the issue clearly
seems to be that it is not intentional that a diff embedded in the
commit message will be applied.
Nobody ever guessed that and that `sleep 1` that was in the commit
message made it into debian unstable because people assumed it to work
as intended.

I call that sheer luck, that it was only a `sleep 1` and not a "here is
how I made this into a backdoor and here is a patch to fix it",
ultimately getting the backdoor in which was written as a diff in the
commit message, instead of the "fix" in the "patch part" of the email.


That said, I am no expert in either C or the git codebase at all, but
from what I saw from reading the git-am codebase, it looks like it tries
to find the patch by looking for three dashes on a line with a linebreak
behind ("---\n").
From what I read, it looks for that from the first line.
What I would think of here is looking for that "patchbreak" from the
_end_ of the email rather than from the top, that would have prevented
this issue, right?

Best,
Matthias

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  7:43 git-am applies commit message diffs Matthias Beyer
  2026-02-06  8:04 ` Jacob Keller
@ 2026-02-06  8:43 ` Kristoffer Haugsbakk
  2026-02-06 17:45   ` Jakob Haufe
  2026-02-07 21:44 ` Kristoffer Haugsbakk
  2026-02-08  0:11 ` [PATCH] doc: add caveat about roundtripping format-patch kristofferhaugsbakk
  3 siblings, 1 reply; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-06  8:43 UTC (permalink / raw)
  To: Matthias Beyer, git

On Fri, Feb 6, 2026, at 08:43, Matthias Beyer wrote:
> Hi,
>
> I am not sure whether this was already reported, searching the lore did
> not yield anything for me, but I might have overlooked it...
>
> This was just posted on mastodon[0]:
>
>     PSA: Did you know that it’s **unsafe** to put code diffs into your
> commit messages?
>
>     Like https://
>     github.com/i3/i3/pull/6564 for example
>
>     Such diffs will be applied by patch(1) (also git-am(1)) as part of
> the code change!
>
>     This is how a sleep(1) made it into i3 4.25-2 in Debian unstable.
>
> TL;DR: If you put a diff in the commit message, that diff will be
> applied by git-am.
>
> This looks clearly like unintended and might be an attack-vector, right?

Related: https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/

But for the magic string that git-format-patch(1) uses at the start of
each email.

Like Jacob said the cure is to use indentation for code blocks.

https://lore.kernel.org/git/xmqqttcmv8a6.fsf@gitster.g/#t

Indentation for code blocks: just stylistic until it isn’t. ;-)

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  8:04 ` Jacob Keller
  2026-02-06  8:18   ` Matthias Beyer
@ 2026-02-06  8:59   ` Florian Weimer
  2026-02-06  9:24     ` Jeff King
  1 sibling, 1 reply; 65+ messages in thread
From: Florian Weimer @ 2026-02-06  8:59 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Matthias Beyer, git

* Jacob Keller:

> It seems like a good idea to me to improve the format patch output and
> the git am patch splitting to somehow try and detect the end of a
> valid commit message and not treat it as a patch content, but I am
> really uncertain how to go about doing so safely without risking
> backwards compatibility (modifying format-patch to insert a marker
> that properly denotes end of commit would cause issues with older
> versions of git, so we need to use some marker that a well formatted
> patch already does.

Isn't the format-patch output already unambiguous because the sequence
of diffs is preceeded by the non-diff statistics section, and only then
the commit message follows?  It's just not possible to process this
correctly in one pass because only at the end of the input, you know
that you have just seen the to-be-applied diffs.

The other tool to look at is git rebase.  There have been problems with
the lack of "From " encoding in commit messages in the past, which
caused rebases to fail due to commit message contents (but I can totally
imagine that this might have resulted in commit injection with more
carefully crafted commit messages).

Thanks,
Florian


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  8:18   ` Matthias Beyer
@ 2026-02-06  9:03     ` Jeff King
  2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
                         ` (2 more replies)
  0 siblings, 3 replies; 65+ messages in thread
From: Jeff King @ 2026-02-06  9:03 UTC (permalink / raw)
  To: Matthias Beyer; +Cc: Jacob Keller, git, pyokagan

On Fri, Feb 06, 2026 at 09:18:50AM +0100, Matthias Beyer wrote:

> That said, I am no expert in either C or the git codebase at all, but
> from what I saw from reading the git-am codebase, it looks like it tries
> to find the patch by looking for three dashes on a line with a linebreak
> behind ("---\n").

Yes, that is how the split is made.

> From what I read, it looks for that from the first line.
> What I would think of here is looking for that "patchbreak" from the
> _end_ of the email rather than from the top, that would have prevented
> this issue, right?

The patch itself may legitimately contain "---" on a line by itself (it
would indicate that the line "--" was removed from a file). That would
confuse your parser, including in a way that we end up only applying
part of the diff (everything before that fake "---" becomes commit
message, and everything after becomes cover-letter material up to the
next "diff" line).

I suspect it also creates corner cases with cover-letter material
(between the "---" and the diff itself) that itself contains any "---"
marker.

I don't think there is a way to unambiguously parse the single-stream
output that format-patch produces. This is a reasonably well-known
gotcha (at least around here). E.g., some earlier discussions:

  2024: https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
  2022: https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
  2015: https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/

There are probably more, but it's actually a tricky thing to search for
in the archive, so I stopped digging. ;)

I think the general attitude has been that such things are a nuisance
when you trigger them accidentally, but probably an unlikely security
issue if we assume a human is reading the patch (and if they're not, all
bets are off anyway).

Ironically, you can ask format-patch to split the message and patch
using the "--attach" option, which should be unambiguous (they are in
two mime parts). But git-mailinfo (which powers git-am under the hood)
decodes the two parts into a single stream, and still takes a "diff"
line in the commit message part as the start of the diff.

Arguably that could be improved, but I suspect might break other cases
(I think it is trying to be forgiving to folks who have shoved the whole
patch into an attachment). So you'd have to pull the attachments apart
yourself and feed them individually to "git apply" and "git commit -F".

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  8:59   ` git-am applies commit message diffs Florian Weimer
@ 2026-02-06  9:24     ` Jeff King
  2026-02-06  9:48       ` Florian Weimer
  0 siblings, 1 reply; 65+ messages in thread
From: Jeff King @ 2026-02-06  9:24 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Jacob Keller, Matthias Beyer, git

On Fri, Feb 06, 2026 at 09:59:31AM +0100, Florian Weimer wrote:

> Isn't the format-patch output already unambiguous because the sequence
> of diffs is preceeded by the non-diff statistics section, and only then
> the commit message follows?  It's just not possible to process this
> correctly in one pass because only at the end of the input, you know
> that you have just seen the to-be-applied diffs.

That diffstat is optional, and not parsed by the receiving format-patch
at all. Keep in mind that in the world for which it was originally
designed, people were not necessarily using Git to generate their
emails. They could be patches emailed by random folks using "diff"
themselves.

> The other tool to look at is git rebase.  There have been problems with
> the lack of "From " encoding in commit messages in the past, which
> caused rebases to fail due to commit message contents (but I can totally
> imagine that this might have resulted in commit injection with more
> carefully crafted commit messages).

It has been a long time since I looked at it, but IIRC we did have a
problem with fidelity of commit message in git-rebase, since it was
based on a format-patch/am pipeline. And we solved it by teaching "git
am" a magic "--rebasing" flag which tells it to ignore the email
contents and find the actual commit in the object database. Gross, but
it works. But of course the same does not work for a true emailed patch,
since the point is that the receiver does not have the commit object
yet.

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  9:24     ` Jeff King
@ 2026-02-06  9:48       ` Florian Weimer
  2026-02-06 10:08         ` Jeff King
  0 siblings, 1 reply; 65+ messages in thread
From: Florian Weimer @ 2026-02-06  9:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Jacob Keller, Matthias Beyer, git

* Jeff King:

> On Fri, Feb 06, 2026 at 09:59:31AM +0100, Florian Weimer wrote:
>
>> Isn't the format-patch output already unambiguous because the sequence
>> of diffs is preceeded by the non-diff statistics section, and only then
>> the commit message follows?  It's just not possible to process this
>> correctly in one pass because only at the end of the input, you know
>> that you have just seen the to-be-applied diffs.
>
> That diffstat is optional, and not parsed by the receiving format-patch
> at all. Keep in mind that in the world for which it was originally
> designed, people were not necessarily using Git to generate their
> emails. They could be patches emailed by random folks using "diff"
> themselves.

Is the git am format that flexible in practice?  I often have trouble
applying patches with git am that were created with git format-patch
and have to resort to plain old patch instead.  As a user, I definitely
get the impression that it's not a type of tool that gets a patch
out of an email message, no matter what the cost.

Thanks,
Florian


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  9:48       ` Florian Weimer
@ 2026-02-06 10:08         ` Jeff King
  0 siblings, 0 replies; 65+ messages in thread
From: Jeff King @ 2026-02-06 10:08 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Jacob Keller, Matthias Beyer, git

On Fri, Feb 06, 2026 at 10:48:29AM +0100, Florian Weimer wrote:

> > On Fri, Feb 06, 2026 at 09:59:31AM +0100, Florian Weimer wrote:
> >
> >> Isn't the format-patch output already unambiguous because the sequence
> >> of diffs is preceeded by the non-diff statistics section, and only then
> >> the commit message follows?  It's just not possible to process this
> >> correctly in one pass because only at the end of the input, you know
> >> that you have just seen the to-be-applied diffs.
> >
> > That diffstat is optional, and not parsed by the receiving format-patch
> > at all. Keep in mind that in the world for which it was originally
> > designed, people were not necessarily using Git to generate their
> > emails. They could be patches emailed by random folks using "diff"
> > themselves.
> 
> Is the git am format that flexible in practice?  I often have trouble
> applying patches with git am that were created with git format-patch
> and have to resort to plain old patch instead.  As a user, I definitely
> get the impression that it's not a type of tool that gets a patch
> out of an email message, no matter what the cost.

I'm sure there are corner cases it doesn't handle, but it will take
input like this:

git am <<\EOF
From: Jeff King <peff@peff.net>
Date: Fri Feb 6 03:42:12 2026 -0500
Subject: my cool patch

this fixes some stuff

diff -Nru old/file new/file
--- old/file	2026-02-06 04:58:56.148348259 -0500
+++ new/file	2026-02-06 04:58:59.432360938 -0500
@@ -1 +1 @@
-base
+changed
EOF


and happily produce the commit you'd expect. I generated the diff there
with GNU diff, and typed the rest. Likewise for this version with
attachments, which I generated with mutt:

git am <<\EOF
Date: Fri, 6 Feb 2026 05:03:31 -0500
From: Jeff King <peff@peff.net>
To: Jeff King <peff@peff.net>
Subject: my cool patch
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="tveeCB9LAhLXuhMJ"
Content-Disposition: inline


--tveeCB9LAhLXuhMJ
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline

see the attached patch, which does blah blah blah

--tveeCB9LAhLXuhMJ
Content-Type: text/plain; charset=utf-8
Content-Disposition: attachment; filename=patch

diff -Nru old/file new/file
--- old/file	2026-02-06 04:58:56.148348259 -0500
+++ new/file	2026-02-06 04:58:59.432360938 -0500
@@ -1 +1 @@
-base
+changed

--tveeCB9LAhLXuhMJ--
EOF


I expect that Linus saw a lot of this kind of stuff in the early days.
I'd guess it's pretty rare now, but I won't be surprised if there are
some die-hards generating kernel patches with who-knows-what. ;)

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  8:43 ` Kristoffer Haugsbakk
@ 2026-02-06 17:45   ` Jakob Haufe
  2026-02-07 10:08     ` Kristoffer Haugsbakk
  0 siblings, 1 reply; 65+ messages in thread
From: Jakob Haufe @ 2026-02-06 17:45 UTC (permalink / raw)
  To: git; +Cc: Kristoffer Haugsbakk

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Fri, 06 Feb 2026 09:43:04 +0100
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> wrote:

> Like Jacob said the cure is to use indentation for code blocks.

That doesn't help here as stated by Michael on GH and his Mastodon
post. Also, to make sure this doesn't get lost:

From patch(1):

---
If the entire diff is indented by a consistent amount, if lines end in CRLF,
or if a diff is encapsulated one or more times by prepending "- " to lines
starting with "-" as specified by Internet RFC 934, this is taken into account.
After removing indenting or encapsulation, lines beginning with # are ignored,
as they are considered to be comments.
---

It's not exactly written in a straightforward way, but it show that the
behavior from patch is intentional. So even if git-am gets a fix, it
only partly mitigates the problem as I'm pretty sure I will not be the
last one to pass "git show"/"git format-patch" to "patch".

Cheers,
sur5r

-- 
ceterum censeo microsoftem esse delendam.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06 17:45   ` Jakob Haufe
@ 2026-02-07 10:08     ` Kristoffer Haugsbakk
  0 siblings, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-07 10:08 UTC (permalink / raw)
  To: Jakob Haufe, git; +Cc: Matthias Beyer

On Fri, Feb 6, 2026, at 18:45, Jakob Haufe wrote:
> On Fri, 06 Feb 2026 09:43:04 +0100
> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> wrote:
>
>> Like Jacob said the cure is to use indentation for code blocks.
>
> That doesn't help here as stated by Michael on GH and his Mastodon
> post. Also, to make sure this doesn't get lost:
>
> From patch(1):
>
> ---
> If the entire diff is indented by a consistent amount, if lines end in CRLF,
> or if a diff is encapsulated one or more times by prepending "- " to lines
> starting with "-" as specified by Internet RFC 934, this is taken into account.
> After removing indenting or encapsulation, lines beginning with # are ignored,
> as they are considered to be comments.
> ---

Yeah, I think I understand now.

• patch(1) will apply all the diffs from git-format-patch(1), including
  from the commit message
• git-am(1) will do the same
• git-am(1) will do the expected thing if you indent the diff in the
  commit message
• For the git-format-patch(1) output with an indented diff in the commit
  message: `git patch -p1` (I guess to strip the `a/` and `b/` from
  git(1) diffs?) applies everything, including the `sleep(1)`[1]

My hodgepodge assumptions from 2024[2] were off. I thought that as long
as you did the following:

• Do not put the magic `From` string at the start of any line in the
  commit message
• Do not put `---` at the start of the line in the commit message
• Do not put diff output unindented in the commit message since
  git-am(1) will think that is the diff and not care about finding any
  `–––`[3]

Then git-am(1) would apply the commit message and the diff part as
expected.

† 1: Related is https://github.com/i3/i3/pull/6564#issuecomment-3863278059 ,
     specifically the link https://lists.gnu.org/archive/html/bug-patch/2026-02/msg00000.html
[2]: https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
† 3: And my assumption here that only the diff in the commit message
     would be applied in this case was wrong. Or else it would have been
     more immediately obvious that the resulting commit was wrong.

> It's not exactly written in a straightforward way,

Yeah it’s not straightforward at all.

Something useful might be to apply all patches if they are all at the
same indentation level. I don’t see how it is useful to apparently strip
all indentation and find all the diffs that way.

> but it show that the behavior from patch is intentional. So even if
> git-am gets a fix, it only partly mitigates the problem as I'm pretty
> sure I will not be the last one to pass "git show"/"git format-patch"
> to "patch".

I don’t pass output from git(1) to patch(1). But I have often (like the
handful of times I’ve needed it) fallen back on using patch(1) for
patches/diffs that are thrown into email messages since it is more
forgiving than git-apply(1), and I guess also git-am(1).

The diff in the commit message doesn’t have the trailing whitespace that
I thought would be needed for patch application. Since git-commit(1) by
default cleans up trailing whitespace. But apparently patch(1) is fine
with that.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am"
  2026-02-06  9:03     ` Jeff King
@ 2026-02-07 14:57       ` Phillip Wood
  2026-02-07 14:58         ` [PATCH 1/3] templates: add .gitattributes entry for sample hooks Phillip Wood
                           ` (3 more replies)
  2026-02-09 15:58       ` git-am applies commit message diffs Patrick Steinhardt
  2026-02-13 14:34       ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
  2 siblings, 4 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-07 14:57 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Matthias Beyer, Jacob Keller, pyokagan

From: Phillip Wood <phillip.wood@dunelm.org.uk>

On 06/02/2026 09:03, Jeff King wrote:
> I don't think there is a way to unambiguously parse the single-stream
> output that format-patch produces. This is a reasonably well-known
> gotcha (at least around here). E.g., some earlier discussions:
>
>    2024:https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
>    2022:https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
>    2015:https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/

If we cannot improve "git am" perhaps we should update our sample
"commit-msg" hook to reject messages that will cause problems. Here
are some patches to do that.

We could perhaps think about adding a more prominent warning to the
"git am" and "git format-patch" documentation. The docs for "git am"
mention that it splits the message on a line starting with "diff -"
but maybe we should spell out what that means for commit messages that
include a diff. In principle "git format-patch" could also warn or
error out if it creates a mail that "git am" cannot import verbatim,
I don't know how hard that would be in implement.

Base-Commit: b2826b52eb7caff9f4ed6e85ec45e338bf02ad09
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fsample-commit-msg-reject-diff%2Fv1
View-Changes-At: https://github.com/phillipwood/git/compare/b2826b52e...83c100a73
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/sample-commit-msg-reject-diff/v1


Phillip Wood (3):
  templates: add .gitattributes entry for sample hooks
  templates: detect commit messages containing diffs
  templates: detect messages that contain a separator line

 .editorconfig                     |  2 +-
 .gitattributes                    |  1 +
 templates/hooks/commit-msg.sample | 38 +++++++++++++++++++++++++++++--
 3 files changed, 38 insertions(+), 3 deletions(-)

-- 
2.52.0.362.g884e03848a9


^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH 1/3] templates: add .gitattributes entry for sample hooks
  2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
@ 2026-02-07 14:58         ` Phillip Wood
  2026-02-07 14:58         ` [PATCH 2/3] templates: detect commit messages containing diffs Phillip Wood
                           ` (2 subsequent siblings)
  3 siblings, 0 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-07 14:58 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Matthias Beyer, Jacob Keller, pyokagan, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

The sample hooks are shell scripts but the filenames end with ".sample"
so they need their own .gitattributes rule. Update our editorconfig
settings to match the attributes as well.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 .editorconfig  | 2 +-
 .gitattributes | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.editorconfig b/.editorconfig
index 2d3929b5916..6e4eaa8e955 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -4,7 +4,7 @@ insert_final_newline = true
 
 # The settings for C (*.c and *.h) files are mirrored in .clang-format.  Keep
 # them in sync.
-[{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]
+[{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile,templates/hooks/*.sample}]
 indent_style = tab
 tab_width = 8
 
diff --git a/.gitattributes b/.gitattributes
index 38b1c52fe0e..556322be01b 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -18,3 +18,4 @@ CODE_OF_CONDUCT.md -whitespace
 /Documentation/user-manual.adoc conflict-marker-size=32
 /t/t????-*.sh conflict-marker-size=32
 /t/unit-tests/clar/test/expected/* whitespace=-blank-at-eof
+/templates/hooks/*.sample whitespace=indent,trail,space,incomplete text eol=lf
-- 
2.52.0.362.g884e03848a9


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH 2/3] templates: detect commit messages containing diffs
  2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
  2026-02-07 14:58         ` [PATCH 1/3] templates: add .gitattributes entry for sample hooks Phillip Wood
@ 2026-02-07 14:58         ` Phillip Wood
  2026-02-07 14:58         ` [PATCH 3/3] templates: detect messages that contain a separator line Phillip Wood
  2026-02-09  6:57         ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Jeff King
  3 siblings, 0 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-07 14:58 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Matthias Beyer, Jacob Keller, pyokagan, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If a commit message contains a diff that is not indented then "git
am" will treat that diff as part of the patch rather than as part
of the commit message. This allows it to apply email messages that
were created by adding a commit message in front of a regular diff
without adding the "---" separator used by "git format-patch". This
often surprises users [1-4] so add a check to the sample "commit-msg"
hook to reject messages that would confuse "git am".

Detecting if the message contains a diff is complicated by the hook
being passed the message before it is cleaned up so we need to ignore
any diffs below the scissors line. There are also two possible
config keys to check to find the comment character at the start
of the scissors line.

[1] https://lore.kernel.org/git/bcqvh7ahjjgzpgxwnr4kh3hfkksfruf54refyry3ha7qk7dldf@fij5calmscvm
[2] https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
[3] https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
[4] https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 templates/hooks/commit-msg.sample | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/templates/hooks/commit-msg.sample b/templates/hooks/commit-msg.sample
index b58d1184a9d..099cc58c303 100755
--- a/templates/hooks/commit-msg.sample
+++ b/templates/hooks/commit-msg.sample
@@ -15,10 +15,37 @@
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
 
-# This example catches duplicate Signed-off-by lines.
+# This example catches duplicate Signed-off-by lines and messages that
+# would confuse 'git am'.
+
+ret=0
 
 test "" = "$(grep '^Signed-off-by: ' "$1" |
 	 sort | uniq -c | sed -e '/^[ 	]*1[ 	]/d')" || {
 	echo >&2 Duplicate Signed-off-by lines.
-	exit 1
+	ret=1
 }
+
+comment_re="$(
+	{
+		git config --get-regexp "^core\.comment(char|string)\$" ||
+			echo '#'
+	} | sed -n -e '
+		${
+			s/^[^ ]* //
+			s|[][*./\]|\\&|g
+			s/^auto$/[#;@!$%^&|:]/
+			p
+		}'
+)"
+line="$(sed -n -e "/^${comment_re} -\{8,\} >8 -\{8,\}\$/q
+		   /^diff -/{p;q;}
+		   /^Index: /{p;q;}" "$1")"
+if test -n "$line"
+then
+	echo >&2 "Message contains a diff that will confuse 'git am'."
+	echo >&2 "To fix this indent the diff."
+	ret=1
+fi
+
+exit $ret
-- 
2.52.0.362.g884e03848a9


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH 3/3] templates: detect messages that contain a separator line
  2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
  2026-02-07 14:58         ` [PATCH 1/3] templates: add .gitattributes entry for sample hooks Phillip Wood
  2026-02-07 14:58         ` [PATCH 2/3] templates: detect commit messages containing diffs Phillip Wood
@ 2026-02-07 14:58         ` Phillip Wood
  2026-02-07 21:27           ` Junio C Hamano
  2026-02-09  6:57         ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Jeff King
  3 siblings, 1 reply; 65+ messages in thread
From: Phillip Wood @ 2026-02-07 14:58 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Matthias Beyer, Jacob Keller, pyokagan, Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Messages that contain "---" separator lines will be truncated by
"git am". This often surprises users so add a check to the sample
"commit-msg" hook to reject such messages. As it's conceivable that
someone is using "---" as their comment string we delete any commented
lines before checking for a separator. The trailing ".*" when matching
commented lines ensures that if the comment string ends with a "$"
it is not treated as an anchor.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 templates/hooks/commit-msg.sample | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/templates/hooks/commit-msg.sample b/templates/hooks/commit-msg.sample
index 099cc58c303..c7a9db88cb9 100755
--- a/templates/hooks/commit-msg.sample
+++ b/templates/hooks/commit-msg.sample
@@ -39,9 +39,16 @@ comment_re="$(
 		}'
 )"
 line="$(sed -n -e "/^${comment_re} -\{8,\} >8 -\{8,\}\$/q
+		   /^${comment_re}.*/d
+		   /^---\$/{p;q;}
 		   /^diff -/{p;q;}
 		   /^Index: /{p;q;}" "$1")"
-if test -n "$line"
+if test "$line" = "---"
+then
+	echo >&2 "Message contains a '---' separator line that will confuse"
+	echo >&2 "'git am'. To fix this indent the '---' line."
+	ret=1
+elif test -n "$line"
 then
 	echo >&2 "Message contains a diff that will confuse 'git am'."
 	echo >&2 "To fix this indent the diff."
-- 
2.52.0.362.g884e03848a9


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH 3/3] templates: detect messages that contain a separator line
  2026-02-07 14:58         ` [PATCH 3/3] templates: detect messages that contain a separator line Phillip Wood
@ 2026-02-07 21:27           ` Junio C Hamano
  2026-02-07 21:38             ` Kristoffer Haugsbakk
  2026-02-09  7:00             ` Jeff King
  0 siblings, 2 replies; 65+ messages in thread
From: Junio C Hamano @ 2026-02-07 21:27 UTC (permalink / raw)
  To: Phillip Wood; +Cc: git, Jeff King, Matthias Beyer, Jacob Keller, pyokagan

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Messages that contain "---" separator lines will be truncated by
> "git am". This often surprises users so add a check to the sample
> "commit-msg" hook to reject such messages. As it's conceivable that
> someone is using "---" as their comment string we delete any commented
> lines before checking for a separator. The trailing ".*" when matching
> commented lines ensures that if the comment string ends with a "$"
> it is not treated as an anchor.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  templates/hooks/commit-msg.sample | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

I have no qualms about the topic up to the previous step, but I know
one of the things that I sometimes do will be broken with the change
in this step, namely, when I know what I want to write below the
three-dash lines, I would commit with "---" and additional notes
below it, so that I do not forget during "format-patch".

When the commit is turned into a patch email, possibly with some
other material like "--notes=<ref>" that adds notes there, the
resulting message will have two three-dashes lines, but because "am"
cuts at the first one, and "apply" knows that the garbage lines at
front, including three-dash lines, do not matter until it sees "^diff",
this works out perfectly well.

Admittedly, I myself do not send out so many patches as I used to,
but I suspect that there are others who have discovered this trick
independently, and they would be unhappy to be interrupted by
commit-msg hook like this.

A saving grace is that when the user is stopped with this,
pre-commit hook that inspects the contents to be committed
have already run successfully, so rerunning with "--no-verify"
is not with too much risk.  But still, I am not sure if this is a
good thing to do overall.

> diff --git a/templates/hooks/commit-msg.sample b/templates/hooks/commit-msg.sample
> index 099cc58c303..c7a9db88cb9 100755
> --- a/templates/hooks/commit-msg.sample
> +++ b/templates/hooks/commit-msg.sample
> @@ -39,9 +39,16 @@ comment_re="$(
>  		}'
>  )"
>  line="$(sed -n -e "/^${comment_re} -\{8,\} >8 -\{8,\}\$/q
> +		   /^${comment_re}.*/d
> +		   /^---\$/{p;q;}
>  		   /^diff -/{p;q;}
>  		   /^Index: /{p;q;}" "$1")"
> -if test -n "$line"
> +if test "$line" = "---"
> +then
> +	echo >&2 "Message contains a '---' separator line that will confuse"
> +	echo >&2 "'git am'. To fix this indent the '---' line."
> +	ret=1
> +elif test -n "$line"
>  then
>  	echo >&2 "Message contains a diff that will confuse 'git am'."
>  	echo >&2 "To fix this indent the diff."

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 3/3] templates: detect messages that contain a separator line
  2026-02-07 21:27           ` Junio C Hamano
@ 2026-02-07 21:38             ` Kristoffer Haugsbakk
  2026-02-09  0:17               ` Junio C Hamano
  2026-02-09  7:00             ` Jeff King
  1 sibling, 1 reply; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-07 21:38 UTC (permalink / raw)
  To: Junio C Hamano, Phillip Wood
  Cc: git, Jeff King, Matthias Beyer, Jacob Keller, pyokagan

On Sat, Feb 7, 2026, at 22:27, Junio C Hamano wrote:
>>[snip]
>
> I have no qualms about the topic up to the previous step, but I know
> one of the things that I sometimes do will be broken with the change
> in this step, namely, when I know what I want to write below the
> three-dash lines, I would commit with "---" and additional notes
> below it, so that I do not forget during "format-patch".
>
> When the commit is turned into a patch email, possibly with some
> other material like "--notes=<ref>" that adds notes there, the
> resulting message will have two three-dashes lines, but because "am"
> cuts at the first one, and "apply" knows that the garbage lines at
> front, including three-dash lines, do not matter until it sees "^diff",
> this works out perfectly well.
>
> Admittedly, I myself do not send out so many patches as I used to,
> but I suspect that there are others who have discovered this trick
> independently, and they would be unhappy to be interrupted by
> commit-msg hook like this.
>
> A saving grace is that when the user is stopped with this,
> pre-commit hook that inspects the contents to be committed
> have already run successfully, so rerunning with "--no-verify"
> is not with too much risk.  But still, I am not sure if this is a
> good thing to do overall.

Maybe this is not the right tool[1] but perhaps the hook could respect
an env. variable to disable this check and hint about it in the error
output?

🔗 1: https://lore.kernel.org/git/cover.1709495964.git.code@khaugsbakk.name/

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  7:43 git-am applies commit message diffs Matthias Beyer
  2026-02-06  8:04 ` Jacob Keller
  2026-02-06  8:43 ` Kristoffer Haugsbakk
@ 2026-02-07 21:44 ` Kristoffer Haugsbakk
  2026-02-08  0:11 ` [PATCH] doc: add caveat about roundtripping format-patch kristofferhaugsbakk
  3 siblings, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-07 21:44 UTC (permalink / raw)
  To: Matthias Beyer, git

On Fri, Feb 6, 2026, at 08:43, Matthias Beyer wrote:
> Hi,
>
> I am not sure whether this was already reported, searching the lore did
> not yield anything for me, but I might have overlooked it...
>
> This was just posted on mastodon[0]:
>
>     PSA: Did you know that it’s **unsafe** to put code diffs into your
> commit messages?
>
>     Like https://
>     github.com/i3/i3/pull/6564 for example
>
>     Such diffs will be applied by patch(1) (also git-am(1)) as part of
> the code change!
>
>     This is how a sleep(1) made it into i3 4.25-2 in Debian unstable.
>
> TL;DR: If you put a diff in the commit message, that diff will be
> applied by git-am.
>
> This looks clearly like unintended and might be an attack-vector, right?

I have an idea for a proposal to note this in the documentation.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-06  7:43 git-am applies commit message diffs Matthias Beyer
                   ` (2 preceding siblings ...)
  2026-02-07 21:44 ` Kristoffer Haugsbakk
@ 2026-02-08  0:11 ` kristofferhaugsbakk
  2026-02-08  1:39   ` Junio C Hamano
                     ` (3 more replies)
  3 siblings, 4 replies; 65+ messages in thread
From: kristofferhaugsbakk @ 2026-02-08  0:11 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Matthias Beyer, Christoph Anton Mitterer,
	Matheus Tavares, Chris Packham, Jakob Haufe

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

git-format-patch(1), git-send-email(1), and git-am(1) deal with
formatting commits as patches, sending them (perhaps directly), and
applying them, respectively. Naturally they use a few delimiters to mark
where the commit message ends. This can lead to surprising behavior when
these delimiters are used in the commit message itself.

git-format-patch(1) and git-send-email(1) will accept any commit message
and not warn or error about these delimiters being used.[1]

Moreover, the presence of unindented diffs in the commit message will
cause git-am(1) to apply both the diffs from the commit message as well
as the patch section.[2]

It is unclear whether any commands in this chain will learn to warn
about this. One concern could be that users have learned to rely on
the three-dash line rule to conveniently add extra-commit message
information in the commit message, knowing that git-am(1) will
ignore it.[4]

All of this is covered already, technically, However, we should spell
out the implications.

† 1: There is also git-commit(1) to consider. However, making that
     command warn or error out over such delimiters would be disruptive
     to all Git users who never use email in their workflow.
[2]: Recently patch(1) caused this issue for a project, but it was noted
     that git-am(1) has the same behavior[3]
[3]: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
[4]: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/

Reported-by: Matthias Beyer <mail@beyermatthias.de>
Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
Reported-by: Matheus Tavares <matheus.tavb@gmail.com>
Reported-by: Chris Packham <judge.packham@gmail.com>
Helped-by: Jakob Haufe <sur5r@sur5r.net>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---

Notes (series):
    There might be other things to do here. Mention it in gitfaq(5)?
    
    § Trailers
    
    • Reported-by: Matthias Beyer <mail@beyermatthias.de>
      • From this thread
    Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
      • From https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/T/#u
    Reported-by: Matheus Tavares <matheus.bernardino@usp.br>
      • From https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/#t
    Reported-by: Chris Packham <judge.packham@gmail.com>
      • From https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
    
    (These were all linked in https://lore.kernel.org/git/20260206090358.GA2761602@coredump.intra.peff.net/ )
    
    Helped-by: Jakob Haufe <sur5r@sur5r.net>
      • For the part about patch(1): https://lore.kernel.org/git/f6e4cdb4-ff82-4853-aca5-0c152f287286@app.fastmail.com/T/#mc389dbd2ae02a007cbe57cd16ca4790ecc5a84f7

 Documentation/format-patch-caveats.adoc | 39 +++++++++++++++++++++++++
 Documentation/git-am.adoc               |  9 ++++++
 Documentation/git-format-patch.adoc     |  4 +++
 Documentation/git-send-email.adoc       |  5 ++++
 4 files changed, 57 insertions(+)
 create mode 100644 Documentation/format-patch-caveats.adoc

diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
new file mode 100644
index 00000000000..2accf2763fd
--- /dev/null
+++ b/Documentation/format-patch-caveats.adoc
@@ -0,0 +1,39 @@
+Patches produced by linkgit:git-format-patch[1] or
+linkgit:git-send-email[1] are inline. This means that the output of
+these two commands can lead to a different commit message when applied
+with linkgit:git-am[1]. It can also mean that the patch is not applied
+correctly.
+
+The commit message might contain a three-dash line (`---`) which was
+perhaps meant to be a thematic break. That means that the commit message
+will be cut short. The presence of a line starting with "Index: " can
+cause the patch not to be found, giving an error about an empty patch.
+
+Furthermore, the presence of an unindented diff in the commit message
+will not only cut the message short but cause that very diff to be
+applied, along with the patch in the patch section. The commit message
+might for example have a diff in a GitHub MarkDown code fence:
+
+----
+```
+diff ...
+```
+----
+
+The solution for this is to indent the diff instead:
+
+----
+    diff ...
+----
+
+This loss of fidelity might be simple to notice if you are applying
+patches directly from a mailbox. However, a commit authored long ago
+might be applied in a different context, perhaps because many changes
+are being integrated via patch files and the
+linkgit:git-format-patch[1] format is trusted to import changes of a
+Git origin.
+
+One might want to use a general-purpose utility like patch(1) instead,
+given these limitations. However, patch(1) will not only look for
+unindented diffs (like linkgit:git-am[1]) but will try to apply indented
+diffs as well.
diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
index 0c94776e296..18f5b950825 100644
--- a/Documentation/git-am.adoc
+++ b/Documentation/git-am.adoc
@@ -259,10 +259,13 @@ message.  Any line that is of the form:
 * a line that begins with "Index: "
 
 is taken as the beginning of a patch, and the commit log message
 is terminated before the first occurrence of such a line.
 
+This means that the content of the commit message can inadverently
+interrupt the processing (see the <<caveats,CAVEATS>> section below).
+
 When initially invoking `git am`, you give it the names of the mailboxes
 to process.  Upon seeing the first patch that does not apply, it
 aborts in the middle.  You can recover from this in one of two ways:
 
 . skip the current patch by re-running the command with the `--skip`
@@ -281,10 +284,16 @@ Before any patches are applied, ORIG_HEAD is set to the tip of the
 current branch.  This is useful if you have problems with multiple
 commits, like running 'git am' on the wrong branch or an error in the
 commits that is more easily fixed by changing the mailbox (e.g.
 errors in the "From:" lines).
 
+[[caveats]]
+CAVEATS
+-------
+
+include::format-patch-caveats.adoc[]
+
 HOOKS
 -----
 This command can run `applypatch-msg`, `pre-applypatch`,
 and `post-applypatch` hooks.  See linkgit:githooks[5] for more
 information.
diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
index 9a7807ca71a..36851aaf5e1 100644
--- a/Documentation/git-format-patch.adoc
+++ b/Documentation/git-format-patch.adoc
@@ -796,10 +796,14 @@ CAVEATS
 Note that `format-patch` will omit merge commits from the output, even
 if they are part of the requested range. A simple "patch" does not
 include enough information for the receiving end to reproduce the same
 merge commit.
 
+'''
+
+include::format-patch-caveats.adoc[]
+
 SEE ALSO
 --------
 linkgit:git-am[1], linkgit:git-send-email[1]
 
 GIT
diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc
index ebe8853e9f5..0b118df6498 100644
--- a/Documentation/git-send-email.adoc
+++ b/Documentation/git-send-email.adoc
@@ -690,10 +690,15 @@ Links of a few such community maintained helpers are:
 	  (cross platform client that can send emails using the ProtonMail API)
 
 	- https://github.com/AdityaGarg8/git-credential-email[git-msgraph]
 	  (cross platform client that can send emails using the Microsoft Graph API)
 
+CAVEATS
+-------
+
+include::format-patch-caveats.adoc[]
+
 SEE ALSO
 --------
 linkgit:git-format-patch[1], linkgit:git-imap-send[1], mbox(5)
 
 GIT

base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc
-- 
2.53.0.26.g2afa8602a26


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-08  0:11 ` [PATCH] doc: add caveat about roundtripping format-patch kristofferhaugsbakk
@ 2026-02-08  1:39   ` Junio C Hamano
  2026-02-08 17:18     ` Kristoffer Haugsbakk
  2026-02-09 16:42   ` Phillip Wood
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2026-02-08  1:39 UTC (permalink / raw)
  To: kristofferhaugsbakk
  Cc: git, Kristoffer Haugsbakk, Matthias Beyer,
	Christoph Anton Mitterer, Matheus Tavares, Chris Packham,
	Jakob Haufe

kristofferhaugsbakk@fastmail.com writes:

> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> git-format-patch(1), git-send-email(1), and git-am(1) deal with
> formatting commits as patches, sending them (perhaps directly), and
> applying them, respectively. Naturally they use a few delimiters to mark
> where the commit message ends. This can lead to surprising behavior when
> these delimiters are used in the commit message itself.
>
> git-format-patch(1) and git-send-email(1) will accept any commit message
> and not warn or error about these delimiters being used.[1]
>
> Moreover, the presence of unindented diffs in the commit message will
> cause git-am(1) to apply both the diffs from the commit message as well
> as the patch section.[2]
>
> It is unclear whether any commands in this chain will learn to warn
> about this. One concern could be that users have learned to rely on
> the three-dash line rule to conveniently add extra-commit message
> information in the commit message, knowing that git-am(1) will
> ignore it.[4]
>
> All of this is covered already, technically, However, we should spell
> out the implications.
>
> † 1: There is also git-commit(1) to consider. However, making that
>      command warn or error out over such delimiters would be disruptive
>      to all Git users who never use email in their workflow.
> [2]: Recently patch(1) caused this issue for a project, but it was noted
>      that git-am(1) has the same behavior[3]
> [3]: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
> [4]: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
>
> Reported-by: Matthias Beyer <mail@beyermatthias.de>
> Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
> Reported-by: Matheus Tavares <matheus.tavb@gmail.com>
> Reported-by: Chris Packham <judge.packham@gmail.com>
> Helped-by: Jakob Haufe <sur5r@sur5r.net>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
>
> Notes (series):
>     There might be other things to do here. Mention it in gitfaq(5)?
>     
>     § Trailers
>     
>     • Reported-by: Matthias Beyer <mail@beyermatthias.de>
>       • From this thread
>     Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
>       • From https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/T/#u
>     Reported-by: Matheus Tavares <matheus.bernardino@usp.br>
>       • From https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/#t
>     Reported-by: Chris Packham <judge.packham@gmail.com>
>       • From https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
>     
>     (These were all linked in https://lore.kernel.org/git/20260206090358.GA2761602@coredump.intra.peff.net/ )
>     
>     Helped-by: Jakob Haufe <sur5r@sur5r.net>
>       • For the part about patch(1): https://lore.kernel.org/git/f6e4cdb4-ff82-4853-aca5-0c152f287286@app.fastmail.com/T/#mc389dbd2ae02a007cbe57cd16ca4790ecc5a84f7

The space after three-dash line is to give additional information to
help readers, but the above does not qualify as one.

> +Furthermore, the presence of an unindented diff in the commit message
> +will not only cut the message short but cause that very diff to be
> +applied, along with the patch in the patch section.

A line that matches "^diff " is taken as the end of the log message,
and everything that follows is passed to the patch application
machinery, and the above description is a consequence of that.  If
you have more than one such diff, they may be either applied, or
some of them may not match the patch target and the whole thing may
be rejected.  Neither is a happy outcome.

Queued.  Thanks.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-08  1:39   ` Junio C Hamano
@ 2026-02-08 17:18     ` Kristoffer Haugsbakk
  0 siblings, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-08 17:18 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Kristoffer Haugsbakk, Matthias Beyer,
	Christoph Anton Mitterer, Matheus Tavares, Chris Packham,
	Jakob Haufe

On Sun, Feb 8, 2026, at 02:39, Junio C Hamano wrote:
>>[snip]
>> ---
>>
>> Notes (series):
>>     There might be other things to do here. Mention it in gitfaq(5)?
>>
>>     § Trailers
>>
>>     • Reported-by: Matthias Beyer <mail@beyermatthias.de>
>>       • From this thread
>>     Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
>>       • From https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/T/#u
>>     Reported-by: Matheus Tavares <matheus.bernardino@usp.br>
>>       • From https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/#t
>>     Reported-by: Chris Packham <judge.packham@gmail.com>
>>       • From https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
>>
>>     (These were all linked in https://lore.kernel.org/git/20260206090358.GA2761602@coredump.intra.peff.net/ )
>>
>>     Helped-by: Jakob Haufe <sur5r@sur5r.net>
>>       • For the part about patch(1): https://lore.kernel.org/git/f6e4cdb4-ff82-4853-aca5-0c152f287286@app.fastmail.com/T/#mc389dbd2ae02a007cbe57cd16ca4790ecc5a84f7
>
> The space after three-dash line is to give additional information to
> help readers, but the above does not qualify as one.

It’s not for you. It’s for the people that got CCd to inform them of the
years-ago context where they reported this and why they are mentioned by
name in the email.

I do go overboard with the writing though sometimes.

>[snip]

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 3/3] templates: detect messages that contain a separator line
  2026-02-07 21:38             ` Kristoffer Haugsbakk
@ 2026-02-09  0:17               ` Junio C Hamano
  0 siblings, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2026-02-09  0:17 UTC (permalink / raw)
  To: Kristoffer Haugsbakk
  Cc: Phillip Wood, git, Jeff King, Matthias Beyer, Jacob Keller,
	pyokagan

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

>> A saving grace is that when the user is stopped with this,
>> pre-commit hook that inspects the contents to be committed
>> have already run successfully, so rerunning with "--no-verify"
>> is not with too much risk.  But still, I am not sure if this is a
>> good thing to do overall.
>
> Maybe this is not the right tool[1] but perhaps the hook could respect
> an env. variable to disable this check and hint about it in the error
> output?

It is merely a sample script shipped with the rest of Git, so people
can choose to install better alternatives.  I think it is fine to
keep the sample script simple and understandable.

It however is still a little worrysome that the behaviour of the
sample commit-msg hook updated with the third patch may be used
against helpful suggestions people in projects that employ the
e-mail based workflow may make to their colleages to deliberately
commit a three-dash line followed by material not meant for the
commit log proper, which is a useful trick if you are making your
commits to be sent over e-mail and never to be merged directly to
your target branch.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am"
  2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
                           ` (2 preceding siblings ...)
  2026-02-07 14:58         ` [PATCH 3/3] templates: detect messages that contain a separator line Phillip Wood
@ 2026-02-09  6:57         ` Jeff King
  2026-02-09 10:43           ` Phillip Wood
  3 siblings, 1 reply; 65+ messages in thread
From: Jeff King @ 2026-02-09  6:57 UTC (permalink / raw)
  To: Phillip Wood; +Cc: git, Matthias Beyer, Jacob Keller, pyokagan

On Sat, Feb 07, 2026 at 02:57:59PM +0000, Phillip Wood wrote:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> On 06/02/2026 09:03, Jeff King wrote:
> > I don't think there is a way to unambiguously parse the single-stream
> > output that format-patch produces. This is a reasonably well-known
> > gotcha (at least around here). E.g., some earlier discussions:
> >
> >    2024:https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
> >    2022:https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
> >    2015:https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
> 
> If we cannot improve "git am" perhaps we should update our sample
> "commit-msg" hook to reject messages that will cause problems. Here
> are some patches to do that.

I'm not entirely opposed to it, but my initial reaction was two bits of
skepticism:

  1. I imagine that hardly anybody runs commit-msg hooks in the first
     place, let alone our sample hook. So I doubt this will get the
     attention of many people.

  2. I'd guess that these days only a small minority of people care
     about sending patches by email. So for most people, a warning about
     their commit message containing a diff or "---" will be mostly
     useless, if not outright confusing.

I'd imagine that documentation updates would be more likely to get read
by users than the sample hook. And a warning in git-commit itself would
be even more obvious (but fall even more afoul of (2) above). Adding a
warning to format-patch would help with (2), but at that point it may be
too late to change the commit message.

> We could perhaps think about adding a more prominent warning to the
> "git am" and "git format-patch" documentation. The docs for "git am"
> mention that it splits the message on a line starting with "diff -"
> but maybe we should spell out what that means for commit messages that
> include a diff. In principle "git format-patch" could also warn or
> error out if it creates a mail that "git am" cannot import verbatim,
> I don't know how hard that would be in implement.

I think the patch from Matheus linked above added that format-patch
check.

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 3/3] templates: detect messages that contain a separator line
  2026-02-07 21:27           ` Junio C Hamano
  2026-02-07 21:38             ` Kristoffer Haugsbakk
@ 2026-02-09  7:00             ` Jeff King
  2026-02-09 10:42               ` Phillip Wood
  1 sibling, 1 reply; 65+ messages in thread
From: Jeff King @ 2026-02-09  7:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Phillip Wood, git, Matthias Beyer, Jacob Keller, pyokagan

On Sat, Feb 07, 2026 at 01:27:01PM -0800, Junio C Hamano wrote:

> I have no qualms about the topic up to the previous step, but I know
> one of the things that I sometimes do will be broken with the change
> in this step, namely, when I know what I want to write below the
> three-dash lines, I would commit with "---" and additional notes
> below it, so that I do not forget during "format-patch".
>
> When the commit is turned into a patch email, possibly with some
> other material like "--notes=<ref>" that adds notes there, the
> resulting message will have two three-dashes lines, but because "am"
> cuts at the first one, and "apply" knows that the garbage lines at
> front, including three-dash lines, do not matter until it sees "^diff",
> this works out perfectly well.
> 
> Admittedly, I myself do not send out so many patches as I used to,
> but I suspect that there are others who have discovered this trick
> independently, and they would be unhappy to be interrupted by
> commit-msg hook like this.

I do it, too, though not all that often. Once upon a time I had a patch
to teach git-commit to auto-convert lines after "---" into a note (which
would then be formatted back out via format-patch). But I found for my
git.git workflow that just letting the "---" ride along in the commit
object was simpler and easier (since I don't care about having pristine
commit objects, as their ultimate fate is to be dropped in favor of what
is applied upstream).

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 3/3] templates: detect messages that contain a separator line
  2026-02-09  7:00             ` Jeff King
@ 2026-02-09 10:42               ` Phillip Wood
  2026-02-10  6:44                 ` Jeff King
  0 siblings, 1 reply; 65+ messages in thread
From: Phillip Wood @ 2026-02-09 10:42 UTC (permalink / raw)
  To: Jeff King, Junio C Hamano
  Cc: git, Matthias Beyer, Jacob Keller, pyokagan, Kristoffer Haugsbakk

On 09/02/2026 07:00, Jeff King wrote:
> On Sat, Feb 07, 2026 at 01:27:01PM -0800, Junio C Hamano wrote:
> 
>> I have no qualms about the topic up to the previous step, but I know
>> one of the things that I sometimes do will be broken with the change
>> in this step, namely, when I know what I want to write below the
>> three-dash lines, I would commit with "---" and additional notes
>> below it, so that I do not forget during "format-patch".
>>
>> When the commit is turned into a patch email, possibly with some
>> other material like "--notes=<ref>" that adds notes there, the
>> resulting message will have two three-dashes lines, but because "am"
>> cuts at the first one, and "apply" knows that the garbage lines at
>> front, including three-dash lines, do not matter until it sees "^diff",
>> this works out perfectly well.
>>
>> Admittedly, I myself do not send out so many patches as I used to,
>> but I suspect that there are others who have discovered this trick
>> independently, and they would be unhappy to be interrupted by
>> commit-msg hook like this.
> 
> I do it, too, though not all that often. Once upon a time I had a patch
> to teach git-commit to auto-convert lines after "---" into a note (which
> would then be formatted back out via format-patch). But I found for my
> git.git workflow that just letting the "---" ride along in the commit
> object was simpler and easier (since I don't care about having pristine
> commit objects, as their ultimate fate is to be dropped in favor of what
> is applied upstream).

I do it too occasionally. I had planned just to use "--no-verify" when I 
did that but maybe we should just drop this patch. We could make it 
configurable as Kristoffer suggested, or, as we have the raw message, we 
could look for a special comment like "# allow ---" but I'm not sure I 
want to spend much more time on this. At least "---" only truncates the 
message rather than applying an unwanted patch.

Thanks

Phillip


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am"
  2026-02-09  6:57         ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Jeff King
@ 2026-02-09 10:43           ` Phillip Wood
  2026-02-09 11:07             ` Matthias Beyer
  2026-02-10  6:46             ` Jeff King
  0 siblings, 2 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-09 10:43 UTC (permalink / raw)
  To: Jeff King, Phillip Wood; +Cc: git, Matthias Beyer, Jacob Keller, pyokagan

On 09/02/2026 06:57, Jeff King wrote:
> On Sat, Feb 07, 2026 at 02:57:59PM +0000, Phillip Wood wrote:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> On 06/02/2026 09:03, Jeff King wrote:
>>> I don't think there is a way to unambiguously parse the single-stream
>>> output that format-patch produces. This is a reasonably well-known
>>> gotcha (at least around here). E.g., some earlier discussions:
>>>
>>>     2024:https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
>>>     2022:https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
>>>     2015:https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
>>
>> If we cannot improve "git am" perhaps we should update our sample
>> "commit-msg" hook to reject messages that will cause problems. Here
>> are some patches to do that.
> 
> I'm not entirely opposed to it, but my initial reaction was two bits of
> skepticism:
> 
>    1. I imagine that hardly anybody runs commit-msg hooks in the first
>       place, let alone our sample hook. So I doubt this will get the
>       attention of many people.

I think that's fair, but having it in the sample hook doesn't do any harm.

>    2. I'd guess that these days only a small minority of people care
>       about sending patches by email. So for most people, a warning about
>       their commit message containing a diff or "---" will be mostly
>       useless, if not outright confusing.

People do download patches from github and apply them even if they're 
not using a email based workflow. I'm not entirely clear but I think 
that's what happened in the post Matthias linked to. Though if they're 
using "patch" rather than "git am" to apply them indenting the diff wont 
help.

> I'd imagine that documentation updates would be more likely to get read
> by users than the sample hook. And a warning in git-commit itself would
> be even more obvious (but fall even more afoul of (2) above). Adding a
> warning to format-patch would help with (2), but at that point it may be
> too late to change the commit message.

Kristoffer has kindly updated the documentation. I'm wary of adding a 
warning to "git commit" for the reason you gave above. We could make it 
opt-in but then hardly anyone would probably set that config option.

Thanks

Phillip

>> We could perhaps think about adding a more prominent warning to the
>> "git am" and "git format-patch" documentation. The docs for "git am"
>> mention that it splits the message on a line starting with "diff -"
>> but maybe we should spell out what that means for commit messages that
>> include a diff. In principle "git format-patch" could also warn or
>> error out if it creates a mail that "git am" cannot import verbatim,
>> I don't know how hard that would be in implement.
> 
> I think the patch from Matheus linked above added that format-patch
> check.
> 
> -Peff
> 


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am"
  2026-02-09 10:43           ` Phillip Wood
@ 2026-02-09 11:07             ` Matthias Beyer
  2026-02-10  6:46             ` Jeff King
  1 sibling, 0 replies; 65+ messages in thread
From: Matthias Beyer @ 2026-02-09 11:07 UTC (permalink / raw)
  To: phillip.wood; +Cc: Jeff King, git, Jacob Keller, pyokagan

[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]

On Mon, Feb 09, 2026 at 10:43:23AM +0000, Phillip Wood wrote:
> >    2. I'd guess that these days only a small minority of people care
> >       about sending patches by email. So for most people, a warning about
> >       their commit message containing a diff or "---" will be mostly
> >       useless, if not outright confusing.
> 
> People do download patches from github and apply them even if they're not
> using a email based workflow. I'm not entirely clear but I think that's what
> happened in the post Matthias linked to. Though if they're using "patch"
> rather than "git am" to apply them indenting the diff wont help.

Yes, the original post was exactly that issue.

I can add that distributions also do that quite often when they apply
fixes from upstream that have not made it into a package release yet. At
least for the NixOS distribution, we do that quite a lot (a totally
unscientific grep through nixpkgs gave ~2800 instances where we fetch
patches).

Of course it is the obligation of the distribution to check the patches
that are applied to packages. But in this case they of course use
`patch` rather than `git am`. Still, that the diff from a commit message
will be applied as well is something even advanced users do not know
(I myself am using git for over 15 years, and I am comfortable with
email patch based workflows - though, I didn't know about that fact and
would have definitively fallen into that "trap").

> Kristoffer has kindly updated the documentation. I'm wary of adding a
> warning to "git commit" for the reason you gave above. We could make it
> opt-in but then hardly anyone would probably set that config option.

I agree on that part.

Matthias

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-06  9:03     ` Jeff King
  2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
@ 2026-02-09 15:58       ` Patrick Steinhardt
  2026-02-10  2:16         ` Jacob Keller
  2026-02-10  6:56         ` Jeff King
  2026-02-13 14:34       ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
  2 siblings, 2 replies; 65+ messages in thread
From: Patrick Steinhardt @ 2026-02-09 15:58 UTC (permalink / raw)
  To: Jeff King; +Cc: Matthias Beyer, Jacob Keller, git, pyokagan

On Fri, Feb 06, 2026 at 04:03:58AM -0500, Jeff King wrote:
> On Fri, Feb 06, 2026 at 09:18:50AM +0100, Matthias Beyer wrote:
> 
> > That said, I am no expert in either C or the git codebase at all, but
> > from what I saw from reading the git-am codebase, it looks like it tries
> > to find the patch by looking for three dashes on a line with a linebreak
> > behind ("---\n").
> 
> Yes, that is how the split is made.
> 
> > From what I read, it looks for that from the first line.
> > What I would think of here is looking for that "patchbreak" from the
> > _end_ of the email rather than from the top, that would have prevented
> > this issue, right?
> 
> The patch itself may legitimately contain "---" on a line by itself (it
> would indicate that the line "--" was removed from a file). That would
> confuse your parser, including in a way that we end up only applying
> part of the diff (everything before that fake "---" becomes commit
> message, and everything after becomes cover-letter material up to the
> next "diff" line).
> 
> I suspect it also creates corner cases with cover-letter material
> (between the "---" and the diff itself) that itself contains any "---"
> marker.
> 
> I don't think there is a way to unambiguously parse the single-stream
> output that format-patch produces. This is a reasonably well-known
> gotcha (at least around here). E.g., some earlier discussions:
> 
>   2024: https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
>   2022: https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
>   2015: https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
> 
> There are probably more, but it's actually a tricky thing to search for
> in the archive, so I stopped digging. ;)

Maybe we can't parse it unambiguously. But what we _can_ detect is that
a patch is ambiguous in the first place, right? So maybe we could extend
git-am(1) to bail by default with a hint that tells the user that:

  - They ought to double-check the patch.

  - They can override the check with "--accept-ambiguous-patch".

It at least notifies the user that something potentially-fishy is going
on, even though it still shifts the burden onto the person that applies
the patch. But I guess that cannot ever be avoided anyway, at least in
the general case.

Patrick

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-08  0:11 ` [PATCH] doc: add caveat about roundtripping format-patch kristofferhaugsbakk
  2026-02-08  1:39   ` Junio C Hamano
@ 2026-02-09 16:42   ` Phillip Wood
  2026-02-09 17:59     ` Kristoffer Haugsbakk
  2026-02-09 22:37   ` [PATCH v2] " kristofferhaugsbakk
  2026-02-10  0:53   ` [PATCH] doc: add caveat about roundtripping format-patch Christoph Anton Mitterer
  3 siblings, 1 reply; 65+ messages in thread
From: Phillip Wood @ 2026-02-09 16:42 UTC (permalink / raw)
  To: kristofferhaugsbakk, git
  Cc: Kristoffer Haugsbakk, Matthias Beyer, Christoph Anton Mitterer,
	Matheus Tavares, Chris Packham, Jakob Haufe

Hi Kristoffer

Thanks for working on this. I've left a few comments below but I think 
what you have here is pretty good already.

On 08/02/2026 00:11, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> 
> git-format-patch(1), git-send-email(1), and git-am(1) deal with

I found the mention of git-send-email here and in the documentation a 
bit distracting as it doesn't do any formatting itself - it just runs 
"git format-patch"

> † 1: There is also git-commit(1) to consider. However, making that
>       command warn or error out over such delimiters would be disruptive
>       to all Git users who never use email in their workflow.

This reference is formatted differently to the rest.

> [2]: Recently patch(1) caused this issue for a project, but it was noted
>       that git-am(1) has the same behavior[3]
> [3]: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
> [4]: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/

> diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
> new file mode 100644
> index 00000000000..2accf2763fd
> --- /dev/null
> +++ b/Documentation/format-patch-caveats.adoc
> @@ -0,0 +1,39 @@
> +Patches produced by linkgit:git-format-patch[1] or
> +linkgit:git-send-email[1] are inline. This means that the output of
> +these two commands can lead to a different commit message when applied
> +with linkgit:git-am[1]. It can also mean that the patch is not applied
> +correctly.

Is this last sentence referring to diffs in the commit message being 
applied? I don't think there are circumstances where the patch itself is 
not applied correctly.

> +The commit message might contain a three-dash line (`---`) which was
> +perhaps meant to be a thematic break. That means that the commit message
> +will be cut short. The presence of a line starting with "Index: " can
> +cause the patch not to be found, giving an error about an empty patch.
> +
> +Furthermore, the presence of an unindented diff in the commit message
> +will not only cut the message short but cause that very diff to be
> +applied, along with the patch in the patch section. The commit message
> +might for example have a diff in a GitHub MarkDown code fence:
> +
> +----
> +```
> +diff ...
> +```
> +----

I'm not sure the markdown really adds anything here

> +The solution for this is to indent the diff instead:
> +
> +----
> +    diff ...
> +----
> +
> +This loss of fidelity might be simple to notice if you are applying
> +patches directly from a mailbox. However, a commit authored long ago
> +might be applied in a different context, perhaps because many changes
> +are being integrated via patch files and the
> +linkgit:git-format-patch[1] format is trusted to import changes of a
> +Git origin.

This last sentence lost me a bit. Is this talking about commits that 
have been pushed to a forge and then some downloads it as a patch? It 
would certainly be helpful to explain that even if you're not using an 
email based workflow, it is possible to be caught out by these issues.

> +One might want to use a general-purpose utility like patch(1) instead,

"Given these limitations, one might be tempted to ..."?

> +given these limitations. However, patch(1) will not only look for
> +unindented diffs (like linkgit:git-am[1]) but will try to apply indented
> +diffs as well.

This is useful context.

Thanks

Phillip

> diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
> index 0c94776e296..18f5b950825 100644
> --- a/Documentation/git-am.adoc
> +++ b/Documentation/git-am.adoc
> @@ -259,10 +259,13 @@ message.  Any line that is of the form:
>   * a line that begins with "Index: "
>   
>   is taken as the beginning of a patch, and the commit log message
>   is terminated before the first occurrence of such a line.
>   
> +This means that the content of the commit message can inadverently
> +interrupt the processing (see the <<caveats,CAVEATS>> section below).
> +
>   When initially invoking `git am`, you give it the names of the mailboxes
>   to process.  Upon seeing the first patch that does not apply, it
>   aborts in the middle.  You can recover from this in one of two ways:
>   
>   . skip the current patch by re-running the command with the `--skip`
> @@ -281,10 +284,16 @@ Before any patches are applied, ORIG_HEAD is set to the tip of the
>   current branch.  This is useful if you have problems with multiple
>   commits, like running 'git am' on the wrong branch or an error in the
>   commits that is more easily fixed by changing the mailbox (e.g.
>   errors in the "From:" lines).
>   
> +[[caveats]]
> +CAVEATS
> +-------
> +
> +include::format-patch-caveats.adoc[]
> +
>   HOOKS
>   -----
>   This command can run `applypatch-msg`, `pre-applypatch`,
>   and `post-applypatch` hooks.  See linkgit:githooks[5] for more
>   information.
> diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
> index 9a7807ca71a..36851aaf5e1 100644
> --- a/Documentation/git-format-patch.adoc
> +++ b/Documentation/git-format-patch.adoc
> @@ -796,10 +796,14 @@ CAVEATS
>   Note that `format-patch` will omit merge commits from the output, even
>   if they are part of the requested range. A simple "patch" does not
>   include enough information for the receiving end to reproduce the same
>   merge commit.
>   
> +'''
> +
> +include::format-patch-caveats.adoc[]
> +
>   SEE ALSO
>   --------
>   linkgit:git-am[1], linkgit:git-send-email[1]
>   
>   GIT
> diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc
> index ebe8853e9f5..0b118df6498 100644
> --- a/Documentation/git-send-email.adoc
> +++ b/Documentation/git-send-email.adoc
> @@ -690,10 +690,15 @@ Links of a few such community maintained helpers are:
>   	  (cross platform client that can send emails using the ProtonMail API)
>   
>   	- https://github.com/AdityaGarg8/git-credential-email[git-msgraph]
>   	  (cross platform client that can send emails using the Microsoft Graph API)
>   
> +CAVEATS
> +-------
> +
> +include::format-patch-caveats.adoc[]
> +
>   SEE ALSO
>   --------
>   linkgit:git-format-patch[1], linkgit:git-imap-send[1], mbox(5)
>   
>   GIT
> 
> base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-09 16:42   ` Phillip Wood
@ 2026-02-09 17:59     ` Kristoffer Haugsbakk
  2026-02-10 10:57       ` Phillip Wood
  0 siblings, 1 reply; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-09 17:59 UTC (permalink / raw)
  To: Phillip Wood, git
  Cc: Kristoffer Haugsbakk, Matthias Beyer, Christoph Anton Mitterer,
	Matheus Tavares, Chris Packham, Jakob Haufe

Hi Phillip

On Mon, Feb 9, 2026, at 17:42, Phillip Wood wrote:
>[snip]
> On 08/02/2026 00:11, kristofferhaugsbakk@fastmail.com wrote:
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>
>> git-format-patch(1), git-send-email(1), and git-am(1) deal with
>
> I found the mention of git-send-email here and in the documentation a
> bit distracting as it doesn't do any formatting itself - it just runs
> "git format-patch"

Okay, I see now that git-send-email(1) already says that it uses
git-format-patch(1). So we can scratch that command mention. The user
can see from the rest of the git-send-email(1) doc why we have a caveat
about git-format-patch(1).

I first thought that it wouldn’t be obvious why we are talking
about git-format-patch(1) here.

>
>> † 1: There is also git-commit(1) to consider. However, making that
>>       command warn or error out over such delimiters would be disruptive
>>       to all Git users who never use email in their workflow.
>
> This reference is formatted differently to the rest.

Okay, thanks.

I will change to using just one style in the next round. :)

( https://lore.kernel.org/git/doc_am_gitlinks_and_am.messageId.321@msgid.xyz/T/#m38026ad670e866b9ef1a0ef3827fd69316bb1aa3 )

>>[snip]
>> +Patches produced by linkgit:git-format-patch[1] or
>> +linkgit:git-send-email[1] are inline. This means that the output of
>> +these two commands can lead to a different commit message when applied
>> +with linkgit:git-am[1]. It can also mean that the patch is not applied
>> +correctly.
>
> Is this last sentence referring to diffs in the commit message being
> applied? I don't think there are circumstances where the patch itself is
> not applied correctly.

I tested with a line like

    Index x

Yesterday and got an empty patch when running git-am(1). But I couldn’t
reproduce now. I must have made a mistake.

I think this should be changed to:

    It can also mean that the patch that is applied is not the same as
    the one that was generated.

(generated = shorthand for made by git-format-patch(1))

This sentence would then serve as an introduction for the “Furthermore,”
paragraph later.

>>[snip]
>> +----
>> +```
>> +diff ...
>> +```
>> +----
>
> I'm not sure the markdown really adds anything here

I don’t understand? It demonstrates a markup for code which does not use
indentation.

Well, maybe it should be:

    ----
    ```
    diff ...
    ...
    ```
    ----

Or maybe...

    ----
    ```
    diff --git a/example.txt b/example.txt
    ...
    ```
    ----

I’m leaning towards the latter.

>>[snip]
>> +One might want to use a general-purpose utility like patch(1) instead,
>
> "Given these limitations, one might be tempted to ..."?

That’s good. That leads with the problem instead letting it trail off at
the end of the sentence. I’ll use that.

>> +given these limitations. However, patch(1) will not only look for
>> +unindented diffs (like linkgit:git-am[1]) but will try to apply indented
>> +diffs as well.
>
> This is useful context.
>
> Thanks
>
> Phillip

Thanks for taking a look. It’s always appreciated.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2] doc: add caveat about roundtripping format-patch
  2026-02-08  0:11 ` [PATCH] doc: add caveat about roundtripping format-patch kristofferhaugsbakk
  2026-02-08  1:39   ` Junio C Hamano
  2026-02-09 16:42   ` Phillip Wood
@ 2026-02-09 22:37   ` kristofferhaugsbakk
  2026-02-09 22:59     ` Junio C Hamano
                       ` (3 more replies)
  2026-02-10  0:53   ` [PATCH] doc: add caveat about roundtripping format-patch Christoph Anton Mitterer
  3 siblings, 4 replies; 65+ messages in thread
From: kristofferhaugsbakk @ 2026-02-09 22:37 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Matthias Beyer, Christoph Anton Mitterer,
	Matheus Tavares, Chris Packham, Jakob Haufe, Phillip Wood

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

git-format-patch(1) and git-am(1) deal with formatting commits as
patches and applying them, respectively. Naturally they use a few
delimiters to mark where the commit message ends. This can lead to
surprising behavior when these delimiters are used in the commit
message itself.

git-format-patch(1) will accept any commit message and not warn or error
about these delimiters being used.[1]

Especially problematic is the presence of unindented diffs in the commit
message; the patch machinery will naturally (since the commit message
has ended) try to apply that diff and everything after it.[2]

It is unclear whether any commands in this chain will learn to warn
about this. One concern could be that users have learned to rely on
the three-dash line rule to conveniently add extra-commit message
information in the commit message, knowing that git-am(1) will
ignore it.[4]

All of this is covered already, technically. However, we should spell
out the implications.

† 1: There is also git-commit(1) to consider. However, making that
     command warn or error out over such delimiters would be disruptive
     to all Git users who never use email in their workflow.
† 2: Recently patch(1) caused this issue for a project, but it was noted
     that git-am(1) has the same behavior[3]
† 3: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
† 4: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
     https://lore.kernel.org/git/V2_format-patch_caveats.34b@msgid.xyz/

Reported-by: Matthias Beyer <mail@beyermatthias.de>
Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
Reported-by: Matheus Tavares <matheus.tavb@gmail.com>
Reported-by: Chris Packham <judge.packham@gmail.com>
Helped-by: Jakob Haufe <sur5r@sur5r.net>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>

---

v2:

Address feedback from Phillip Wood.

Cc: Phillip Wood <phillip.wood@dunelm.org.uk>

• Drop the code blocks with the diffs; the prose speaks for itself, no need
  to take up space
• Don’t discuss git-send-email(1). We already know that git-format-patch(1)
  is the generator. It is mentioned in git-send-email(1).
• Try to be more clear about the case where someone might be applying a
  diff. Use the example from Matthias Beyer in:

      https://lore.kernel.org/git/gfxpnecn2cdtmeiape2d4x5aybuyyqi4c7m6te3khgct34dd44@wqusigna2nsp/

  Hopefully I explained it correctly?
• Add a “this goes to show...”... which seems to emphasize the point
  without being redundant. Hopefully.

Try to address feedback from Junio C Hamano by adding more nuance: the diff
in the commit message might be applied as well, or the patch machinery
might trip on something and fail.

Finally, in the middle of discussing the three possible cmt. message
delimiters, I noticed that the three points were drifting apart. So I
decided to use the list already used in git-am(1) and be done with it in
one place.

---

It seems that the section break in git-format-patch(1) does not get
applied in the man output (according to `Documentation/doc-diff`
apparently)? Maybe this is the wrong construct? I couldn’t find any
other thematic breaks here (though there are several variations).
---
 Documentation/format-patch-caveats.adoc       | 36 +++++++++++++++++++
 .../format-patch-end-of-commit-message.adoc   |  3 ++
 Documentation/git-am.adoc                     | 15 ++++++--
 Documentation/git-format-patch.adoc           |  4 +++
 Documentation/git-send-email.adoc             |  5 +++
 5 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/format-patch-caveats.adoc
 create mode 100644 Documentation/format-patch-end-of-commit-message.adoc

diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
new file mode 100644
index 00000000000..c666d709742
--- /dev/null
+++ b/Documentation/format-patch-caveats.adoc
@@ -0,0 +1,36 @@
+Patches produced by linkgit:git-format-patch[1] are inline. This means
+that the output from that command can lead to a different commit message
+when applied with linkgit:git-am[1]. It can also mean that the patch
+that is applied is not the same as the one that was generated, or that
+the patch application fails outright.
+ifdef::git-am[]
+See the <<discussion,DISCUSSION>> section above for the syntactic rules.
+endif::git-am[]
+
+ifndef::git-am[]
+Any line that is of the form:
+
+include::format-patch-end-of-commit-message.adoc[]
+
+will terminate the commit message and cause the patch machinery to start
+searching for patches to apply.
+endif::git-am[]
+
+Note that this is especially problematic for unindented diffs that occur
+in the commit message; the diff in the commit message might get applied
+along with the patch section, or the patch application machinery might
+trip up because the patch target doesn't apply. This could for example
+be caused by a diff in a GitHub Markdown code block.
+
+This loss of fidelity might be simple to notice if you are applying
+patches directly from a mailbox. However, changes originating from Git
+could be applied in bulk, in which case this would be much harder to
+notice. This could for example be a Linux distribution which uses patch
+files to apply changes on top of the commits from the upstream
+repositories. This goes to show that this behavior does not only impact
+email workflows.
+
+Given these limitations, one might be tempted to use a general-purpose
+utility like patch(1) instead. However, patch(1) will not only look for
+unindented diffs (like linkgit:git-am[1]) but will try to apply indented
+diffs as well.
diff --git a/Documentation/format-patch-end-of-commit-message.adoc b/Documentation/format-patch-end-of-commit-message.adoc
new file mode 100644
index 00000000000..47399ae7266
--- /dev/null
+++ b/Documentation/format-patch-end-of-commit-message.adoc
@@ -0,0 +1,3 @@
+* three-dashes and end-of-line, or
+* a line that begins with "diff -", or
+* a line that begins with "Index: "
diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
index 0c94776e296..756dfd722b9 100644
--- a/Documentation/git-am.adoc
+++ b/Documentation/git-am.adoc
@@ -231,10 +231,11 @@ applying.
 --allow-empty::
 	After a patch failure on an input e-mail message lacking a patch,
 	create an empty commit with the contents of the e-mail message
 	as its log message.
 
+[[discussion]]
 DISCUSSION
 ----------
 
 The commit author name is taken from the "From: " line of the
 message, and commit author date is taken from the "Date: " line
@@ -252,17 +253,18 @@ where the patch begins.  Excess whitespace at the end of each
 line is automatically stripped.
 
 The patch is expected to be inline, directly following the
 message.  Any line that is of the form:
 
-* three-dashes and end-of-line, or
-* a line that begins with "diff -", or
-* a line that begins with "Index: "
+include::format-patch-end-of-commit-message.adoc[]
 
 is taken as the beginning of a patch, and the commit log message
 is terminated before the first occurrence of such a line.
 
+This means that the contents of the commit message can inadvertently
+interrupt the processing (see the <<caveats,CAVEATS>> section below).
+
 When initially invoking `git am`, you give it the names of the mailboxes
 to process.  Upon seeing the first patch that does not apply, it
 aborts in the middle.  You can recover from this in one of two ways:
 
 . skip the current patch by re-running the command with the `--skip`
@@ -281,10 +283,17 @@ Before any patches are applied, ORIG_HEAD is set to the tip of the
 current branch.  This is useful if you have problems with multiple
 commits, like running 'git am' on the wrong branch or an error in the
 commits that is more easily fixed by changing the mailbox (e.g.
 errors in the "From:" lines).
 
+[[caveats]]
+CAVEATS
+-------
+
+:git-am: 1
+include::format-patch-caveats.adoc[]
+
 HOOKS
 -----
 This command can run `applypatch-msg`, `pre-applypatch`,
 and `post-applypatch` hooks.  See linkgit:githooks[5] for more
 information.
diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
index 9a7807ca71a..36851aaf5e1 100644
--- a/Documentation/git-format-patch.adoc
+++ b/Documentation/git-format-patch.adoc
@@ -796,10 +796,14 @@ CAVEATS
 Note that `format-patch` will omit merge commits from the output, even
 if they are part of the requested range. A simple "patch" does not
 include enough information for the receiving end to reproduce the same
 merge commit.
 
+'''
+
+include::format-patch-caveats.adoc[]
+
 SEE ALSO
 --------
 linkgit:git-am[1], linkgit:git-send-email[1]
 
 GIT
diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc
index ebe8853e9f5..0b118df6498 100644
--- a/Documentation/git-send-email.adoc
+++ b/Documentation/git-send-email.adoc
@@ -690,10 +690,15 @@ Links of a few such community maintained helpers are:
 	  (cross platform client that can send emails using the ProtonMail API)
 
 	- https://github.com/AdityaGarg8/git-credential-email[git-msgraph]
 	  (cross platform client that can send emails using the Microsoft Graph API)
 
+CAVEATS
+-------
+
+include::format-patch-caveats.adoc[]
+
 SEE ALSO
 --------
 linkgit:git-format-patch[1], linkgit:git-imap-send[1], mbox(5)
 
 GIT

Interdiff against v1:
  diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
  index 2accf2763fd..c666d709742 100644
  --- a/Documentation/format-patch-caveats.adoc
  +++ b/Documentation/format-patch-caveats.adoc
  @@ -1,39 +1,36 @@
  -Patches produced by linkgit:git-format-patch[1] or
  -linkgit:git-send-email[1] are inline. This means that the output of
  -these two commands can lead to a different commit message when applied
  -with linkgit:git-am[1]. It can also mean that the patch is not applied
  -correctly.
  +Patches produced by linkgit:git-format-patch[1] are inline. This means
  +that the output from that command can lead to a different commit message
  +when applied with linkgit:git-am[1]. It can also mean that the patch
  +that is applied is not the same as the one that was generated, or that
  +the patch application fails outright.
  +ifdef::git-am[]
  +See the <<discussion,DISCUSSION>> section above for the syntactic rules.
  +endif::git-am[]
   
  -The commit message might contain a three-dash line (`---`) which was
  -perhaps meant to be a thematic break. That means that the commit message
  -will be cut short. The presence of a line starting with "Index: " can
  -cause the patch not to be found, giving an error about an empty patch.
  +ifndef::git-am[]
  +Any line that is of the form:
   
  -Furthermore, the presence of an unindented diff in the commit message
  -will not only cut the message short but cause that very diff to be
  -applied, along with the patch in the patch section. The commit message
  -might for example have a diff in a GitHub MarkDown code fence:
  +include::format-patch-end-of-commit-message.adoc[]
   
  -----
  -```
  -diff ...
  -```
  -----
  +will terminate the commit message and cause the patch machinery to start
  +searching for patches to apply.
  +endif::git-am[]
   
  -The solution for this is to indent the diff instead:
  -
  -----
  -    diff ...
  -----
  +Note that this is especially problematic for unindented diffs that occur
  +in the commit message; the diff in the commit message might get applied
  +along with the patch section, or the patch application machinery might
  +trip up because the patch target doesn't apply. This could for example
  +be caused by a diff in a GitHub Markdown code block.
   
   This loss of fidelity might be simple to notice if you are applying
  -patches directly from a mailbox. However, a commit authored long ago
  -might be applied in a different context, perhaps because many changes
  -are being integrated via patch files and the
  -linkgit:git-format-patch[1] format is trusted to import changes of a
  -Git origin.
  +patches directly from a mailbox. However, changes originating from Git
  +could be applied in bulk, in which case this would be much harder to
  +notice. This could for example be a Linux distribution which uses patch
  +files to apply changes on top of the commits from the upstream
  +repositories. This goes to show that this behavior does not only impact
  +email workflows.
   
  -One might want to use a general-purpose utility like patch(1) instead,
  -given these limitations. However, patch(1) will not only look for
  +Given these limitations, one might be tempted to use a general-purpose
  +utility like patch(1) instead. However, patch(1) will not only look for
   unindented diffs (like linkgit:git-am[1]) but will try to apply indented
   diffs as well.
  diff --git a/Documentation/format-patch-end-of-commit-message.adoc b/Documentation/format-patch-end-of-commit-message.adoc
  new file mode 100644
  index 00000000000..47399ae7266
  --- /dev/null
  +++ b/Documentation/format-patch-end-of-commit-message.adoc
  @@ -0,0 +1,3 @@
  +* three-dashes and end-of-line, or
  +* a line that begins with "diff -", or
  +* a line that begins with "Index: "
  diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
  index 18f5b950825..756dfd722b9 100644
  --- a/Documentation/git-am.adoc
  +++ b/Documentation/git-am.adoc
  @@ -231,10 +231,11 @@ applying.
   --allow-empty::
   	After a patch failure on an input e-mail message lacking a patch,
   	create an empty commit with the contents of the e-mail message
   	as its log message.
   
  +[[discussion]]
   DISCUSSION
   ----------
   
   The commit author name is taken from the "From: " line of the
   message, and commit author date is taken from the "Date: " line
  @@ -252,18 +253,16 @@ where the patch begins.  Excess whitespace at the end of each
   line is automatically stripped.
   
   The patch is expected to be inline, directly following the
   message.  Any line that is of the form:
   
  -* three-dashes and end-of-line, or
  -* a line that begins with "diff -", or
  -* a line that begins with "Index: "
  +include::format-patch-end-of-commit-message.adoc[]
   
   is taken as the beginning of a patch, and the commit log message
   is terminated before the first occurrence of such a line.
   
  -This means that the content of the commit message can inadverently
  +This means that the contents of the commit message can inadvertently
   interrupt the processing (see the <<caveats,CAVEATS>> section below).
   
   When initially invoking `git am`, you give it the names of the mailboxes
   to process.  Upon seeing the first patch that does not apply, it
   aborts in the middle.  You can recover from this in one of two ways:
  @@ -288,10 +287,11 @@ errors in the "From:" lines).
   
   [[caveats]]
   CAVEATS
   -------
   
  +:git-am: 1
   include::format-patch-caveats.adoc[]
   
   HOOKS
   -----
   This command can run `applypatch-msg`, `pre-applypatch`,

Range-diff against v1:
1:  4bed8f55b98 ! 1:  c54f394bb33 doc: add caveat about roundtripping format-patch
    @@ Metadata
      ## Commit message ##
         doc: add caveat about roundtripping format-patch
     
    -    git-format-patch(1), git-send-email(1), and git-am(1) deal with
    -    formatting commits as patches, sending them (perhaps directly), and
    -    applying them, respectively. Naturally they use a few delimiters to mark
    -    where the commit message ends. This can lead to surprising behavior when
    -    these delimiters are used in the commit message itself.
    +    git-format-patch(1) and git-am(1) deal with formatting commits as
    +    patches and applying them, respectively. Naturally they use a few
    +    delimiters to mark where the commit message ends. This can lead to
    +    surprising behavior when these delimiters are used in the commit
    +    message itself.
     
    -    git-format-patch(1) and git-send-email(1) will accept any commit message
    -    and not warn or error about these delimiters being used.[1]
    +    git-format-patch(1) will accept any commit message and not warn or error
    +    about these delimiters being used.[1]
     
    -    Moreover, the presence of unindented diffs in the commit message will
    -    cause git-am(1) to apply both the diffs from the commit message as well
    -    as the patch section.[2]
    +    Especially problematic is the presence of unindented diffs in the commit
    +    message; the patch machinery will naturally (since the commit message
    +    has ended) try to apply that diff and everything after it.[2]
     
         It is unclear whether any commands in this chain will learn to warn
         about this. One concern could be that users have learned to rely on
    @@ Commit message
         information in the commit message, knowing that git-am(1) will
         ignore it.[4]
     
    -    All of this is covered already, technically, However, we should spell
    +    All of this is covered already, technically. However, we should spell
         out the implications.
     
         † 1: There is also git-commit(1) to consider. However, making that
              command warn or error out over such delimiters would be disruptive
              to all Git users who never use email in their workflow.
    -    [2]: Recently patch(1) caused this issue for a project, but it was noted
    +    † 2: Recently patch(1) caused this issue for a project, but it was noted
              that git-am(1) has the same behavior[3]
    -    [3]: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
    -    [4]: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
    +    † 3: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
    +    † 4: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
    +         https://lore.kernel.org/git/V2_format-patch_caveats.34b@msgid.xyz/
     
         Reported-by: Matthias Beyer <mail@beyermatthias.de>
         Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
    @@ Commit message
         Helped-by: Jakob Haufe <sur5r@sur5r.net>
         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
     
    +    ---
    +
    +    v2:
    +
    +    Address feedback from Phillip Wood.
    +
    +    Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
    +
    +    • Drop the code blocks with the diffs; the prose speaks for itself, no need
    +      to take up space
    +    • Don’t discuss git-send-email(1). We already know that git-format-patch(1)
    +      is the generator. It is mentioned in git-send-email(1).
    +    • Try to be more clear about the case where someone might be applying a
    +      diff. Use the example from Matthias Beyer in:
    +
    +          https://lore.kernel.org/git/gfxpnecn2cdtmeiape2d4x5aybuyyqi4c7m6te3khgct34dd44@wqusigna2nsp/
    +
    +      Hopefully I explained it correctly?
    +    • Add a “this goes to show...”... which seems to emphasize the point
    +      without being redundant. Hopefully.
    +
    +    Try to address feedback from Junio C Hamano by adding more nuance: the diff
    +    in the commit message might be applied as well, or the patch machinery
    +    might trip on something and fail.
    +
    +    Finally, in the middle of discussing the three possible cmt. message
    +    delimiters, I noticed that the three points were drifting apart. So I
    +    decided to use the list already used in git-am(1) and be done with it in
    +    one place.
    +
    +    ---
    +
    +    It seems that the section break in git-format-patch(1) does not get
    +    applied in the man output (according to `Documentation/doc-diff`
    +    apparently)? Maybe this is the wrong construct? I couldn’t find any
    +    other thematic breaks here (though there are several variations).
    +
      ## Documentation/format-patch-caveats.adoc (new) ##
     @@
    -+Patches produced by linkgit:git-format-patch[1] or
    -+linkgit:git-send-email[1] are inline. This means that the output of
    -+these two commands can lead to a different commit message when applied
    -+with linkgit:git-am[1]. It can also mean that the patch is not applied
    -+correctly.
    -+
    -+The commit message might contain a three-dash line (`---`) which was
    -+perhaps meant to be a thematic break. That means that the commit message
    -+will be cut short. The presence of a line starting with "Index: " can
    -+cause the patch not to be found, giving an error about an empty patch.
    ++Patches produced by linkgit:git-format-patch[1] are inline. This means
    ++that the output from that command can lead to a different commit message
    ++when applied with linkgit:git-am[1]. It can also mean that the patch
    ++that is applied is not the same as the one that was generated, or that
    ++the patch application fails outright.
    ++ifdef::git-am[]
    ++See the <<discussion,DISCUSSION>> section above for the syntactic rules.
    ++endif::git-am[]
     +
    -+Furthermore, the presence of an unindented diff in the commit message
    -+will not only cut the message short but cause that very diff to be
    -+applied, along with the patch in the patch section. The commit message
    -+might for example have a diff in a GitHub MarkDown code fence:
    ++ifndef::git-am[]
    ++Any line that is of the form:
     +
    -+----
    -+```
    -+diff ...
    -+```
    -+----
    ++include::format-patch-end-of-commit-message.adoc[]
     +
    -+The solution for this is to indent the diff instead:
    ++will terminate the commit message and cause the patch machinery to start
    ++searching for patches to apply.
    ++endif::git-am[]
     +
    -+----
    -+    diff ...
    -+----
    ++Note that this is especially problematic for unindented diffs that occur
    ++in the commit message; the diff in the commit message might get applied
    ++along with the patch section, or the patch application machinery might
    ++trip up because the patch target doesn't apply. This could for example
    ++be caused by a diff in a GitHub Markdown code block.
     +
     +This loss of fidelity might be simple to notice if you are applying
    -+patches directly from a mailbox. However, a commit authored long ago
    -+might be applied in a different context, perhaps because many changes
    -+are being integrated via patch files and the
    -+linkgit:git-format-patch[1] format is trusted to import changes of a
    -+Git origin.
    ++patches directly from a mailbox. However, changes originating from Git
    ++could be applied in bulk, in which case this would be much harder to
    ++notice. This could for example be a Linux distribution which uses patch
    ++files to apply changes on top of the commits from the upstream
    ++repositories. This goes to show that this behavior does not only impact
    ++email workflows.
     +
    -+One might want to use a general-purpose utility like patch(1) instead,
    -+given these limitations. However, patch(1) will not only look for
    ++Given these limitations, one might be tempted to use a general-purpose
    ++utility like patch(1) instead. However, patch(1) will not only look for
     +unindented diffs (like linkgit:git-am[1]) but will try to apply indented
     +diffs as well.
     
    + ## Documentation/format-patch-end-of-commit-message.adoc (new) ##
    +@@
    ++* three-dashes and end-of-line, or
    ++* a line that begins with "diff -", or
    ++* a line that begins with "Index: "
    +
      ## Documentation/git-am.adoc ##
    -@@ Documentation/git-am.adoc: message.  Any line that is of the form:
    +@@ Documentation/git-am.adoc: applying.
    + 	create an empty commit with the contents of the e-mail message
    + 	as its log message.
    + 
    ++[[discussion]]
    + DISCUSSION
    + ----------
    + 
    +@@ Documentation/git-am.adoc: line is automatically stripped.
    + The patch is expected to be inline, directly following the
    + message.  Any line that is of the form:
    + 
    +-* three-dashes and end-of-line, or
    +-* a line that begins with "diff -", or
    +-* a line that begins with "Index: "
    ++include::format-patch-end-of-commit-message.adoc[]
    + 
      is taken as the beginning of a patch, and the commit log message
      is terminated before the first occurrence of such a line.
      
    -+This means that the content of the commit message can inadverently
    ++This means that the contents of the commit message can inadvertently
     +interrupt the processing (see the <<caveats,CAVEATS>> section below).
     +
      When initially invoking `git am`, you give it the names of the mailboxes
    @@ Documentation/git-am.adoc: commits, like running 'git am' on the wrong branch or
     +CAVEATS
     +-------
     +
    ++:git-am: 1
     +include::format-patch-caveats.adoc[]
     +
      HOOKS

base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc
-- 
2.53.0.26.g2afa8602a26


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH v2] doc: add caveat about roundtripping format-patch
  2026-02-09 22:37   ` [PATCH v2] " kristofferhaugsbakk
@ 2026-02-09 22:59     ` Junio C Hamano
  2026-02-09 23:11       ` Kristoffer Haugsbakk
  2026-02-10 11:02     ` Phillip Wood
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2026-02-09 22:59 UTC (permalink / raw)
  To: kristofferhaugsbakk
  Cc: git, Kristoffer Haugsbakk, Matthias Beyer,
	Christoph Anton Mitterer, Matheus Tavares, Chris Packham,
	Jakob Haufe, Phillip Wood

kristofferhaugsbakk@fastmail.com writes:

> diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
> new file mode 100644
> index 00000000000..c666d709742
> --- /dev/null
> +++ b/Documentation/format-patch-caveats.adoc
> @@ -0,0 +1,36 @@
> +Patches produced by linkgit:git-format-patch[1] are inline. This means
> +that the output from that command can lead to a different commit message
> +when applied with linkgit:git-am[1]. It can also mean that the patch
> +that is applied is not the same as the one that was generated, or that
> +the patch application fails outright.
> +ifdef::git-am[]
> +See the <<discussion,DISCUSSION>> section above for the syntactic rules.
> +endif::git-am[]

It is news to me that adjective "inline" has such a meaning.

Whenever I see somebody writes "X. This means Y", I try to see if it
makes the result easier to understand to more people by just saying
"Y" without mentioning X, and to me, this is such an occasion.  I'd
rather see that sentence, plus "This means", taken away.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2] doc: add caveat about roundtripping format-patch
  2026-02-09 22:59     ` Junio C Hamano
@ 2026-02-09 23:11       ` Kristoffer Haugsbakk
  0 siblings, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-09 23:11 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Kristoffer Haugsbakk, Matthias Beyer,
	Christoph Anton Mitterer, Matheus Tavares, Chris Packham,
	Jakob Haufe, Phillip Wood

On Mon, Feb 9, 2026, at 23:59, Junio C Hamano wrote:
> kristofferhaugsbakk@fastmail.com writes:
>
>> diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
>> new file mode 100644
>> index 00000000000..c666d709742
>> --- /dev/null
>> +++ b/Documentation/format-patch-caveats.adoc
>> @@ -0,0 +1,36 @@
>> +Patches produced by linkgit:git-format-patch[1] are inline. This means
>> +that the output from that command can lead to a different commit message
>> +when applied with linkgit:git-am[1]. It can also mean that the patch
>> +that is applied is not the same as the one that was generated, or that
>> +the patch application fails outright.
>> +ifdef::git-am[]
>> +See the <<discussion,DISCUSSION>> section above for the syntactic rules.
>> +endif::git-am[]
>
> It is news to me that adjective "inline" has such a meaning.

The original intent was to emphasize that the commit message and the
patch being in the same “string” means that there has to be some
delimiter. And that can trip things up since there is no escaping.

But you’re right. This part can be dropped. It is already clear that we
are talking about delimiters that can occur in the commit message.

>
> Whenever I see somebody writes "X. This means Y", I try to see if it
> makes the result easier to understand to more people by just saying
> "Y" without mentioning X, and to me, this is such an occasion.  I'd
> rather see that sentence, plus "This means", taken away.

So write it like this:

    The output from git-patch-format(1) can lead to a different commit
    message ...

I’ll make that change.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-08  0:11 ` [PATCH] doc: add caveat about roundtripping format-patch kristofferhaugsbakk
                     ` (2 preceding siblings ...)
  2026-02-09 22:37   ` [PATCH v2] " kristofferhaugsbakk
@ 2026-02-10  0:53   ` Christoph Anton Mitterer
  2026-02-10 16:00     ` Kristoffer Haugsbakk
  3 siblings, 1 reply; 65+ messages in thread
From: Christoph Anton Mitterer @ 2026-02-10  0:53 UTC (permalink / raw)
  To: git

Hey.

While it's nice to see it getting documented (thanks for that)...
wouldn't it be even better to actually fix the underlying issue? :-)

I mean it's all but guaranteed that everyone reads this,... and IMO the
problem might even be exploited security wise.

Cheers,
Chris.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-09 15:58       ` git-am applies commit message diffs Patrick Steinhardt
@ 2026-02-10  2:16         ` Jacob Keller
  2026-02-10 14:22           ` Patrick Steinhardt
  2026-02-10  6:56         ` Jeff King
  1 sibling, 1 reply; 65+ messages in thread
From: Jacob Keller @ 2026-02-10  2:16 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Jeff King, Matthias Beyer, git, pyokagan

On Mon, Feb 9, 2026 at 7:59 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Fri, Feb 06, 2026 at 04:03:58AM -0500, Jeff King wrote:
> > On Fri, Feb 06, 2026 at 09:18:50AM +0100, Matthias Beyer wrote:
> >
> > > That said, I am no expert in either C or the git codebase at all, but
> > > from what I saw from reading the git-am codebase, it looks like it tries
> > > to find the patch by looking for three dashes on a line with a linebreak
> > > behind ("---\n").
> >
> > Yes, that is how the split is made.
> >
> > > From what I read, it looks for that from the first line.
> > > What I would think of here is looking for that "patchbreak" from the
> > > _end_ of the email rather than from the top, that would have prevented
> > > this issue, right?
> >
> > The patch itself may legitimately contain "---" on a line by itself (it
> > would indicate that the line "--" was removed from a file). That would
> > confuse your parser, including in a way that we end up only applying
> > part of the diff (everything before that fake "---" becomes commit
> > message, and everything after becomes cover-letter material up to the
> > next "diff" line).
> >
> > I suspect it also creates corner cases with cover-letter material
> > (between the "---" and the diff itself) that itself contains any "---"
> > marker.
> >
> > I don't think there is a way to unambiguously parse the single-stream
> > output that format-patch produces. This is a reasonably well-known
> > gotcha (at least around here). E.g., some earlier discussions:
> >
> >   2024: https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
> >   2022: https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
> >   2015: https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
> >
> > There are probably more, but it's actually a tricky thing to search for
> > in the archive, so I stopped digging. ;)
>
> Maybe we can't parse it unambiguously. But what we _can_ detect is that
> a patch is ambiguous in the first place, right? So maybe we could extend
> git-am(1) to bail by default with a hint that tells the user that:
>

I think it might make sense in a breaking change to update format
patch and git am to have an "unambiguous" mode which would allow
somehow to unambiguously distinguish between commit message contents
and patch data. I'm not 100% sure how to do this, and it likely
requires some sort of breaking changes to both tools to allow
distinguishing properly between the two points. Obviously if you're
sending the contents together, a malicious user could edit the
formatted patch to move or copy whatever the "signifier" for patch vs
commit separator is... but at least we'd prevent the cases where
someone accidentally includes diffs without intending to.

>   - They ought to double-check the patch.
>
>   - They can override the check with "--accept-ambiguous-patch".
>
> It at least notifies the user that something potentially-fishy is going
> on, even though it still shifts the burden onto the person that applies
> the patch. But I guess that cannot ever be avoided anyway, at least in
> the general case.
>
> Patrick

These steps also make sense... check the commit content for a diff and
if we see one, make sure to warn and not allow it by default?

I'm unsure how the receiver end could detect the patch actually is
unambiguous since multiple different diff hunks can exist to handle
each file. We could improve the parser to complain about the extra ---
separators though?

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 3/3] templates: detect messages that contain a separator line
  2026-02-09 10:42               ` Phillip Wood
@ 2026-02-10  6:44                 ` Jeff King
  0 siblings, 0 replies; 65+ messages in thread
From: Jeff King @ 2026-02-10  6:44 UTC (permalink / raw)
  To: phillip.wood
  Cc: Junio C Hamano, git, Matthias Beyer, Jacob Keller, pyokagan,
	Kristoffer Haugsbakk

On Mon, Feb 09, 2026 at 10:42:49AM +0000, Phillip Wood wrote:

> > I do it, too, though not all that often. Once upon a time I had a patch
> > to teach git-commit to auto-convert lines after "---" into a note (which
> > would then be formatted back out via format-patch). But I found for my
> > git.git workflow that just letting the "---" ride along in the commit
> > object was simpler and easier (since I don't care about having pristine
> > commit objects, as their ultimate fate is to be dropped in favor of what
> > is applied upstream).
> 
> I do it too occasionally. I had planned just to use "--no-verify" when I did
> that but maybe we should just drop this patch. We could make it configurable
> as Kristoffer suggested, or, as we have the raw message, we could look for a
> special comment like "# allow ---" but I'm not sure I want to spend much
> more time on this. At least "---" only truncates the message rather than
> applying an unwanted patch.

Just to be clear, I am OK either way, as I do not use the sample
commit-msg hook. ;) Since the sample is mostly for illustrative
purposes, maybe it is fine to potentially over-reach and let people trim
it as they see fit.

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am"
  2026-02-09 10:43           ` Phillip Wood
  2026-02-09 11:07             ` Matthias Beyer
@ 2026-02-10  6:46             ` Jeff King
  1 sibling, 0 replies; 65+ messages in thread
From: Jeff King @ 2026-02-10  6:46 UTC (permalink / raw)
  To: phillip.wood; +Cc: git, Matthias Beyer, Jacob Keller, pyokagan

On Mon, Feb 09, 2026 at 10:43:23AM +0000, Phillip Wood wrote:

> >    2. I'd guess that these days only a small minority of people care
> >       about sending patches by email. So for most people, a warning about
> >       their commit message containing a diff or "---" will be mostly
> >       useless, if not outright confusing.
> 
> People do download patches from github and apply them even if they're not
> using a email based workflow. I'm not entirely clear but I think that's what
> happened in the post Matthias linked to. Though if they're using "patch"
> rather than "git am" to apply them indenting the diff wont help.

Yeah, true. I have done that (thought not very often). I think limiting
our thinking to "git am" in that case is probably OK. We have to draw
the line somewhere.

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-09 15:58       ` git-am applies commit message diffs Patrick Steinhardt
  2026-02-10  2:16         ` Jacob Keller
@ 2026-02-10  6:56         ` Jeff King
  1 sibling, 0 replies; 65+ messages in thread
From: Jeff King @ 2026-02-10  6:56 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Matthias Beyer, Jacob Keller, git, pyokagan

On Mon, Feb 09, 2026 at 04:58:51PM +0100, Patrick Steinhardt wrote:

> > I don't think there is a way to unambiguously parse the single-stream
> > output that format-patch produces. This is a reasonably well-known
> > gotcha (at least around here). E.g., some earlier discussions:
> > 
> >   2024: https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
> >   2022: https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
> >   2015: https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
> > 
> > There are probably more, but it's actually a tricky thing to search for
> > in the archive, so I stopped digging. ;)
> 
> Maybe we can't parse it unambiguously. But what we _can_ detect is that
> a patch is ambiguous in the first place, right? So maybe we could extend
> git-am(1) to bail by default with a hint that tells the user that:
> 
>   - They ought to double-check the patch.
> 
>   - They can override the check with "--accept-ambiguous-patch".
> 
> It at least notifies the user that something potentially-fishy is going
> on, even though it still shifts the burden onto the person that applies
> the patch. But I guess that cannot ever be avoided anyway, at least in
> the general case.

Yes, I think you could detect ambiguous cases on the receiving side. You
might need some heuristics to reduce false positives, though, since it
is permitted to include extra content between and after diffs (e.g.,
format-patch writes signature lines by default).

So you'd probably need some rules like:

  - Multiple instances of "---" always generate a warning. Though I
    won't be surprised if it turns out that people often do:

       the commit message

       Signed-off-by: etc...
       ---
       Here is some cover letter material.

       ---
         [diffstat goes here]

    That's totally fine, but indistinguishable from the case that the
    commit message contains a "---" and is being truncated.

  - Presence of "diff" header before "---", which means there is
    probably a diff inside the commit message. But then what about when
    there is no "---" at all (as in a non-git patch)? Maybe the rule
    needs to be "there is a --- line after a diff header" or something.

  - Presence of non-empty text lines after a "diff" header (but not at
    the end, which would trigger pointlessly on signature lines). We
    would never generate this with format-patch, but it is historically
    allowed. I sometimes use it when talking through a "something like
    this..." patch. I don't expect those to become real commits, but I
    imagine people do apply them sometimes.

Of course you can sweep all of the false positives under the "well,
you'll have to re-run with --accept-ambiguous-patch" rug. But we would
want to make sure we do not require that often enough to be annoying.

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-09 17:59     ` Kristoffer Haugsbakk
@ 2026-02-10 10:57       ` Phillip Wood
  2026-02-10 16:00         ` Kristoffer Haugsbakk
  0 siblings, 1 reply; 65+ messages in thread
From: Phillip Wood @ 2026-02-10 10:57 UTC (permalink / raw)
  To: Kristoffer Haugsbakk, Phillip Wood, git
  Cc: Kristoffer Haugsbakk, Matthias Beyer, Christoph Anton Mitterer,
	Matheus Tavares, Chris Packham, Jakob Haufe

Hi Kristoffer

On 09/02/2026 17:59, Kristoffer Haugsbakk wrote:
> Hi Phillip
> On Mon, Feb 9, 2026, at 17:42, Phillip Wood wrote:
>> On 08/02/2026 00:11, kristofferhaugsbakk@fastmail.com wrote:
>>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>>
>>> [snip]
>>> +Patches produced by linkgit:git-format-patch[1] or
>>> +linkgit:git-send-email[1] are inline. This means that the output of
>>> +these two commands can lead to a different commit message when applied
>>> +with linkgit:git-am[1]. It can also mean that the patch is not applied
>>> +correctly.
>>
>> Is this last sentence referring to diffs in the commit message being
>> applied? I don't think there are circumstances where the patch itself is
>> not applied correctly.
> 
> I tested with a line like
> 
>      Index x
> 
> Yesterday and got an empty patch when running git-am(1). But I couldn’t
> reproduce now. I must have made a mistake.

Oh, if you use "Index: x" (with a colon) does that mess up the patch 
application?

> 
> I think this should be changed to:
> 
>      It can also mean that the patch that is applied is not the same as
>      the one that was generated.

That's a nice concise way of putting it

> 
> (generated = shorthand for made by git-format-patch(1))
> 
> This sentence would then serve as an introduction for the “Furthermore,”
> paragraph later.
> 
>>> [snip]
>>> +----
>>> +```
>>> +diff ...
>>> +```
>>> +----
>>
>> I'm not sure the markdown really adds anything here
> 
> I don’t understand? It demonstrates a markup for code which does not use
> indentation.

But I think the markup is a distraction from the problem which is that 
the diff is not indented. Also calling it "Github MarkDown" is 
unfortunate as we try not to favor one forge over another and many sites 
support that syntax.


Thanks

Phillip

> Well, maybe it should be:
> 
>      ----
>      ```
>      diff ...
>      ...
>      ```
>      ----
> 
> Or maybe...
> 
>      ----
>      ```
>      diff --git a/example.txt b/example.txt
>      ...
>      ```
>      ----
> 
> I’m leaning towards the latter.
> 
>>> [snip]
>>> +One might want to use a general-purpose utility like patch(1) instead,
>>
>> "Given these limitations, one might be tempted to ..."?
> 
> That’s good. That leads with the problem instead letting it trail off at
> the end of the sentence. I’ll use that.
> 
>>> +given these limitations. However, patch(1) will not only look for
>>> +unindented diffs (like linkgit:git-am[1]) but will try to apply indented
>>> +diffs as well.
>>
>> This is useful context.
>>
>> Thanks
>>
>> Phillip
> 
> Thanks for taking a look. It’s always appreciated.


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2] doc: add caveat about roundtripping format-patch
  2026-02-09 22:37   ` [PATCH v2] " kristofferhaugsbakk
  2026-02-09 22:59     ` Junio C Hamano
@ 2026-02-10 11:02     ` Phillip Wood
  2026-02-10 18:20     ` Kristoffer Haugsbakk
  2026-02-12 22:28     ` [PATCH v3] doc: add caveat about round-tripping format-patch kristofferhaugsbakk
  3 siblings, 0 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-10 11:02 UTC (permalink / raw)
  To: kristofferhaugsbakk, git
  Cc: Kristoffer Haugsbakk, Matthias Beyer, Christoph Anton Mitterer,
	Matheus Tavares, Chris Packham, Jakob Haufe, Phillip Wood

Hi Kristoffer

This looks good to me modulo the comments about "Github MarkDown" I 
mentioned in my other mail.

Thanks for working on this, it is a nice improvement to our documentation.

Phillip

On 09/02/2026 22:37, kristofferhaugsbakk@fastmail.com wrote:
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> 
> git-format-patch(1) and git-am(1) deal with formatting commits as
> patches and applying them, respectively. Naturally they use a few
> delimiters to mark where the commit message ends. This can lead to
> surprising behavior when these delimiters are used in the commit
> message itself.
> 
> git-format-patch(1) will accept any commit message and not warn or error
> about these delimiters being used.[1]
> 
> Especially problematic is the presence of unindented diffs in the commit
> message; the patch machinery will naturally (since the commit message
> has ended) try to apply that diff and everything after it.[2]
> 
> It is unclear whether any commands in this chain will learn to warn
> about this. One concern could be that users have learned to rely on
> the three-dash line rule to conveniently add extra-commit message
> information in the commit message, knowing that git-am(1) will
> ignore it.[4]
> 
> All of this is covered already, technically. However, we should spell
> out the implications.
> 
> † 1: There is also git-commit(1) to consider. However, making that
>       command warn or error out over such delimiters would be disruptive
>       to all Git users who never use email in their workflow.
> † 2: Recently patch(1) caused this issue for a project, but it was noted
>       that git-am(1) has the same behavior[3]
> † 3: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
> † 4: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
>       https://lore.kernel.org/git/V2_format-patch_caveats.34b@msgid.xyz/
> 
> Reported-by: Matthias Beyer <mail@beyermatthias.de>
> Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
> Reported-by: Matheus Tavares <matheus.tavb@gmail.com>
> Reported-by: Chris Packham <judge.packham@gmail.com>
> Helped-by: Jakob Haufe <sur5r@sur5r.net>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> 
> ---
> 
> v2:
> 
> Address feedback from Phillip Wood.
> 
> Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
> 
> • Drop the code blocks with the diffs; the prose speaks for itself, no need
>    to take up space
> • Don’t discuss git-send-email(1). We already know that git-format-patch(1)
>    is the generator. It is mentioned in git-send-email(1).
> • Try to be more clear about the case where someone might be applying a
>    diff. Use the example from Matthias Beyer in:
> 
>        https://lore.kernel.org/git/gfxpnecn2cdtmeiape2d4x5aybuyyqi4c7m6te3khgct34dd44@wqusigna2nsp/
> 
>    Hopefully I explained it correctly?
> • Add a “this goes to show...”... which seems to emphasize the point
>    without being redundant. Hopefully.
> 
> Try to address feedback from Junio C Hamano by adding more nuance: the diff
> in the commit message might be applied as well, or the patch machinery
> might trip on something and fail.
> 
> Finally, in the middle of discussing the three possible cmt. message
> delimiters, I noticed that the three points were drifting apart. So I
> decided to use the list already used in git-am(1) and be done with it in
> one place.
> 
> ---
> 
> It seems that the section break in git-format-patch(1) does not get
> applied in the man output (according to `Documentation/doc-diff`
> apparently)? Maybe this is the wrong construct? I couldn’t find any
> other thematic breaks here (though there are several variations).
> ---
>   Documentation/format-patch-caveats.adoc       | 36 +++++++++++++++++++
>   .../format-patch-end-of-commit-message.adoc   |  3 ++
>   Documentation/git-am.adoc                     | 15 ++++++--
>   Documentation/git-format-patch.adoc           |  4 +++
>   Documentation/git-send-email.adoc             |  5 +++
>   5 files changed, 60 insertions(+), 3 deletions(-)
>   create mode 100644 Documentation/format-patch-caveats.adoc
>   create mode 100644 Documentation/format-patch-end-of-commit-message.adoc
> 
> diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
> new file mode 100644
> index 00000000000..c666d709742
> --- /dev/null
> +++ b/Documentation/format-patch-caveats.adoc
> @@ -0,0 +1,36 @@
> +Patches produced by linkgit:git-format-patch[1] are inline. This means
> +that the output from that command can lead to a different commit message
> +when applied with linkgit:git-am[1]. It can also mean that the patch
> +that is applied is not the same as the one that was generated, or that
> +the patch application fails outright.
> +ifdef::git-am[]
> +See the <<discussion,DISCUSSION>> section above for the syntactic rules.
> +endif::git-am[]
> +
> +ifndef::git-am[]
> +Any line that is of the form:
> +
> +include::format-patch-end-of-commit-message.adoc[]
> +
> +will terminate the commit message and cause the patch machinery to start
> +searching for patches to apply.
> +endif::git-am[]
> +
> +Note that this is especially problematic for unindented diffs that occur
> +in the commit message; the diff in the commit message might get applied
> +along with the patch section, or the patch application machinery might
> +trip up because the patch target doesn't apply. This could for example
> +be caused by a diff in a GitHub Markdown code block.
> +
> +This loss of fidelity might be simple to notice if you are applying
> +patches directly from a mailbox. However, changes originating from Git
> +could be applied in bulk, in which case this would be much harder to
> +notice. This could for example be a Linux distribution which uses patch
> +files to apply changes on top of the commits from the upstream
> +repositories. This goes to show that this behavior does not only impact
> +email workflows.
> +
> +Given these limitations, one might be tempted to use a general-purpose
> +utility like patch(1) instead. However, patch(1) will not only look for
> +unindented diffs (like linkgit:git-am[1]) but will try to apply indented
> +diffs as well.
> diff --git a/Documentation/format-patch-end-of-commit-message.adoc b/Documentation/format-patch-end-of-commit-message.adoc
> new file mode 100644
> index 00000000000..47399ae7266
> --- /dev/null
> +++ b/Documentation/format-patch-end-of-commit-message.adoc
> @@ -0,0 +1,3 @@
> +* three-dashes and end-of-line, or
> +* a line that begins with "diff -", or
> +* a line that begins with "Index: "
> diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
> index 0c94776e296..756dfd722b9 100644
> --- a/Documentation/git-am.adoc
> +++ b/Documentation/git-am.adoc
> @@ -231,10 +231,11 @@ applying.
>   --allow-empty::
>   	After a patch failure on an input e-mail message lacking a patch,
>   	create an empty commit with the contents of the e-mail message
>   	as its log message.
>   
> +[[discussion]]
>   DISCUSSION
>   ----------
>   
>   The commit author name is taken from the "From: " line of the
>   message, and commit author date is taken from the "Date: " line
> @@ -252,17 +253,18 @@ where the patch begins.  Excess whitespace at the end of each
>   line is automatically stripped.
>   
>   The patch is expected to be inline, directly following the
>   message.  Any line that is of the form:
>   
> -* three-dashes and end-of-line, or
> -* a line that begins with "diff -", or
> -* a line that begins with "Index: "
> +include::format-patch-end-of-commit-message.adoc[]
>   
>   is taken as the beginning of a patch, and the commit log message
>   is terminated before the first occurrence of such a line.
>   
> +This means that the contents of the commit message can inadvertently
> +interrupt the processing (see the <<caveats,CAVEATS>> section below).
> +
>   When initially invoking `git am`, you give it the names of the mailboxes
>   to process.  Upon seeing the first patch that does not apply, it
>   aborts in the middle.  You can recover from this in one of two ways:
>   
>   . skip the current patch by re-running the command with the `--skip`
> @@ -281,10 +283,17 @@ Before any patches are applied, ORIG_HEAD is set to the tip of the
>   current branch.  This is useful if you have problems with multiple
>   commits, like running 'git am' on the wrong branch or an error in the
>   commits that is more easily fixed by changing the mailbox (e.g.
>   errors in the "From:" lines).
>   
> +[[caveats]]
> +CAVEATS
> +-------
> +
> +:git-am: 1
> +include::format-patch-caveats.adoc[]
> +
>   HOOKS
>   -----
>   This command can run `applypatch-msg`, `pre-applypatch`,
>   and `post-applypatch` hooks.  See linkgit:githooks[5] for more
>   information.
> diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
> index 9a7807ca71a..36851aaf5e1 100644
> --- a/Documentation/git-format-patch.adoc
> +++ b/Documentation/git-format-patch.adoc
> @@ -796,10 +796,14 @@ CAVEATS
>   Note that `format-patch` will omit merge commits from the output, even
>   if they are part of the requested range. A simple "patch" does not
>   include enough information for the receiving end to reproduce the same
>   merge commit.
>   
> +'''
> +
> +include::format-patch-caveats.adoc[]
> +
>   SEE ALSO
>   --------
>   linkgit:git-am[1], linkgit:git-send-email[1]
>   
>   GIT
> diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc
> index ebe8853e9f5..0b118df6498 100644
> --- a/Documentation/git-send-email.adoc
> +++ b/Documentation/git-send-email.adoc
> @@ -690,10 +690,15 @@ Links of a few such community maintained helpers are:
>   	  (cross platform client that can send emails using the ProtonMail API)
>   
>   	- https://github.com/AdityaGarg8/git-credential-email[git-msgraph]
>   	  (cross platform client that can send emails using the Microsoft Graph API)
>   
> +CAVEATS
> +-------
> +
> +include::format-patch-caveats.adoc[]
> +
>   SEE ALSO
>   --------
>   linkgit:git-format-patch[1], linkgit:git-imap-send[1], mbox(5)
>   
>   GIT
> 
> Interdiff against v1:
>    diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
>    index 2accf2763fd..c666d709742 100644
>    --- a/Documentation/format-patch-caveats.adoc
>    +++ b/Documentation/format-patch-caveats.adoc
>    @@ -1,39 +1,36 @@
>    -Patches produced by linkgit:git-format-patch[1] or
>    -linkgit:git-send-email[1] are inline. This means that the output of
>    -these two commands can lead to a different commit message when applied
>    -with linkgit:git-am[1]. It can also mean that the patch is not applied
>    -correctly.
>    +Patches produced by linkgit:git-format-patch[1] are inline. This means
>    +that the output from that command can lead to a different commit message
>    +when applied with linkgit:git-am[1]. It can also mean that the patch
>    +that is applied is not the same as the one that was generated, or that
>    +the patch application fails outright.
>    +ifdef::git-am[]
>    +See the <<discussion,DISCUSSION>> section above for the syntactic rules.
>    +endif::git-am[]
>     
>    -The commit message might contain a three-dash line (`---`) which was
>    -perhaps meant to be a thematic break. That means that the commit message
>    -will be cut short. The presence of a line starting with "Index: " can
>    -cause the patch not to be found, giving an error about an empty patch.
>    +ifndef::git-am[]
>    +Any line that is of the form:
>     
>    -Furthermore, the presence of an unindented diff in the commit message
>    -will not only cut the message short but cause that very diff to be
>    -applied, along with the patch in the patch section. The commit message
>    -might for example have a diff in a GitHub MarkDown code fence:
>    +include::format-patch-end-of-commit-message.adoc[]
>     
>    -----
>    -```
>    -diff ...
>    -```
>    -----
>    +will terminate the commit message and cause the patch machinery to start
>    +searching for patches to apply.
>    +endif::git-am[]
>     
>    -The solution for this is to indent the diff instead:
>    -
>    -----
>    -    diff ...
>    -----
>    +Note that this is especially problematic for unindented diffs that occur
>    +in the commit message; the diff in the commit message might get applied
>    +along with the patch section, or the patch application machinery might
>    +trip up because the patch target doesn't apply. This could for example
>    +be caused by a diff in a GitHub Markdown code block.
>     
>     This loss of fidelity might be simple to notice if you are applying
>    -patches directly from a mailbox. However, a commit authored long ago
>    -might be applied in a different context, perhaps because many changes
>    -are being integrated via patch files and the
>    -linkgit:git-format-patch[1] format is trusted to import changes of a
>    -Git origin.
>    +patches directly from a mailbox. However, changes originating from Git
>    +could be applied in bulk, in which case this would be much harder to
>    +notice. This could for example be a Linux distribution which uses patch
>    +files to apply changes on top of the commits from the upstream
>    +repositories. This goes to show that this behavior does not only impact
>    +email workflows.
>     
>    -One might want to use a general-purpose utility like patch(1) instead,
>    -given these limitations. However, patch(1) will not only look for
>    +Given these limitations, one might be tempted to use a general-purpose
>    +utility like patch(1) instead. However, patch(1) will not only look for
>     unindented diffs (like linkgit:git-am[1]) but will try to apply indented
>     diffs as well.
>    diff --git a/Documentation/format-patch-end-of-commit-message.adoc b/Documentation/format-patch-end-of-commit-message.adoc
>    new file mode 100644
>    index 00000000000..47399ae7266
>    --- /dev/null
>    +++ b/Documentation/format-patch-end-of-commit-message.adoc
>    @@ -0,0 +1,3 @@
>    +* three-dashes and end-of-line, or
>    +* a line that begins with "diff -", or
>    +* a line that begins with "Index: "
>    diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
>    index 18f5b950825..756dfd722b9 100644
>    --- a/Documentation/git-am.adoc
>    +++ b/Documentation/git-am.adoc
>    @@ -231,10 +231,11 @@ applying.
>     --allow-empty::
>     	After a patch failure on an input e-mail message lacking a patch,
>     	create an empty commit with the contents of the e-mail message
>     	as its log message.
>     
>    +[[discussion]]
>     DISCUSSION
>     ----------
>     
>     The commit author name is taken from the "From: " line of the
>     message, and commit author date is taken from the "Date: " line
>    @@ -252,18 +253,16 @@ where the patch begins.  Excess whitespace at the end of each
>     line is automatically stripped.
>     
>     The patch is expected to be inline, directly following the
>     message.  Any line that is of the form:
>     
>    -* three-dashes and end-of-line, or
>    -* a line that begins with "diff -", or
>    -* a line that begins with "Index: "
>    +include::format-patch-end-of-commit-message.adoc[]
>     
>     is taken as the beginning of a patch, and the commit log message
>     is terminated before the first occurrence of such a line.
>     
>    -This means that the content of the commit message can inadverently
>    +This means that the contents of the commit message can inadvertently
>     interrupt the processing (see the <<caveats,CAVEATS>> section below).
>     
>     When initially invoking `git am`, you give it the names of the mailboxes
>     to process.  Upon seeing the first patch that does not apply, it
>     aborts in the middle.  You can recover from this in one of two ways:
>    @@ -288,10 +287,11 @@ errors in the "From:" lines).
>     
>     [[caveats]]
>     CAVEATS
>     -------
>     
>    +:git-am: 1
>     include::format-patch-caveats.adoc[]
>     
>     HOOKS
>     -----
>     This command can run `applypatch-msg`, `pre-applypatch`,
> 
> Range-diff against v1:
> 1:  4bed8f55b98 ! 1:  c54f394bb33 doc: add caveat about roundtripping format-patch
>      @@ Metadata
>        ## Commit message ##
>           doc: add caveat about roundtripping format-patch
>       
>      -    git-format-patch(1), git-send-email(1), and git-am(1) deal with
>      -    formatting commits as patches, sending them (perhaps directly), and
>      -    applying them, respectively. Naturally they use a few delimiters to mark
>      -    where the commit message ends. This can lead to surprising behavior when
>      -    these delimiters are used in the commit message itself.
>      +    git-format-patch(1) and git-am(1) deal with formatting commits as
>      +    patches and applying them, respectively. Naturally they use a few
>      +    delimiters to mark where the commit message ends. This can lead to
>      +    surprising behavior when these delimiters are used in the commit
>      +    message itself.
>       
>      -    git-format-patch(1) and git-send-email(1) will accept any commit message
>      -    and not warn or error about these delimiters being used.[1]
>      +    git-format-patch(1) will accept any commit message and not warn or error
>      +    about these delimiters being used.[1]
>       
>      -    Moreover, the presence of unindented diffs in the commit message will
>      -    cause git-am(1) to apply both the diffs from the commit message as well
>      -    as the patch section.[2]
>      +    Especially problematic is the presence of unindented diffs in the commit
>      +    message; the patch machinery will naturally (since the commit message
>      +    has ended) try to apply that diff and everything after it.[2]
>       
>           It is unclear whether any commands in this chain will learn to warn
>           about this. One concern could be that users have learned to rely on
>      @@ Commit message
>           information in the commit message, knowing that git-am(1) will
>           ignore it.[4]
>       
>      -    All of this is covered already, technically, However, we should spell
>      +    All of this is covered already, technically. However, we should spell
>           out the implications.
>       
>           † 1: There is also git-commit(1) to consider. However, making that
>                command warn or error out over such delimiters would be disruptive
>                to all Git users who never use email in their workflow.
>      -    [2]: Recently patch(1) caused this issue for a project, but it was noted
>      +    † 2: Recently patch(1) caused this issue for a project, but it was noted
>                that git-am(1) has the same behavior[3]
>      -    [3]: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
>      -    [4]: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
>      +    † 3: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
>      +    † 4: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
>      +         https://lore.kernel.org/git/V2_format-patch_caveats.34b@msgid.xyz/
>       
>           Reported-by: Matthias Beyer <mail@beyermatthias.de>
>           Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
>      @@ Commit message
>           Helped-by: Jakob Haufe <sur5r@sur5r.net>
>           Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
>       
>      +    ---
>      +
>      +    v2:
>      +
>      +    Address feedback from Phillip Wood.
>      +
>      +    Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
>      +
>      +    • Drop the code blocks with the diffs; the prose speaks for itself, no need
>      +      to take up space
>      +    • Don’t discuss git-send-email(1). We already know that git-format-patch(1)
>      +      is the generator. It is mentioned in git-send-email(1).
>      +    • Try to be more clear about the case where someone might be applying a
>      +      diff. Use the example from Matthias Beyer in:
>      +
>      +          https://lore.kernel.org/git/gfxpnecn2cdtmeiape2d4x5aybuyyqi4c7m6te3khgct34dd44@wqusigna2nsp/
>      +
>      +      Hopefully I explained it correctly?
>      +    • Add a “this goes to show...”... which seems to emphasize the point
>      +      without being redundant. Hopefully.
>      +
>      +    Try to address feedback from Junio C Hamano by adding more nuance: the diff
>      +    in the commit message might be applied as well, or the patch machinery
>      +    might trip on something and fail.
>      +
>      +    Finally, in the middle of discussing the three possible cmt. message
>      +    delimiters, I noticed that the three points were drifting apart. So I
>      +    decided to use the list already used in git-am(1) and be done with it in
>      +    one place.
>      +
>      +    ---
>      +
>      +    It seems that the section break in git-format-patch(1) does not get
>      +    applied in the man output (according to `Documentation/doc-diff`
>      +    apparently)? Maybe this is the wrong construct? I couldn’t find any
>      +    other thematic breaks here (though there are several variations).
>      +
>        ## Documentation/format-patch-caveats.adoc (new) ##
>       @@
>      -+Patches produced by linkgit:git-format-patch[1] or
>      -+linkgit:git-send-email[1] are inline. This means that the output of
>      -+these two commands can lead to a different commit message when applied
>      -+with linkgit:git-am[1]. It can also mean that the patch is not applied
>      -+correctly.
>      -+
>      -+The commit message might contain a three-dash line (`---`) which was
>      -+perhaps meant to be a thematic break. That means that the commit message
>      -+will be cut short. The presence of a line starting with "Index: " can
>      -+cause the patch not to be found, giving an error about an empty patch.
>      ++Patches produced by linkgit:git-format-patch[1] are inline. This means
>      ++that the output from that command can lead to a different commit message
>      ++when applied with linkgit:git-am[1]. It can also mean that the patch
>      ++that is applied is not the same as the one that was generated, or that
>      ++the patch application fails outright.
>      ++ifdef::git-am[]
>      ++See the <<discussion,DISCUSSION>> section above for the syntactic rules.
>      ++endif::git-am[]
>       +
>      -+Furthermore, the presence of an unindented diff in the commit message
>      -+will not only cut the message short but cause that very diff to be
>      -+applied, along with the patch in the patch section. The commit message
>      -+might for example have a diff in a GitHub MarkDown code fence:
>      ++ifndef::git-am[]
>      ++Any line that is of the form:
>       +
>      -+----
>      -+```
>      -+diff ...
>      -+```
>      -+----
>      ++include::format-patch-end-of-commit-message.adoc[]
>       +
>      -+The solution for this is to indent the diff instead:
>      ++will terminate the commit message and cause the patch machinery to start
>      ++searching for patches to apply.
>      ++endif::git-am[]
>       +
>      -+----
>      -+    diff ...
>      -+----
>      ++Note that this is especially problematic for unindented diffs that occur
>      ++in the commit message; the diff in the commit message might get applied
>      ++along with the patch section, or the patch application machinery might
>      ++trip up because the patch target doesn't apply. This could for example
>      ++be caused by a diff in a GitHub Markdown code block.
>       +
>       +This loss of fidelity might be simple to notice if you are applying
>      -+patches directly from a mailbox. However, a commit authored long ago
>      -+might be applied in a different context, perhaps because many changes
>      -+are being integrated via patch files and the
>      -+linkgit:git-format-patch[1] format is trusted to import changes of a
>      -+Git origin.
>      ++patches directly from a mailbox. However, changes originating from Git
>      ++could be applied in bulk, in which case this would be much harder to
>      ++notice. This could for example be a Linux distribution which uses patch
>      ++files to apply changes on top of the commits from the upstream
>      ++repositories. This goes to show that this behavior does not only impact
>      ++email workflows.
>       +
>      -+One might want to use a general-purpose utility like patch(1) instead,
>      -+given these limitations. However, patch(1) will not only look for
>      ++Given these limitations, one might be tempted to use a general-purpose
>      ++utility like patch(1) instead. However, patch(1) will not only look for
>       +unindented diffs (like linkgit:git-am[1]) but will try to apply indented
>       +diffs as well.
>       
>      + ## Documentation/format-patch-end-of-commit-message.adoc (new) ##
>      +@@
>      ++* three-dashes and end-of-line, or
>      ++* a line that begins with "diff -", or
>      ++* a line that begins with "Index: "
>      +
>        ## Documentation/git-am.adoc ##
>      -@@ Documentation/git-am.adoc: message.  Any line that is of the form:
>      +@@ Documentation/git-am.adoc: applying.
>      + 	create an empty commit with the contents of the e-mail message
>      + 	as its log message.
>      +
>      ++[[discussion]]
>      + DISCUSSION
>      + ----------
>      +
>      +@@ Documentation/git-am.adoc: line is automatically stripped.
>      + The patch is expected to be inline, directly following the
>      + message.  Any line that is of the form:
>      +
>      +-* three-dashes and end-of-line, or
>      +-* a line that begins with "diff -", or
>      +-* a line that begins with "Index: "
>      ++include::format-patch-end-of-commit-message.adoc[]
>      +
>        is taken as the beginning of a patch, and the commit log message
>        is terminated before the first occurrence of such a line.
>        
>      -+This means that the content of the commit message can inadverently
>      ++This means that the contents of the commit message can inadvertently
>       +interrupt the processing (see the <<caveats,CAVEATS>> section below).
>       +
>        When initially invoking `git am`, you give it the names of the mailboxes
>      @@ Documentation/git-am.adoc: commits, like running 'git am' on the wrong branch or
>       +CAVEATS
>       +-------
>       +
>      ++:git-am: 1
>       +include::format-patch-caveats.adoc[]
>       +
>        HOOKS
> 
> base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-10  2:16         ` Jacob Keller
@ 2026-02-10 14:22           ` Patrick Steinhardt
  2026-02-10 15:47             ` Junio C Hamano
  0 siblings, 1 reply; 65+ messages in thread
From: Patrick Steinhardt @ 2026-02-10 14:22 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Jeff King, Matthias Beyer, git, pyokagan

On Mon, Feb 09, 2026 at 06:16:35PM -0800, Jacob Keller wrote:
> On Mon, Feb 9, 2026 at 7:59 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Fri, Feb 06, 2026 at 04:03:58AM -0500, Jeff King wrote:
> > > On Fri, Feb 06, 2026 at 09:18:50AM +0100, Matthias Beyer wrote:
> > >
> > > > That said, I am no expert in either C or the git codebase at all, but
> > > > from what I saw from reading the git-am codebase, it looks like it tries
> > > > to find the patch by looking for three dashes on a line with a linebreak
> > > > behind ("---\n").
> > >
> > > Yes, that is how the split is made.
> > >
> > > > From what I read, it looks for that from the first line.
> > > > What I would think of here is looking for that "patchbreak" from the
> > > > _end_ of the email rather than from the top, that would have prevented
> > > > this issue, right?
> > >
> > > The patch itself may legitimately contain "---" on a line by itself (it
> > > would indicate that the line "--" was removed from a file). That would
> > > confuse your parser, including in a way that we end up only applying
> > > part of the diff (everything before that fake "---" becomes commit
> > > message, and everything after becomes cover-letter material up to the
> > > next "diff" line).
> > >
> > > I suspect it also creates corner cases with cover-letter material
> > > (between the "---" and the diff itself) that itself contains any "---"
> > > marker.
> > >
> > > I don't think there is a way to unambiguously parse the single-stream
> > > output that format-patch produces. This is a reasonably well-known
> > > gotcha (at least around here). E.g., some earlier discussions:
> > >
> > >   2024: https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
> > >   2022: https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
> > >   2015: https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
> > >
> > > There are probably more, but it's actually a tricky thing to search for
> > > in the archive, so I stopped digging. ;)
> >
> > Maybe we can't parse it unambiguously. But what we _can_ detect is that
> > a patch is ambiguous in the first place, right? So maybe we could extend
> > git-am(1) to bail by default with a hint that tells the user that:
> >
> 
> I think it might make sense in a breaking change to update format
> patch and git am to have an "unambiguous" mode which would allow
> somehow to unambiguously distinguish between commit message contents
> and patch data. I'm not 100% sure how to do this, and it likely
> requires some sort of breaking changes to both tools to allow
> distinguishing properly between the two points.

That is worth a thought indeed. I guess one of the biggest questions
here is whether we can introduce such an unambiguous mode in such a way
that old Git clients/patch(1) would continue to understand them. I
wouldn't mind much if they would still misinterpret the ambiguous parts.
But if so, we could make this unambiguous mode the default without a
breaking change.

This is all pure speculation though, I have no idea whether such a
backwards-compatible and forwards-safe mode exists.

> Obviously if you're sending the contents together, a malicious user
> could edit the formatted patch to move or copy whatever the
> "signifier" for patch vs commit separator is... but at least we'd
> prevent the cases where someone accidentally includes diffs without
> intending to.

Well, if we had such an unambiguous mode I would say that eventually,
Git should start to refuse patches that have been generated without this
mode by default.

Patrick

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-10 14:22           ` Patrick Steinhardt
@ 2026-02-10 15:47             ` Junio C Hamano
  2026-02-11  2:31               ` Jacob Keller
  0 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2026-02-10 15:47 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: Jacob Keller, Jeff King, Matthias Beyer, git, pyokagan

Patrick Steinhardt <ps@pks.im> writes:

> That is worth a thought indeed. I guess one of the biggest questions
> here is whether we can introduce such an unambiguous mode in such a way
> that old Git clients/patch(1) would continue to understand them. I
> wouldn't mind much if they would still misinterpret the ambiguous parts.
> But if so, we could make this unambiguous mode the default without a
> breaking change.

Yup, if the old versions misinterpret exactly the same way as
before, then it does not even have to be called "unambiguous mode"
that is on by default.  I doubt it is possible, though.

>> Obviously if you're sending the contents together, a malicious user
>> could edit the formatted patch to move or copy whatever the
>> "signifier" for patch vs commit separator is... but at least we'd
>> prevent the cases where someone accidentally includes diffs without
>> intending to.
>
> Well, if we had such an unambiguous mode I would say that eventually,
> Git should start to refuse patches that have been generated without this
> mode by default.

Or any unsigned patch, perhaps?

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-10  0:53   ` [PATCH] doc: add caveat about roundtripping format-patch Christoph Anton Mitterer
@ 2026-02-10 16:00     ` Kristoffer Haugsbakk
  0 siblings, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-10 16:00 UTC (permalink / raw)
  To: Christoph Anton Mitterer, git
  Cc: Matthias Beyer, Christoph Anton Mitterer, Matheus Tavares,
	Chris Packham, Jakob Haufe, Phillip Wood

Hi

On Tue, Feb 10, 2026, at 01:53, Christoph Anton Mitterer wrote:
> While it's nice to see it getting documented (thanks for that)...
> wouldn't it be even better to actually fix the underlying issue? :-)
>
> I mean it's all but guaranteed that everyone reads this,... and IMO the
> problem might even be exploited security wise.

There’s a discussion about fixing it from the start of the thread:

https://lore.kernel.org/git/bcqvh7ahjjgzpgxwnr4kh3hfkksfruf54refyry3ha7qk7dldf@fij5calmscvm/

Please use Reply-All. ;)

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH] doc: add caveat about roundtripping format-patch
  2026-02-10 10:57       ` Phillip Wood
@ 2026-02-10 16:00         ` Kristoffer Haugsbakk
  0 siblings, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-10 16:00 UTC (permalink / raw)
  To: Phillip Wood, Kristoffer Haugsbakk, Phillip Wood, git
  Cc: Matthias Beyer, Christoph Anton Mitterer, Matheus Tavares,
	Chris Packham, Jakob Haufe

On Tue, Feb 10, 2026, at 11:57, Phillip Wood wrote:
>>>[snip]
>>> Is this last sentence referring to diffs in the commit message being
>>> applied? I don't think there are circumstances where the patch itself is
>>> not applied correctly.
>>
>> I tested with a line like
>>
>>      Index x
>>
>> Yesterday and got an empty patch when running git-am(1). But I couldn’t
>> reproduce now. I must have made a mistake.
>
> Oh, if you use "Index: x" (with a colon) does that mess up the patch
> application?

Sorry, I think I made a typo. I did test with something like `Index:
something`. I’m pretty sure I did...

But now I’ve taken the description from git-am(1) for the
delimiters. I’ve moved away from trying to explain each case.

>>[snip]
>> I don’t understand? It demonstrates a markup for code which does not use
>> indentation.
>
> But I think the markup is a distraction from the problem which is that
> the diff is not indented.

I’ve dropped the code blocks in v2 since you don’t need a code block to
show indentation. Or code fences.

> Also calling it "Github MarkDown" is unfortunate as we try not to
> favor one forge over another and many sites support that syntax.

Sure. I can just say MarkDown code fence. Such a code fence does not use
indentation so it’s clear that we are contrasting with the MD
alternative of just indentation.

Thanks!

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2] doc: add caveat about roundtripping format-patch
  2026-02-09 22:37   ` [PATCH v2] " kristofferhaugsbakk
  2026-02-09 22:59     ` Junio C Hamano
  2026-02-10 11:02     ` Phillip Wood
@ 2026-02-10 18:20     ` Kristoffer Haugsbakk
  2026-02-12 22:28     ` [PATCH v3] doc: add caveat about round-tripping format-patch kristofferhaugsbakk
  3 siblings, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-10 18:20 UTC (permalink / raw)
  To: Kristoffer Haugsbakk, git
  Cc: Matthias Beyer, Christoph Anton Mitterer, Matheus Tavares,
	Chris Packham, Jakob Haufe, Phillip Wood

Snipping all my verbiage here.

On Mon, Feb 9, 2026, at 23:37, kristofferhaugsbakk@fastmail.com wrote:
>[snip]
> @@ -796,10 +796,14 @@ CAVEATS
>  Note that `format-patch` will omit merge commits from the output, even
>  if they are part of the requested range. A simple "patch" does not
>  include enough information for the receiving end to reproduce the same
>  merge commit.
>
> +'''
> +
> +include::format-patch-caveats.adoc[]
> +
>  SEE ALSO
>  --------
>[snip]
>     +
>     +    It seems that the section break in git-format-patch(1) does not get
>     +    applied in the man output (according to `Documentation/doc-diff`
>     +    apparently)? Maybe this is the wrong construct? I couldn’t find any
>     +    other thematic breaks here (though there are several variations).
>     +
>[snip]

I want to use a heading instead.

    === Patch application

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-10 15:47             ` Junio C Hamano
@ 2026-02-11  2:31               ` Jacob Keller
  2026-02-11  2:34                 ` Jacob Keller
  0 siblings, 1 reply; 65+ messages in thread
From: Jacob Keller @ 2026-02-11  2:31 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Patrick Steinhardt, Jeff King, Matthias Beyer, git, pyokagan

On Tue, Feb 10, 2026 at 7:47 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Patrick Steinhardt <ps@pks.im> writes:
>
> > That is worth a thought indeed. I guess one of the biggest questions
> > here is whether we can introduce such an unambiguous mode in such a way
> > that old Git clients/patch(1) would continue to understand them. I
> > wouldn't mind much if they would still misinterpret the ambiguous parts.
> > But if so, we could make this unambiguous mode the default without a
> > breaking change.
>
> Yup, if the old versions misinterpret exactly the same way as
> before, then it does not even have to be called "unambiguous mode"
> that is on by default.  I doubt it is possible, though.
>

Hmm. If we add a new unambiguous marker after the ---, old versions
would see '...' and know to cut the description. New versions would
wait for <NEW MARKER> and properly ignore any diff/etc prior to this.

Since <NEW MARKER> is after a ---, it would be ignored and not
inserted as part of the commit message, and because all versions
universally accept cruft between --- and the diff start, this should
be acceptable right?

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-11  2:31               ` Jacob Keller
@ 2026-02-11  2:34                 ` Jacob Keller
  2026-02-11  7:47                   ` Jeff King
  0 siblings, 1 reply; 65+ messages in thread
From: Jacob Keller @ 2026-02-11  2:34 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Patrick Steinhardt, Jeff King, Matthias Beyer, git, pyokagan

On Tue, Feb 10, 2026 at 6:31 PM Jacob Keller <jacob.keller@gmail.com> wrote:
>
> On Tue, Feb 10, 2026 at 7:47 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> > > That is worth a thought indeed. I guess one of the biggest questions
> > > here is whether we can introduce such an unambiguous mode in such a way
> > > that old Git clients/patch(1) would continue to understand them. I
> > > wouldn't mind much if they would still misinterpret the ambiguous parts.
> > > But if so, we could make this unambiguous mode the default without a
> > > breaking change.
> >
> > Yup, if the old versions misinterpret exactly the same way as
> > before, then it does not even have to be called "unambiguous mode"
> > that is on by default.  I doubt it is possible, though.
> >
>
> Hmm. If we add a new unambiguous marker after the ---, old versions
> would see '...' and know to cut the description. New versions would
> wait for <NEW MARKER> and properly ignore any diff/etc prior to this.
>
> Since <NEW MARKER> is after a ---, it would be ignored and not
> inserted as part of the commit message, and because all versions
> universally accept cruft between --- and the diff start, this should
> be acceptable right?

Keeping in mind we'd have to use <NEW MARKER> as something that we
somehow reject as being a valid part of a commit message somehow, so
that you can't accidentally insert it, and we'd need to be careful
about rejecting formatting such a patch, and probably complaining on
the receiving end if we see multiple markers.. Trickier than it sounds
I imagine.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-11  2:34                 ` Jacob Keller
@ 2026-02-11  7:47                   ` Jeff King
  2026-02-11 15:23                     ` Kristoffer Haugsbakk
  2026-02-11 15:47                     ` Junio C Hamano
  0 siblings, 2 replies; 65+ messages in thread
From: Jeff King @ 2026-02-11  7:47 UTC (permalink / raw)
  To: Jacob Keller
  Cc: Junio C Hamano, Patrick Steinhardt, Matthias Beyer, git, pyokagan

On Tue, Feb 10, 2026 at 06:34:05PM -0800, Jacob Keller wrote:

> > Hmm. If we add a new unambiguous marker after the ---, old versions
> > would see '...' and know to cut the description. New versions would
> > wait for <NEW MARKER> and properly ignore any diff/etc prior to this.
> >
> > Since <NEW MARKER> is after a ---, it would be ignored and not
> > inserted as part of the commit message, and because all versions
> > universally accept cruft between --- and the diff start, this should
> > be acceptable right?
> 
> Keeping in mind we'd have to use <NEW MARKER> as something that we
> somehow reject as being a valid part of a commit message somehow, so
> that you can't accidentally insert it, and we'd need to be careful
> about rejecting formatting such a patch, and probably complaining on
> the receiving end if we see multiple markers.. Trickier than it sounds
> I imagine.

Yeah, on reading your first message, I wondered if we would run into a
commit message adding "---" followed by the new marker. If the new
marker is forbidden, I guess that works. But how ugly is that new marker
going to be, then? ;) We'll now see it in every email.

If we are going to modify what format-patch produces, I'd be more
inclined to have it perform some reversible quoting on the commit
message so that "---" and "diff" lines are not recognized. And then that
quoting only has to kick in when a message would be ambiguous, so most
people wouldn't even see it.

If an older version of git-am (that does not understand how to unquote
it) receives the mail, the worst case is you'd see the quoting in the
resulting commit message. So if we make it not-too-ugly, that may not be
so bad. Think something along the lines of seeing ">From" in emails. It
is gross and ugly, but you can still read the email.


All that said, if the main goal is just avoiding accidental diffs in
commit messages (and not worrying about truncation due to "---" in
messages), there may be a simpler receiver-side solution. If the
receiver expects the message to be generated by Git, then it will expect
there to be a "---" line. And we will not expect any diff before then.
So could we just have a "git am --strict" mode (and perhaps matching
config option) that always looks for the "---" separator?

It's not foolproof, but I suspect it would help with the worst cases of
embedded diffs. And it's not that hard to implement.

-Peff

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-11  7:47                   ` Jeff King
@ 2026-02-11 15:23                     ` Kristoffer Haugsbakk
  2026-02-11 15:47                     ` Junio C Hamano
  1 sibling, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-11 15:23 UTC (permalink / raw)
  To: Jeff King, Jacob Keller
  Cc: Junio C Hamano, Patrick Steinhardt, Matthias Beyer, git, pyokagan

On Wed, Feb 11, 2026, at 08:47, Jeff King wrote:
> On Tue, Feb 10, 2026 at 06:34:05PM -0800, Jacob Keller wrote:
>
>> > Hmm. If we add a new unambiguous marker after the ---, old versions
>> > would see '...' and know to cut the description. New versions would
>> > wait for <NEW MARKER> and properly ignore any diff/etc prior to this.
>> >
>> > Since <NEW MARKER> is after a ---, it would be ignored and not
>> > inserted as part of the commit message, and because all versions
>> > universally accept cruft between --- and the diff start, this should
>> > be acceptable right?
>>
>> Keeping in mind we'd have to use <NEW MARKER> as something that we
>> somehow reject as being a valid part of a commit message somehow, so
>> that you can't accidentally insert it, and we'd need to be careful
>> about rejecting formatting such a patch, and probably complaining on
>> the receiving end if we see multiple markers.. Trickier than it sounds
>> I imagine.
>
> Yeah, on reading your first message, I wondered if we would run into a
> commit message adding "---" followed by the new marker. If the new
> marker is forbidden, I guess that works. But how ugly is that new marker
> going to be, then? ;) We'll now see it in every email.

Maybe it could be something like `<symbols><space>`? It’s difficult to accidentally 
get a trailing whitespace into a commit.

> If we are going to modify what format-patch produces, I'd be more
> inclined to have it perform some reversible quoting on the commit
> message so that "---" and "diff" lines are not recognized. And then that
> quoting only has to kick in when a message would be ambiguous, so most
> people wouldn't even see it.

This sounds better anyway.

>[snip]

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: git-am applies commit message diffs
  2026-02-11  7:47                   ` Jeff King
  2026-02-11 15:23                     ` Kristoffer Haugsbakk
@ 2026-02-11 15:47                     ` Junio C Hamano
  1 sibling, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2026-02-11 15:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Jacob Keller, Patrick Steinhardt, Matthias Beyer, git, pyokagan

Jeff King <peff@peff.net> writes:

> All that said, if the main goal is just avoiding accidental diffs in
> commit messages (and not worrying about truncation due to "---" in
> messages), there may be a simpler receiver-side solution. If the
> receiver expects the message to be generated by Git, then it will expect
> there to be a "---" line. And we will not expect any diff before then.
> So could we just have a "git am --strict" mode (and perhaps matching
> config option) that always looks for the "---" separator?
>
> It's not foolproof, but I suspect it would help with the worst cases of
> embedded diffs. And it's not that hard to implement.

Yup, if the payload is known to be generated by "git", which is much
more likely these days than back when "git am" (or "git applymbox")
was written, can ignore "Index:" and "diff -" when deciding when the
log message part of a piece of e-mail ends.  I like that approach.


^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v3] doc: add caveat about round-tripping format-patch
  2026-02-09 22:37   ` [PATCH v2] " kristofferhaugsbakk
                       ` (2 preceding siblings ...)
  2026-02-10 18:20     ` Kristoffer Haugsbakk
@ 2026-02-12 22:28     ` kristofferhaugsbakk
  2026-02-12 23:19       ` Junio C Hamano
  3 siblings, 1 reply; 65+ messages in thread
From: kristofferhaugsbakk @ 2026-02-12 22:28 UTC (permalink / raw)
  To: git
  Cc: Kristoffer Haugsbakk, Matthias Beyer, Christoph Anton Mitterer,
	Matheus Tavares, Chris Packham, Jakob Haufe, Phillip Wood

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

git-format-patch(1) and git-am(1) deal with formatting commits as
patches and applying them, respectively. Naturally they use a few
delimiters to mark where the commit message ends. This can lead to
surprising behavior when these delimiters are used in the commit
message itself.

git-format-patch(1) will accept any commit message and not warn or error
about these delimiters being used.[1]

Especially problematic is the presence of unindented diffs in the commit
message; the patch machinery will naturally (since the commit message
has ended) try to apply that diff and everything after it.[2]

It is unclear whether any commands in this chain will learn to warn
about this. One concern could be that users have learned to rely on
the three-dash line rule to conveniently add extra-commit message
information in the commit message, knowing that git-am(1) will
ignore it.[4]

All of this is covered already, technically. However, we should spell
out the implications.

† 1: There is also git-commit(1) to consider. However, making that
     command warn or error out over such delimiters would be disruptive
     to all Git users who never use email in their workflow.
† 2: Recently patch(1) caused this issue for a project, but it was noted
     that git-am(1) has the same behavior[3]
† 3: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
† 4: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
     https://lore.kernel.org/git/V3_format-patch_caveats.354@msgid.xyz/

Reported-by: Matthias Beyer <mail@beyermatthias.de>
Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
Reported-by: Matheus Tavares <matheus.tavb@gmail.com>
Reported-by: Chris Packham <judge.packham@gmail.com>
Helped-by: Jakob Haufe <sur5r@sur5r.net>
Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>

---

v3:

Drop “[GitHub] Markdown” based on discussion with Phillip.

Address Junio’s feedback about not using a needless
“this means” construct.

Also:
• Pull out the whole syntactic rules section instead of just the list
• Add back the solution paragraph (indent diff or other problematic
  text) that I accidentally deleted in v2.
• Resolve the thematic break problem (not rendered in man) by using a
  subheading (Patch Application)
• Fine, Merriam Webster says that roundtripping is “less commonly
  used” (use round-trip)

Link to v2: https://lore.kernel.org/git/V2_format-patch_caveats.34b@msgid.xyz/
Link to v1: https://lore.kernel.org/git/format-patch_caveats.281@msgid.xyz/
---
 Documentation/format-patch-caveats.adoc       | 33 +++++++++++++++++++
 .../format-patch-end-of-commit-message.adoc   |  8 +++++
 Documentation/git-am.adoc                     | 19 +++++++----
 Documentation/git-format-patch.adoc           |  4 +++
 Documentation/git-send-email.adoc             |  5 +++
 5 files changed, 62 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/format-patch-caveats.adoc
 create mode 100644 Documentation/format-patch-end-of-commit-message.adoc

diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
new file mode 100644
index 00000000000..807a65b885b
--- /dev/null
+++ b/Documentation/format-patch-caveats.adoc
@@ -0,0 +1,33 @@
+The output from linkgit:git-format-patch[1] can lead to a different
+commit message when applied with linkgit:git-am[1]. The patch that is
+applied may also be different from the one that was generated, or patch
+application may fail outright.
+ifdef::git-am[]
+See the <<discussion,DISCUSSION>> section above for the syntactic rules.
+endif::git-am[]
+
+ifndef::git-am[]
+include::format-patch-end-of-commit-message.adoc[]
+endif::git-am[]
+
+Note that this is especially problematic for unindented diffs that occur
+in the commit message; the diff in the commit message might get applied
+along with the patch section, or the patch application machinery might
+trip up because the patch target doesn't apply. This could for example
+be caused by a diff in a Markdown code block.
+
+The solution for this is to indent the diff or other text that could
+cause problems.
+
+This loss of fidelity might be simple to notice if you are applying
+patches directly from a mailbox. However, changes originating from Git
+could be applied in bulk, in which case this would be much harder to
+notice. This could for example be a Linux distribution which uses patch
+files to apply changes on top of the commits from the upstream
+repositories. This goes to show that this behavior does not only impact
+email workflows.
+
+Given these limitations, one might be tempted to use a general-purpose
+utility like patch(1) instead. However, patch(1) will not only look for
+unindented diffs (like linkgit:git-am[1]) but will try to apply indented
+diffs as well.
diff --git a/Documentation/format-patch-end-of-commit-message.adoc b/Documentation/format-patch-end-of-commit-message.adoc
new file mode 100644
index 00000000000..ec1ef79f5e3
--- /dev/null
+++ b/Documentation/format-patch-end-of-commit-message.adoc
@@ -0,0 +1,8 @@
+Any line that is of the form:
+
+* three-dashes and end-of-line, or
+* a line that begins with "diff -", or
+* a line that begins with "Index: "
+
+is taken as the beginning of a patch, and the commit log message
+is terminated before the first occurrence of such a line.
diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
index 0c94776e296..972398d4575 100644
--- a/Documentation/git-am.adoc
+++ b/Documentation/git-am.adoc
@@ -231,10 +231,11 @@ applying.
 --allow-empty::
 	After a patch failure on an input e-mail message lacking a patch,
 	create an empty commit with the contents of the e-mail message
 	as its log message.
 
+[[discussion]]
 DISCUSSION
 ----------
 
 The commit author name is taken from the "From: " line of the
 message, and commit author date is taken from the "Date: " line
@@ -250,18 +251,15 @@ The commit message is formed by the title taken from the
 "Subject: ", a blank line and the body of the message up to
 where the patch begins.  Excess whitespace at the end of each
 line is automatically stripped.
 
 The patch is expected to be inline, directly following the
-message.  Any line that is of the form:
+message.
+include::format-patch-end-of-commit-message.adoc[]
 
-* three-dashes and end-of-line, or
-* a line that begins with "diff -", or
-* a line that begins with "Index: "
-
-is taken as the beginning of a patch, and the commit log message
-is terminated before the first occurrence of such a line.
+This means that the contents of the commit message can inadvertently
+interrupt the processing (see the <<caveats,CAVEATS>> section below).
 
 When initially invoking `git am`, you give it the names of the mailboxes
 to process.  Upon seeing the first patch that does not apply, it
 aborts in the middle.  You can recover from this in one of two ways:
 
@@ -281,10 +279,17 @@ Before any patches are applied, ORIG_HEAD is set to the tip of the
 current branch.  This is useful if you have problems with multiple
 commits, like running 'git am' on the wrong branch or an error in the
 commits that is more easily fixed by changing the mailbox (e.g.
 errors in the "From:" lines).
 
+[[caveats]]
+CAVEATS
+-------
+
+:git-am: 1
+include::format-patch-caveats.adoc[]
+
 HOOKS
 -----
 This command can run `applypatch-msg`, `pre-applypatch`,
 and `post-applypatch` hooks.  See linkgit:githooks[5] for more
 information.
diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
index 9a7807ca71a..bac9b818f3b 100644
--- a/Documentation/git-format-patch.adoc
+++ b/Documentation/git-format-patch.adoc
@@ -796,10 +796,14 @@ CAVEATS
 Note that `format-patch` will omit merge commits from the output, even
 if they are part of the requested range. A simple "patch" does not
 include enough information for the receiving end to reproduce the same
 merge commit.
 
+=== PATCH APPLICATION
+
+include::format-patch-caveats.adoc[]
+
 SEE ALSO
 --------
 linkgit:git-am[1], linkgit:git-send-email[1]
 
 GIT
diff --git a/Documentation/git-send-email.adoc b/Documentation/git-send-email.adoc
index ebe8853e9f5..0b118df6498 100644
--- a/Documentation/git-send-email.adoc
+++ b/Documentation/git-send-email.adoc
@@ -690,10 +690,15 @@ Links of a few such community maintained helpers are:
 	  (cross platform client that can send emails using the ProtonMail API)
 
 	- https://github.com/AdityaGarg8/git-credential-email[git-msgraph]
 	  (cross platform client that can send emails using the Microsoft Graph API)
 
+CAVEATS
+-------
+
+include::format-patch-caveats.adoc[]
+
 SEE ALSO
 --------
 linkgit:git-format-patch[1], linkgit:git-imap-send[1], mbox(5)
 
 GIT

Interdiff against v2:
  diff --git a/Documentation/format-patch-caveats.adoc b/Documentation/format-patch-caveats.adoc
  index c666d709742..807a65b885b 100644
  --- a/Documentation/format-patch-caveats.adoc
  +++ b/Documentation/format-patch-caveats.adoc
  @@ -1,28 +1,25 @@
  -Patches produced by linkgit:git-format-patch[1] are inline. This means
  -that the output from that command can lead to a different commit message
  -when applied with linkgit:git-am[1]. It can also mean that the patch
  -that is applied is not the same as the one that was generated, or that
  -the patch application fails outright.
  +The output from linkgit:git-format-patch[1] can lead to a different
  +commit message when applied with linkgit:git-am[1]. The patch that is
  +applied may also be different from the one that was generated, or patch
  +application may fail outright.
   ifdef::git-am[]
   See the <<discussion,DISCUSSION>> section above for the syntactic rules.
   endif::git-am[]
   
   ifndef::git-am[]
  -Any line that is of the form:
  -
   include::format-patch-end-of-commit-message.adoc[]
  -
  -will terminate the commit message and cause the patch machinery to start
  -searching for patches to apply.
   endif::git-am[]
   
   Note that this is especially problematic for unindented diffs that occur
   in the commit message; the diff in the commit message might get applied
   along with the patch section, or the patch application machinery might
   trip up because the patch target doesn't apply. This could for example
  -be caused by a diff in a GitHub Markdown code block.
  +be caused by a diff in a Markdown code block.
  +
  +The solution for this is to indent the diff or other text that could
  +cause problems.
   
   This loss of fidelity might be simple to notice if you are applying
   patches directly from a mailbox. However, changes originating from Git
   could be applied in bulk, in which case this would be much harder to
   notice. This could for example be a Linux distribution which uses patch
  diff --git a/Documentation/format-patch-end-of-commit-message.adoc b/Documentation/format-patch-end-of-commit-message.adoc
  index 47399ae7266..ec1ef79f5e3 100644
  --- a/Documentation/format-patch-end-of-commit-message.adoc
  +++ b/Documentation/format-patch-end-of-commit-message.adoc
  @@ -1,3 +1,8 @@
  +Any line that is of the form:
  +
   * three-dashes and end-of-line, or
   * a line that begins with "diff -", or
   * a line that begins with "Index: "
  +
  +is taken as the beginning of a patch, and the commit log message
  +is terminated before the first occurrence of such a line.
  diff --git a/Documentation/git-am.adoc b/Documentation/git-am.adoc
  index 756dfd722b9..972398d4575 100644
  --- a/Documentation/git-am.adoc
  +++ b/Documentation/git-am.adoc
  @@ -251,17 +251,13 @@ The commit message is formed by the title taken from the
   "Subject: ", a blank line and the body of the message up to
   where the patch begins.  Excess whitespace at the end of each
   line is automatically stripped.
   
   The patch is expected to be inline, directly following the
  -message.  Any line that is of the form:
  -
  +message.
   include::format-patch-end-of-commit-message.adoc[]
   
  -is taken as the beginning of a patch, and the commit log message
  -is terminated before the first occurrence of such a line.
  -
   This means that the contents of the commit message can inadvertently
   interrupt the processing (see the <<caveats,CAVEATS>> section below).
   
   When initially invoking `git am`, you give it the names of the mailboxes
   to process.  Upon seeing the first patch that does not apply, it
  diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
  index 36851aaf5e1..bac9b818f3b 100644
  --- a/Documentation/git-format-patch.adoc
  +++ b/Documentation/git-format-patch.adoc
  @@ -796,11 +796,11 @@ CAVEATS
   Note that `format-patch` will omit merge commits from the output, even
   if they are part of the requested range. A simple "patch" does not
   include enough information for the receiving end to reproduce the same
   merge commit.
   
  -'''
  +=== PATCH APPLICATION
   
   include::format-patch-caveats.adoc[]
   
   SEE ALSO
   --------

Range-diff against v2:
1:  c54f394bb33 ! 1:  b51273c82c9 doc: add caveat about roundtripping format-patch
    @@ Metadata
     Author: Kristoffer Haugsbakk <code@khaugsbakk.name>
     
      ## Commit message ##
    -    doc: add caveat about roundtripping format-patch
    +    doc: add caveat about round-tripping format-patch
     
         git-format-patch(1) and git-am(1) deal with formatting commits as
         patches and applying them, respectively. Naturally they use a few
    @@ Commit message
              that git-am(1) has the same behavior[3]
         † 3: https://github.com/i3/i3/pull/6564#issuecomment-3858381425
         † 4: https://lore.kernel.org/git/xmqqldh4b5y2.fsf@gitster.g/
    -         https://lore.kernel.org/git/V2_format-patch_caveats.34b@msgid.xyz/
    +         https://lore.kernel.org/git/V3_format-patch_caveats.354@msgid.xyz/
     
         Reported-by: Matthias Beyer <mail@beyermatthias.de>
         Reported-by: Christoph Anton Mitterer <calestyo@scientia.org>
         Reported-by: Matheus Tavares <matheus.tavb@gmail.com>
         Reported-by: Chris Packham <judge.packham@gmail.com>
         Helped-by: Jakob Haufe <sur5r@sur5r.net>
    +    Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
     
         ---
     
    -    v2:
    +    v3:
     
    -    Address feedback from Phillip Wood.
    +    Drop “[GitHub] Markdown” based on discussion with Phillip.
     
    -    Cc: Phillip Wood <phillip.wood@dunelm.org.uk>
    +    Address Junio’s feedback about not using a needless
    +    “this means” construct.
     
    -    • Drop the code blocks with the diffs; the prose speaks for itself, no need
    -      to take up space
    -    • Don’t discuss git-send-email(1). We already know that git-format-patch(1)
    -      is the generator. It is mentioned in git-send-email(1).
    -    • Try to be more clear about the case where someone might be applying a
    -      diff. Use the example from Matthias Beyer in:
    +    Also:
    +    • Pull out the whole syntactic rules section instead of just the list
    +    • Add back the solution paragraph (indent diff or other problematic
    +      text) that I accidentally deleted in v2.
    +    • Resolve the thematic break problem (not rendered in man) by using a
    +      subheading (Patch Application)
    +    • Fine, Merriam Webster says that roundtripping is “less commonly
    +      used” (use round-trip)
     
    -          https://lore.kernel.org/git/gfxpnecn2cdtmeiape2d4x5aybuyyqi4c7m6te3khgct34dd44@wqusigna2nsp/
    -
    -      Hopefully I explained it correctly?
    -    • Add a “this goes to show...”... which seems to emphasize the point
    -      without being redundant. Hopefully.
    -
    -    Try to address feedback from Junio C Hamano by adding more nuance: the diff
    -    in the commit message might be applied as well, or the patch machinery
    -    might trip on something and fail.
    -
    -    Finally, in the middle of discussing the three possible cmt. message
    -    delimiters, I noticed that the three points were drifting apart. So I
    -    decided to use the list already used in git-am(1) and be done with it in
    -    one place.
    -
    -    ---
    -
    -    It seems that the section break in git-format-patch(1) does not get
    -    applied in the man output (according to `Documentation/doc-diff`
    -    apparently)? Maybe this is the wrong construct? I couldn’t find any
    -    other thematic breaks here (though there are several variations).
    +    Link to v2: https://lore.kernel.org/git/V2_format-patch_caveats.34b@msgid.xyz/
    +    Link to v1: https://lore.kernel.org/git/format-patch_caveats.281@msgid.xyz/
     
      ## Documentation/format-patch-caveats.adoc (new) ##
     @@
    -+Patches produced by linkgit:git-format-patch[1] are inline. This means
    -+that the output from that command can lead to a different commit message
    -+when applied with linkgit:git-am[1]. It can also mean that the patch
    -+that is applied is not the same as the one that was generated, or that
    -+the patch application fails outright.
    ++The output from linkgit:git-format-patch[1] can lead to a different
    ++commit message when applied with linkgit:git-am[1]. The patch that is
    ++applied may also be different from the one that was generated, or patch
    ++application may fail outright.
     +ifdef::git-am[]
     +See the <<discussion,DISCUSSION>> section above for the syntactic rules.
     +endif::git-am[]
     +
     +ifndef::git-am[]
    -+Any line that is of the form:
    -+
     +include::format-patch-end-of-commit-message.adoc[]
    -+
    -+will terminate the commit message and cause the patch machinery to start
    -+searching for patches to apply.
     +endif::git-am[]
     +
     +Note that this is especially problematic for unindented diffs that occur
     +in the commit message; the diff in the commit message might get applied
     +along with the patch section, or the patch application machinery might
     +trip up because the patch target doesn't apply. This could for example
    -+be caused by a diff in a GitHub Markdown code block.
    ++be caused by a diff in a Markdown code block.
    ++
    ++The solution for this is to indent the diff or other text that could
    ++cause problems.
     +
     +This loss of fidelity might be simple to notice if you are applying
     +patches directly from a mailbox. However, changes originating from Git
    @@ Documentation/format-patch-caveats.adoc (new)
     
      ## Documentation/format-patch-end-of-commit-message.adoc (new) ##
     @@
    ++Any line that is of the form:
    ++
     +* three-dashes and end-of-line, or
     +* a line that begins with "diff -", or
     +* a line that begins with "Index: "
    ++
    ++is taken as the beginning of a patch, and the commit log message
    ++is terminated before the first occurrence of such a line.
     
      ## Documentation/git-am.adoc ##
     @@ Documentation/git-am.adoc: applying.
    @@ Documentation/git-am.adoc: applying.
      DISCUSSION
      ----------
      
    -@@ Documentation/git-am.adoc: line is automatically stripped.
    +@@ Documentation/git-am.adoc: where the patch begins.  Excess whitespace at the end of each
    + line is automatically stripped.
    + 
      The patch is expected to be inline, directly following the
    - message.  Any line that is of the form:
    +-message.  Any line that is of the form:
    ++message.
    ++include::format-patch-end-of-commit-message.adoc[]
      
     -* three-dashes and end-of-line, or
     -* a line that begins with "diff -", or
     -* a line that begins with "Index: "
    -+include::format-patch-end-of-commit-message.adoc[]
    - 
    - is taken as the beginning of a patch, and the commit log message
    - is terminated before the first occurrence of such a line.
    - 
    +-
    +-is taken as the beginning of a patch, and the commit log message
    +-is terminated before the first occurrence of such a line.
     +This means that the contents of the commit message can inadvertently
     +interrupt the processing (see the <<caveats,CAVEATS>> section below).
    -+
    + 
      When initially invoking `git am`, you give it the names of the mailboxes
      to process.  Upon seeing the first patch that does not apply, it
    - aborts in the middle.  You can recover from this in one of two ways:
     @@ Documentation/git-am.adoc: commits, like running 'git am' on the wrong branch or an error in the
      commits that is more easily fixed by changing the mailbox (e.g.
      errors in the "From:" lines).
    @@ Documentation/git-format-patch.adoc: if they are part of the requested range. A
      include enough information for the receiving end to reproduce the same
      merge commit.
      
    -+'''
    ++=== PATCH APPLICATION
     +
     +include::format-patch-caveats.adoc[]
     +

base-commit: 3e0db84c88c57e70ac8be8c196dfa92c5d656fbc
-- 
2.53.0.26.g2afa8602a26


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH v3] doc: add caveat about round-tripping format-patch
  2026-02-12 22:28     ` [PATCH v3] doc: add caveat about round-tripping format-patch kristofferhaugsbakk
@ 2026-02-12 23:19       ` Junio C Hamano
  2026-02-13 14:41         ` Phillip Wood
  0 siblings, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2026-02-12 23:19 UTC (permalink / raw)
  To: kristofferhaugsbakk
  Cc: git, Kristoffer Haugsbakk, Matthias Beyer,
	Christoph Anton Mitterer, Matheus Tavares, Chris Packham,
	Jakob Haufe, Phillip Wood

kristofferhaugsbakk@fastmail.com writes:

> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ...
> All of this is covered already, technically. However, we should spell
> out the implications.

I've read the new text (without formatting, I have to admit) again,
and did not see anything questionable.  Nicely written.

Shall we mark this for 'next'?

Thanks.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am"
  2026-02-06  9:03     ` Jeff King
  2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
  2026-02-09 15:58       ` git-am applies commit message diffs Patrick Steinhardt
@ 2026-02-13 14:34       ` Phillip Wood
  2026-02-13 14:34         ` [PATCH v2 1/2] templates: add .gitattributes entry for sample hooks Phillip Wood
                           ` (2 more replies)
  2 siblings, 3 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-13 14:34 UTC (permalink / raw)
  To: git, Jeff King; +Cc: Matthias Beyer, Jacob Keller, pyokagan, Junio C Hamano

From: Phillip Wood <phillip.wood@dunelm.org.uk>

This series adds a check to the sample commit-msg hook to reject commit
messages where the body of the message contains lines starting with
"diff -" and "Index: ". Such lines confuse "git am".

Changes since V1:

 - Allow subjects to start with "diff -" as they end up in an email
   header and so do not confuse "git am"

 - Allow "---" lines as they are useful when preparing patches.

Base-Commit: b2826b52eb7caff9f4ed6e85ec45e338bf02ad09
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fsample-commit-msg-reject-diff%2Fv2
View-Changes-At: https://github.com/phillipwood/git/compare/b2826b52e...494f4df68
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/sample-commit-msg-reject-diff/v2


Phillip Wood (2):
  templates: add .gitattributes entry for sample hooks
  templates: detect commit messages containing diffs

 .editorconfig                     |  2 +-
 .gitattributes                    |  1 +
 templates/hooks/commit-msg.sample | 54 +++++++++++++++++++++++++++++--
 3 files changed, 54 insertions(+), 3 deletions(-)

Range-diff against v1:
1:  5f5e3091435 = 1:  5f5e3091435 templates: add .gitattributes entry for sample hooks
2:  e75978b9591 ! 2:  494f4df6865 templates: detect commit messages containing diffs
    @@ Metadata
      ## Commit message ##
         templates: detect commit messages containing diffs
     
    -    If a commit message contains a diff that is not indented then "git
    -    am" will treat that diff as part of the patch rather than as part
    -    of the commit message. This allows it to apply email messages that
    -    were created by adding a commit message in front of a regular diff
    +    If the body of a commit message contains a diff that is not indented
    +    then "git am" will treat that diff as part of the patch rather than
    +    as part of the commit message. This allows it to apply email messages
    +    that were created by adding a commit message in front of a regular diff
         without adding the "---" separator used by "git format-patch". This
         often surprises users [1-4] so add a check to the sample "commit-msg"
    -    hook to reject messages that would confuse "git am".
    -
    -    Detecting if the message contains a diff is complicated by the hook
    -    being passed the message before it is cleaned up so we need to ignore
    -    any diffs below the scissors line. There are also two possible
    -    config keys to check to find the comment character at the start
    -    of the scissors line.
    +    hook to reject messages that would confuse "git am". Even if a project
    +    does not use an email based workflow it is not uncommon for people
    +    to generate patches from it and apply them with "git am". Therefore
    +    it is still worth discouraging the creation of commit messages that
    +    would not be applied correctly.
    +
    +    A further source of confusion when applying patches with "git am" is
    +    the "---" separator that is added by "git format patch". If a commit
    +    message body contains that line then it will be truncated by "git am".
    +    As this is often used by patch authors to add some commentary that
    +    they do not want to end up in the commit message when the patch is
    +    applied, the hook does not complain about the presence of "---" lines
    +    in the message.
    +
    +    Detecting if the message contains a diff is complicated by the
    +    hook being passed the message before it is cleaned up so we need to
    +    ignore any diffs below the scissors line. There are also two possible
    +    config keys to check to find the comment character at the start of
    +    the scissors line. The first paragraph of the commit message becomes
    +    the email subject header which beings "Subject: " and so does not
    +    need to be checked. The trailing ".*" when matching commented lines
    +    ensures that if the comment string ends with a "$" it is not treated
    +    as an anchor.
     
         [1] https://lore.kernel.org/git/bcqvh7ahjjgzpgxwnr4kh3hfkksfruf54refyry3ha7qk7dldf@fij5calmscvm
         [2] https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
    @@ templates/hooks/commit-msg.sample
     +			p
     +		}'
     +)"
    -+line="$(sed -n -e "/^${comment_re} -\{8,\} >8 -\{8,\}\$/q
    -+		   /^diff -/{p;q;}
    -+		   /^Index: /{p;q;}" "$1")"
    ++scissors_line="^${comment_re} -\{8,\} >8 -\{8,\}\$"
    ++comment_line="^${comment_re}.*"
    ++blank_line='^[ 	]*$'
    ++# Disallow lines starting with "diff -" or "Index: " in the body of the
    ++# message. Stop looking if we see a scissors line.
    ++line="$(sed -n -e "
    ++	# Skip comments and blank lines at the start of the file.
    ++	/${scissors_line}/q
    ++	/${comment_line}/d
    ++	/${blank_line}/d
    ++	# The first paragraph will become the subject header so
    ++	# does not need to be checked.
    ++	: subject
    ++	n
    ++	/${scissors_line}/q
    ++	/${blank_line}/!b subject
    ++	# Check the body of the message for problematic
    ++	# prefixes.
    ++	: body
    ++	n
    ++	/${scissors_line}/q
    ++	/${comment_line}/b body
    ++	/^diff -/{p;q;}
    ++	/^Index: /{p;q;}
    ++	b body
    ++	" "$1")"
     +if test -n "$line"
     +then
     +	echo >&2 "Message contains a diff that will confuse 'git am'."
3:  83c100a73ec < -:  ----------- templates: detect messages that contain a separator line
-- 
2.52.0.362.g884e03848a9


^ permalink raw reply	[flat|nested] 65+ messages in thread

* [PATCH v2 1/2] templates: add .gitattributes entry for sample hooks
  2026-02-13 14:34       ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
@ 2026-02-13 14:34         ` Phillip Wood
  2026-02-13 14:34         ` [PATCH v2 2/2] templates: detect commit messages containing diffs Phillip Wood
  2026-02-13 17:41         ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Junio C Hamano
  2 siblings, 0 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-13 14:34 UTC (permalink / raw)
  To: git, Jeff King
  Cc: Matthias Beyer, Jacob Keller, pyokagan, Junio C Hamano,
	Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

The sample hooks are shell scripts but the filenames end with ".sample"
so they need their own .gitattributes rule. Update our editorconfig
settings to match the attributes as well.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 .editorconfig  | 2 +-
 .gitattributes | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/.editorconfig b/.editorconfig
index 2d3929b5916..6e4eaa8e955 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -4,7 +4,7 @@ insert_final_newline = true
 
 # The settings for C (*.c and *.h) files are mirrored in .clang-format.  Keep
 # them in sync.
-[{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile}]
+[{*.{c,h,sh,bash,perl,pl,pm,txt,adoc},config.mak.*,Makefile,templates/hooks/*.sample}]
 indent_style = tab
 tab_width = 8
 
diff --git a/.gitattributes b/.gitattributes
index 38b1c52fe0e..556322be01b 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -18,3 +18,4 @@ CODE_OF_CONDUCT.md -whitespace
 /Documentation/user-manual.adoc conflict-marker-size=32
 /t/t????-*.sh conflict-marker-size=32
 /t/unit-tests/clar/test/expected/* whitespace=-blank-at-eof
+/templates/hooks/*.sample whitespace=indent,trail,space,incomplete text eol=lf
-- 
2.52.0.362.g884e03848a9


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* [PATCH v2 2/2] templates: detect commit messages containing diffs
  2026-02-13 14:34       ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
  2026-02-13 14:34         ` [PATCH v2 1/2] templates: add .gitattributes entry for sample hooks Phillip Wood
@ 2026-02-13 14:34         ` Phillip Wood
  2026-02-13 16:42           ` Kristoffer Haugsbakk
  2026-02-13 17:59           ` Junio C Hamano
  2026-02-13 17:41         ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Junio C Hamano
  2 siblings, 2 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-13 14:34 UTC (permalink / raw)
  To: git, Jeff King
  Cc: Matthias Beyer, Jacob Keller, pyokagan, Junio C Hamano,
	Phillip Wood

From: Phillip Wood <phillip.wood@dunelm.org.uk>

If the body of a commit message contains a diff that is not indented
then "git am" will treat that diff as part of the patch rather than
as part of the commit message. This allows it to apply email messages
that were created by adding a commit message in front of a regular diff
without adding the "---" separator used by "git format-patch". This
often surprises users [1-4] so add a check to the sample "commit-msg"
hook to reject messages that would confuse "git am". Even if a project
does not use an email based workflow it is not uncommon for people
to generate patches from it and apply them with "git am". Therefore
it is still worth discouraging the creation of commit messages that
would not be applied correctly.

A further source of confusion when applying patches with "git am" is
the "---" separator that is added by "git format patch". If a commit
message body contains that line then it will be truncated by "git am".
As this is often used by patch authors to add some commentary that
they do not want to end up in the commit message when the patch is
applied, the hook does not complain about the presence of "---" lines
in the message.

Detecting if the message contains a diff is complicated by the
hook being passed the message before it is cleaned up so we need to
ignore any diffs below the scissors line. There are also two possible
config keys to check to find the comment character at the start of
the scissors line. The first paragraph of the commit message becomes
the email subject header which beings "Subject: " and so does not
need to be checked. The trailing ".*" when matching commented lines
ensures that if the comment string ends with a "$" it is not treated
as an anchor.

[1] https://lore.kernel.org/git/bcqvh7ahjjgzpgxwnr4kh3hfkksfruf54refyry3ha7qk7dldf@fij5calmscvm
[2] https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
[3] https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
[4] https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 templates/hooks/commit-msg.sample | 54 +++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/templates/hooks/commit-msg.sample b/templates/hooks/commit-msg.sample
index b58d1184a9d..f7458efe62f 100755
--- a/templates/hooks/commit-msg.sample
+++ b/templates/hooks/commit-msg.sample
@@ -15,10 +15,60 @@
 # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
 # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
 
-# This example catches duplicate Signed-off-by lines.
+# This example catches duplicate Signed-off-by lines and messages that
+# would confuse 'git am'.
+
+ret=0
 
 test "" = "$(grep '^Signed-off-by: ' "$1" |
 	 sort | uniq -c | sed -e '/^[ 	]*1[ 	]/d')" || {
 	echo >&2 Duplicate Signed-off-by lines.
-	exit 1
+	ret=1
 }
+
+comment_re="$(
+	{
+		git config --get-regexp "^core\.comment(char|string)\$" ||
+			echo '#'
+	} | sed -n -e '
+		${
+			s/^[^ ]* //
+			s|[][*./\]|\\&|g
+			s/^auto$/[#;@!$%^&|:]/
+			p
+		}'
+)"
+scissors_line="^${comment_re} -\{8,\} >8 -\{8,\}\$"
+comment_line="^${comment_re}.*"
+blank_line='^[ 	]*$'
+# Disallow lines starting with "diff -" or "Index: " in the body of the
+# message. Stop looking if we see a scissors line.
+line="$(sed -n -e "
+	# Skip comments and blank lines at the start of the file.
+	/${scissors_line}/q
+	/${comment_line}/d
+	/${blank_line}/d
+	# The first paragraph will become the subject header so
+	# does not need to be checked.
+	: subject
+	n
+	/${scissors_line}/q
+	/${blank_line}/!b subject
+	# Check the body of the message for problematic
+	# prefixes.
+	: body
+	n
+	/${scissors_line}/q
+	/${comment_line}/b body
+	/^diff -/{p;q;}
+	/^Index: /{p;q;}
+	b body
+	" "$1")"
+if test -n "$line"
+then
+	echo >&2 "Message contains a diff that will confuse 'git am'."
+	echo >&2 "To fix this indent the diff."
+	ret=1
+fi
+
+exit $ret
-- 
2.52.0.362.g884e03848a9


^ permalink raw reply related	[flat|nested] 65+ messages in thread

* Re: [PATCH v3] doc: add caveat about round-tripping format-patch
  2026-02-12 23:19       ` Junio C Hamano
@ 2026-02-13 14:41         ` Phillip Wood
  2026-02-13 14:43           ` Kristoffer Haugsbakk
  2026-02-13 18:02           ` Junio C Hamano
  0 siblings, 2 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-13 14:41 UTC (permalink / raw)
  To: Junio C Hamano, kristofferhaugsbakk
  Cc: git, Kristoffer Haugsbakk, Matthias Beyer,
	Christoph Anton Mitterer, Matheus Tavares, Chris Packham,
	Jakob Haufe, Phillip Wood

On 12/02/2026 23:19, Junio C Hamano wrote:
> kristofferhaugsbakk@fastmail.com writes:
> 
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>> ...
>> All of this is covered already, technically. However, we should spell
>> out the implications.
> 
> I've read the new text (without formatting, I have to admit) again,
> and did not see anything questionable.  Nicely written.
> 
> Shall we mark this for 'next'?

I'm happy with this version - thanks for working on it Kristoffer

Phillip


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v3] doc: add caveat about round-tripping format-patch
  2026-02-13 14:41         ` Phillip Wood
@ 2026-02-13 14:43           ` Kristoffer Haugsbakk
  2026-02-13 18:02           ` Junio C Hamano
  1 sibling, 0 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-13 14:43 UTC (permalink / raw)
  To: Phillip Wood, Junio C Hamano, Kristoffer Haugsbakk
  Cc: git, Matthias Beyer, Christoph Anton Mitterer, Matheus Tavares,
	Chris Packham, Jakob Haufe

On Fri, Feb 13, 2026, at 15:41, Phillip Wood wrote:
> On 12/02/2026 23:19, Junio C Hamano wrote:
>> kristofferhaugsbakk@fastmail.com writes:
>>
>>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>> ...
>>> All of this is covered already, technically. However, we should spell
>>> out the implications.
>>
>> I've read the new text (without formatting, I have to admit) again,
>> and did not see anything questionable.  Nicely written.
>>
>> Shall we mark this for 'next'?
>
> I'm happy with this version - thanks for working on it Kristoffer

Thanks for the helpful reviews, Phillip and Junio. :)

-- 
  Kristoffer Haugsbakk

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/2] templates: detect commit messages containing diffs
  2026-02-13 14:34         ` [PATCH v2 2/2] templates: detect commit messages containing diffs Phillip Wood
@ 2026-02-13 16:42           ` Kristoffer Haugsbakk
  2026-02-13 18:08             ` Junio C Hamano
  2026-02-14 14:46             ` Phillip Wood
  2026-02-13 17:59           ` Junio C Hamano
  1 sibling, 2 replies; 65+ messages in thread
From: Kristoffer Haugsbakk @ 2026-02-13 16:42 UTC (permalink / raw)
  To: Phillip Wood, git, Jeff King
  Cc: Matthias Beyer, Jacob Keller, pyokagan, Junio C Hamano,
	Phillip Wood

On Fri, Feb 13, 2026, at 15:34, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> If the body of a commit message contains a diff that is not indented
> then "git am" will treat that diff as part of the patch rather than
> as part of the commit message. This allows it to apply email messages
> that were created by adding a commit message in front of a regular diff
> without adding the "---" separator used by "git format-patch". This
> often surprises users [1-4] so add a check to the sample "commit-msg"
> hook to reject messages that would confuse "git am". Even if a project
> does not use an email based workflow it is not uncommon for people
> to generate patches from it and apply them with "git am". Therefore
> it is still worth discouraging the creation of commit messages that
> would not be applied correctly.
>
> A further source of confusion when applying patches with "git am" is
> the "---" separator that is added by "git format patch". If a commit
> message body contains that line then it will be truncated by "git am".
> As this is often used by patch authors to add some commentary that
> they do not want to end up in the commit message when the patch is
> applied, the hook does not complain about the presence of "---" lines
> in the message.
>
> Detecting if the message contains a diff is complicated by the
> hook being passed the message before it is cleaned up so we need to
> ignore any diffs below the scissors line. There are also two possible
> config keys to check to find the comment character at the start of
> the scissors line. The first paragraph of the commit message becomes
> the email subject header which beings "Subject: " and so does not
> need to be checked. The trailing ".*" when matching commented lines
> ensures that if the comment string ends with a "$" it is not treated
> as an anchor.
>
> [1]
> https://lore.kernel.org/git/bcqvh7ahjjgzpgxwnr4kh3hfkksfruf54refyry3ha7qk7dldf@fij5calmscvm
> [2]
> https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
> [3]
> https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
> [4]
> https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  templates/hooks/commit-msg.sample | 54 +++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
>[snip]

This works for me with `git commit --cleanup=scissors --verbose`.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am"
  2026-02-13 14:34       ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
  2026-02-13 14:34         ` [PATCH v2 1/2] templates: add .gitattributes entry for sample hooks Phillip Wood
  2026-02-13 14:34         ` [PATCH v2 2/2] templates: detect commit messages containing diffs Phillip Wood
@ 2026-02-13 17:41         ` Junio C Hamano
  2 siblings, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2026-02-13 17:41 UTC (permalink / raw)
  To: Phillip Wood; +Cc: git, Jeff King, Matthias Beyer, Jacob Keller, pyokagan

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> This series adds a check to the sample commit-msg hook to reject commit
> messages where the body of the message contains lines starting with
> "diff -" and "Index: ". Such lines confuse "git am".
>
> Changes since V1:
>
>  - Allow subjects to start with "diff -" as they end up in an email
>    header and so do not confuse "git am"
>
>  - Allow "---" lines as they are useful when preparing patches.

I see some mention of scissors line that is also new.


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/2] templates: detect commit messages containing diffs
  2026-02-13 14:34         ` [PATCH v2 2/2] templates: detect commit messages containing diffs Phillip Wood
  2026-02-13 16:42           ` Kristoffer Haugsbakk
@ 2026-02-13 17:59           ` Junio C Hamano
  2026-02-14 14:36             ` Phillip Wood
  1 sibling, 1 reply; 65+ messages in thread
From: Junio C Hamano @ 2026-02-13 17:59 UTC (permalink / raw)
  To: Phillip Wood; +Cc: git, Jeff King, Matthias Beyer, Jacob Keller, pyokagan

Phillip Wood <phillip.wood123@gmail.com> writes:

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> If the body of a commit message contains a diff that is not indented
> then "git am" will treat that diff as part of the patch rather than
> as part of the commit message. This allows it to apply email messages
> that were created by adding a commit message in front of a regular diff
> without adding the "---" separator used by "git format-patch". This
> often surprises users [1-4] so add a check to the sample "commit-msg"
> hook to reject messages that would confuse "git am". Even if a project
> does not use an email based workflow it is not uncommon for people
> to generate patches from it and apply them with "git am". Therefore
> it is still worth discouraging the creation of commit messages that
> would not be applied correctly.
>
> A further source of confusion when applying patches with "git am" is
> the "---" separator that is added by "git format patch". If a commit
> message body contains that line then it will be truncated by "git am".
> As this is often used by patch authors to add some commentary that
> they do not want to end up in the commit message when the patch is
> applied, the hook does not complain about the presence of "---" lines
> in the message.

"git format match" -> "git format-patch".

> Detecting if the message contains a diff is complicated by the
> hook being passed the message before it is cleaned up so we need to
> ignore any diffs below the scissors line.

Sorry, but I do not quite understand the logic here.  In e-mailed
messages, the way the scissors line is most commonly used is to have
something like this.

	Hi, I read your problem report, and I think what is going on
	is ... (lengthy discussion here).

	Can you try this patch?

	--- >8 ---
	Subject: frotz: try working around nitfol

	As we cannot easily tell if the gostak will distim these
	patciular doshes, let's be careful to see ...

	diff - will be used to confuse the mailinfo

	Signed-off-by: a.u.thour
	---
	(diffstat here)
	(patch here)

and "diff - will be used to confuse" is something we would want to
notice.  But I am not sure if the use case of committing a scissors
line.  You help those who write a three-dash line and materials
meant to be kept outside of the final commit at the end, so if is
this an attempt to help those who write a scissors line and
materials meant to be kept outside of the final commit at the
beginning, I can understand, but then don't you want to notice "diff
-" that appears after the scissors line?  I do not offhand remember
what happens to a "diff -" that appears before the scissors (i.e.,
if you write "diff -" before "Can you try this patch?"), but I
wouldn't be surprised if mailinfo stopped there long before it sees
the scissors.

> There are also two possible
> config keys to check to find the comment character at the start of
> the scissors line.

Also I do not think scissors requires to be a comment.

So, I am a bit confused.

> The first paragraph of the commit message becomes
> the email subject header which beings "Subject: " and so does not
> need to be checked.

Great.

> The trailing ".*" when matching commented lines
> ensures that if the comment string ends with a "$" it is not treated
> as an anchor.

I am not sure what this means.  Wouldn't these three

	sed -e '/^#/d'
	sed -e '/^#.*/d'
	sed -e '/^#.*$/d'

work exactly the same way?

Thanks.

> [1] https://lore.kernel.org/git/bcqvh7ahjjgzpgxwnr4kh3hfkksfruf54refyry3ha7qk7dldf@fij5calmscvm
> [2] https://lore.kernel.org/git/ca13705ae4817ffba16f97530637411b59c9eb19.camel@scientia.org/
> [3] https://lore.kernel.org/git/d0b577825124ac684ab304d3a1395f3d2d0708e8.1662333027.git.matheus.bernardino@usp.br/
> [4] https://lore.kernel.org/git/CAFOYHZC6Qd9wkoWPcTJDxAs9u=FGpHQTkjE-guhwkya0DRVA6g@mail.gmail.com/
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  templates/hooks/commit-msg.sample | 54 +++++++++++++++++++++++++++++--
>  1 file changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/templates/hooks/commit-msg.sample b/templates/hooks/commit-msg.sample
> index b58d1184a9d..f7458efe62f 100755
> --- a/templates/hooks/commit-msg.sample
> +++ b/templates/hooks/commit-msg.sample
> @@ -15,10 +15,60 @@
>  # SOB=$(git var GIT_AUTHOR_IDENT | sed -n 's/^\(.*>\).*$/Signed-off-by: \1/p')
>  # grep -qs "^$SOB" "$1" || echo "$SOB" >> "$1"
>  
> -# This example catches duplicate Signed-off-by lines.
> +# This example catches duplicate Signed-off-by lines and messages that
> +# would confuse 'git am'.
> +
> +ret=0
>  
>  test "" = "$(grep '^Signed-off-by: ' "$1" |
>  	 sort | uniq -c | sed -e '/^[ 	]*1[ 	]/d')" || {
>  	echo >&2 Duplicate Signed-off-by lines.
> -	exit 1
> +	ret=1
>  }
> +
> +comment_re="$(
> +	{
> +		git config --get-regexp "^core\.comment(char|string)\$" ||
> +			echo '#'
> +	} | sed -n -e '
> +		${
> +			s/^[^ ]* //
> +			s|[][*./\]|\\&|g
> +			s/^auto$/[#;@!$%^&|:]/
> +			p
> +		}'
> +)"
> +scissors_line="^${comment_re} -\{8,\} >8 -\{8,\}\$"
> +comment_line="^${comment_re}.*"
> +blank_line='^[ 	]*$'
> +# Disallow lines starting with "diff -" or "Index: " in the body of the
> +# message. Stop looking if we see a scissors line.
> +line="$(sed -n -e "
> +	# Skip comments and blank lines at the start of the file.
> +	/${scissors_line}/q
> +	/${comment_line}/d
> +	/${blank_line}/d
> +	# The first paragraph will become the subject header so
> +	# does not need to be checked.
> +	: subject
> +	n
> +	/${scissors_line}/q
> +	/${blank_line}/!b subject
> +	# Check the body of the message for problematic
> +	# prefixes.
> +	: body
> +	n
> +	/${scissors_line}/q
> +	/${comment_line}/b body
> +	/^diff -/{p;q;}
> +	/^Index: /{p;q;}
> +	b body
> +	" "$1")"
> +if test -n "$line"
> +then
> +	echo >&2 "Message contains a diff that will confuse 'git am'."
> +	echo >&2 "To fix this indent the diff."
> +	ret=1
> +fi
> +
> +exit $ret

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v3] doc: add caveat about round-tripping format-patch
  2026-02-13 14:41         ` Phillip Wood
  2026-02-13 14:43           ` Kristoffer Haugsbakk
@ 2026-02-13 18:02           ` Junio C Hamano
  1 sibling, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2026-02-13 18:02 UTC (permalink / raw)
  To: Phillip Wood
  Cc: kristofferhaugsbakk, git, Kristoffer Haugsbakk, Matthias Beyer,
	Christoph Anton Mitterer, Matheus Tavares, Chris Packham,
	Jakob Haufe, Phillip Wood

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 12/02/2026 23:19, Junio C Hamano wrote:
>> kristofferhaugsbakk@fastmail.com writes:
>> 
>>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>> ...
>>> All of this is covered already, technically. However, we should spell
>>> out the implications.
>> 
>> I've read the new text (without formatting, I have to admit) again,
>> and did not see anything questionable.  Nicely written.
>> 
>> Shall we mark this for 'next'?
>
> I'm happy with this version - thanks for working on it Kristoffer
>
> Phillip

Thanks for reviewing, Phillip, and thanks for writing, Kristoffer.

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/2] templates: detect commit messages containing diffs
  2026-02-13 16:42           ` Kristoffer Haugsbakk
@ 2026-02-13 18:08             ` Junio C Hamano
  2026-02-14 14:46             ` Phillip Wood
  1 sibling, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2026-02-13 18:08 UTC (permalink / raw)
  To: Kristoffer Haugsbakk
  Cc: Phillip Wood, git, Jeff King, Matthias Beyer, Jacob Keller,
	pyokagan, Phillip Wood

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:

> This works for me with `git commit --cleanup=scissors --verbose`.

Ahhhh, OK, my previous message was talking aoubt completely
different kind of scissors.  Yes, what we take out of the log
message editor will have these "git commit" generated comments and
crufts, and we do need to strip them out ourselves.

Phillip, sorry for a confused message earlier, and please forget
everything I said about scissors (I may have said worthy-to-listen
things on other things, but I do not offhand recall).

Kristoffer, thanks for a review.


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/2] templates: detect commit messages containing diffs
  2026-02-13 17:59           ` Junio C Hamano
@ 2026-02-14 14:36             ` Phillip Wood
  2026-02-14 15:42               ` Junio C Hamano
  0 siblings, 1 reply; 65+ messages in thread
From: Phillip Wood @ 2026-02-14 14:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Jeff King, Matthias Beyer, Jacob Keller, pyokagan

On 13/02/2026 17:59, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> If the body of a commit message contains a diff that is not indented
>> then "git am" will treat that diff as part of the patch rather than
>> as part of the commit message. This allows it to apply email messages
>> that were created by adding a commit message in front of a regular diff
>> without adding the "---" separator used by "git format-patch". This
>> often surprises users [1-4] so add a check to the sample "commit-msg"
>> hook to reject messages that would confuse "git am". Even if a project
>> does not use an email based workflow it is not uncommon for people
>> to generate patches from it and apply them with "git am". Therefore
>> it is still worth discouraging the creation of commit messages that
>> would not be applied correctly.
>>
>> A further source of confusion when applying patches with "git am" is
>> the "---" separator that is added by "git format patch". If a commit
>> message body contains that line then it will be truncated by "git am".
>> As this is often used by patch authors to add some commentary that
>> they do not want to end up in the commit message when the patch is
>> applied, the hook does not complain about the presence of "---" lines
>> in the message.
> 
> "git format match" -> "git format-patch".

Thanks (I was confused for a minute because it says "format patch" above 
not "format match" but you're pointing out that it should be hypenated)

>> The trailing ".*" when matching commented lines
>> ensures that if the comment string ends with a "$" it is not treated
>> as an anchor.
> 
> I am not sure what this means.  Wouldn't these three
> 
> 	sed -e '/^#/d'
> 	sed -e '/^#.*/d'
> 	sed -e '/^#.*$/d'
> 
> work exactly the same way?

They do, but if the comment string is '$' then these two

	sed -e '/^$/d'
	sed -e '/^$.*/d'

have different meanings

Thanks

Phillip


^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/2] templates: detect commit messages containing diffs
  2026-02-13 16:42           ` Kristoffer Haugsbakk
  2026-02-13 18:08             ` Junio C Hamano
@ 2026-02-14 14:46             ` Phillip Wood
  1 sibling, 0 replies; 65+ messages in thread
From: Phillip Wood @ 2026-02-14 14:46 UTC (permalink / raw)
  To: Kristoffer Haugsbakk, Phillip Wood, git, Jeff King
  Cc: Matthias Beyer, Jacob Keller, pyokagan, Junio C Hamano

On 13/02/2026 16:42, Kristoffer Haugsbakk wrote:
> 
> This works for me with `git commit --cleanup=scissors --verbose`.

Thanks for testing it.

Phillip

^ permalink raw reply	[flat|nested] 65+ messages in thread

* Re: [PATCH v2 2/2] templates: detect commit messages containing diffs
  2026-02-14 14:36             ` Phillip Wood
@ 2026-02-14 15:42               ` Junio C Hamano
  0 siblings, 0 replies; 65+ messages in thread
From: Junio C Hamano @ 2026-02-14 15:42 UTC (permalink / raw)
  To: Phillip Wood; +Cc: git, Jeff King, Matthias Beyer, Jacob Keller, pyokagan

Phillip Wood <phillip.wood123@gmail.com> writes:

>>> The trailing ".*" when matching commented lines
>>> ensures that if the comment string ends with a "$" it is not treated
>>> as an anchor.
>> 
>> I am not sure what this means.  Wouldn't these three
>> 
>> 	sed -e '/^#/d'
>> 	sed -e '/^#.*/d'
>> 	sed -e '/^#.*$/d'
>> 
>> work exactly the same way?
>
> They do, but if the comment string is '$' then these two
>
> 	sed -e '/^$/d'
> 	sed -e '/^$.*/d'
>
> have different meanings

Ahh, that is what you meant.

Having to write "/^\$/" is inconvenient, because others characters
do not generally need the backslash (e.g., "/^\#/" and "/^#/" are
the same) and some characters even may become nonsensical if we
blindly add backslash to everybody (e.g., "/^\1/").

Nobody would use "[" as a commentchar, I hope, as "/^[/d" would not
work, and neither "/^[.*/d" does.

Thanks.

^ permalink raw reply	[flat|nested] 65+ messages in thread

end of thread, other threads:[~2026-02-14 15:42 UTC | newest]

Thread overview: 65+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-06  7:43 git-am applies commit message diffs Matthias Beyer
2026-02-06  8:04 ` Jacob Keller
2026-02-06  8:18   ` Matthias Beyer
2026-02-06  9:03     ` Jeff King
2026-02-07 14:57       ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
2026-02-07 14:58         ` [PATCH 1/3] templates: add .gitattributes entry for sample hooks Phillip Wood
2026-02-07 14:58         ` [PATCH 2/3] templates: detect commit messages containing diffs Phillip Wood
2026-02-07 14:58         ` [PATCH 3/3] templates: detect messages that contain a separator line Phillip Wood
2026-02-07 21:27           ` Junio C Hamano
2026-02-07 21:38             ` Kristoffer Haugsbakk
2026-02-09  0:17               ` Junio C Hamano
2026-02-09  7:00             ` Jeff King
2026-02-09 10:42               ` Phillip Wood
2026-02-10  6:44                 ` Jeff King
2026-02-09  6:57         ` [PATCH 0/3] commit-msg.sample: reject messages that would confuse "git am" Jeff King
2026-02-09 10:43           ` Phillip Wood
2026-02-09 11:07             ` Matthias Beyer
2026-02-10  6:46             ` Jeff King
2026-02-09 15:58       ` git-am applies commit message diffs Patrick Steinhardt
2026-02-10  2:16         ` Jacob Keller
2026-02-10 14:22           ` Patrick Steinhardt
2026-02-10 15:47             ` Junio C Hamano
2026-02-11  2:31               ` Jacob Keller
2026-02-11  2:34                 ` Jacob Keller
2026-02-11  7:47                   ` Jeff King
2026-02-11 15:23                     ` Kristoffer Haugsbakk
2026-02-11 15:47                     ` Junio C Hamano
2026-02-10  6:56         ` Jeff King
2026-02-13 14:34       ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Phillip Wood
2026-02-13 14:34         ` [PATCH v2 1/2] templates: add .gitattributes entry for sample hooks Phillip Wood
2026-02-13 14:34         ` [PATCH v2 2/2] templates: detect commit messages containing diffs Phillip Wood
2026-02-13 16:42           ` Kristoffer Haugsbakk
2026-02-13 18:08             ` Junio C Hamano
2026-02-14 14:46             ` Phillip Wood
2026-02-13 17:59           ` Junio C Hamano
2026-02-14 14:36             ` Phillip Wood
2026-02-14 15:42               ` Junio C Hamano
2026-02-13 17:41         ` [PATCH v2 0/2] commit-msg.sample: reject messages that would confuse "git am" Junio C Hamano
2026-02-06  8:59   ` git-am applies commit message diffs Florian Weimer
2026-02-06  9:24     ` Jeff King
2026-02-06  9:48       ` Florian Weimer
2026-02-06 10:08         ` Jeff King
2026-02-06  8:43 ` Kristoffer Haugsbakk
2026-02-06 17:45   ` Jakob Haufe
2026-02-07 10:08     ` Kristoffer Haugsbakk
2026-02-07 21:44 ` Kristoffer Haugsbakk
2026-02-08  0:11 ` [PATCH] doc: add caveat about roundtripping format-patch kristofferhaugsbakk
2026-02-08  1:39   ` Junio C Hamano
2026-02-08 17:18     ` Kristoffer Haugsbakk
2026-02-09 16:42   ` Phillip Wood
2026-02-09 17:59     ` Kristoffer Haugsbakk
2026-02-10 10:57       ` Phillip Wood
2026-02-10 16:00         ` Kristoffer Haugsbakk
2026-02-09 22:37   ` [PATCH v2] " kristofferhaugsbakk
2026-02-09 22:59     ` Junio C Hamano
2026-02-09 23:11       ` Kristoffer Haugsbakk
2026-02-10 11:02     ` Phillip Wood
2026-02-10 18:20     ` Kristoffer Haugsbakk
2026-02-12 22:28     ` [PATCH v3] doc: add caveat about round-tripping format-patch kristofferhaugsbakk
2026-02-12 23:19       ` Junio C Hamano
2026-02-13 14:41         ` Phillip Wood
2026-02-13 14:43           ` Kristoffer Haugsbakk
2026-02-13 18:02           ` Junio C Hamano
2026-02-10  0:53   ` [PATCH] doc: add caveat about roundtripping format-patch Christoph Anton Mitterer
2026-02-10 16:00     ` Kristoffer Haugsbakk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox