Re: [PATCH v3 7/7] documentation: format-patch: clarify requirements for patch-ids to match

From: Jerry Zhang <jerry@skydio.com>
Date: Oct 18, 2022, 21:57 UTC
Message-ID: <CAMKO5CuqLSowSo3fhOux-fY8ek-CL4zudgA0fBjXAt+9CBhs9g@mail.gmail.com>
In-Reply-To: <xmqqo7ua1nrz.fsf@gitster.g>

On Mon, Oct 17, 2022 at 8:19 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 21 quoted lines

>
> "Jerry Zhang via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> >  The 'base commit' is shown as "base-commit: " followed by the 40-hex of
> >  the commit object name.  A 'prerequisite patch' is shown as
> >  "prerequisite-patch-id: " followed by the 40-hex 'patch id', which can
> > -be obtained by passing the patch through the `git patch-id --stable`
> > -command.
> > +be obtained by passing the patch (generated with -U3 --full-index) through
> > +the `git patch-id --stable` command.
>
> This is not incorrect per-se, but I wonder how much it would help or
> mislead people in practice.
>
> I understand that the update means well to help those who complain
> "'patch-id' produces wrong result when I feed the output of 'git
> diff -U1'" by making them suspect that their -U1 may be the culprit.
> But the new description does not cover everything that can affect
> the resulting patch ID (the choice of --diff-algorithm affects how
> common lines are matched up between the preimage and the postimage,
> for example).

I can add a note about diff algorithm as well. Its slightly different from the other two options in that there should be fewer cases where diff algorithm is a problem. U3 and full-index aren't the default options to "git diff" so people are more likely to have the wrong options, whereas they would explicitly have to run the two diffs with different algorithms to run into a problem. But there's no harm in being more specific.

>
> So, I dunno.

Keeping patch-ids the same is difficult especially between different git versions, as changes could be made to "diff" that seem like improvements but would slightly change patch-id even with the same args. I expect that people generally aren't keeping the compatibility of patch-id in mind when changing diff. Nevertheless since we've already advertised that they would match, we ought to give users the best advice possible to minimize if not eliminate confusion.