* [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed
@ 2011-03-20 21:10 Maxin john
2011-03-21 11:29 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Maxin john @ 2011-03-20 21:10 UTC (permalink / raw)
To: Git Mailing List
Hi,
While using "git bisect visualize" on my PC running Ubuntu 10.10, I
came across this error:
$ git bisect visualize
eval: 1: gitk: not found
git: 'bisect' is not a git command. See 'git --help'.
Did you mean this?
bisect
$
As this distribution don't keep "gitk" as a dependency for git,we will
have to install "gitk" as a separate package.
However, I found the error message a bit confusing. So,I have prepared
a "quick and dirty" patch to solve it.
Please let me know your comments.
Signed-off-by: Maxin B. John <maxin@maxinbjohn.info>
---
diff --git a/git-bisect.sh b/git-bisect.sh
index c21e33c..fefe212 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -290,7 +290,8 @@ bisect_visualize() {
then
case
"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}"
in
'') set git log ;;
- set*) set gitk ;;
+ set*) is_gitk_present
+ set gitk ;;
esac
else
case "$1" in
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index aa16b83..5e78b54 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -132,6 +132,14 @@ is_bare_repository () {
git rev-parse --is-bare-repository
}
+is_gitk_present () {
+ GIT_GITK=$(which gitk)
+ test -n "$GIT_GITK" || {
+ echo >&2 "Cannot find 'gitk' in the PATH"
+ exit 1
+ }
+}
+
cd_to_toplevel () {
cdup=$(git rev-parse --show-toplevel) &&
cd "$cdup" || {
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed
2011-03-20 21:10 [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed Maxin john
@ 2011-03-21 11:29 ` Jeff King
2011-03-21 12:26 ` Maxin john
0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2011-03-21 11:29 UTC (permalink / raw)
To: Maxin john; +Cc: Git Mailing List
On Sun, Mar 20, 2011 at 11:10:55PM +0200, Maxin john wrote:
> While using "git bisect visualize" on my PC running Ubuntu 10.10, I
> came across this error:
>
> $ git bisect visualize
> eval: 1: gitk: not found
> git: 'bisect' is not a git command. See 'git --help'.
>
> Did you mean this?
> bisect
> $
Yuck. Definitely non-optimal.
> diff --git a/git-bisect.sh b/git-bisect.sh
> index c21e33c..fefe212 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -290,7 +290,8 @@ bisect_visualize() {
> then
> case
> "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}"
> in
> '') set git log ;;
> - set*) set gitk ;;
> + set*) is_gitk_present
> + set gitk ;;
> esac
The point of this code is to use "gitk" if we can (i.e., if we have a
grahpical display of some sort), and "git log" otherwise. Shouldn't "we
are missing gitk" also cause us to fallback to using "git log"? IOW,
something like:
if test -n "${DISPLAY+set}..." && is_gitk_present; then
set gitk
else
set git log
fi
> +is_gitk_present () {
> + GIT_GITK=$(which gitk)
> + test -n "$GIT_GITK" || {
> + echo >&2 "Cannot find 'gitk' in the PATH"
> + exit 1
> + }
> +}
I don't think this is a portable use of which. In particular, I seem to
recall SunOS which printing some junk to stderr like "no foo in /bin
/usr/bin etc...". I think it even then returns a successful exit code,
just to make it totally useless.
I think we tend to use the shell's "type" builtin for this, which has a
usable exit code.
So the patch would look like:
diff --git a/git-bisect.sh b/git-bisect.sh
index c21e33c..3b3156f 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -288,10 +288,12 @@ bisect_visualize() {
if test $# = 0
then
- case "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" in
- '') set git log ;;
- set*) set gitk ;;
- esac
+ if test -n "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" &&
+ type gitk >/dev/null 2>&1; then
+ set gitk
+ else
+ set git log
+ fi
else
case "$1" in
git*|tig) ;;
but I didn't test it at all.
-Peff
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed
2011-03-21 11:29 ` Jeff King
@ 2011-03-21 12:26 ` Maxin john
2011-03-21 13:14 ` [PATCH] bisect: visualize with git-log if gitk is unavailable Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Maxin john @ 2011-03-21 12:26 UTC (permalink / raw)
To: Jeff King; +Cc: Git Mailing List
Hi Jeff,
I have tested the patch and it works like a charm!.
Tested-by: Maxin B. John <maxin@maxinbjohn.info>
On Mon, Mar 21, 2011 at 12:29 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Mar 20, 2011 at 11:10:55PM +0200, Maxin john wrote:
>
>> While using "git bisect visualize" on my PC running Ubuntu 10.10, I
>> came across this error:
>>
>> $ git bisect visualize
>> eval: 1: gitk: not found
>> git: 'bisect' is not a git command. See 'git --help'.
>>
>> Did you mean this?
>> bisect
>> $
>
> Yuck. Definitely non-optimal.
>
>> diff --git a/git-bisect.sh b/git-bisect.sh
>> index c21e33c..fefe212 100755
>> --- a/git-bisect.sh
>> +++ b/git-bisect.sh
>> @@ -290,7 +290,8 @@ bisect_visualize() {
>> then
>> case
>> "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}"
>> in
>> '') set git log ;;
>> - set*) set gitk ;;
>> + set*) is_gitk_present
>> + set gitk ;;
>> esac
>
> The point of this code is to use "gitk" if we can (i.e., if we have a
> grahpical display of some sort), and "git log" otherwise. Shouldn't "we
> are missing gitk" also cause us to fallback to using "git log"? IOW,
> something like:
>
> if test -n "${DISPLAY+set}..." && is_gitk_present; then
> set gitk
> else
> set git log
> fi
>
Yes. it is much better than just exiting if "gitk" is not present.
>> +is_gitk_present () {
>> + GIT_GITK=$(which gitk)
>> + test -n "$GIT_GITK" || {
>> + echo >&2 "Cannot find 'gitk' in the PATH"
>> + exit 1
>> + }
>> +}
>
> I don't think this is a portable use of which. In particular, I seem to
> recall SunOS which printing some junk to stderr like "no foo in /bin
> /usr/bin etc...". I think it even then returns a successful exit code,
> just to make it totally useless.
>
> I think we tend to use the shell's "type" builtin for this, which has a
> usable exit code.
"type" seems to be a better choice than using "which" for this case.
> So the patch would look like:
>
> diff --git a/git-bisect.sh b/git-bisect.sh
> index c21e33c..3b3156f 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -288,10 +288,12 @@ bisect_visualize() {
>
> if test $# = 0
> then
> - case "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" in
> - '') set git log ;;
> - set*) set gitk ;;
> - esac
> + if test -n "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" &&
> + type gitk >/dev/null 2>&1; then
> + set gitk
> + else
> + set git log
> + fi
> else
> case "$1" in
> git*|tig) ;;
>
> but I didn't test it at all.
>
> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Warm Regards,
Maxin B. John
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] bisect: visualize with git-log if gitk is unavailable
2011-03-21 12:26 ` Maxin john
@ 2011-03-21 13:14 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2011-03-21 13:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Maxin john
If gitk is not available in the PATH, bisect ends up
exiting with the shell's 127 error code, confusing the git
wrapper into thinking that bisect is not a git command.
We already fallback to git-log if there doesn't seem to be a
graphical display available. We should do the same if gitk
is not available in our PATH at all. This not only fixes the
ugly error message, but is a much more sensible default than
failing to show the user anything.
Reported by Maxin John.
Tested-by: Maxin B. John <maxin@maxinbjohn.info>
Signed-off-by: Jeff King <peff@peff.net>
---
git-bisect.sh | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/git-bisect.sh b/git-bisect.sh
index c21e33c..415a8d0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -288,10 +288,12 @@ bisect_visualize() {
if test $# = 0
then
- case "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" in
- '') set git log ;;
- set*) set gitk ;;
- esac
+ if test -n "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" &&
+ type gitk >/dev/null 2>&1; then
+ set gitk
+ else
+ set git log
+ fi
else
case "$1" in
git*|tig) ;;
--
1.7.2.5.22.g853c5
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-21 13:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-20 21:10 [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed Maxin john
2011-03-21 11:29 ` Jeff King
2011-03-21 12:26 ` Maxin john
2011-03-21 13:14 ` [PATCH] bisect: visualize with git-log if gitk is unavailable Jeff King
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).