git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Christoph Anton Mitterer]

Hey there.

Could it be that git-sh-prompt no longer works properly?

With git 2.31.1:
$ . /usr/lib/git-core/git-sh-prompt
$ PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
$ cd someGitRepo
bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable
$ 


Cheers,
Chris.

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Junio C Hamano]

Christoph Anton Mitterer <calestyo@scientia.net> writes:

> Could it be that git-sh-prompt no longer works properly?
>
> With git 2.31.1:
> $ . /usr/lib/git-core/git-sh-prompt
> $ PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
> $ cd someGitRepo
> bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable
> $ 

Would

    $ set +u

fix it, I have to wonder?

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Christoph Anton Mitterer <calestyo@scientia.net> writes:
>
>> Could it be that git-sh-prompt no longer works properly?
>>
>> With git 2.31.1:
>> $ . /usr/lib/git-core/git-sh-prompt
>> $ PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
>> $ cd someGitRepo
>> bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable
>> $ 
>
> Would
>
>     $ set +u
>
> fix it, I have to wonder?

Assuming that the answer is yes,...

I do not know who maintains this contrib/ script, but here is what

$ git grep -e '\$GIT_PS1_' -e '\${GIT_PS1_[^}-]*}' contrib/completion

gave me a handful candidates for fixes.  Randomly picking Elijah from
the output of

    $ git shortlog --no-merges -sn --since=18.months  \
	contrib/completion/git-prompt.sh |
      head -n 1

for ideas.

Thanks.

 contrib/completion/git-prompt.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git i/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
index 4640a1535d..b9485f4016 100644
--- i/contrib/completion/git-prompt.sh
+++ w/contrib/completion/git-prompt.sh
@@ -139,7 +139,7 @@ __git_ps1_show_upstream ()
 
 	# parse configuration values
 	local option
-	for option in ${GIT_PS1_SHOWUPSTREAM}; do
+	for option in ${GIT_PS1_SHOWUPSTREAM-}; do
 		case "$option" in
 		git|svn) upstream="$option" ;;
 		verbose) verbose=1 ;;
@@ -433,8 +433,8 @@ __git_ps1 ()
 	fi
 
 	local sparse=""
