On Wed, May 09, 2012 at 12:43:33PM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder@xxxxxxxxxx> writes:
>
> > + __gitdir >/dev/null
>
> If this becomes the only call site of __gitdir helper function (and that
> was the way I read the log message), it would be sane to rename it to
> a more descriptive __setup_dash_dash_git_dir function and lose the need to
> redirect its output, no?
There might be user-defined completion functions using __gitdir(), and
I didn't want to break them by changing the function's name or
altering its existing behavior, i.e. printing the path.
> > @@ -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.
All completion functions are called either from _git() or from
_gitk(), where $__git_dir is declared as local. So no matter how deep
is $__git_dir set in the callchain, it can't leak into the
environment. It won't even survive between two subsequent completions
on the same command line.
Now, it would definitely be simpler to just initialize $__git_dir in
the two toplevel functions. But there are many codepatch that don't
need $__git_dir at all, and would only be slowed down by an additional
$(git rev-parse --git-dir).
Gábor
--
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