Re: [PATCH v2 4/5] fetch: teach independent negotiation (no packfile)

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

Derrick Stolee <stolee@gmail.com> writes:

>> Moving to lookup_object() made me look around, but at this point the
>> object in question (which comes from the negotiation_tips) has been
>> instantiated, so it is OK.
>> 
>>     Side note. The big difference between lookup and parse is what
>>     happens when this process hasn't seen the object yet---the
>>     former will just return NULL instead of instantiating the
>>     in-core copy; for that reason, it is easier on the readers to
>>     use parse_object() if we know we want an in-core object *and*
>>     when we use it we want to see it parsed already).
>
> Please forgive my incorrect recommendation here. I was expecting
> lookup_object() to behave like lookup_commit(), which creates the
> object if it is not already in the cached set.

It is not wrong per-se to recommend or use lookup_object() in this
case, as long as who recommends or uses the function is certain that
the object should already exist in-core.

But it is just that the lookup_object() is often not what the caller
wants to use; it is a very rare use case for a caller to be able to
say "I am OK for you to return NULL to me for this object, even if
it is in our object store, when we do not yet have it in-core".  The
only exception is the implementation of lookup_<type>() helpers,
where they handle "not loaded yet" answer from lookup_object() with
create_object() and always return usable in-core object to the
caller.

So the sole practical use case for lookup_object() outside the
lookup_<type>() implementation becomes "I am 100% sure that an
object with this object name has already been loaded in-core, so
please let me access it", which may be optimized for a wrong case
(because most of the API pass in-core objects, not object names,
around).  I am not sure if this particular API strangeness can be
remedied, though.

Thanks.