Skip to contentSkip to conversation

bash-completion: Print a useful error when called in a non-bash shell

PATCH In review Andrew Sayers 6 participants 8 messages Jun 15, 2016 Documentation

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 bash-completion: Print a useful error when called in a non-bash shell

Detect whether the shell supports process substitution with <() Shells that fail the test will not be able to load git-completion.bash

If a bad shell is found, print a warning which gives the user as much debugging information as possible.

This was added in response to a bug report on the git mailing list:
http://permalink.gmane.org/gmane.comp.version-control.git/151723

---

My thanks to solsTiCe d'Hiver for reporting this bug.

 contrib/completion/git-completion.bash |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)
contrib/completion/git-completion.bash +25 -0
index 6756990..3bbb4da 100755
@@ -96,6 +96,31 @@ __gitdir ()
 	fi
 }
 
+# Check in case we were called with something like "sh git-completion.bash":
+$(
+	exec 2>/dev/null
+	$(exec < <( ))
+ )
+if [[ 0 -ne $? ]]
+then
+	cat <<EOF
+
+ERROR: you don't seem to be running a full bash shell.
+git-completion.bash is probably about to fail with a syntax error.
+If you are sure that your system is calling git-completion.bash from a bash,
+then please include the following in a bug report to [email protected]:
+
+	BASH_VERSION: {$BASH_VERSION}
+	BASHOPTS: {$BASHOPTS}
+	SHELLOPTS: {$SHELLOPTS}
+	POSIXLY_CORRECT: {$POSIXLY_CORRECT}
+EOF
+	echo -n "	command line: {"
+	tr '\0' ' ' < /proc/$$/cmdline
+	echo "}"
+	echo
+fi
+
 # stores the divergence from upstream in $p
 # used by GIT_PS1_SHOWUPSTREAM
 __git_ps1_show_upstream ()
Signature · 1 line
1.7.0.4
#1 lore raw
Quoted 15 lines · Andrew Sayers

Detect whether the shell supports process substitution with <() Shells that fail the test will not be able to load git-completion.bash

If a bad shell is found, print a warning which gives the user as much debugging information as possible.

This was added in response to a bug report on the git mailing list:
http://permalink.gmane.org/gmane.comp.version-control.git/151723

---

contrib/completion/git-completion.bash +3 -0
+# Check in case we were called with something like "sh git-completion.bash":
+$(
+	exec 2>/dev/null

contrib/completion/git-completion.bash __gitdir () · line 111

Quoted 10 lines
contrib/completion/git-completion.bash +10 -0
+	$(exec < <( ))
+ )
+if [[ 0 -ne $? ]]
+then
+	cat <<EOF
+
+ERROR: you don't seem to be running a full bash shell.
+git-completion.bash is probably about to fail with a syntax error.
+If you are sure that your system is calling git-completion.bash from a bash,
+then please include the following in a bug report to [email protected]:

I needed to read this twice to realize that majority of people who will ever see this message on their screen are _not_ expected to send any bug report to us. Also if the user's "full bash" groks <() indirection but still fails to run completion script correctly, this message does not help them to find where to file a bug report at all.

I do agree that there need to be a way to find that information for the end users, especially for those who just use binary-packaged git given by their distros, but this codepath is _not_ the place to do it.

How about replacing these with something simple like:

echo >&2 "You are not running full 'bash'; exiting." ; exit 127

contrib/completion/git-completion.bash __gitdir () · line 119

Quoted 7 lines
contrib/completion/git-completion.bash +7 -0
+	BASH_VERSION: {$BASH_VERSION}
+	BASHOPTS: {$BASHOPTS}
+	SHELLOPTS: {$SHELLOPTS}
+	POSIXLY_CORRECT: {$POSIXLY_CORRECT}
+EOF
+	echo -n "	command line: {"
+	tr '\0' ' ' < /proc/$$/cmdline

This looks like a Linux-ism to me.

#3 lore raw
In reply to Junio C Hamano
Quoted 8 lines · Junio C Hamano

I do agree that there need to be a way to find that information for the end users, especially for those who just use binary-packaged git given by their distros, but this codepath is _not_ the place to do it.

How about replacing these with something simple like:

echo >&2 "You are not running full 'bash'; exiting." ; exit 127

