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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> > 	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.

> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux