* 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 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
* [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: [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 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 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-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 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: [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-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: 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: 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: 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
* 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
* [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 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 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 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-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 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-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
* 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: 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: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 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: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
* 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] 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
* 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] 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
* [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 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: [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
* [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
* 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 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] 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: [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
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