Re: [PATCH 14/26] sha1_file: allow prepare_alt_odb to handle arbitrary repositories
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Feb 14, 2018, 00:37 UTC
- Message-ID
- <CACsJy8DmXrO9bxv10DsDpSWxm2BiZwwfhso4=ZRQs_xhrpAgfg@mail.gmail.com>
- In-Reply-To
- <20180213012241.187007-15-sbeller@google.com>
On Tue, Feb 13, 2018 at 8:22 AM, Stefan Beller <sbeller@google.com> wrote:
Show 41 quoted lines
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> object-store.h | 3 +--
> sha1_file.c | 21 +++++++++++----------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/object-store.h b/object-store.h
> index d96a16edd1..add1d4e27c 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -61,7 +61,6 @@ struct packed_git {
> char pack_name[FLEX_ARRAY]; /* more */
> };
>
> -#define prepare_alt_odb(r) prepare_alt_odb_##r()
> -extern void prepare_alt_odb_the_repository(void);
> +void prepare_alt_odb(struct repository *r);
>
> #endif /* OBJECT_STORE_H */
> diff --git a/sha1_file.c b/sha1_file.c
> index d18ce3aeba..f046d560f8 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -677,21 +677,22 @@ int foreach_alt_odb(alt_odb_fn fn, void *cb)
> return r;
> }
>
> -void prepare_alt_odb_the_repository(void)
> +void prepare_alt_odb(struct repository *r)
> {
> - const char *alt;
> -
> - if (the_repository->objects.alt_odb_tail)
> + if (r->objects.alt_odb_tail)
> return;
>
> - alt = getenv(ALTERNATE_DB_ENVIRONMENT);
> + r->objects.alt_odb_tail = &r->objects.alt_odb_list;
> +
> + if (!r->ignore_env) {
> + const char *alt = getenv(ALTERNATE_DB_ENVIRONMENT);If one day the majority of git moves to use 'struct repository', then ALTERNATE_DB_ENVIRONMENT is always ignored because ignore_env is always true. I think if you ignore_env, then you still need to get this "alt" from 'struct raw_object_store' (or 'struct repository').
Since you get lots of getenv() in repo_setup_env(), I think this getenv(ALTERNATE_DB_ENVIRONMENT) belongs there too. Then here, if ignore_env is true, you read r->objects.alt or something instead of doing getenv().
I really want to kill this getenv() in this code, which is basically delayed initialization because we haven't done proper init on the_repo. I realize that it cannot be done earlier, when prepare_alt_odb() does not even have a 'struct repository *' to work with. Would it be ok if I contributed a patch on top of your series to basically do repo_init(&the_repo) for all builtin/external commands (and fix all the bugs that come with it)? Then we would not need ignore_env here anymore.
Show 18 quoted lines
> + if (!alt) > + alt = ""; > > - the_repository->objects.alt_odb_tail = > - &the_repository->objects.alt_odb_list; > - link_alt_odb_entries(the_repository, alt, > - PATH_SEP, NULL, 0); > + link_alt_odb_entries(r, alt, PATH_SEP, NULL, 0); > + } > > - read_info_alternates(the_repository, get_object_directory(), 0); > + read_info_alternates(r, r->objects.objectdir, 0); > } > > /* Returns 1 if we have successfully freshened the file, 0 otherwise. */ > -- > 2.16.1.73.ga2c3e9663f.dirty >
-- Duy