Web lists-archives.com

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




On Thu, Feb 8, 2018 at 1:06 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>> 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.
>
> I tried that in the beginning and it blew up a couple of times when I realized
> that I forgot to pass through one of these dependencies.
> Maybe we can go to the repository and shrink the scope in a follow up?

I think it's a good thing to do. We need to make these implicit
dependencies explicit sooner or later.

Also, perhaps at the earlier steps you don't need to add everything to
the_respository yet. You can have the_object_store, the_parsed_object
(and we already have get_main_ref_store()), then use them internally
without touching the API, which helps reduce code change. For example,
read_sha1_file() could be converted to take "struct object_store *"
all the way

void *read_sha1_file_extended(const unsigned char *sha1,
      enum object_type *type,
      unsigned long *size,
      int lookup_replace)
{
        struct object_store *store = &the_object_store;
        ...
        // more access to "store"
}

When every piece is in place, the API change can be made be removing
that "store = &the_object_store" line and make "store" an argument of
read_sha1_file().
-- 
Duy