-	if [ -z "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
-	   [ -z "${GIT_PS1_OMITSPARSESTATE}" ] &&
+	if [ -z "${GIT_PS1_COMPRESSSPARSESTATE-}" ] &&
+	   [ -z "${GIT_PS1_OMITSPARSESTATE-}" ] &&
 	   [ "$(git config --bool core.sparseCheckout)" = "true" ]; then
 		sparse="|SPARSE"
 	fi
@@ -543,7 +543,7 @@ __git_ps1 ()
 			u="%${ZSH_VERSION+%}"
 		fi
 
-		if [ -n "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
+		if [ -n "${GIT_PS1_COMPRESSSPARSESTATE-}" ] &&
 		   [ "$(git config --bool core.sparseCheckout)" = "true" ]; then
 			h="?"
 		fi

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Elijah Newren]

On Wed, May 12, 2021 at 9:13 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Christoph Anton Mitterer <calestyo@scientia.net> writes:
> >
> >> Could it be that git-sh-prompt no longer works properly?
> >>
> >> With git 2.31.1:
> >> $ . /usr/lib/git-core/git-sh-prompt
> >> $ PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
> >> $ cd someGitRepo
> >> bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable
> >> $
> >
> > Would
> >
> >     $ set +u
> >
> > fix it, I have to wonder?
>
> Assuming that the answer is yes,...
>
> I do not know who maintains this contrib/ script, but here is what
>
> $ git grep -e '\$GIT_PS1_' -e '\${GIT_PS1_[^}-]*}' contrib/completion
>
> gave me a handful candidates for fixes.  Randomly picking Elijah from
> the output of
>
>     $ git shortlog --no-merges -sn --since=18.months  \
>         contrib/completion/git-prompt.sh |
>       head -n 1
>
> for ideas.

Yeah, I accidentally was relying on undefined-translates-to-empty,
which breaks under set -u.  I can duplicate the precise error, and
your fix below is exactly how I fixed it too, before seeing you posted
the same fix.

I'll post a patch with you as author...and some questions on what to
do with the rest of the commit message and attribution tags.

> Thanks.
>
>  contrib/completion/git-prompt.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git i/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
> index 4640a1535d..b9485f4016 100644
> --- i/contrib/completion/git-prompt.sh
> +++ w/contrib/completion/git-prompt.sh
> @@ -139,7 +139,7 @@ __git_ps1_show_upstream ()
>
>         # parse configuration values
>         local option
> -       for option in ${GIT_PS1_SHOWUPSTREAM}; do
> +       for option in ${GIT_PS1_SHOWUPSTREAM-}; do
>                 case "$option" in
>                 git|svn) upstream="$option" ;;
>                 verbose) verbose=1 ;;
> @@ -433,8 +433,8 @@ __git_ps1 ()
>         fi
>
>         local sparse=""
> -       if [ -z "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
> -          [ -z "${GIT_PS1_OMITSPARSESTATE}" ] &&
> +       if [ -z "${GIT_PS1_COMPRESSSPARSESTATE-}" ] &&
> +          [ -z "${GIT_PS1_OMITSPARSESTATE-}" ] &&
>            [ "$(git config --bool core.sparseCheckout)" = "true" ]; then
>                 sparse="|SPARSE"
>         fi
> @@ -543,7 +543,7 @@ __git_ps1 ()
>                         u="%${ZSH_VERSION+%}"
>                 fi
>
> -               if [ -n "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
> +               if [ -n "${GIT_PS1_COMPRESSSPARSESTATE-}" ] &&
>                    [ "$(git config --bool core.sparseCheckout)" = "true" ]; then
>                         h="?"
>                 fi

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> Yeah, I accidentally was relying on undefined-translates-to-empty,
> which breaks under set -u.  I can duplicate the precise error, and
> your fix below is exactly how I fixed it too, before seeing you posted
> the same fix.
>
> I'll post a patch with you as author...and some questions on what to
> do with the rest of the commit message and attribution tags.

Nah, please take full credit for it.  It is more work to test the
result of the change than just running a grep and coming up with "I
wonder if this would work...".

Thanks for a sanity check.

>
>> Thanks.
>>
>>  contrib/completion/git-prompt.sh | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git i/contrib/completion/git-prompt.sh w/contrib/completion/git-prompt.sh
>> index 4640a1535d..b9485f4016 100644
>> --- i/contrib/completion/git-prompt.sh
>> +++ w/contrib/completion/git-prompt.sh
>> @@ -139,7 +139,7 @@ __git_ps1_show_upstream ()
>>
>>         # parse configuration values
>>         local option
>> -       for option in ${GIT_PS1_SHOWUPSTREAM}; do
>> +       for option in ${GIT_PS1_SHOWUPSTREAM-}; do
>>                 case "$option" in
>>                 git|svn) upstream="$option" ;;
>>                 verbose) verbose=1 ;;
>> @@ -433,8 +433,8 @@ __git_ps1 ()
>>         fi
>>
>>         local sparse=""
>> -       if [ -z "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
>> -          [ -z "${GIT_PS1_OMITSPARSESTATE}" ] &&
>> +       if [ -z "${GIT_PS1_COMPRESSSPARSESTATE-}" ] &&
>> +          [ -z "${GIT_PS1_OMITSPARSESTATE-}" ] &&
>>            [ "$(git config --bool core.sparseCheckout)" = "true" ]; then
>>                 sparse="|SPARSE"
>>         fi
>> @@ -543,7 +543,7 @@ __git_ps1 ()
>>                         u="%${ZSH_VERSION+%}"
>>                 fi
>>
>> -               if [ -n "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
>> +               if [ -n "${GIT_PS1_COMPRESSSPARSESTATE-}" ] &&
>>                    [ "$(git config --bool core.sparseCheckout)" = "true" ]; then
>>                         h="?"
>>                 fi

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Christoph Anton Mitterer]

On Thu, 2021-05-13 at 13:03 +0900, Junio C Hamano wrote:
> Would
>     $ set +u
> fix it, I have to wonder?

Damn, I'm so blind ^^ Had recently set -u to try out whether unset
variables are used anywhere.
Yes it fixes it.

Thanks anyway for going to fix it :-)


Cheers,
Chris.

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Christoph Anton Mitterer]

Hey there.

I think I found another case of an unbound variable:

Completing e.g.:
git commit --[press TAB]

gives:
$ git commit  --bash: GIT_COMPLETION_SHOW_ALL: unbound variable


