On 4 June 2018 at 05:40, Junio C Hamano <gits...@pobox.com> wrote: Rick van Hattem <wo...@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 ... ~rick On 4 June 2018 at 05:40, Junio C Hamano <gits...@pobox.com> wrote: > Rick van Hattem <wo...@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...
