Re: [PATCH 1/4] repository.c: move env-related setup code back to environment.c

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Feb 27, 2018 at 3:30 AM, Stefan Beller <sbeller@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