Re: [PATCH v2] cache-tree: do not lazy-fetch merge tree

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

Jonathan Tan <jonathantanmy@google.com> writes:

> When cherry-picking (for example), new trees may be constructed. During
> this process, Git constructs the new tree in a struct strbuf, computes
> the OID of the new tree, and checks if the new OID already exists on
> disk. However, in a partial clone, the disk check causes a lazy fetch to
> occur, which is both unnecessary (because we have the tree in the struct
> strbuf) and likely to fail (because the remote probably doesn't have
> this tree).

I somehow smell that the above misses the point of the check in the
first place, though.  The reason why we are computing the tree
object's name and seeing if we have it locally on disk is to decide
if we want to record it in the cache tree, *without* writing the
tree out to our object store, no?

It is not just unnecessary but also against the goal of the codepath
to lazily download it, even if the tree is available remotely.  And
it is irrelevant that there are cases the remote does not have
it---we have no need to mention that we must be prepared to see the
lazy fetch to fail.  Even when they do have one, we do not want to
fetch it and write to our object store.

Isn't that what is going on?  I thought I dug up the original that
introduced the has_object_file() call to this codepath to make sure
we understand why we make the check (and I expected the person who
is proposing this change to do the same and record the finding in
the proposed log message).

I am running out of time today, and will revisit later this week
(I'll be down for at least two days starting tomorrow, by the way).

Thanks.