Re: git-am applies commit message diffs

From: Jacob Keller <jacob.keller@gmail.com>
Date: Feb 10, 2026, 02:16 UTC
Message-ID: <CA+P7+xrNycJHTyJwn9AQcJLG0dDAE7KrTvWTHBi+CiQUqK8p5A@mail.gmail.com>
In-Reply-To: <aYoEO0CcVt2Qjgnb@pks.im>

On Mon, Feb 9, 2026 at 7:59 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 42 quoted lines

>
> 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.

Show 10 quoted lines

>   - 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?