Re: `git patch-id --stable' vs quoted-printable

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 22, 2022, 04:06 UTC
Message-ID: <xmqqczcsgbvn.fsf@gitster.g>
In-Reply-To: <20220822022503.M873583@dcvr>

Eric Wong <e@80x24.org> writes:

Show 8 quoted lines

> While poking around at the newish patchid indexing support in
> public-inbox[1], I noticed an inconsistency in how it seems to
> mishandle quoted-printable messages.
> ...
> So, I'm wondering if the search indexing code of public-inbox
> should s/^$/ /mgs before feeding stuff to `git patch-id'; and/or
> if `git patch-id' should be assuming empty lines and lines with a
> single SP are the same...

I suspect that QP is a red herring. I haven't looked at relevant code at all for a while, but what I think is going on is:

 * patch-id algorithm was written back when "unified" format of
   "diff" did not have the extension of GNU origin to allow an empty
   context line to be expressed as a truely empty line, not a single
   whitespace that signals it is a context line, followed by the
   contents of the line that is empty

 * "git apply" hence "git am" was taught to grok the empty context
   line extention, https://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html
   has this:

      It is implementation-defined whether an empty unaffected line is
      written as an empty line or a line containing a single <space> character.

   IIRC, this was added after GNU diff started emitting such an
   output (--suppress-blank-empty) and people complained that such a
   patch is not understood by us.

 * "git diff" was updated to allow this with diff.suppressBlankEmpty
   configuration , but that is never turned on by default.

So, if a patch producer runs "git diff" with diff.suppressBlankEmpty turned on, "git am" accepts it, and then you run "git show" without the configuration, then the "shape" of the patch text would be slightly different. I do not offhand know if we added configuration support to "patch-id", but even with a configuration knob, because once you turn incoming e-mail into a commit, the single bit (i.e. whether suppressBlankEmpty was in use or not) is forever lost, it would not be of much help. After all, the incoming patch can be hand munged to use both "single whitespace and the end of line" and "a completely empty line" to record an empty context line, and "am" has to take such a patch happily.

I *think* the right thing to do is for patch-id that takes text input to normalize the empty context line into one form or the other (as a conservatist, I would say we should probably pretend as if an empty context line is always expressed as a single whitespace on a line by itself) before computing the ID.

René, do you remember if you used diff.suppressBlankEmpty configuration when generating the patch in question at:

    https://public-inbox.org/git/6727daf1-f077-7319-187e-ab4e55de3b2d@web.de/raw

by the way?

Thanks.