Bug 602158

I'd rather not have git pull in gitk and tk if we can avoid it.  Might not be possible though.

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.

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.

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.

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

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.

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

This issue won't be fixed in Fedora 13, moving to Fedora 14.

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

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

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

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.

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.