Re: [PATCH] init-db: init the_repository->hash_algo early from GIT_DEFAULT_HASH

[Junio C Hamano <gitster@pobox.com>]

Han-Wen Nienhuys <hanwen@google.com> writes:

> If the_repository is only half-initialized at this point in init_db(),
> then why are we passing it in refs_init_db() just a couple of lines
> further? At what point the_repository considered initialized?

I would have to say it probably depends on what callees expect.  The
current implementation of refs_init_db() for files backend may not
need anything other than the hash algorithm enum, but many other
fields are missing, and they should ideally be populated, no?  

For example, I see files_ref_store_create() cheats by calling
get_common_dir_noenv() to find out where the commondir is, instead
of ever asking the repository the ref store belongs to. At least,
get_main_ref_store() is told to get the ref store that belongs to
the_repository, and it would be the right place to learn relevant
pieces of information (for that matter, I am not sure why struct
ref_store does not have a pointer to a repository structure; perhaps
we are seeing the result of piecemeal evolution, not a designed
structure?).

> I'm a bit at a loss here; I never learned how to cleanly work with so
> many global variables, so I'm happy to take your suggestion.

I am only interested in giving a clear direction to future
developers where to populate the_repository's members (and nothing
else) if their enhancement needs members other than the hash
algorithm to be populated, as if it were the_repository initialized
in an already working repository (I am not talking about many global
variables, whichever you are referring to).

One way to do so would probably be to do something like the
attached.

The patch that started this thread (or the equivalent one in the
updated reftable series) may want to initialize a bit more members
while at it (looking at how commondir from the_repository is not
used by the refs/files-backend.c::files_init_db() to decide where
the function creates the refs/heads and refs/tags directories, we
probably would need to populate the_repository->commondir before
that codepath can be fixed to look at the member, for example).


 builtin/init-db.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git c/builtin/init-db.c w/builtin/init-db.c
index dcc45bef51..2b5c94596f 100644
--- c/builtin/init-db.c
+++ w/builtin/init-db.c
@@ -438,6 +438,27 @@ int init_db(const char *git_dir, const char *real_git_dir,
 
 	validate_hash_algorithm(&repo_fmt, hash);
 
+	/*
+	 * At this point, the_repository we have in-core does not look
+	 * anything like one that we would see initialized in an already
+	 * working repository after calling setup_git_directory().
+	 *
+	 * Calling repository.c::initialize_the_repository() may have
+	 * prepared the .index .objects and .parsed_objects members, but
+	 * other members like .gitdir, .commondir, etc. have not been
+	 * initialized.
+	 *
+	 * Many API functions assume they are working with the_repository
+	 * that has sensibly been initialized, but because we haven't
+	 * really read from an existing repository, we need to hand-craft
+	 * the necessary members of the structure to get out of this
+	 * chicken-and-egg situation.
+	 *
+	 * For now, we update the hash algorithm member to what the
+	 * validate_hash_algorithm() call decided for us.
+	 */
+	repo_set_hash_algo(the_repository, repo_fmt->hash_algo);
+
 	reinit = create_default_files(template_dir, original_git_dir,
 				      initial_branch, &repo_fmt,
 				      flags & INIT_DB_QUIET);