I take your point that this is not the place to advertise the mailing list, although I prefer to include instructions in error messages. Would you be amenable in principle to creating something like man git-bug? A quick search doesn't turn up any documents specifically about bug fixing/reporting, and a man page was the first place I thought to look. I'll happily withdraw this patch until I can have a crack at such a page.

Quoted 9 lines

contrib/completion/git-completion.bash __gitdir () · line 119

Quoted 7 lines
contrib/completion/git-completion.bash +7 -0
+	BASH_VERSION: {$BASH_VERSION}
+	BASHOPTS: {$BASHOPTS}
+	SHELLOPTS: {$SHELLOPTS}
+	POSIXLY_CORRECT: {$POSIXLY_CORRECT}
+EOF
+	echo -n "	command line: {"
+	tr '\0' ' ' < /proc/$$/cmdline

This looks like a Linux-ism to me.

Caught red-flippered :) I'll use ps next time.

- Andrew

#4 lore raw
Quoted 4 lines · Andrew Sayers

Would you be amenable in principle to creating something like man git-bug? A quick search doesn't turn up any documents specifically about bug fixing/reporting, and a man page was the first place I thought to look.

I'd be perfectly fine with something like this. People may want to add a sentence or two to give tips on how to make sure that they are actual bugs.

 Documentation/git.txt |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Documentation/git.txt +8 -0
index 59f291d..3ba004f 100644
@@ -727,6 +727,14 @@ The documentation for git suite was started by David Greaves
 <[email protected]>, and later enhanced greatly by the
 contributors on the git-list <[email protected]>.
 
+Reporting Bugs
+--------------
+
+If you found a bug, please send a bug report to the Git mailing list
+<[email protected]>, where the development and maintenance is primarily
+done.  You do not have to be subscribed to the list to send a message
+there.
+
 SEE ALSO
 --------
 linkgit:gittutorial[7], linkgit:gittutorial-2[7],
#5 lore raw

Documentation/git.txt The documentation for git suite was started by David Greaves · line 737

Quoted 9 lines · Junio C Hamano
Documentation/git.txt +8 -0
+Reporting Bugs
+--------------
+
+If you found a bug, please send a bug report to the Git mailing list
+<[email protected]>, where the development and maintenance is primarily
+done.  You do not have to be subscribed to the list to send a message
+there.
+

I like it. How do I know if I found a bug though? Probably better to just say:

Report bugs to the Git mailing list<[email protected]> where development and ...

#6 lore raw
In reply to Andrew Sayers
Quoted 4 lines · Andrew Sayers
Quoted 1 line · Junio C Hamano

This looks like a Linux-ism to me.

Caught red-flippered :)  I'll use ps next time.

That's also probably hard to get right on all the mutually incompatible *nix ps(1) implementations out there.

As for this patch in general, I think solving this issue in Git's bash-completion code isn't the right thing to do, I think the right thing is to just ignore it and no nothing.

Any given *nix distribution will include lots of non-POSIX and shell-specific initialization files throughout the system. Trying to detect if the shell is running in POSIX compatibility mode in each of these is going to be redundant and bug-prone.

Instead the user should make sure that he's invoking the shell in non-POSIX mode before evaluating non-POSIX code.

Maybe this is a bigger potential problem than it seems, but it seems like just a one-off error in Arch Linux. I'd be surprised if it didn't also break dozens of other packages in Arch which included bash extensions.

This is not a nack, if you want to pursue this and try to emit a friendlier error message that's great. But maybe it's a bit *too* much effort on our part.

#7 lore raw
In reply to Andrew Sayers
Quoted 5 lines · Andrew Sayers

Detect whether the shell supports process substitution with <() Shells that fail the test will not be able to load git-completion.bash

If a bad shell is found, print a warning which gives the user as much debugging information as possible.

This cannot work the way you implemented it:

contrib/completion/git-completion.bash __gitdir () · line 103

Quoted 4 lines

+$(

contrib/completion/git-completion.bash +3 -0
+	exec 2>/dev/null
+	$(exec < <( ))
+ )

Any shell that does not support the <() syntax will have exited at this time with a syntax error and never get to print something.

This could work:

  (eval : '$(exec < <( ) )' 2>/dev/null)
Quoted 1 line

+if [[ 0 -ne $? ]]

Of course, you must not use unportable [[ ]] syntax here, either.

contrib/completion/git-completion.bash __gitdir () · line 106

Quoted 3 lines
contrib/completion/git-completion.bash +2 -0
+then
+	cat <<EOF
...

-- Hannes