On Tue, Feb 27, 2018 at 3:30 AM, Stefan Beller <sbel...@google.com> wrote: >> diff --git a/setup.c b/setup.c >> index c5d55dcee4..6fac1bb58a 100644 >> --- a/setup.c >> +++ b/setup.c >> @@ -1116,8 +1116,7 @@ const char *setup_git_directory_gently(int *nongit_ok) >> const char *gitdir = getenv(GIT_DIR_ENVIRONMENT); >> if (!gitdir) >> gitdir = DEFAULT_GIT_DIR_ENVIRONMENT; >> - repo_set_gitdir(the_repository, gitdir); >> - setup_git_env(); >> + setup_git_env(gitdir); >> } > > What makes git_dir special, such that we have to pass it in here? > Could the check for getenv(GIT_DIR_ENVIRONMENT) and fallback to > DEFAULT_... be part of setup_git_env() ? > Oh I guess that cannot be done easily as the submodule code > spcifically doesn't want to discover the git dir itself.
No, submodule code already does not care about setup_git_env(). Back to your first question, this is related to the "NEEDSWORK" comment in this code. We _should_ always set_git_dir() before leaving setup_git_directory_gently() then everybody behaves the same way and we don't need this special setup_git_env() here at all. BUT we need to be careful about setting environment variable $GIT_DIR, done inside set_git_dir(), because sub-processes will inherit it and if we're not careful, we'll break .git discovery in those. There's another thing I don't like about this is setup_work_tree() using set_git_dir() the second time to adjust relative $GIT_DIR and friends when it moves $CWD. I'll need to fix this, soon. So in short, it's special because the current situation is messy and (hopefully) temporary. But it should eventually be gone. -- Duy
