Hi Ævar & Peff, On Wed, 13 Mar 2019, Jeff King wrote: > On Wed, Mar 13, 2019 at 05:11:44PM +0100, Ævar Arnfjörð Bjarmason wrote: > > > > 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. Indeed. It could be a typo, for example. Which is why that error message is so helpful. > > 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. I did think about this while developing this patch series, and decided against conflating concerns. And I was totally right to do so! Because I do have an internal ticket that talks about allowing `reset v2.20.1`, which is a tag, not a commit. Granted, it is easy to work around: just use `reset v2.20.1^0`, but it is quite annoying that we do not allow this at the moment: even if we do allow `get_oid()` to resolve the tag, we don't peel it to the commit. #leftoverbits Ciao, Dscho
