On Thu, Feb 8, 2018 at 1:06 AM, Stefan Beller <sbel...@google.com> 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
