{"thread":{"id":"26807","subject":"[PATCH ] \"git bisect visualize\" results in an invalid error if \"gitk\" is not installed","startedAt":"2011-03-20T21:10:55Z","lastAt":"2011-03-21T13:14:22Z","messageCount":4,"participants":["Maxin john","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"163841","messageId":"AANLkTi=HJjqrvv-PFO3VjhrHzBsLZmAbN0yU47WScWd_@mail.gmail.com","threadId":"26807","inReplyTo":null,"subject":"[PATCH ] \"git bisect visualize\" results in an invalid error if \"gitk\" is not installed","fromName":"Maxin john","fromEmail":"maxin@maxinbjohn.info","sentAt":"2011-03-20T21:10:55Z","receivedAt":"2011-03-20T21:10:55Z","isPatch":true,"sender":{"key":"maxin@maxinbjohn.info","avatar":null},"body":"Hi,\n\nWhile using \"git bisect visualize\" on my PC running Ubuntu 10.10, I\ncame across this error:\n\n$ git bisect visualize\neval: 1: gitk: not found\ngit: 'bisect' is not a git command. See 'git --help'.\n\nDid you mean this?\n\tbisect\n$\n\nAs this distribution don't keep \"gitk\" as a dependency for git,we will\nhave to install \"gitk\" as a separate package.\nHowever, I found the error message a bit confusing. So,I have prepared\na \"quick and dirty\" patch to solve it.\nPlease let me know your comments.\n\nSigned-off-by: Maxin B. John <maxin@maxinbjohn.info>\n---\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex c21e33c..fefe212 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -290,7 +290,8 @@ bisect_visualize() {\n        then\n                case\n\"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\"\nin\n                '')     set git log ;;\n-               set*)   set gitk ;;\n+               set*)   is_gitk_present\n+                       set gitk ;;\n                esac\n        else\n                case \"$1\" in\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex aa16b83..5e78b54 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -132,6 +132,14 @@ is_bare_repository () {\n        git rev-parse --is-bare-repository\n }\n\n+is_gitk_present () {\n+       GIT_GITK=$(which gitk)\n+       test -n \"$GIT_GITK\" || {\n+               echo >&2 \"Cannot find 'gitk' in the PATH\"\n+               exit 1\n+       }\n+}\n+\n cd_to_toplevel () {\n        cdup=$(git rev-parse --show-toplevel) &&\n        cd \"$cdup\" || {\n"},{"id":"163898","messageId":"20110321112932.GF16334@sigill.intra.peff.net","threadId":"26807","inReplyTo":"AANLkTi=HJjqrvv-PFO3VjhrHzBsLZmAbN0yU47WScWd_@mail.gmail.com","subject":"Re: [PATCH ] \"git bisect visualize\" results in an invalid error if \"gitk\" is not installed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-21T11:29:32Z","receivedAt":"2011-03-21T11:29:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Mar 20, 2011 at 11:10:55PM +0200, Maxin john wrote:\n\n> While using \"git bisect visualize\" on my PC running Ubuntu 10.10, I\n> came across this error:\n> \n> $ git bisect visualize\n> eval: 1: gitk: not found\n> git: 'bisect' is not a git command. See 'git --help'.\n> \n> Did you mean this?\n> \tbisect\n> $\n\nYuck. Definitely non-optimal.\n\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index c21e33c..fefe212 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -290,7 +290,8 @@ bisect_visualize() {\n>         then\n>                 case\n> \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\"\n> in\n>                 '')     set git log ;;\n> -               set*)   set gitk ;;\n> +               set*)   is_gitk_present\n> +                       set gitk ;;\n>                 esac\n\nThe point of this code is to use \"gitk\" if we can (i.e., if we have a\ngrahpical display of some sort), and \"git log\" otherwise. Shouldn't \"we\nare missing gitk\" also cause us to fallback to using \"git log\"? IOW,\nsomething like:\n\n  if test -n \"${DISPLAY+set}...\" && is_gitk_present; then\n    set gitk\n  else\n    set git log\n  fi\n\n> +is_gitk_present () {\n> +       GIT_GITK=$(which gitk)\n> +       test -n \"$GIT_GITK\" || {\n> +               echo >&2 \"Cannot find 'gitk' in the PATH\"\n> +               exit 1\n> +       }\n> +}\n\nI don't think this is a portable use of which. In particular, I seem to\nrecall SunOS which printing some junk to stderr like \"no foo in /bin\n/usr/bin etc...\". I think it even then returns a successful exit code,\njust to make it totally useless.\n\nI think we tend to use the shell's \"type\" builtin for this, which has a\nusable exit code.\n\nSo the patch would look like:\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex c21e33c..3b3156f 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -288,10 +288,12 @@ bisect_visualize() {\n \n \tif test $# = 0\n \tthen\n-\t\tcase \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" in\n-\t\t'')\tset git log ;;\n-\t\tset*)\tset gitk ;;\n-\t\tesac\n+\t\tif test -n \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" &&\n+\t\t   type gitk >/dev/null 2>&1; then\n+\t\t\tset gitk\n+\t\telse\n+\t\t\tset git log\n+\t\tfi\n \telse\n \t\tcase \"$1\" in\n \t\tgit*|tig) ;;\n\nbut I didn't test it at all.\n\n-Peff\n"},{"id":"163902","messageId":"AANLkTinS87obXcgbcFZ8L-UjZUQL96SzpHp84Y6-yX6v@mail.gmail.com","threadId":"26807","inReplyTo":"20110321112932.GF16334@sigill.intra.peff.net","subject":"Re: [PATCH ] \"git bisect visualize\" results in an invalid error if \"gitk\" is not installed","fromName":"Maxin john","fromEmail":"maxin@maxinbjohn.info","sentAt":"2011-03-21T12:26:33Z","receivedAt":"2011-03-21T12:26:33Z","isPatch":true,"sender":{"key":"maxin@maxinbjohn.info","avatar":null},"body":"Hi Jeff,\n\nI have tested the patch and it works like a charm!.\n\nTested-by: Maxin B. John <maxin@maxinbjohn.info>\n\n\nOn Mon, Mar 21, 2011 at 12:29 PM, Jeff King <peff@peff.net> wrote:\n> On Sun, Mar 20, 2011 at 11:10:55PM +0200, Maxin john wrote:\n>\n>> While using \"git bisect visualize\" on my PC running Ubuntu 10.10, I\n>> came across this error:\n>>\n>> $ git bisect visualize\n>> eval: 1: gitk: not found\n>> git: 'bisect' is not a git command. See 'git --help'.\n>>\n>> Did you mean this?\n>>       bisect\n>> $\n>\n> Yuck. Definitely non-optimal.\n>\n>> diff --git a/git-bisect.sh b/git-bisect.sh\n>> index c21e33c..fefe212 100755\n>> --- a/git-bisect.sh\n>> +++ b/git-bisect.sh\n>> @@ -290,7 +290,8 @@ bisect_visualize() {\n>>         then\n>>                 case\n>> \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\"\n>> in\n>>                 '')     set git log ;;\n>> -               set*)   set gitk ;;\n>> +               set*)   is_gitk_present\n>> +                       set gitk ;;\n>>                 esac\n>\n> The point of this code is to use \"gitk\" if we can (i.e., if we have a\n> grahpical display of some sort), and \"git log\" otherwise. Shouldn't \"we\n> are missing gitk\" also cause us to fallback to using \"git log\"? IOW,\n> something like:\n>\n>  if test -n \"${DISPLAY+set}...\" && is_gitk_present; then\n>    set gitk\n>  else\n>    set git log\n>  fi\n>\n\nYes. it is much better than just exiting if \"gitk\" is not present.\n\n>> +is_gitk_present () {\n>> +       GIT_GITK=$(which gitk)\n>> +       test -n \"$GIT_GITK\" || {\n>> +               echo >&2 \"Cannot find 'gitk' in the PATH\"\n>> +               exit 1\n>> +       }\n>> +}\n>\n> I don't think this is a portable use of which. In particular, I seem to\n> recall SunOS which printing some junk to stderr like \"no foo in /bin\n> /usr/bin etc...\". I think it even then returns a successful exit code,\n> just to make it totally useless.\n>\n> I think we tend to use the shell's \"type\" builtin for this, which has a\n> usable exit code.\n\n\"type\" seems to be a better choice than using \"which\" for this case.\n\n> So the patch would look like:\n>\n> diff --git a/git-bisect.sh b/git-bisect.sh\n> index c21e33c..3b3156f 100755\n> --- a/git-bisect.sh\n> +++ b/git-bisect.sh\n> @@ -288,10 +288,12 @@ bisect_visualize() {\n>\n>        if test $# = 0\n>        then\n> -               case \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" in\n> -               '')     set git log ;;\n> -               set*)   set gitk ;;\n> -               esac\n> +               if test -n \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" &&\n> +                  type gitk >/dev/null 2>&1; then\n> +                       set gitk\n> +               else\n> +                       set git log\n> +               fi\n>        else\n>                case \"$1\" in\n>                git*|tig) ;;\n>\n> but I didn't test it at all.\n>\n> -Peff\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n\nWarm Regards,\nMaxin B. John\n"},{"id":"163906","messageId":"20110321131422.GA24382@sigill.intra.peff.net","threadId":"26807","inReplyTo":"AANLkTinS87obXcgbcFZ8L-UjZUQL96SzpHp84Y6-yX6v@mail.gmail.com","subject":"[PATCH] bisect: visualize with git-log if gitk is unavailable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-03-21T13:14:22Z","receivedAt":"2011-03-21T13:14:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If gitk is not available in the PATH, bisect ends up\nexiting with the shell's 127 error code, confusing the git\nwrapper into thinking that bisect is not a git command.\n\nWe already fallback to git-log if there doesn't seem to be a\ngraphical display available. We should do the same if gitk\nis not available in our PATH at all. This not only fixes the\nugly error message, but is a much more sensible default than\nfailing to show the user anything.\n\nReported by Maxin John.\n\nTested-by: Maxin B. John <maxin@maxinbjohn.info>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git-bisect.sh |   10 ++++++----\n 1 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/git-bisect.sh b/git-bisect.sh\nindex c21e33c..415a8d0 100755\n--- a/git-bisect.sh\n+++ b/git-bisect.sh\n@@ -288,10 +288,12 @@ bisect_visualize() {\n \n \tif test $# = 0\n \tthen\n-\t\tcase \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" in\n-\t\t'')\tset git log ;;\n-\t\tset*)\tset gitk ;;\n-\t\tesac\n+\t\tif test -n \"${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}\" &&\n+\t\t   type gitk >/dev/null 2>&1; then\n+\t\t\tset gitk\n+\t\telse\n+\t\t\tset git log\n+\t\tfi\n \telse\n \t\tcase \"$1\" in\n \t\tgit*|tig) ;;\n-- \n1.7.2.5.22.g853c5\n"}]}