Re: [PATCHv4 01/27] repository: introduce raw object store field

[Stefan Beller <sbeller@google.com>]

On Mon, Feb 26, 2018 at 1:30 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Feb 23, 2018 at 04:47:28PM -0800, Stefan Beller wrote:
>>  /* The main repository */
>>  static struct repository the_repo = {
>> -     NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, &hash_algos[GIT_HASH_SHA1], 0, 0
>> +     NULL, NULL,
>> +     RAW_OBJECT_STORE_INIT,
>> +     NULL, NULL, NULL,
>> +     NULL, NULL, NULL,
>> +     &the_index,
>> +     &hash_algos[GIT_HASH_SHA1],
>> +     0, 0
>>  };
>>  struct repository *the_repository = &the_repo;
>
> I wonder if we should do something like this. It makes the_repo
> initialization easier to read and it helps unify the init code with
> submodule.
>
> No don't reroll. If you think it's a good idea, you can do something
> like this in the next series instead.
>
> -- 8< --
> diff --git a/check-racy.c b/check-racy.c
> index 24b6542352..47cbb4eb6d 100644
> --- a/check-racy.c
> +++ b/check-racy.c

totally offtopic: Do we want to move this file into t/helper?

> --- a/common-main.c
> +++ b/common-main.c
> @@ -1,6 +1,7 @@
>  #include "cache.h"
>  #include "exec_cmd.h"
>  #include "attr.h"
> +#include "repository.h"
>
>  /*
>   * Many parts of Git have subprograms communicate via pipe, expect the
> @@ -32,6 +33,8 @@ int main(int argc, const char **argv)
>          */
>         sanitize_stdfds();
>
> +       init_the_repository();
> +

So this is the real deal.

> -#define RAW_OBJECT_STORE_INIT { NULL }
>
> +void raw_object_store_init(struct raw_object_store *o);
>  void raw_object_store_clear(struct raw_object_store *o);
>
>  #endif /* OBJECT_STORE_H */
> diff --git a/object.c b/object.c
> index 11d904c033..8a4d01dd5f 100644
> --- a/object.c
> +++ b/object.c
> @@ -446,6 +446,11 @@ void clear_commit_marks_all(unsigned int flags)
>         }
>  }
>
> +void raw_object_store_init(struct raw_object_store *o)
> +{
> +       memset(o, 0, sizeof(*o));
> +}

We'll have to adapt this for the list that we use for packed_git_mru,
but that should be no problem.

> +
> +static void repo_pre_init(struct repository *repo)
> +{
> +       memset(repo, 0, sizeof(*repo));
> +       raw_object_store_init(&repo->objects);
> +}
> +
> +void init_the_repository(void)
> +{
> +       the_repository = &the_repo;
> +       repo_pre_init(the_repository);
> +       the_repository->index = &the_index;
> +       repo_set_hash_algo(the_repository, GIT_HASH_SHA1);
> +}
>
>  static char *git_path_from_env(const char *envvar, const char *git_dir,
>                                const char *path, int fromenv)
> @@ -138,7 +144,8 @@ static int read_and_verify_repository_format(struct repository_format *format,
>  int repo_init(struct repository *repo, const char *gitdir, const char *worktree)
>  {
>         struct repository_format format;
> -       memset(repo, 0, sizeof(*repo));
> +
> +       repo_pre_init(repo);
>
>         repo->ignore_env = 1;
>

I like the approach. Though as you say, let's have it in the next series.

Thanks for the thoughts and guidance,
Stefan