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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 14, 2019, 13:06 UTC
Message-ID: <nycvar.QRO.7.76.6.1903141403360.41@tvgsbejvaqbjf.bet>
In-Reply-To: <0f7870b7-f92a-ad63-7854-300c202cb4a8@kdbg.org>

Hi Hannes,

On Thu, 14 Mar 2019, Johannes Sixt wrote:

Show 14 quoted lines

> Am 13.03.19 um 17:35 schrieb Jeff King:
> > On Wed, Mar 13, 2019 at 05:11:44PM +0100, Ævar Arnfjörð Bjarmason wrote:
> >> 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.
> 
> 'pick' needs all the power of revision expressions. Not all too
> infrequently I do insert a pick line with a rev expression argument.
> Assuming that the resolved object is a commit is too strict.

No need to worry. `get_oid_committish()` can resolve all kinds of crazy expressions. In a way, it is surprising that it will even resolve "HEAD:Makefile". The only thing (that I know of) that `get_oid_committish()` does that `get_oid()` does not is when you pass a short hash which is unambiguous *only* when you exclude blobs and trees from the disambiguation.

So I think `get_oid_committish()` will serve us really well, as we really want to parse the given hash name into the `struct todo_item`'s `commit` field.

Ciao, Dscho