[PATCH 0/8] make gitk work better in non-top-level directory

[PATCH 0/8] make gitk work better in non-top-level directory

[Martin von Zweigbergk]

This series fixes a few different bugs in gitk related to its working
directory.

I started working on patch 1, which fixes "Highlight this only/too"
when gitk is started in a subdirectory. This problem has bothered me
for a long time, but I had heard that the Tcl code in gitk was hard to
maintain, so I didn't have a look at it until now. I have to say that
it was a lot easier to follow the code than I had feared.

While testing that the fix in patch 1 worked, I found that gitk does
not work very well when the work tree is not at ".git/..", so most of
the other patches try to improve that situation.

I think I have tested most combinations of setups (top-level dir,
subdir, separate work tree, bare repo, .git) and operations (highlight
file, blame, external diff, show origin of line).


Martin von Zweigbergk (8):
  gitk: fix file highlight when run in subdirectory
  gitk: fix "show origin of this line" with separate work tree
  gitk: fix "blame parent commit" with separate work tree
  gitk: fix "External diff" with separate work tree
  gitk: put temporary directory inside .git
  gitk: run 'git rev-parse --git-dir' only once
  gitk: simplify calculation of gitdir
  gitk: show modified files with separate work tree

 gitk-git/gitk |   59 +++++++++++++++++++++++++++++++-------------------------
 1 files changed, 33 insertions(+), 26 deletions(-)

-- 
1.7.4.79.gcbe20

[PATCH 1/8] gitk: fix file highlight when run in subdirectory

[Martin von Zweigbergk]

The "highlight this only" and "highlight this too" commands in gitk
add the path relative to $GIT_WORK_TREE to the "Find" input box. When
the search (using git-diff-tree) is run, the paths are used
unmodified, except for some shell escaping. Since the search is run
from gitk's working directory, no commits matching the paths will be
found if gitk was started in a subdirectory.

Make the paths passed to git-diff-tree relative to gitk's working
directory instead of being relative to $GIT_WORK_TREE. If, however,
gitk is run outside of the working directory (e.g. with $GIT_WORK_TREE
set), we still need to use the path relative to $GIT_WORK_TREE.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---

This could also have been fixed by cd-ing to the work tree
directory. That would also make the "Local changes checked in to index
but not committed" and "Local uncommitted changes, not checked in to
index" show up properly when running with GIT_WORK_TREE defined.

