libgit2/libgit2 — pull request #1782 by vmg (2013-08-14 08:44:27+00:00)
Title: Do not use the "experimental" lookup mode

As seen on this mailing list thread:

http://git.661346.n2.nabble.com/Make-GIT-USE-LOOKUP-default-td7579902.html

The `sha1_entry_pos` function we've been using for packfile lookups is still considered experimental on Core Git, mainly because it hasn't been proven to be reliably faster than a simple binary search.

This wasn't a problem for us (since both functions seem to be just as fast), but we've just hit a corner case in production. The repository at https://github.com/rcarmo/core-plot has a specific commit, `4ea4acbcb0930ac42acc87a0d203864dec1a9697` that fails to be looked up both with libgit2 **and** with Core Git when using `GIT_USE_LOOKUP`. This has caused a meltdown on fs23b when a friendly bot kept spamming our API asking for the specific commit (cc @scottjg).

For starters, I'm reverting the `sha1_entry_pos` implementation and bringing back the old school binary search, which should be just as fast. Old implementation doesn't fail with that commit anymore.

I've also dugged in into why is `sha1_entry_pos` failing in libgit2 and in Git. This [specific assertion](https://github.com/git/git/blob/master/sha1-lookup.c#L222) (`assert(lov < hiv);`) fails during the lookup because `lov = hiv = 78`. This doesn't seem to be a bug or general corruption, but simply the fact that the assertion is not correct. When changed to `assert(lov <= hiv)`, the lookup succeeds.

I've added that change to this commit, despite the fact that `sha1_entry_pos` is disabled. @peff I'd like your advice here. Do you want to bring this assertion fix upstream, at least to discuss with whoever wrote this infernal piece of code from hell?