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

[Junio C Hamano <gitster@pobox.com>]

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?