# [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed 4 messages from 2011-03-20 to 2011-03-21. Participants: Maxin john, Jeff King. Thread: https://gitlist.dev/t/26807 ## Maxin john, 2011-03-20 21:10 Subject: [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed Message-ID: URL: https://gitlist.dev/e/AANLkTi%3DHJjqrvv-PFO3VjhrHzBsLZmAbN0yU47WScWd_%40mail.gmail.com ``` 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 --- 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" || { ``` ## Jeff King, 2011-03-21 11:29 Subject: Re: [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed Message-ID: <20110321112932.GF16334@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110321112932.GF16334%40sigill.intra.peff.net In-Reply-To: ``` 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 ``` ## Maxin john, 2011-03-21 12:26 Subject: Re: [PATCH ] "git bisect visualize" results in an invalid error if "gitk" is not installed Message-ID: URL: https://gitlist.dev/e/AANLkTinS87obXcgbcFZ8L-UjZUQL96SzpHp84Y6-yX6v%40mail.gmail.com In-Reply-To: <20110321112932.GF16334@sigill.intra.peff.net> ``` Hi Jeff, I have tested the patch and it works like a charm!. Tested-by: Maxin B. John On Mon, Mar 21, 2011 at 12:29 PM, Jeff King 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 ``` ## Jeff King, 2011-03-21 13:14 Subject: [PATCH] bisect: visualize with git-log if gitk is unavailable Message-ID: <20110321131422.GA24382@sigill.intra.peff.net> URL: https://gitlist.dev/e/20110321131422.GA24382%40sigill.intra.peff.net In-Reply-To: ``` 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 Signed-off-by: Jeff King --- 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 ```