Re: [PATCH 3/3] Replace setenv(GIT_DIR_ENVIRONMENT, ...) with set_git_dir()

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Nov 22, 2007, at 3:34 AM, Junio C Hamano wrote:
>>
>>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>>
>>>> Does this not have a fundamental issue?  When you call other git
>>>> programs
>>>> with run_command(), you _need_ GIT_DIR to be set, no?
>>>
>>> It is much worse.  set_git_dir() does not just setenv() but does
>>> setup_git_env() as well.
>>
>> What do your comments mean?
>>
>> My understanding is that set_git_dir() sets the environment and
>> then calls setup_git_env() to cache all pointers.  This call
>> updates dangling pointer if they have been cached earlier.
>
> Well, I was agreeing with you.  "Worse" was about what the
> current code does _not_ do.
>
> If there are earlier calls that obtain locations relative to the
> earlier definition of GIT_DIR, the locations they obtained are
> not just stored in memory that is "dangling" (which was what
> your proposed log message described) but they are also
> inconsistent with the updated definition of GIT_DIR.
>
> I suspect Johannes mistook set_git_dir() was only local
> (i.e. per calling process) matter without noticing that it has
> its own setenv() when he made that comment, hence my response to
> point out that the current code only calls setenv(), but
> set_git_dir() does setup_git_env() too, which should hide the
> inconsistency problem.
>
> HOWEVER.
>
> I suspect that if there are even earlier callers than these
> early parts in the codepaths (handle_options, enter_repo, and
> setup_git_directory_gently), maybe these earlier callers are
> doing something wrong.  Logically, if you are somewhere very
> early in the codepath that you can still change the value of
> GIT_DIR, you shouldn't have assumed the unknown value of GIT_DIR
> and cached locations relative to that directory, no?  What are
> the problematic callers?  What values do they access and why?