Bugzilla will be upgraded to version 5.0 on a still to be determined date in the near future. The original upgrade date has been delayed.
Bug 602158 - git bisect calls gitk without checking whether it is available
git bisect calls gitk without checking whether it is available
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: git (Show other bugs)
14
All Linux
low Severity medium
: ---
: ---
Assigned To: Adam Tkac
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-09 05:38 EDT by Dhaval Giani
Modified: 2013-01-10 05:42 EST (History)
7 users (show)

See Also:
Fixed In Version: git-1.7.4.2-1.fc14
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2011-04-05 17:35:10 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Dhaval Giani 2010-06-09 05:38:14 EDT
[dhaval@gondor libcg]$ git bisect visualize
/usr/libexec/git-core/git-bisect: line 303: gitk: command not found
git: 'bisect' is not a git command. See 'git --help'.

Did you mean this?
        bisect


As can be seen, git bisect depends on gitk, so please make the appropriate git package (git-core in this case if i am correct) have a dependency on gitk.

[dhaval@gondor tmp]$ rpm -qv git
git-1.7.0.1-1.fc13.x86_64
[dhaval@gondor tmp]$
Comment 1 Josh Boyer 2010-06-09 08:18:04 EDT
I'd rather not have git pull in gitk and tk if we can avoid it.  Might not be possible though.
Comment 2 Adam Tkac 2010-06-09 08:52:54 EDT
Actually there are two ways how to solve this issue:

1. add a distribution-specific patch to git-bisect to check if gitk is present. If no then write something like "please install gitk package to get this functionality working".

2. simply add Requires: gitk

In my opinion we can safely choose the second way because gitk has only tcl and tk packages as "non-standard dependencies". Tcl and tk depends on "standard dependencies" only, like libc and libX11 which are present on vast majority of systems. Per `yum install gitk` gitk + tk + tcl needs only 8.3 M of additional space which is not a big issue nowadays.
Comment 3 Todd Zullinger 2010-06-09 09:33:03 EDT
Hmm, gitk requires git, so making git require gitk might be fun.  I haven't woken up enough to think much about it yet.

My first thought is that the bisect script ought to check that gitk exists before attempting to execute it.  Then it could warn the user that gitk is needed for this one feature.  I must admit, I've used bisect a lot of times and never missed gitk.  And seeing that visualize has been around for 2 or 3 years and we've not had any bug reports so far, it seems not many others miss it either.

The code in question in git-bisect looks like this (squashed for space):

bisect_visualize() {
    bisect_next_check fail

    if test $# = 0
    then
        case "${DISPLAY+set}${SESSIONNAME+set}${MSYSTEM+set}${SECURITYSESSIONID+set}" in
        '')     set git log ;;
        set*)   set gitk ;;
        esac
    else
        case "$1" in
        git*|tig) ;;
        -*)     set git log "$@" ;;
        *)      set git "$@" ;;
        esac
    fi

    eval '"$@"' --bisect -- $(cat "$GIT_DIR/BISECT_NAMES")
}

Somewhere, a test for gitk's presence is needed I think. Though note that `which gitk` isn't portable across the many systems git runs on, and isn't used in any of the git tools which use shell, AFAIK.
Comment 4 Todd Zullinger 2010-06-09 09:35:36 EDT
Adam, why would we consider adding a distro specific patch?  If it's worth solving with a patch, it should be a patch that's something we can push upstream.
Comment 5 Todd Zullinger 2010-06-09 10:03:12 EDT
Further, requiring gitk will pull in libX11.  There are certainly folks who install git on servers where libX11 is not installed nor desired.  Making that a requirement for one small function in one of git's many tools seems like a bad trade to me.

Anyone with the itch to scratch is encouraged to suggest an upstream-able patch to check for gitk before using it in the bisect script.  Bonus points if you suggest it directly to the git list, following the patch submission guidelines in http://repo.or.cz/w/git.git?a=blob;f=Documentation/SubmittingPatches;hb=HEAD
Comment 6 Adam Tkac 2010-06-09 11:20:15 EDT
I proposed distro specific patch because it will be pretty simple, just something like `which gitk` or `rpm -q gitk`. It won't be accepted by upstream, of course. \

Although generic patch must be sophisticated enough, I think searching for "gitk" in $PATH should be sufficient and portable.
Comment 7 Ondrej Vasik 2010-06-10 02:55:46 EDT
I would suggest the similar way which is used by autoconf ( e.g. expanded macro AC_PATH_PROG() ) ... it should be portable and sophisticated enough ...
Comment 8 Adam Tkac 2011-03-28 10:46:30 EDT
This issue won't be fixed in Fedora 13, moving to Fedora 14.
Comment 9 Fedora Update System 2011-03-28 10:53:45 EDT
git-1.7.4.2-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/git-1.7.4.2-1.fc15
Comment 10 Fedora Update System 2011-03-28 10:54:03 EDT
git-1.7.4.2-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/git-1.7.4.2-1.fc14
Comment 11 Fedora Update System 2011-03-28 15:19:26 EDT
Package git-1.7.4.2-1.fc14:
* should fix your issue,
* was pushed to the Fedora 14 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing git-1.7.4.2-1.fc14'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/git-1.7.4.2-1.fc14
then log in and leave karma (feedback).
Comment 12 Fedora Update System 2011-04-05 17:34:53 EDT
git-1.7.4.2-1.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 13 Fedora Update System 2011-04-06 18:36:07 EDT
git-1.7.4.2-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.