with some debugging:
$ set -xv
$ git commit --+ __git_func_wrap __git_main
+ local cur words cword prev
+ _get_comp_words_by_ref -n =: cur words cword prev
+ local exclude flag i OPTIND=1
+ words=()
+ local cur cword words
+ upargs=()
+ upvars=()
+ local upargs upvars vcur vcword vprev vwords
+ getopts c:i:n:p:w: flag -n =: cur words cword prev
+ case $flag in
+ exclude==:
+ getopts c:i:n:p:w: flag -n =: cur words cword prev
+ [[ 6 -ge 3 ]]
+ case ${!OPTIND} in
+ vcur=cur
+ (( OPTIND += 1 ))
+ [[ 6 -ge 4 ]]
+ case ${!OPTIND} in
+ vwords=words
+ (( OPTIND += 1 ))
+ [[ 6 -ge 5 ]]
+ case ${!OPTIND} in
+ vcword=cword
+ (( OPTIND += 1 ))
+ [[ 6 -ge 6 ]]
+ case ${!OPTIND} in
+ vprev=prev
+ (( OPTIND += 1 ))
+ [[ 6 -ge 7 ]]
+ __get_cword_at_cursor_by_ref =: words cword cur
+ words=()
+ local cword words
+ __reassemble_comp_words_by_ref =: words cword
+ local exclude i j line ref
+ [[ -n =: ]]
+ exclude='[=:]'
+ printf -v cword %s 2
+ [[ -v exclude ]]
+ line='git commit --'
+ (( i = 0, j = 0 ))
+ (( i < 3 ))
+ [[ 0 -gt 0 ]]
+ ref='words[0]'
+ printf -v 'words[0]' %s git
+ line=' commit --'
+ (( i == COMP_CWORD ))
+ (( i++, j++ ))
+ (( i < 3 ))
+ [[ 1 -gt 0 ]]
+ [[ commit == +([=:]) ]]
+ ref='words[1]'
+ printf -v 'words[1]' %s commit
+ line=' --'
+ (( i == COMP_CWORD ))
+ (( i++, j++ ))
+ (( i < 3 ))
+ [[ 2 -gt 0 ]]
+ [[ -- == +([=:]) ]]
+ ref='words[2]'
+ printf -v 'words[2]' %s --
+ line=
+ (( i == COMP_CWORD ))
+ printf -v cword %s 2
+ (( i++, j++ ))
+ (( i < 3 ))
+ (( i == COMP_CWORD ))
+ local i cur= index=13 'lead=git commit --'
+ [[ 13 -gt 0 ]]
+ [[ -n git commit -- ]]
+ [[ -n gitcommit-- ]]
+ cur='git commit --'
+ (( i = 0 ))
+ (( i <= cword ))
+ [[ 13 -ge 3 ]]
+ [[ git != \g\i\t ]]
+ (( i < cword ))
+ local old_size=13
+ cur=' commit --'
+ local new_size=10
+ (( index -= old_size - new_size ))
+ (( ++i ))
+ (( i <= cword ))
+ [[ 10 -ge 6 ]]
+ [[  commi != \c\o\m\m\i\t ]]
+ cur='commit --'
+ (( index > 0 ))
+ (( index-- ))
+ [[ 9 -ge 6 ]]
+ [[ commit != \c\o\m\m\i\t ]]
+ (( i < cword ))
+ local old_size=9
+ cur=' --'
+ local new_size=3
+ (( index -= old_size - new_size ))
+ (( ++i ))
+ (( i <= cword ))
+ [[ 3 -ge 2 ]]
+ [[  - != \-\- ]]
+ cur=--
+ (( index > 0 ))
+ (( index-- ))
+ [[ 2 -ge 2 ]]
+ [[ -- != \-\- ]]
+ (( i < cword ))
+ (( ++i ))
+ (( i <= cword ))
+ [[ -n -- ]]
+ [[ ! -n -- ]]
+ (( index < 0 ))
+ local words cword cur
+ _upvars -a3 words git commit -- -v cword 2 -v cur --
+ (( 11 ))
+ (( 11 ))
+ case $1 in
+ [[ -n 3 ]]
+ printf %d 3
+ [[ -n words ]]
+ unset -v words
+ eval 'words=("${@:3:3}")'
words=("${@:3:3}")
++ words=("${@:3:3}")
+ shift 5
+ (( 6 ))
+ case $1 in
+ [[ -n cword ]]
+ unset -v cword
+ eval 'cword="$3"'
cword="$3"
++ cword=2
+ shift 3
+ (( 3 ))
+ case $1 in
+ [[ -n cur ]]
+ unset -v cur
+ eval 'cur="$3"'
cur="$3"
++ cur=--
+ shift 3
+ (( 0 ))
+ [[ -v vcur ]]
+ upvars+=("$vcur")
+ upargs+=(-v $vcur "$cur")
+ [[ -v vcword ]]
+ upvars+=("$vcword")
+ upargs+=(-v $vcword "$cword")
+ [[ -v vprev ]]
+ [[ 2 -ge 1 ]]
+ upvars+=("$vprev")
+ upargs+=(-v $vprev "${words[cword - 1]}")
+ [[ -v vwords ]]
+ upvars+=("$vwords")
+ upargs+=(-a${#words[@]} $vwords ${words+"${words[@]}"})
+ (( 4 ))
+ local cur cword prev words
+ _upvars -v cur -- -v cword 2 -v prev commit -a3 words git commit --
+ (( 14 ))
+ (( 14 ))
+ case $1 in
+ [[ -n cur ]]
+ unset -v cur
+ eval 'cur="$3"'
cur="$3"
++ cur=--
+ shift 3
+ (( 11 ))
+ case $1 in
+ [[ -n cword ]]
+ unset -v cword
+ eval 'cword="$3"'
cword="$3"
++ cword=2
+ shift 3
+ (( 8 ))
+ case $1 in
+ [[ -n prev ]]
+ unset -v prev
+ eval 'prev="$3"'
prev="$3"
++ prev=commit
+ shift 3
+ (( 5 ))
+ case $1 in
+ [[ -n 3 ]]
+ printf %d 3
+ [[ -n words ]]
+ unset -v words
+ eval 'words=("${@:3:3}")'
words=("${@:3:3}")
++ words=("${@:3:3}")
+ shift 5
+ (( 0 ))
+ __git_main
+ local i c=1 command __git_dir __git_repo_path
+ local __git_C_args C_args_count=0
+ '[' 1 -lt 2 ']'
+ i=commit
+ case "$i" in
+ command=commit
+ break
+ '[' -z commit ']'
+ __git_complete_command commit
+ local command=commit
+ local completion_func=_git_commit
+ __git_have_func _git_commit
+ declare -f -- _git_commit
+ __git_have_func _git_commit
+ declare -f -- _git_commit
+ _git_commit
+ case "$prev" in
+ case "$cur" in
+ __gitcomp_builtin commit
+ local cmd=commit
+ local incl=
+ local excl=
+ local var=__gitcomp_builtin_commit
+ local options
+ eval 'options=${__gitcomp_builtin_commit-}'
options=${__gitcomp_builtin_commit-}
++ options=
+ '[' -z '' ']'
+ local completion_helper
bash: GIT_COMPLETION_SHOW_ALL: unbound variable

These seem to be in: 
/usr/share/bash-completion/completions/git
and possibly also:
/usr/share/bash-completion/completions/gitk


Cheers,
Chris.

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Junio C Hamano]

Christoph Anton Mitterer <calestyo@scientia.net> writes:

> Hey there.
>
> I think I found another case of an unbound variable:
>
> Completing e.g.:
> git commit --[press TAB]
>
> gives:
> $ git commit  --bash: GIT_COMPLETION_SHOW_ALL: unbound variable

It seems that OMIT_SPARSESTATE would have the same issue.

	if [ -z "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
	   [ -z "${GIT_PS1_OMITSPARSESTATE}" ] &&

all coming from afda36db (git-prompt: include sparsity state as
well, 2020-06-21).

But I think we have already seen the fix in 5c0cbdb1 (git-prompt:
work under set -u, 2021-05-13), which may or may not appear in the
upcoming release.

There still are unprotected mentions of GIT_PS1_SHOWUPSTREAM even
with that fix, though.

Re: git-sh-prompt: bash: GIT_PS1_COMPRESSSPARSESTATE: unbound variable

[Elijah Newren]

On Wed, May 19, 2021 at 4:29 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Christoph Anton Mitterer <calestyo@scientia.net> writes:
>
> > Hey there.
> >
> > I think I found another case of an unbound variable:
> >
> > Completing e.g.:
> > git commit --[press TAB]
> >
> > gives:
> > $ git commit  --bash: GIT_COMPLETION_SHOW_ALL: unbound variable

That particular case was fixed by Ville Skyttä in commit c5c0548d793e
(completion: audit and guard $GIT_* against unset use, 2021-04-08).

> It seems that OMIT_SPARSESTATE would have the same issue.
>
>         if [ -z "${GIT_PS1_COMPRESSSPARSESTATE}" ] &&
>            [ -z "${GIT_PS1_OMITSPARSESTATE}" ] &&
>
> all coming from afda36db (git-prompt: include sparsity state as
> well, 2020-06-21).
>
> But I think we have already seen the fix in 5c0cbdb1 (git-prompt:
> work under set -u, 2021-05-13), which may or may not appear in the
> upcoming release.

Yeah, I fixed the ones I introduced in git-prompt.sh --
GIT_PS1_COMPRESSSPARSESTATE and GIT_PS1_OMITSPARSESTATE.

> There still are unprotected mentions of GIT_PS1_SHOWUPSTREAM even
> with that fix, though.

Yeah, neither my fix (which was only trying to fix the problems I
introduced in git-prompt.sh) nor Ville's fix (which was focused on
git-completion.bash) caught that one.

Do you want to make a patch for that, Christoph?  If not, #leftoverbits?