Re: [RFC PATCH 0/2] extend --abbrev support to diff-patch format

From: Jeff King <peff@peff.net>
Date: Aug 10, 2020, 15:27 UTC
Message-ID: <20200810152705.GA61606@coredump.intra.peff.net>
In-Reply-To: <xmqqy2mmcz1e.fsf@gitster.c.googlers.com>

On Mon, Aug 10, 2020 at 08:15:41AM -0700, Junio C Hamano wrote:

Show 17 quoted lines

> > 	A lot of those patches couldn't be applied cleanly to old
> > 	versions of said software, thus requires some changes from
> > 	developer and they needs to be regenerated from their trimmed
> > 	tree. Because the archive tree has significantly fewer
> > 	objects, the abbreviation in the index line is usually shorter
> > 	than the original patch. Thus, it generates some noise when
> > 	said developers try to compare the new patch with the original
> > 	patch if there's an exact file-hunk match.
> >
> > 	Make the object name's abbreviation length configurable to
> > 	lower those noise.
> 
> I agree with Peff that with the above as the sole motivating use
> case, the "--full-index" option is the right approach.  It is a much
> more robust solution than "--abbrev=16 would be long enough for all
> project participants to avoid length drift".  IOW these four
> paragraphs do not argue _for_ this change, at least to me.

Yeah, that's what I was getting at: if you care about robust machine-readability, then the full index is the best solution. Reading between the lines, I think the argument may be "using --full-index is too long and therefore ugly, so people like the short-ish names but with a bit of extra safety".

There's an extra challenge here, which is that you have to convince the sender to use the extra --abbrev option, even though they themselves won't be the ones running into the problem when applying. But I don't think there's an elegant solution to that (we could just bump the default abbrev everywhere to 12+, which is enough in practice).

Though I'm not 100% sure that "git apply" is smart enough to only look at blobs (i.e., if "1234abcd" is ambiguous between a tree and a blob, ignore the tree since patches always apply to blobs). That might be another avenue that would make things more likely to work without anybody having to configure anything.

Show 6 quoted lines

> On the other hand, I think you could argue that "--full-index" is
> merely a synonym for "--abbrev=40", and the patch fixes the
> inconsistency between the object names on the "index" line, which
> can choose only between the default abbrev length and the full
> abbrev length, and all the other places we show object names, which
> uniformly honor the "--abbrev" option.

Yeah, I certainly don't mind the extra flexibility between "full" and "default" for "index" lines. I do wonder if people want to configure the abbreviations for those lines separately from other parts. I don't know that I've ever particularly cared about that flexibility, but the fact that they were set up separately all those years ago makes me think somebody might.

-Peff