Re: [PATCH] bisect: don't use invalid oid as rev when starting

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> A slightly bigger question is whether `git_oid_committish()` would be okay
> enough, or whether we do need `get_oidf(&oid, "%s^{commit}", arg)` (as
> your `SQUASH???` does).
>
> I'm not quite sure: aren't those two calls idempotent, with the latter
> going through some unnecessary string processing dances?

I tend to agree that get_oidf() is not quite satisfactory but it is
a handy short-hand that is readily available to us to parse a string
into the name of the object of type commit the string peels into.

While get_oid_X_ish() makes sure the result can be peeled to type X,
it gives the outer object back to the caller to allow it to behave
differently and it is up to the caller to peel the tag down to
commit.

I audited all uses of get_oid_X_ish(), in the hope that perhaps we
have enough callers that want to get peeled object back, and more
importantly, no caller that wants the original.  Then we could
change these to peel for the callers, and we may be able to lose a
few lines from the callers.

But unfortunately that was not the case.  A new helper

	int oid_of_type(struct object_id *result_oid,
			const char *name_text,
			enum object_type peel_to);

is a possibility, but I have a suspicion that the API in question
encourages to manipulate and swap objects at their hash level for
too long with little upside.  If we were considering to clean the
callers up, we would want to encourage them to work with in-core
objects longer.

IOW, repo_peel_to_type() might be a better fit for some callers that
use get_oid_X_ish() and then peel the result to obtain an object of
desired type.

Thanks.