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

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

[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

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(-)

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

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

[Junio C Hamano]

Andrew Sayers <andrew-git@pileofstuff.org> writes:

> 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>
> ---

> +# 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

> +	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.

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

[solsTiCe d'Hiver]

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.

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

[Andrew Sayers]

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

>> +	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

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

[Ævar Arnfjörð Bjarmason]

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.

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

[Junio C Hamano]

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(-)

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],

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

[Stephen Boyd]

  On 07/29/2010 04:38 PM, Junio C Hamano wrote:
>
> +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 ...

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

[Johannes Sixt]

On Donnerstag, 29. Juli 2010, Andrew Sayers wrote:
> 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