On Wed, May 09, 2012 at 01:56:02PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@xxxxxxxxxx> writes:
>
> >> > @@ -962,7 +967,8 @@ __git_aliases ()
> >> > # __git_aliased_command requires 1 argument
> >> > __git_aliased_command ()
> >> > {
> >> > - local word cmdline=$(git --git-dir="$(__gitdir)" \
> >> > + __gitdir >/dev/null
> >> > + local word cmdline=$(git --git-dir="$__git_dir" \
> >> > config --get "alias.$1")
> >> > for word in $cmdline; do
> >> > case "$word" in
> >>
> >> Now this worries me. The way I read 07/19 was that the local __git_dir=""
> >> declarations in __git_ps1 and __git were what protected this whole
> >> machinery to protect us against surprises from user doing "cd" between
> >> interactive commands, but you have the same __gitdir call to set up the
> >> global $__git_dir variable there, without the initialization to "".
> >>
> >> Having to have a call to __gitdir seems to indicate to me that you cannot
> >> assume that the other initialization sites may not have been called before
> >> we get to this point. Then why is 'local __git_dir=""' unneeded here?
> >
> > Your comments to the previous patch apply here.
>
> Not really. __git_ps1 and __gitk seems to do __gitdir very early to make
> sure anybody that use $__git_dir can rely on it
No, __git_ps1() and __gitk() do __gitdir() early, because they need
the path to the repository very early.
> but having to sprinkle
> "set up $__git_dir variable" everywhere means anybody who wants to update
> need to know if it is already called, which defeats the point of "we can
> use $__git_dir instead of calling $(__gitdir)" from maintainability's
> point of view.
Well, the point is better explained in the body of the commit message:
just call __gitdir() directly and then use $__git_dir instead of
doing 'dir="$(__gitdir)"' command substitution
Unfortunately, I just couldn't manage to squeeze all this into a
one-line short description. So it really is about performance, and
not maintainability.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html