[PATCH 0/10] helping people resolve ambiguous sha1s

From: Jeff King <peff@peff.net>
Date: Sep 26, 2016, 11:57 UTC
Message-ID: <20160926115720.p2yb22lcq37gboon@sigill.intra.peff.net>
In-Reply-To: <CAPc5daV1YJaEqH5eZCej3nkg8htHVDWQu0V0uoC4gVmPYpDL9Q@mail.gmail.com>

On Sun, Sep 25, 2016 at 09:45:18PM -0700, Junio C Hamano wrote:

Show 7 quoted lines

> On Sun, Sep 25, 2016 at 9:34 PM, Jeff King <peff@peff.net> wrote:
> >
> > An easier (but less automatic) tool would be to improve our error
> > message for the ambiguous case, and actually report details of the
> > candidates. I'm working up a patch now.
> 
> That sounds like a fun little lunch-break project. Thanks.

That's what I thought, but it turned out to be quite involved. :)

I started by trying to teach get_short_sha1() to remember all of the candidates it sees, but it turns out to be surprisingly complicated. I did have something working, but I scrapped it in favor of just looking at the object database again. It's the error code path, so it's OK to be slower (especially if it keeps the non-error code path much simpler).

But then being the diligent programmer that I am, I added a tests. And that failed because of an unrelated bug. Fixing that revealed another bug. And so on.

The good news is that I think I've finally cleared up all of the long-standing bugs where git will print the same error message twice. Those have been annoying me for yours (and apparently others[1]).

Patches 2-4 and 9 are all bugfixes. Patch 10 is the interesting part. The rest are just cleanups and refactoring.

  [01/10]: get_sha1: detect buggy calls with multiple disambiguators
  [02/10]: get_sha1: avoid repeating ourselves via ONLY_TO_DIE
  [03/10]: get_sha1: propagate flags to child functions
  [04/10]: get_short_sha1: peel tags when looking for treeish
  [05/10]: get_short_sha1: refactor init of disambiguation code
  [06/10]: get_short_sha1: NUL-terminate hex prefix
  [07/10]: get_short_sha1: mark ambiguity error for translation
  [08/10]: sha1_array: let callbacks interrupt iteration
  [09/10]: for_each_abbrev: drop duplicate objects
  [10/10]: get_short_sha1: list ambiguous objects on error

Of course this is all totally orthogonal to Linus's original question. I hope it will make things more pleasant when somebody does end up having to look up a too-short sha1, but it's probably still a good idea to bump the default.

-Peff

[1] http://public-inbox.org/git/504B91B7.1000406@avtalion.name/