On Wed, May 31, 2017 at 2:43 PM, Brandon Williams <bmw...@google.com> wrote:
> 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.
Lazy init is usually there for a reason. (As in: it took too long to perform
it at all times, so it has been optimized to only perform the init when needed).
Reading the code of setup_git_env, this is the additional cost incurred to us:
* reading a file ('git_dir' to determine if it is a gitlink or actual directory)
* reading another file to determine the commondir
* some environment variable reading (<10).
Reading the environment should be fast, though in the implementation
of libc that I just looked up, it is still a linear search, so O(n) with n the
number of environment variables. (No actual syscalls though.)
I would think that this is ok as it incurs not much cost. Maybe the
lazyloading implementation was just a habit of the contributors at the time.
> const char *get_git_dir(void)
> {
> if (!git_dir)
> - setup_git_env();
NEEDSWORK: eventually (say after release 2.16)
we can remove all these
if (!git_dir)
BUG(...);
calls. I will read on, as maybe this series touches
this further
