setup: don't perform lazy initialization of repository state

Under some circumstances (bogus GIT_DIR value or the discovered gitdir
is '.git') 'setup_git_directory()' won't initialize key repository
state.  This leads to inconsistent state after running the setup code.
To account for this inconsistent state, lazy initialization is done once
a caller asks for the repository's gitdir or some other piece of
repository state.  This is confusing and can be error prone.

Instead let's tighten the expected outcome of 'setup_git_directory()'
and ensure that it initializes repository state in all cases that would
have been handled by lazy initialization.

This also lets us drop the requirement to have 'have_git_dir()' check if
the environment variable GIT_DIR was set as that will be handled by the
end of the setup code.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/cache.h b/cache.h
index 96055c222929e4ba307f3162b58740eccf165f2c..7c81749a9800062c0f3d812060968151a7e42951 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -462,6 +462,8 @@ static inline enum object_type object_type(unsigned int mode)
  */
 extern const char * const local_repo_env[];
 
+extern void setup_git_env(void);
+
 /*
  * Returns true iff we have a configured git repository (either via
  * setup_git_directory, or in the environment via $GIT_DIR).

diff --git a/environment.c b/environment.c
index d40b21fb72168f32f1aaeac553347859e32f90c0..a73b08f5d9fa61afe691dd489b951fd341142850 100644 (file)
--- a/environment.c
+++ b/environment.c
@@ -160,7 +160,7 @@ static char *git_path_from_env(const char *envvar, const char *git_dir,
        return xstrdup(value);
 }
 
-static void setup_git_env(void)
+void setup_git_env(void)
 {
        struct strbuf sb = STRBUF_INIT;
        const char *gitfile;
@@ -205,28 +205,27 @@ int is_bare_repository(void)
 int have_git_dir(void)
 {
        return startup_info->have_repository
-               || git_dir
-               || getenv(GIT_DIR_ENVIRONMENT);
+               || git_dir;
 }
 
 const char *get_git_dir(void)
 {
        if (!git_dir)
-               setup_git_env();
+               BUG("git environment hasn't been setup");
        return git_dir;
 }
 
 const char *get_git_common_dir(void)
 {
        if (!git_dir)
-               setup_git_env();
+               BUG("git environment hasn't been setup");
        return git_common_dir;
 }
 
 const char *get_git_namespace(void)
 {
        if (!namespace)
-               setup_git_env();
+               BUG("git environment hasn't been setup");
        return namespace;
 }
 
@@ -276,7 +275,7 @@ const char *get_git_work_tree(void)
 char *get_object_directory(void)
 {
        if (!git_object_dir)
-               setup_git_env();
+               BUG("git environment hasn't been setup");
        return git_object_dir;
 }
 
@@ -316,14 +315,14 @@ int odb_pack_keep(const char *name)
 char *get_index_file(void)
 {
        if (!git_index_file)
-               setup_git_env();
+               BUG("git environment hasn't been setup");
        return git_index_file;
 }
 
 char *get_graft_file(void)
 {
        if (!git_graft_file)
-               setup_git_env();
+               BUG("git environment hasn't been setup");
        return git_graft_file;
 }
 

diff --git a/setup.c b/setup.c
index 358fbc2e5301d9412b55463065b912b5708c3ceb..24a738b0d657a85fdd85d6561352331fa8c4570e 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1091,6 +1091,20 @@ const char *setup_git_directory_gently(int *nongit_ok)
        startup_info->have_repository = !nongit_ok || !*nongit_ok;
        startup_info->prefix = prefix;
 
+       /*
+        * Not all paths through the setup code will call 'set_git_dir()' (which
+        * directly sets up the environment) so in order to guarantee that the
+        * environment is in a consistent state after setup, explicitly setup
+        * the environment if we have a repository.
+        *
+        * NEEDSWORK: currently we allow bogus GIT_DIR values to be set in some
+        * code paths so we also need to explicitly setup the environment if
+        * the user has set GIT_DIR.  It may be beneficial to disallow bogus
+        * GIT_DIR values at some point in the future.
+        */
+       if (startup_info->have_repository || getenv(GIT_DIR_ENVIRONMENT))
+               setup_git_env();
+
        strbuf_release(&dir);
        strbuf_release(&gitdir);