threads / patch / 24564

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

Subject: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

## tl;dr

8 messages between Jul 28, 2010 and Aug 6, 2010. Diffs are folded; open one to read it.

replies: 7people: 6as markdown or json

Andrew Sayers· Jul 28, 2010, 22:32 UTC · lore

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
Signed-off-by: Andrew Sayers <andrew-git@pileofstuff.org>
---
My thanks to solsTiCe d'Hiver for reporting this bug.
 contrib/completion/git-completion.bash |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)
Show changes to contrib/completion/git-completion.bash +25 −0
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6756990..3bbb4da 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -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 git@vger.kernel.org:
+
+	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 ()
-- 
1.7.0.4
Junio C Hamano· Jul 29, 2010, 16:25 UTC · re: Andrew Sayers · lore

Re: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

Andrew Sayers <andrew-git@pileofstuff.org> writes:
Show 11 quoted lines
> 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
>
> Signed-off-by: Andrew Sayers <andrew-git@pileofstuff.org>
> ---
Show 13 quoted lines
> +# 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 git@vger.kernel.org:

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
Show 7 quoted lines
> +	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.
solsTiCe d'Hiver· Jul 29, 2010, 19:05 UTC · re: Junio C Hamano · lore

Re: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

I must re-emphasize that the bug was an archlinux bug in /etc/profile. http://bugs.archlinux.org/task/20288

Having said that, I don't see why you're trying to do anything about that. If I were you, I wouldn't bother to try to print any message at all.

Andrew Sayers· Jul 29, 2010, 21:29 UTC · re: Junio C Hamano · lore

Re: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

On 29/07/10 17:25, Junio C Hamano wrote:
Show 8 quoted lines
> 
> 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.

Show 9 quoted lines
>> +	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
Ævar Arnfjörð Bjarmason· Jul 29, 2010, 22:16 UTC · re: Andrew Sayers · lore

Re: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

On Thu, Jul 29, 2010 at 21:29, Andrew Sayers <andrew-git@pileofstuff.org> wrote:
> On 29/07/10 17:25, Junio C Hamano wrote:
>> 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.

Junio C Hamano· Jul 29, 2010, 23:38 UTC · re: Andrew Sayers · lore

Re: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

Andrew Sayers <andrew-git@pileofstuff.org> writes:
> 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(-)
Show changes to Documentation/git.txt +8 −0
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 59f291d..3ba004f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -727,6 +727,14 @@ The documentation for git suite was started by David Greaves
 <david@dgreaves.com>, and later enhanced greatly by the
 contributors on the git-list <git@vger.kernel.org>.
 
+Reporting Bugs
+--------------
+
+If you found a bug, please send a bug report to the Git mailing list
+<git@vger.kernel.org>, 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],
Stephen Boyd· Jul 30, 2010, 06:02 UTC · re: Junio C Hamano · lore

Re: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

  On 07/29/2010 04:38 PM, Junio C Hamano wrote:
Show 9 quoted lines
>
> +Reporting Bugs
> +--------------
> +
> +If you found a bug, please send a bug report to the Git mailing list
> +<git@vger.kernel.org>, 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<git@vger.kernel.org>
	where development and ...
Johannes Sixt· Aug 6, 2010, 21:31 UTC · re: Andrew Sayers · lore

Re: [PATCH] bash-completion: Print a useful error when called in a non-bash shell

On Donnerstag, 29. Juli 2010, Andrew Sayers wrote:
Show 5 quoted lines
> 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:
> +$( 
> +	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)
> +if [[ 0 -ne $? ]]
Of course, you must not use unportable [[ ]] syntax here, either.
> +then
> +	cat <<EOF
> ...
-- Hannes

← back to recent threads