Re: [PATCH] Use ZSH_NAME instead of ZSH_VERSION because it's redefined by git-completion.zsh

> On 4 June 2018 at 05:40, Junio C Hamano <gitster@pobox.com> wrote:
> Rick van Hattem <wolph@wol.ph> writes:
> 
> > > The `git-completion.zsh` unsets the `$ZSH_VERSION` which makes this check moot. The result (at least for me) is that zsh segfaults because of all the variables it's unsetting.
> > > ---
> >
> > Overlong line, lack of sign-off.
> 
> Apologies for the long lines, I wrote the message on Github where this
> message is properly formatted, apparently the submitgit script can be
> considered broken as it truncates the message when converting to email.
> 
> The original message can be found here: https://github.com/git/git/pull/500

> Below is the original message with proper formatting:
> 
> > A recent change (94408dc) broke zsh for me (actually segfault zsh when
> > trying to use git completion)
> >
> > The reason is that the `git-completion.zsh` sets the `ZSH_VERSION`
> > variable to an empty string:
> >     ...
> >     ZSH_VERSION='' . "$script"
> >     ...
> >
> > I'm not sure if @szeder or @gitster used a different zsh version for
> > testing commit 94408dc but it segfaults zsh 5.5.1
> > (x86_64-apple-darwin15.6.0) on my OS X 10.11.6 machine.

> > The proposed fix is quite simple and shouldn't break any backwards
> > compatibility.
> 
> Hopefully that clears a little bit of the confusion.
> 
> > >  # Clear the variables caching builtins' options when (re-)sourcing
> > >  # the completion script.
> > > -if [[ -n ${ZSH_VERSION-} ]]; then
> > > +if [[ -n ${ZSH_NAME-} ]]; then
> >
> > I am not a zsh user, and I do not know how reliable $ZSH_NAME can be
> > taken as an indication that we are running zsh and have already
> > found a usable git-completion-.bash script.
> 
> >From what I gathered this variable has been available since 1995. But
> I'm not ZSH expert...
> 
> You can search for ZSH_NAME in the 3.0 changelog:
> http://zsh.sourceforge.net/Etc/changelog-3.0.html
> 
> > I think what the proposed log message refers to as "unsets" is this
> > part of the script:
> 
> As mentioned above, I was referring to commit 94408dc which changed the
> behaviour of the bash completion script.
> 
> Specifically:
> 
>     ...
>     if [[ -n ${ZSH_VERSION-} ]]; then
>         unset $(set |sed -ne
> 's/^\(__gitcomp_builtin_[a-zA-Z0-9_][a-zA-Z0-9_]*\)=.*/\1/p')
> 2>/dev/null
>     else
>         unset $(compgen -v __gitcomp_builtin_)
>     fi
>     ...
> 
> Because the ZSH script unsets the ZSH_VERSION variable (which is needed
> because the bash script checks for that later in the script) it defaults
> to the bash behaviour resulting in a segfault.

> > If your ZSH_VERSION is empty, doesn't it indicate that the script
> > did not find a usable git-completion.bash script (to which it
> > outsources the bulk of the completion work)?  I do agree segfaulting
> > is not a friendly way to tell you that your setup is lacking to make
> > it work, but I have a feeling that what you are seeing is an
> > indication of a bigger problem, which will be sweeped under the rug
> > with this patch but without getting fixed...
> 
> The git-completion.zsh script purposefully unsets the ZSH_VERSION
> before including the git-completion.bash script like this:
> 
>     ...
>     ZSH_VERSION='' . "$script"
>     ...

> The reason for that is (presumably) the check that's used within the
> git-completion.bash script to warn ZSH users:
> 
>     ...
>     if [[ -n ${ZSH_VERSION-} ]]; then
>     echo "WARNING: this script is deprecated, please see
> git-completion.zsh" 1>&2
>     ...