Web lists-archives.com

Re: [RFC PATCH 000/194] Moving global state into the repository object




On Tue, Feb 6, 2018 at 6:51 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> This series moves a lot of global state into the repository struct.
> It applies on top of 2512f15446149235156528dafbe75930c712b29e (2.16.0)
> It can be found at https://github.com/stefanbeller/git/tree/object-store
>
> Motivation for this series:
> * Easier to reason about code when all state is stored in one place,
>   for example for multithreading
> * Easier to move to processing submodules in-process instead of
>   calling a processes for the submodule handling.
>   The last patch demonstrates this.

I have a mixed feeling about this. The end game is to keep
"the_repository" references as minimum as possible, right? Like we
only need to mention in in cmd_xxx() and not all over the place. If
so, yay!!

Something else.. maybe "struct repository *" should not be a universal
function argument to avoid global states. Like sha1_file.c is mostly about the
object store, and I see that you added object store struct (nice!).
These sha1 related function should take the object_store* (which
probably is a combination of both packed and loose stores since they
depend on each other), not repository*. This way if a function needs
both access to object store and ref store, we can see the two
dependencies from function arguments.

The alternate object store, if modeled right, could share the same
object store interface. But I don't think we should do anything that
big right now, just put alternate object store inside object_store.

Similarly those functions in blob.c, commit.c, tree.c... should take
object_parser* as argument instead of repository*. Maybe there's a
better name for this than object_parser. parsed_object_store I guess.
Or maybe just object_pool.
-- 
Duy