Re: [PATCH v3 5/7] builtin: patch-id: add --include-whitespace as a command mode

[Jerry Zhang <jerry@skydio.com>]

On Fri, Oct 14, 2022 at 2:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Jerry Zhang via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > +--include-whitespace::
> > +     Use the "stable" algorithm described below and also don't strip whitespace
> > +     from lines when calculating the patch-id.
> > +
> > +     This is the default if patchid.includeWhitespace is true and implies
> > +     patchid.stable.
>
> This seems very much orthogonal to "--stable/--unstable.
>
> Because the "--stable" variant is more expensive than "--unstable",
I didn't realize it was more expensive, I'm assuming you mean in terms
of time, maybe it does
slightly more hashing operations under the hood?  I tried timing some
runs locally
and they were a wash:

time /bin/sh -c "git show | git patch-id --stable"
dddea79ee68d62a32cf8c0d7bb6691bcd0445628
4677fe858366a51ff3c5a0c0893418e32e934262

real 0m0.011s
user 0m0.003s
sys 0m0.012s
time /bin/sh -c "git show | git patch-id"
6602a3b2fe8b17d5bc295c2703901ad3e18eee18
4677fe858366a51ff3c5a0c0893418e32e934262

real 0m0.012s
user 0m0.009s
sys 0m0.007s

The operation is probably bound by process / disk overhead quite a bit
and a small
amount of cpu use wouldn't really be user-visible. Based on these
results I don't think
a user would choose --unstable just for the speed gain (if any).

> I am not sure why such an implication is a good thing to have.  Why
> can we not have
>
>     --include-whitespace --stable
>     --include-whitespace --unstable
>
> both combinations valid?
If you accept my point above, then a user would only choose
"--unstable" if they actually
had a need for backwards compatibility, such as for a persistent
database. Trying to include
whitespace on top of that would break the compatibility they're
relying on. So my conclusion was
that there isn't any usecase for the combination "--include-whitespace
--unstable", and it's better for
usability and not needing to always maintain compatibility if we don't
expose it to users at all.