@matheusbsbs / matheus-bagliano / commits / 73f192c991

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>

Brandon Williams committed Jun 20, 2017 at 12:19 UTC 73f192c991016bf88a9416cdf0e949f8b946f7e2
3 files changed +24 -9
cache.h
+2
@@ -462,6 +462,8 @@ static inline enum object_type object_type(unsigned int mode)
462 */
463 extern const char * const local_repo_env[];
464
465 +extern void setup_git_env(void);
466 +
467 /*
468 * Returns true iff we have a configured git repository (either via
469 * setup_git_directory, or in the environment via $GIT_DIR).
environment.c
+8 -9
@@ -160,7 +160,7 @@ static char *git_path_from_env(const char *envvar, const char *git_dir,
160 return xstrdup(value);
161 }
162
163 -static void setup_git_env(void)
163 +void setup_git_env(void)
164 {
165 struct strbuf sb = STRBUF_INIT;
166 const char *gitfile;
@@ -205,28 +205,27 @@ int is_bare_repository(void)
205 int have_git_dir(void)
206 {
207 return startup_info->have_repository
208 - || git_dir
209 - || getenv(GIT_DIR_ENVIRONMENT);
208 + || git_dir;
209 }
210
211 const char *get_git_dir(void)
212 {
213 if (!git_dir)
215 - setup_git_env();
214 + BUG("git environment hasn't been setup");
215 return git_dir;
216 }
217
218 const char *get_git_common_dir(void)
219 {
220 if (!git_dir)
222 - setup_git_env();
221 + BUG("git environment hasn't been setup");
222 return git_common_dir;
223 }
224
225 const char *get_git_namespace(void)
226 {
227 if (!namespace)
229 - setup_git_env();
228 + BUG("git environment hasn't been setup");
229 return namespace;
230 }
231
@@ -276,7 +275,7 @@ const char *get_git_work_tree(void)
275 char *get_object_directory(void)
276 {
277 if (!git_object_dir)
279 - setup_git_env();
278 + BUG("git environment hasn't been setup");
279 return git_object_dir;
280 }
281
@@ -316,14 +315,14 @@ int odb_pack_keep(const char *name)
315 char *get_index_file(void)
316 {
317 if (!git_index_file)
319 - setup_git_env();
318 + BUG("git environment hasn't been setup");
319 return git_index_file;
320 }
321
322 char *get_graft_file(void)
323 {
324 if (!git_graft_file)
326 - setup_git_env();
325 + BUG("git environment hasn't been setup");
326 return git_graft_file;
327 }
328
setup.c
+14
@@ -1091,6 +1091,20 @@ const char *setup_git_directory_gently(int *nongit_ok)
1091 startup_info->have_repository = !nongit_ok || !*nongit_ok;
1092 startup_info->prefix = prefix;
1093
1094 + /*
1095 + * Not all paths through the setup code will call 'set_git_dir()' (which
1096 + * directly sets up the environment) so in order to guarantee that the
1097 + * environment is in a consistent state after setup, explicitly setup
1098 + * the environment if we have a repository.
1099 + *
1100 + * NEEDSWORK: currently we allow bogus GIT_DIR values to be set in some
1101 + * code paths so we also need to explicitly setup the environment if
1102 + * the user has set GIT_DIR. It may be beneficial to disallow bogus
1103 + * GIT_DIR values at some point in the future.
1104 + */
1105 + if (startup_info->have_repository || getenv(GIT_DIR_ENVIRONMENT))
1106 + setup_git_env();
1107 +
1108 strbuf_release(&dir);
1109 strbuf_release(&gitdir);
1110