Re: [PATCH] Use ZSH_NAME instead of ZSH_VERSION because it's redefined by git-completion.zsh
- From
Rick van Hattem <wolph@wol.ph>
- Date
- Jun 4, 2018, 14:04 UTC
- Message-ID
- <CAJAwA=xjS6bWO2Cy+-pz-Es_RjvSyno9JNBGdPAve1L9ctOy-A@mail.gmail.com>
- In-Reply-To
- <xmqqr2ln5ide.fsf@gitster-ct.c.googlers.com>
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:
Show 15 quoted lines
> 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.
Show 8 quoted lines
> > # 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.
Show 7 quoted lines
> 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
...~rick
On 4 June 2018 at 05:40, Junio C Hamano <gitster@pobox.com> wrote:
Show 42 quoted lines
> 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.
>
>> # 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.
>
> I think what the proposed log message refers to as "unsets" is this
> part of the script:
>
> ...
> zstyle -s ":completion:*:*:git:*" script script
> if [ -z "$script" ]; then
> local -a locations
> local e
> locations=(
> $(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
> ...
> )
> for e in $locations; do
> test -f $e && script="$e" && break
> done
> fi
> ZSH_VERSION='' . "$script"
> ...
>
> 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...