I wasn't sure if other parts of gitk depend on the working directory,
or if there are plans to make something depend on it, so I thought
changing it only for the specific case of file highlighting would be
safer. What do you think?


 gitk-git/gitk |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index e82c6bf..ce96294 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -4528,12 +4528,17 @@ proc makepatterns {l} {
 
 proc do_file_hl {serial} {
     global highlight_files filehighlight highlight_paths gdttype fhl_list
+    global cdup
 
     if {$gdttype eq [mc "touching paths:"]} {
 	if {[catch {set paths [shellsplit $highlight_files]}]} return
 	set highlight_paths [makepatterns $paths]
 	highlight_filelist
-	set gdtargs [concat -- $paths]
+	set relative_paths {}
+	foreach path $paths {
+	    lappend relative_paths [file join $cdup $path]
+	}
+	set gdtargs [concat -- $relative_paths]
     } elseif {$gdttype eq [mc "adding/removing string:"]} {
 	set gdtargs [list "-S$highlight_files"]
     } else {
@@ -11625,6 +11630,10 @@ set stuffsaved 0
 set patchnum 0
 set lserial 0
 set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == "true"}]
+set cdup {}
+if {$isworktree} {
+    set cdup [exec git rev-parse --show-cdup]
+}
 setcoords
 makewindow
 catch {
-- 
1.7.4.79.gcbe20

Re: [PATCH 1/8] gitk: fix file highlight when run in subdirectory

[Paul Mackerras]

On Mon, Apr 04, 2011 at 10:14:12PM -0400, Martin von Zweigbergk wrote:

> The "highlight this only" and "highlight this too" commands in gitk
> add the path relative to $GIT_WORK_TREE to the "Find" input box. When
> the search (using git-diff-tree) is run, the paths are used
> unmodified, except for some shell escaping. Since the search is run
> from gitk's working directory, no commits matching the paths will be
> found if gitk was started in a subdirectory.
> 
> Make the paths passed to git-diff-tree relative to gitk's working
> directory instead of being relative to $GIT_WORK_TREE. If, however,
> gitk is run outside of the working directory (e.g. with $GIT_WORK_TREE
> set), we still need to use the path relative to $GIT_WORK_TREE.
> 
> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
> ---
> 
> This could also have been fixed by cd-ing to the work tree
> directory. That would also make the "Local changes checked in to index
> but not committed" and "Local uncommitted changes, not checked in to
> index" show up properly when running with GIT_WORK_TREE defined.
> 
> I wasn't sure if other parts of gitk depend on the working directory,
> or if there are plans to make something depend on it, so I thought
> changing it only for the specific case of file highlighting would be
> safer. What do you think?

I have to admit I wasn't aware of GIT_WORK_TREE before I saw your
patches.  The patches look OK, but I wonder how many of the problems
would go away if gitk were simply to set GIT_WORK_TREE in the
environment for the programs it runs, if it is not already set.
Something like this (untested):

 # check that we can find a .git directory somewhere...
 if {[catch {set gitdir [gitdir]}]} {
     show_error {} . [mc "Cannot find a git repository here."]
     exit 1
 }
 if {![file isdirectory $gitdir]} {
     show_error {} . [mc "Cannot find the git directory \"%s\"." $gitdir]
     exit 1
 }
+if {![info exists env(GIT_WORK_TREE)]} {
+    set worktree [file dirname $gitdir]
+    if {$worktree ne "."} {
+	set env(GIT_WORK_TREE) $worktree
+    }
+}

Paul.

Re: [PATCH 1/8] gitk: fix file highlight when run in subdirectory

[Martin von Zweigbergk]

Hi Paul,

Thanks for your input. I will work on a re-roll of this series based
on, at least,
your input on patch 8/8. However, I am going away for 4 weeks today and I will
probably not be able to do that until some time after I come back. If
anyone else
cares to do it before then, please go ahead.

On Sat, Apr 9, 2011 at 9:54 PM, Paul Mackerras <paulus@samba.org> wrote:
> I have to admit I wasn't aware of GIT_WORK_TREE before I saw your
> patches.  The patches look OK, but I wonder how many of the problems
> would go away if gitk were simply to set GIT_WORK_TREE in the
> environment for the programs it runs, if it is not already set.

Most of the problems I have tried to fix in this series only appear when
GIT_WORK_TREE _is_ set, so I don't think setting it would help. When
it it not set,
setting it to the directory that git detected should have no effect as
far as I can see.


/Martin

[PATCH 2/8] gitk: fix "show origin of this line" with separate work tree

[Martin von Zweigbergk]

Running "show origin of this line" currently fails when the the work
tree is not the parent of the git directory. Fix it by feeding
git-blame paths relative to $GIT_WORK_TREE instead of "$GIT_DIR/..".

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 gitk-git/gitk |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index ce96294..a0f0f37 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3590,7 +3590,7 @@ proc external_blame {parent_idx {line {}}} {
 proc show_line_source {} {
     global cmitmode currentid parents curview blamestuff blameinst
     global diff_menu_line diff_menu_filebase flist_menu_file
-    global nullid nullid2 gitdir
+    global nullid nullid2 gitdir cdup
 
     set from_index {}
     if {$cmitmode eq "tree"} {
@@ -3643,7 +3643,7 @@ proc show_line_source {} {
     } else {
 	lappend blameargs $id
     }
-    lappend blameargs -- [file join [file dirname $gitdir] $flist_menu_file]
+    lappend blameargs -- [file join $cdup $flist_menu_file]
     if {[catch {
 	set f [open $blameargs r]
     } err]} {
-- 
1.7.4.79.gcbe20

[PATCH 3/8] gitk: fix "blame parent commit" with separate work tree

[Martin von Zweigbergk]

Running "blame parent commit" currently brings up an empty blame view
when the the work tree is not the parent of the git directory. Fix it
by feeding git-blame paths relative to $GIT_WORK_TREE instead of
"$GIT_DIR/..".

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 gitk-git/gitk |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index a0f0f37..b1696de 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3558,7 +3558,7 @@ proc make_relative {f} {
 }
 
 proc external_blame {parent_idx {line {}}} {
-    global flist_menu_file gitdir
+    global flist_menu_file cdup
     global nullid nullid2
     global parentlist selectedline currentid
 
@@ -3577,7 +3577,7 @@ proc external_blame {parent_idx {line {}}} {
     if {$line ne {} && $line > 1} {
 	lappend cmdline "--line=$line"
     }
-    set f [file join [file dirname $gitdir] $flist_menu_file]
+    set f [file join $cdup $flist_menu_file]
     # Unfortunately it seems git gui blame doesn't like
     # being given an absolute path...
     set f [make_relative $f]
-- 
1.7.4.79.gcbe20

[PATCH 4/8] gitk: fix "External diff" with separate work tree

[Martin von Zweigbergk]

Running "External diff" to compare the index and work tree currently
brings up an empty blame view when the work tree is not the parent of
the git directory. This is because the file that is taken from the
work tree is assumed to be in
$GIT_DIR/../<repo-relative-file-name>. Fix it by feeding the diff tool
a path under $GIT_WORK_TREE instead of "$GIT_DIR/..".

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 gitk-git/gitk |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index b1696de..58b98df 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3365,10 +3365,10 @@ proc save_file_from_commit {filename output what} {
 
 proc external_diff_get_one_file {diffid filename diffdir} {
     global nullid nullid2 nullfile
-    global gitdir
+    global worktree
 
     if {$diffid == $nullid} {
-        set difffile [file join [file dirname $gitdir] $filename]
+        set difffile [file join $worktree $filename]
 	if {[file exists $difffile]} {
 	    return $difffile
 	}
@@ -11634,6 +11634,7 @@ set cdup {}
 if {$isworktree} {
     set cdup [exec git rev-parse --show-cdup]
 }
+set worktree [exec git rev-parse --show-toplevel]
 setcoords
 makewindow
 catch {
-- 
1.7.4.79.gcbe20

[PATCH 5/8] gitk: put temporary directory inside .git

[Martin von Zweigbergk]

When running "External diff" from gitk, the "from" and "to" files will
first be copied into a directory that is currently
".git/../.gitk-tmp.$pid". When gitk is closed, the directory is
deleted. When the work tree is not at ".git/.." (which is supported
since the previous commit), that directory may not even be git-related
and it does not seem unlikely that permissions may not allow the
temporary directory to be created there. Move the directory inside
.git instead.

This patch introduces a regression in the case that the .git directory
is readonly, but .git/.. is writeable.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 gitk-git/gitk |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 58b98df..b925f3e 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -3332,8 +3332,7 @@ proc gitknewtmpdir {} {
     global diffnum gitktmpdir gitdir
 
     if {![info exists gitktmpdir]} {
-	set gitktmpdir [file join [file dirname $gitdir] \
-			    [format ".gitk-tmp.%s" [pid]]]
+	set gitktmpdir [file join $gitdir [format ".gitk-tmp.%s" [pid]]]
 	if {[catch {file mkdir $gitktmpdir} err]} {
 	    error_popup "[mc "Error creating temporary directory %s:" $gitktmpdir] $err"
 	    unset gitktmpdir
-- 
1.7.4.79.gcbe20

[PATCH 6/8] gitk: run 'git rev-parse --git-dir' only once

[Martin von Zweigbergk]

It seems like gitk has been setting the global variable 'gitdir' at
startup since aa81d97 (gitk: Fix Update menu item, 2006-02-28). It
should therefore no longer be necessary to call the procedure with the
same name (more than once to set the global variable). Remove the
other call sites and use the global variable instead.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 gitk-git/gitk |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index b925f3e..0c1c4df 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -9045,6 +9045,7 @@ proc exec_citool {tool_args {baseid {}}} {
 proc cherrypick {} {
     global rowmenuid curview
     global mainhead mainheadid
+    global gitdir
 
     set oldhead [exec git rev-parse HEAD]
     set dheads [descheads $rowmenuid]
@@ -9073,7 +9074,7 @@ proc cherrypick {} {
 			conflict.\nDo you wish to run git citool to\
 			resolve it?"]]} {
 		# Force citool to read MERGE_MSG
-		file delete [file join [gitdir] "GITGUI_MSG"]
+		file delete [file join $gitdir "GITGUI_MSG"]
 		exec_citool {} $rowmenuid
 	    }
 	} else {
@@ -9439,6 +9440,7 @@ proc refill_reflist {} {
 proc getallcommits {} {
     global allcommits nextarc seeds allccache allcwait cachedarcs allcupdate
     global idheads idtags idotherrefs allparents tagobjid
+    global gitdir
 
     if {![info exists allcommits]} {
 	set nextarc 0
@@ -9446,7 +9448,7 @@ proc getallcommits {} {
 	set seeds {}
 	set allcwait 0
 	set cachedarcs 0
-	set allccache [file join [gitdir] "gitk.cache"]
+	set allccache [file join $gitdir "gitk.cache"]
 	if {![catch {
 	    set f [open $allccache r]
 	    set allcwait 1
-- 
1.7.4.79.gcbe20

[PATCH 7/8] gitk: simplify calculation of gitdir

[Martin von Zweigbergk]

Since 5024baa ([PATCH] Make gitk work when launched in a subdirectory,
2007-01-09), gitk has used 'git rev-parse --git-dir' to find the .git
directory. However, gitk still first checks for the $GIT_DIR
environment variable and that the value returned from git-rev-parse
does not point to a file. Since git-rev-parse does both of these
checks already, the checks can safely be removed from gitk. This makes
the gitdir procedure small enough to inline.

This cleanup introduces a UI regression in that the error message will
now be "Cannot find a git repository here." even in the case where
GIT_DIR points to a file, for which the error message was previously
"Cannot find the git directory \"%s\".". It should be noted, though,
that even before this patch, 'gitk --git-dir=path/to/some/file' would
give the former error message.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---
 gitk-git/gitk |   15 +--------------
 1 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 0c1c4df..232ea6e 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -9,15 +9,6 @@ exec wish "$0" -- "$@"
 
 package require Tk
 
-proc gitdir {} {
-    global env
-    if {[info exists env(GIT_DIR)]} {
-	return $env(GIT_DIR)
-    } else {
-	return [exec git rev-parse --git-dir]
-    }
-}
-
 # A simple scheduler for compute-intensive stuff.
 # The aim is to make sure that event handlers for GUI actions can
 # run at least every 50-100 ms.  Unfortunately fileevent handlers are
@@ -11507,14 +11498,10 @@ setui $uicolor
 setoptions
 
 # check that we can find a .git directory somewhere...
-if {[catch {set gitdir [gitdir]}]} {
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
     show_error {} . [mc "Cannot find a git repository here."]
     exit 1
 }
-if {![file isdirectory $gitdir]} {
-    show_error {} . [mc "Cannot find the git directory \"%s\"." $gitdir]
-    exit 1
-}
 
 set selecthead {}
 set selectheadid {}
-- 
1.7.4.79.gcbe20

[PATCH 8/8] gitk: show modified files with separate work tree

[Martin von Zweigbergk]

"git rev-parse --is-inside-work-tree" is currently used to determine
whether to show modified files in gitk (the red and green fake
commits). This does not work if the current directory is not inside
the work tree, as can be the case e.g. if GIT_WORK_TREE is
set. Instead, check if the repository is not bare and that we are not
inside the .git directory.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>

---
Is the test in proc hasworktree good?

Why do git commands that need a work tree not work under .git? Why
don't they show the same output as if they had been run from the work
tree? (Btw, the check for valid work tree does not work for aliases,
so e.g. 'git st', with 'st' as alias for 'status' will show all files
as deleted.)

How do I simplify the Tcl code to just return the boolean right away?

Why is the hasworktree variable reset in updatecommits? The only reason
I can think of is when 'core.worktree' is set/changed, but I don't
think that case worked very well before this series anyway. Should
gitdir also be recalculated?

 gitk-git/gitk |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 232ea6e..914de8d 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -9,6 +9,15 @@ exec wish "$0" -- "$@"
 
 package require Tk
 
+proc hasworktree {} {
+    if {[expr {[exec git rev-parse --is-bare-repository] == "false"}] &&
+	[expr {[exec git rev-parse --is-inside-git-dir] == "false"}]} {
+	return 1
+    } else {
+	return 0
+    }
+}
+
 # A simple scheduler for compute-intensive stuff.
 # The aim is to make sure that event handlers for GUI actions can
 # run at least every 50-100 ms.  Unfortunately fileevent handlers are
@@ -459,11 +468,11 @@ proc updatecommits {} {
     global viewactive viewcomplete tclencoding
     global startmsecs showneartags showlocalchanges
     global mainheadid viewmainheadid viewmainheadid_orig pending_select
-    global isworktree
+    global hasworktree
     global varcid vposids vnegids vflags vrevs
     global show_notes
 
-    set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == "true"}]
+    set hasworktree [hasworktree]
     rereadrefs
     set view $curview
     if {$mainheadid ne $viewmainheadid_orig($view)} {
@@ -5025,9 +5034,9 @@ proc dohidelocalchanges {} {
 # spawn off a process to do git diff-index --cached HEAD
 proc dodiffindex {} {
     global lserial showlocalchanges vfilelimit curview
-    global isworktree
+    global hasworktree
 
-    if {!$showlocalchanges || !$isworktree} return
+    if {!$showlocalchanges || !$hasworktree} return
     incr lserial
     set cmd "|git diff-index --cached HEAD"
     if {$vfilelimit($curview) ne {}} {
@@ -11617,9 +11626,9 @@ set stopped 0
 set stuffsaved 0
 set patchnum 0
 set lserial 0
-set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == "true"}]
+set hasworktree [hasworktree]
 set cdup {}
-if {$isworktree} {
+if {[expr {[exec git rev-parse --is-inside-work-tree] == "true"}]} {
     set cdup [exec git rev-parse --show-cdup]
 }
 set worktree [exec git rev-parse --show-toplevel]
-- 
1.7.4.79.gcbe20

Re: [PATCH 8/8] gitk: show modified files with separate work tree

[Paul Mackerras]

On Mon, Apr 04, 2011 at 10:14:19PM -0400, Martin von Zweigbergk wrote:

> Is the test in proc hasworktree good?

The first parameter to 'if' is evaluated as an expression, so you
don't need the extra exprs.

> Why do git commands that need a work tree not work under .git? Why
> don't they show the same output as if they had been run from the work
> tree? (Btw, the check for valid work tree does not work for aliases,
> so e.g. 'git st', with 'st' as alias for 'status' will show all files
> as deleted.)

Don't know, ask Junio. :)

> How do I simplify the Tcl code to just return the boolean right away?

You can do:

    return [expr {[exec git rev-parse --is-bare-repository] == "false" &&
		  [exec git rev-parse --is-inside-git-dir] == "false"}]

> Why is the hasworktree variable reset in updatecommits? The only reason
> I can think of is when 'core.worktree' is set/changed, but I don't
> think that case worked very well before this series anyway. Should
> gitdir also be recalculated?

I don't know that there's any particularly strong reason to do it in
updatecommits.  It could probably be done once at startup.

Paul.

Re: [PATCH 8/8] gitk: show modified files with separate work tree

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

>> Why do git commands that need a work tree not work under .git?
> ...
> Don't know, ask Junio. :)

Whatever the current behaviour is, I am reasonably sure that it is coming
more from "meh -- who cares such a case?" than "it should work like this
when you are in .git because of such and such reasons".

For example, what does it mean to be able to do this?

	$ edit Makefile
        $ git add Makefile
        $ edit Makefile
	$ cd .git
        $ git grep frotz Makefile

Perhaps the last step needs to be

	$ git grep frotz ../Makefile

instead, but a more important point is, how would that be useful?

If you have both GIT_DIR and GIT_WORK_TREE set up to point at the correct
places, I think it is sensible to make the above (the "../Makefile"
version, not the one without dot-dot) work as expected.

I suspect (but would not bother to dig the history myself to find out)
that "we require a working tree" semantics that in fact often means "we
require you to be in the working tree" was a misdesign that did not matter
that came from the days back when GIT_WORK_TREE was not either present or
not widely used.  Now more people seem to be using GIT_WORK_TREE for some
reason, I don't have anything against a patch series that defines and
implements a more desirable behaviour clearly.

Thanks.

[PATCH v2 8/8] gitk: show modified files with separate work tree

[Martin von Zweigbergk]

"git rev-parse --is-inside-work-tree" is currently used to determine
whether to show modified files in gitk (the red and green fake
commits). This does not work if the current directory is not inside
the work tree, as can be the case e.g. if GIT_WORK_TREE is
set. Instead, check if the repository is not bare and that we are not
inside the .git directory.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---

The only change since v1 is that the hasworktree procedure has been
simplified. I was conservative and left the recalculatation of
hasworktree in updatecommits(). There are no changes to any of the
other patches, so I didn't bother resending them. Sorry about the long
delay for such a trivial fixup.


 gitk-git/gitk |   17 +++++++++++------
 1 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 10b2bca..01b63e5 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -9,6 +9,11 @@ exec wish "$0" -- "$@"
 
 package require Tk
 
+proc hasworktree {} {
+    return [expr {[exec git rev-parse --is-bare-repository] == "false" &&
+		  [exec git rev-parse --is-inside-git-dir] == "false"}]
+}
+
 # A simple scheduler for compute-intensive stuff.
 # The aim is to make sure that event handlers for GUI actions can
 # run at least every 50-100 ms.  Unfortunately fileevent handlers are
@@ -459,11 +464,11 @@ proc updatecommits {} {
     global viewactive viewcomplete tclencoding
     global startmsecs showneartags showlocalchanges
     global mainheadid viewmainheadid viewmainheadid_orig pending_select
-    global isworktree
+    global hasworktree
     global varcid vposids vnegids vflags vrevs
     global show_notes
 
-    set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == "true"}]
+    set hasworktree [hasworktree]
     rereadrefs
     set view $curview
     if {$mainheadid ne $viewmainheadid_orig($view)} {
@@ -5026,9 +5031,9 @@ proc dohidelocalchanges {} {
 # spawn off a process to do git diff-index --cached HEAD
 proc dodiffindex {} {
     global lserial showlocalchanges vfilelimit curview
-    global isworktree
+    global hasworktree
 
-    if {!$showlocalchanges || !$isworktree} return
+    if {!$showlocalchanges || !$hasworktree} return
     incr lserial
     set cmd "|git diff-index --cached HEAD"
     if {$vfilelimit($curview) ne {}} {
@@ -11621,9 +11626,9 @@ set stopped 0
 set stuffsaved 0
 set patchnum 0
 set lserial 0
-set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == "true"}]
+set hasworktree [hasworktree]
 set cdup {}
-if {$isworktree} {
+if {[expr {[exec git rev-parse --is-inside-work-tree] == "true"}]} {
     set cdup [exec git rev-parse --show-cdup]
 }
 set worktree [exec git rev-parse --show-toplevel]
-- 
1.7.4.79.gcbe20

Re: [PATCH 0/8] make gitk work better in non-top-level directory

[Peter Baumann]

On Mon, Apr 04, 2011 at 10:14:11PM -0400, Martin von Zweigbergk wrote:
> This series fixes a few different bugs in gitk related to its working
> directory.
> 
> I started working on patch 1, which fixes "Highlight this only/too"
> when gitk is started in a subdirectory. This problem has bothered me
> for a long time, but I had heard that the Tcl code in gitk was hard to
> maintain, so I didn't have a look at it until now. I have to say that
> it was a lot easier to follow the code than I had feared.
> 
> While testing that the fix in patch 1 worked, I found that gitk does
> not work very well when the work tree is not at ".git/..", so most of
> the other patches try to improve that situation.
> 
> I think I have tested most combinations of setups (top-level dir,
> subdir, separate work tree, bare repo, .git) and operations (highlight
> file, blame, external diff, show origin of line).
> 

This is something which anoyed me a long time. I even tried to fix
it [1], but with my limited Tcl knowledge I failed misserably.

Your patch series seems to fix all the problems I was having.
Nice job!

-Peter

[1]: http://article.gmane.org/gmane.comp.version-control.git/120391