threads / patch / 26992

patch, 8 partsmake gitk work better in non-top-level directory

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

## tl;dr

15 messages between Apr 5, 2011 and May 24, 2011. Diffs are folded; open one to read it.

replies: 14people: 4as markdown or json

Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · lore

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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to gitk-git/gitk +10 −1
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
Paul Mackerras· Apr 10, 2011, 01:54 UTC · re: Martin von Zweigbergk · lore

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

On Mon, Apr 04, 2011 at 10:14:12PM -0400, Martin von Zweigbergk wrote:
Show 24 quoted lines
> 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.
Martin von Zweigbergk· Apr 10, 2011, 18:03 UTC · re: Paul Mackerras · lore

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

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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to gitk-git/gitk +2 −2
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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to gitk-git/gitk +2 −2
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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to gitk-git/gitk +3 −2
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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to gitk-git/gitk +1 −2
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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to gitk-git/gitk +4 −2
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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

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(-)
Show changes to gitk-git/gitk +1 −14
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
Martin von Zweigbergk· Apr 5, 2011, 02:14 UTC · re: Martin von Zweigbergk · lore

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

"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(-)
Show changes to gitk-git/gitk +15 −6
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
Paul Mackerras· Apr 10, 2011, 02:03 UTC · re: Martin von Zweigbergk · lore

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

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.

Show 5 quoted lines
> 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.
Junio C Hamano· Apr 11, 2011, 19:15 UTC · re: Paul Mackerras · lore

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

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.
Martin von Zweigbergk· May 24, 2011, 02:44 UTC · re: Martin von Zweigbergk · lore

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

"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(-)
Show changes to gitk-git/gitk +11 −6
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
Peter Baumann· Apr 5, 2011, 18:48 UTC · re: Martin von Zweigbergk · lore

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

On Mon, Apr 04, 2011 at 10:14:11PM -0400, Martin von Zweigbergk wrote:
Show 17 quoted lines
> 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

← back to recent threads