Subject: Re: [PATCH v2 8/9] object-store: remove global array of cached objects

From: Junio C Hamano <[email-masked]>
Date: Fri, 11 Apr 2025 15:58:03 -0700

Patrick Steinhardt <[email-masked]> writes:

> Cached objects are virtual objects that can be set up without writing
> anything into the object store directly. This mechanism for example
> allows us to create fake commits in git-blame(1).
>
> The cached objects are stored in a global variable. Refactor the code so
> that we instead store the array as part of the raw object store. This is
> another step into the direction of libifying our object database.

While we do need some execution context object to hang these virtual
objects, once we decide that it cannot be global, I am not sure if
epository objects are good home for them.  If your application
running in a repository needs to give one object name to a virtual
object, and then that same application wants to access a submodule
of that repository in the same process image, wouldn't you have one
in-core repository object for the top-level superproject, and one
for each submodule?  If a submodule commit bound to a path in the
superproject's tree is a viertual "pretend" commit object or if it
has a virtual "pretend" tree object, don't you need to expose these
to both submodule and superproject repositories, if your application
wants to seamlessly cross the module boundary (think "git grep
--recurse-submodules" or something)?

For now, as long as the_repository is being used as that "execution
context object", and not a repository instance passed along the call
chain, then the globalness of these virtual objects is maintained,
so this change will not cause breakage (e.g., such an application
may want to pick up the virtual object from the repository instance
for the superproject and it may find it, but when traversing down to
a submdoule, the same virtual object may not be found in the
repository instance for the submodule it descended into and working
in, if you make it per repository and pass repository instance
around along the call chain).  But eventually somebody will start
saying "let's remove USE_THE_REPOSITORY_VARIABLE", at which point I
am not sure how subtle such a bug would become.