Re: [PATCH 1/4] rebase -i: demonstrate obscure loose object cache bug

From: Jeff King <peff@peff.net>
Date: Mar 13, 2019, 16:35 UTC
Message-ID: <20190313163516.GA26045@sigill.intra.peff.net>
In-Reply-To: <87k1h2bvpb.fsf@evledraar.gmail.com>

On Wed, Mar 13, 2019 at 05:11:44PM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 9 quoted lines

> > And this is where the loose object cache interferes with this feature:
> > if *some* loose object was read whose hash shares the same first two
> > digits with a commit that was not yet created when that loose object was
> > created, then we fail to find that new commit by its short name in
> > `get_oid()`, and the interactive rebase fails with an obscure error
> > message like:
> >
> > 	error: invalid line 1: pick 6568fef
> > 	error: please fix this using 'git rebase --edit-todo'.

Are we 100% sure this part is necessary? From my understanding of the problem, even without any ambiguity get_oid() could fail due to just plain not finding the object in question.

> As a further improvement, is there a good reason for why we wouldn't
> pass something down to the oid machinery to say "we're only interested
> in commits". I have a WIP series somewhere to generalize that more, but
> e.g.  here locally:

We have get_oid_commit() and get_oid_committish() already. Should rebase just be using those? (I think we probably want "commit()", because we do not expect a "pick" line to have a tag, for example.

-Peff