Re: [PATCH] config: Introduce --patience config variable

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 7, 2012, 02:57 UTC
Message-ID: <7vlindp17d.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120306114914.GB6733@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> On Tue, Mar 06, 2012 at 11:59:42AM +0100, Michal Privoznik wrote:
>
>> Some users like the patience diff more than the bare diff. However,
>> specifying the '--patience' argument every time diff is to be used
>> is impractical. Moreover, creating an alias doesn't play with other
>> tools nice, e.g. git-show; Therefore we need a configuration variable
>> to turn this on/off across whole git tools.
>
> The idea of turning on patience diff via config makes sense to me, and
> it is not a problem for plumbing tools to have patience diff on, since
> patience diffs are syntactically identical to non-patience diffs.

Even though I do not strongly object to the general conclusion, I have to point out that the last line above is a dangerous thought.

If you change the default diff algorithm, you will invalidate long term caches that computed their keys based on the shape of patches produced in the past. The prime example being the rerere database, but I wouldn't be surprised if somebody has a notes tree to record patch ids for existing commits to speed up "git cherry" (hence "git rebase"). Also kup tool kernel.org folks use to optimize the uploading of inter-release diff relies on having a stable output from "git diff A..B", so that the uploader can run the command to produce a diff locally, GPG sign it, and upload only the signature and have the k.org side produce the same diff between two tags, without having to upload the huge patchfile over the wire.

IOW, "syntactically identical so it is OK" is not the right thought process. "It may change the shape of the patch, which is a potential problem for various tools, but as long as users understand that, it should be allowed." is OK.

Cached patch-id database for "git cherry" would be a local and does not affect the correctness, so I would consider breaking it is fine.

About kup use case, the potential problem can be worked around as long as the receiving end keeps the setting vanilla (and I do not see any reason it wouldn't) and the uploader remembers to choose the matching variant when he locally generates the patch, so I think it would be also OK.

Unnecessarily invalidating rerere database may be more frustrating, but that again is local and personal, so the end user may suffer worse than cached patch-id use case, but that hopefully is just one time pain.

My preference however is to limit this to Porcelains only, though.