{"thread":{"id":"26992","subject":"[PATCH 0/8] make gitk work better in non-top-level directory","startedAt":"2011-04-05T02:14:11Z","lastAt":"2011-05-24T02:44:08Z","messageCount":15,"participants":["Martin von Zweigbergk","Peter Baumann","Paul Mackerras","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"165151","messageId":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":null,"subject":"[PATCH 0/8] make gitk work better in non-top-level directory","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:11Z","receivedAt":"2011-04-05T02:14:11Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"This series fixes a few different bugs in gitk related to its working\ndirectory.\n\nI started working on patch 1, which fixes \"Highlight this only/too\"\nwhen gitk is started in a subdirectory. This problem has bothered me\nfor a long time, but I had heard that the Tcl code in gitk was hard to\nmaintain, so I didn't have a look at it until now. I have to say that\nit was a lot easier to follow the code than I had feared.\n\nWhile testing that the fix in patch 1 worked, I found that gitk does\nnot work very well when the work tree is not at \".git/..\", so most of\nthe other patches try to improve that situation.\n\nI think I have tested most combinations of setups (top-level dir,\nsubdir, separate work tree, bare repo, .git) and operations (highlight\nfile, blame, external diff, show origin of line).\n\n\nMartin von Zweigbergk (8):\n  gitk: fix file highlight when run in subdirectory\n  gitk: fix \"show origin of this line\" with separate work tree\n  gitk: fix \"blame parent commit\" with separate work tree\n  gitk: fix \"External diff\" with separate work tree\n  gitk: put temporary directory inside .git\n  gitk: run 'git rev-parse --git-dir' only once\n  gitk: simplify calculation of gitdir\n  gitk: show modified files with separate work tree\n\n gitk-git/gitk |   59 +++++++++++++++++++++++++++++++-------------------------\n 1 files changed, 33 insertions(+), 26 deletions(-)\n\n-- \n1.7.4.79.gcbe20\n"},{"id":"165152","messageId":"1301969659-19703-2-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 1/8] gitk: fix file highlight when run in subdirectory","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:12Z","receivedAt":"2011-04-05T02:14:12Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"The \"highlight this only\" and \"highlight this too\" commands in gitk\nadd the path relative to $GIT_WORK_TREE to the \"Find\" input box. When\nthe search (using git-diff-tree) is run, the paths are used\nunmodified, except for some shell escaping. Since the search is run\nfrom gitk's working directory, no commits matching the paths will be\nfound if gitk was started in a subdirectory.\n\nMake the paths passed to git-diff-tree relative to gitk's working\ndirectory instead of being relative to $GIT_WORK_TREE. If, however,\ngitk is run outside of the working directory (e.g. with $GIT_WORK_TREE\nset), we still need to use the path relative to $GIT_WORK_TREE.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n\nThis could also have been fixed by cd-ing to the work tree\ndirectory. That would also make the \"Local changes checked in to index\nbut not committed\" and \"Local uncommitted changes, not checked in to\nindex\" show up properly when running with GIT_WORK_TREE defined.\n\nI wasn't sure if other parts of gitk depend on the working directory,\nor if there are plans to make something depend on it, so I thought\nchanging it only for the specific case of file highlighting would be\nsafer. What do you think?\n\n\n gitk-git/gitk |   11 ++++++++++-\n 1 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex e82c6bf..ce96294 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -4528,12 +4528,17 @@ proc makepatterns {l} {\n \n proc do_file_hl {serial} {\n     global highlight_files filehighlight highlight_paths gdttype fhl_list\n+    global cdup\n \n     if {$gdttype eq [mc \"touching paths:\"]} {\n \tif {[catch {set paths [shellsplit $highlight_files]}]} return\n \tset highlight_paths [makepatterns $paths]\n \thighlight_filelist\n-\tset gdtargs [concat -- $paths]\n+\tset relative_paths {}\n+\tforeach path $paths {\n+\t    lappend relative_paths [file join $cdup $path]\n+\t}\n+\tset gdtargs [concat -- $relative_paths]\n     } elseif {$gdttype eq [mc \"adding/removing string:\"]} {\n \tset gdtargs [list \"-S$highlight_files\"]\n     } else {\n@@ -11625,6 +11630,10 @@ set stuffsaved 0\n set patchnum 0\n set lserial 0\n set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == \"true\"}]\n+set cdup {}\n+if {$isworktree} {\n+    set cdup [exec git rev-parse --show-cdup]\n+}\n setcoords\n makewindow\n catch {\n-- \n1.7.4.79.gcbe20\n"},{"id":"165158","messageId":"1301969659-19703-3-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 2/8] gitk: fix \"show origin of this line\" with separate work tree","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:13Z","receivedAt":"2011-04-05T02:14:13Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Running \"show origin of this line\" currently fails when the the work\ntree is not the parent of the git directory. Fix it by feeding\ngit-blame paths relative to $GIT_WORK_TREE instead of \"$GIT_DIR/..\".\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n gitk-git/gitk |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex ce96294..a0f0f37 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -3590,7 +3590,7 @@ proc external_blame {parent_idx {line {}}} {\n proc show_line_source {} {\n     global cmitmode currentid parents curview blamestuff blameinst\n     global diff_menu_line diff_menu_filebase flist_menu_file\n-    global nullid nullid2 gitdir\n+    global nullid nullid2 gitdir cdup\n \n     set from_index {}\n     if {$cmitmode eq \"tree\"} {\n@@ -3643,7 +3643,7 @@ proc show_line_source {} {\n     } else {\n \tlappend blameargs $id\n     }\n-    lappend blameargs -- [file join [file dirname $gitdir] $flist_menu_file]\n+    lappend blameargs -- [file join $cdup $flist_menu_file]\n     if {[catch {\n \tset f [open $blameargs r]\n     } err]} {\n-- \n1.7.4.79.gcbe20\n"},{"id":"165155","messageId":"1301969659-19703-4-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 3/8] gitk: fix \"blame parent commit\" with separate work tree","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:14Z","receivedAt":"2011-04-05T02:14:14Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Running \"blame parent commit\" currently brings up an empty blame view\nwhen the the work tree is not the parent of the git directory. Fix it\nby feeding git-blame paths relative to $GIT_WORK_TREE instead of\n\"$GIT_DIR/..\".\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n gitk-git/gitk |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex a0f0f37..b1696de 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -3558,7 +3558,7 @@ proc make_relative {f} {\n }\n \n proc external_blame {parent_idx {line {}}} {\n-    global flist_menu_file gitdir\n+    global flist_menu_file cdup\n     global nullid nullid2\n     global parentlist selectedline currentid\n \n@@ -3577,7 +3577,7 @@ proc external_blame {parent_idx {line {}}} {\n     if {$line ne {} && $line > 1} {\n \tlappend cmdline \"--line=$line\"\n     }\n-    set f [file join [file dirname $gitdir] $flist_menu_file]\n+    set f [file join $cdup $flist_menu_file]\n     # Unfortunately it seems git gui blame doesn't like\n     # being given an absolute path...\n     set f [make_relative $f]\n-- \n1.7.4.79.gcbe20\n"},{"id":"165154","messageId":"1301969659-19703-5-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 4/8] gitk: fix \"External diff\" with separate work tree","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:15Z","receivedAt":"2011-04-05T02:14:15Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Running \"External diff\" to compare the index and work tree currently\nbrings up an empty blame view when the work tree is not the parent of\nthe git directory. This is because the file that is taken from the\nwork tree is assumed to be in\n$GIT_DIR/../<repo-relative-file-name>. Fix it by feeding the diff tool\na path under $GIT_WORK_TREE instead of \"$GIT_DIR/..\".\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n gitk-git/gitk |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex b1696de..58b98df 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -3365,10 +3365,10 @@ proc save_file_from_commit {filename output what} {\n \n proc external_diff_get_one_file {diffid filename diffdir} {\n     global nullid nullid2 nullfile\n-    global gitdir\n+    global worktree\n \n     if {$diffid == $nullid} {\n-        set difffile [file join [file dirname $gitdir] $filename]\n+        set difffile [file join $worktree $filename]\n \tif {[file exists $difffile]} {\n \t    return $difffile\n \t}\n@@ -11634,6 +11634,7 @@ set cdup {}\n if {$isworktree} {\n     set cdup [exec git rev-parse --show-cdup]\n }\n+set worktree [exec git rev-parse --show-toplevel]\n setcoords\n makewindow\n catch {\n-- \n1.7.4.79.gcbe20\n"},{"id":"165156","messageId":"1301969659-19703-6-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 5/8] gitk: put temporary directory inside .git","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:16Z","receivedAt":"2011-04-05T02:14:16Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"When running \"External diff\" from gitk, the \"from\" and \"to\" files will\nfirst be copied into a directory that is currently\n\".git/../.gitk-tmp.$pid\". When gitk is closed, the directory is\ndeleted. When the work tree is not at \".git/..\" (which is supported\nsince the previous commit), that directory may not even be git-related\nand it does not seem unlikely that permissions may not allow the\ntemporary directory to be created there. Move the directory inside\n.git instead.\n\nThis patch introduces a regression in the case that the .git directory\nis readonly, but .git/.. is writeable.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n gitk-git/gitk |    3 +--\n 1 files changed, 1 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 58b98df..b925f3e 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -3332,8 +3332,7 @@ proc gitknewtmpdir {} {\n     global diffnum gitktmpdir gitdir\n \n     if {![info exists gitktmpdir]} {\n-\tset gitktmpdir [file join [file dirname $gitdir] \\\n-\t\t\t    [format \".gitk-tmp.%s\" [pid]]]\n+\tset gitktmpdir [file join $gitdir [format \".gitk-tmp.%s\" [pid]]]\n \tif {[catch {file mkdir $gitktmpdir} err]} {\n \t    error_popup \"[mc \"Error creating temporary directory %s:\" $gitktmpdir] $err\"\n \t    unset gitktmpdir\n-- \n1.7.4.79.gcbe20\n"},{"id":"165159","messageId":"1301969659-19703-7-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 6/8] gitk: run 'git rev-parse --git-dir' only once","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:17Z","receivedAt":"2011-04-05T02:14:17Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"It seems like gitk has been setting the global variable 'gitdir' at\nstartup since aa81d97 (gitk: Fix Update menu item, 2006-02-28). It\nshould therefore no longer be necessary to call the procedure with the\nsame name (more than once to set the global variable). Remove the\nother call sites and use the global variable instead.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n gitk-git/gitk |    6 ++++--\n 1 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex b925f3e..0c1c4df 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -9045,6 +9045,7 @@ proc exec_citool {tool_args {baseid {}}} {\n proc cherrypick {} {\n     global rowmenuid curview\n     global mainhead mainheadid\n+    global gitdir\n \n     set oldhead [exec git rev-parse HEAD]\n     set dheads [descheads $rowmenuid]\n@@ -9073,7 +9074,7 @@ proc cherrypick {} {\n \t\t\tconflict.\\nDo you wish to run git citool to\\\n \t\t\tresolve it?\"]]} {\n \t\t# Force citool to read MERGE_MSG\n-\t\tfile delete [file join [gitdir] \"GITGUI_MSG\"]\n+\t\tfile delete [file join $gitdir \"GITGUI_MSG\"]\n \t\texec_citool {} $rowmenuid\n \t    }\n \t} else {\n@@ -9439,6 +9440,7 @@ proc refill_reflist {} {\n proc getallcommits {} {\n     global allcommits nextarc seeds allccache allcwait cachedarcs allcupdate\n     global idheads idtags idotherrefs allparents tagobjid\n+    global gitdir\n \n     if {![info exists allcommits]} {\n \tset nextarc 0\n@@ -9446,7 +9448,7 @@ proc getallcommits {} {\n \tset seeds {}\n \tset allcwait 0\n \tset cachedarcs 0\n-\tset allccache [file join [gitdir] \"gitk.cache\"]\n+\tset allccache [file join $gitdir \"gitk.cache\"]\n \tif {![catch {\n \t    set f [open $allccache r]\n \t    set allcwait 1\n-- \n1.7.4.79.gcbe20\n"},{"id":"165157","messageId":"1301969659-19703-8-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 7/8] gitk: simplify calculation of gitdir","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:18Z","receivedAt":"2011-04-05T02:14:18Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Since 5024baa ([PATCH] Make gitk work when launched in a subdirectory,\n2007-01-09), gitk has used 'git rev-parse --git-dir' to find the .git\ndirectory. However, gitk still first checks for the $GIT_DIR\nenvironment variable and that the value returned from git-rev-parse\ndoes not point to a file. Since git-rev-parse does both of these\nchecks already, the checks can safely be removed from gitk. This makes\nthe gitdir procedure small enough to inline.\n\nThis cleanup introduces a UI regression in that the error message will\nnow be \"Cannot find a git repository here.\" even in the case where\nGIT_DIR points to a file, for which the error message was previously\n\"Cannot find the git directory \\\"%s\\\".\". It should be noted, though,\nthat even before this patch, 'gitk --git-dir=path/to/some/file' would\ngive the former error message.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n gitk-git/gitk |   15 +--------------\n 1 files changed, 1 insertions(+), 14 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 0c1c4df..232ea6e 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -9,15 +9,6 @@ exec wish \"$0\" -- \"$@\"\n \n package require Tk\n \n-proc gitdir {} {\n-    global env\n-    if {[info exists env(GIT_DIR)]} {\n-\treturn $env(GIT_DIR)\n-    } else {\n-\treturn [exec git rev-parse --git-dir]\n-    }\n-}\n-\n # A simple scheduler for compute-intensive stuff.\n # The aim is to make sure that event handlers for GUI actions can\n # run at least every 50-100 ms.  Unfortunately fileevent handlers are\n@@ -11507,14 +11498,10 @@ setui $uicolor\n setoptions\n \n # check that we can find a .git directory somewhere...\n-if {[catch {set gitdir [gitdir]}]} {\n+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {\n     show_error {} . [mc \"Cannot find a git repository here.\"]\n     exit 1\n }\n-if {![file isdirectory $gitdir]} {\n-    show_error {} . [mc \"Cannot find the git directory \\\"%s\\\".\" $gitdir]\n-    exit 1\n-}\n \n set selecthead {}\n set selectheadid {}\n-- \n1.7.4.79.gcbe20\n"},{"id":"165153","messageId":"1301969659-19703-9-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH 8/8] gitk: show modified files with separate work tree","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-05T02:14:19Z","receivedAt":"2011-04-05T02:14:19Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\"git rev-parse --is-inside-work-tree\" is currently used to determine\nwhether to show modified files in gitk (the red and green fake\ncommits). This does not work if the current directory is not inside\nthe work tree, as can be the case e.g. if GIT_WORK_TREE is\nset. Instead, check if the repository is not bare and that we are not\ninside the .git directory.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n\n---\nIs the test in proc hasworktree good?\n\nWhy do git commands that need a work tree not work under .git? Why\ndon't they show the same output as if they had been run from the work\ntree? (Btw, the check for valid work tree does not work for aliases,\nso e.g. 'git st', with 'st' as alias for 'status' will show all files\nas deleted.)\n\nHow do I simplify the Tcl code to just return the boolean right away?\n\nWhy is the hasworktree variable reset in updatecommits? The only reason\nI can think of is when 'core.worktree' is set/changed, but I don't\nthink that case worked very well before this series anyway. Should\ngitdir also be recalculated?\n\n gitk-git/gitk |   21 +++++++++++++++------\n 1 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 232ea6e..914de8d 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -9,6 +9,15 @@ exec wish \"$0\" -- \"$@\"\n \n package require Tk\n \n+proc hasworktree {} {\n+    if {[expr {[exec git rev-parse --is-bare-repository] == \"false\"}] &&\n+\t[expr {[exec git rev-parse --is-inside-git-dir] == \"false\"}]} {\n+\treturn 1\n+    } else {\n+\treturn 0\n+    }\n+}\n+\n # A simple scheduler for compute-intensive stuff.\n # The aim is to make sure that event handlers for GUI actions can\n # run at least every 50-100 ms.  Unfortunately fileevent handlers are\n@@ -459,11 +468,11 @@ proc updatecommits {} {\n     global viewactive viewcomplete tclencoding\n     global startmsecs showneartags showlocalchanges\n     global mainheadid viewmainheadid viewmainheadid_orig pending_select\n-    global isworktree\n+    global hasworktree\n     global varcid vposids vnegids vflags vrevs\n     global show_notes\n \n-    set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == \"true\"}]\n+    set hasworktree [hasworktree]\n     rereadrefs\n     set view $curview\n     if {$mainheadid ne $viewmainheadid_orig($view)} {\n@@ -5025,9 +5034,9 @@ proc dohidelocalchanges {} {\n # spawn off a process to do git diff-index --cached HEAD\n proc dodiffindex {} {\n     global lserial showlocalchanges vfilelimit curview\n-    global isworktree\n+    global hasworktree\n \n-    if {!$showlocalchanges || !$isworktree} return\n+    if {!$showlocalchanges || !$hasworktree} return\n     incr lserial\n     set cmd \"|git diff-index --cached HEAD\"\n     if {$vfilelimit($curview) ne {}} {\n@@ -11617,9 +11626,9 @@ set stopped 0\n set stuffsaved 0\n set patchnum 0\n set lserial 0\n-set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == \"true\"}]\n+set hasworktree [hasworktree]\n set cdup {}\n-if {$isworktree} {\n+if {[expr {[exec git rev-parse --is-inside-work-tree] == \"true\"}]} {\n     set cdup [exec git rev-parse --show-cdup]\n }\n set worktree [exec git rev-parse --show-toplevel]\n-- \n1.7.4.79.gcbe20\n"},{"id":"165199","messageId":"20110405184803.GA19515@m62s10.vlinux.de","threadId":"26992","inReplyTo":"1301969659-19703-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH 0/8] make gitk work better in non-top-level directory","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2011-04-05T18:48:03Z","receivedAt":"2011-04-05T18:48:03Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Mon, Apr 04, 2011 at 10:14:11PM -0400, Martin von Zweigbergk wrote:\n> This series fixes a few different bugs in gitk related to its working\n> directory.\n> \n> I started working on patch 1, which fixes \"Highlight this only/too\"\n> when gitk is started in a subdirectory. This problem has bothered me\n> for a long time, but I had heard that the Tcl code in gitk was hard to\n> maintain, so I didn't have a look at it until now. I have to say that\n> it was a lot easier to follow the code than I had feared.\n> \n> While testing that the fix in patch 1 worked, I found that gitk does\n> not work very well when the work tree is not at \".git/..\", so most of\n> the other patches try to improve that situation.\n> \n> I think I have tested most combinations of setups (top-level dir,\n> subdir, separate work tree, bare repo, .git) and operations (highlight\n> file, blame, external diff, show origin of line).\n> \n\nThis is something which anoyed me a long time. I even tried to fix\nit [1], but with my limited Tcl knowledge I failed misserably.\n\nYour patch series seems to fix all the problems I was having.\nNice job!\n\n-Peter\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/120391\n"},{"id":"165527","messageId":"20110410015410.GA25368@brick.ozlabs.ibm.com","threadId":"26992","inReplyTo":"1301969659-19703-2-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH 1/8] gitk: fix file highlight when run in subdirectory","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2011-04-10T01:54:10Z","receivedAt":"2011-04-10T01:54:10Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Mon, Apr 04, 2011 at 10:14:12PM -0400, Martin von Zweigbergk wrote:\n\n> The \"highlight this only\" and \"highlight this too\" commands in gitk\n> add the path relative to $GIT_WORK_TREE to the \"Find\" input box. When\n> the search (using git-diff-tree) is run, the paths are used\n> unmodified, except for some shell escaping. Since the search is run\n> from gitk's working directory, no commits matching the paths will be\n> found if gitk was started in a subdirectory.\n> \n> Make the paths passed to git-diff-tree relative to gitk's working\n> directory instead of being relative to $GIT_WORK_TREE. If, however,\n> gitk is run outside of the working directory (e.g. with $GIT_WORK_TREE\n> set), we still need to use the path relative to $GIT_WORK_TREE.\n> \n> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n> ---\n> \n> This could also have been fixed by cd-ing to the work tree\n> directory. That would also make the \"Local changes checked in to index\n> but not committed\" and \"Local uncommitted changes, not checked in to\n> index\" show up properly when running with GIT_WORK_TREE defined.\n> \n> I wasn't sure if other parts of gitk depend on the working directory,\n> or if there are plans to make something depend on it, so I thought\n> changing it only for the specific case of file highlighting would be\n> safer. What do you think?\n\nI have to admit I wasn't aware of GIT_WORK_TREE before I saw your\npatches.  The patches look OK, but I wonder how many of the problems\nwould go away if gitk were simply to set GIT_WORK_TREE in the\nenvironment for the programs it runs, if it is not already set.\nSomething like this (untested):\n\n # check that we can find a .git directory somewhere...\n if {[catch {set gitdir [gitdir]}]} {\n     show_error {} . [mc \"Cannot find a git repository here.\"]\n     exit 1\n }\n if {![file isdirectory $gitdir]} {\n     show_error {} . [mc \"Cannot find the git directory \\\"%s\\\".\" $gitdir]\n     exit 1\n }\n+if {![info exists env(GIT_WORK_TREE)]} {\n+    set worktree [file dirname $gitdir]\n+    if {$worktree ne \".\"} {\n+\tset env(GIT_WORK_TREE) $worktree\n+    }\n+}\n\nPaul.\n"},{"id":"165526","messageId":"20110410020318.GB25368@brick.ozlabs.ibm.com","threadId":"26992","inReplyTo":"1301969659-19703-9-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH 8/8] gitk: show modified files with separate work tree","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2011-04-10T02:03:18Z","receivedAt":"2011-04-10T02:03:18Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Mon, Apr 04, 2011 at 10:14:19PM -0400, Martin von Zweigbergk wrote:\n\n> Is the test in proc hasworktree good?\n\nThe first parameter to 'if' is evaluated as an expression, so you\ndon't need the extra exprs.\n\n> Why do git commands that need a work tree not work under .git? Why\n> don't they show the same output as if they had been run from the work\n> tree? (Btw, the check for valid work tree does not work for aliases,\n> so e.g. 'git st', with 'st' as alias for 'status' will show all files\n> as deleted.)\n\nDon't know, ask Junio. :)\n\n> How do I simplify the Tcl code to just return the boolean right away?\n\nYou can do:\n\n    return [expr {[exec git rev-parse --is-bare-repository] == \"false\" &&\n\t\t  [exec git rev-parse --is-inside-git-dir] == \"false\"}]\n\n> Why is the hasworktree variable reset in updatecommits? The only reason\n> I can think of is when 'core.worktree' is set/changed, but I don't\n> think that case worked very well before this series anyway. Should\n> gitdir also be recalculated?\n\nI don't know that there's any particularly strong reason to do it in\nupdatecommits.  It could probably be done once at startup.\n\nPaul.\n"},{"id":"165562","messageId":"BANLkTinZCCMQbQBF-j3bq3Wfxm2YRksCYQ@mail.gmail.com","threadId":"26992","inReplyTo":"20110410015410.GA25368@brick.ozlabs.ibm.com","subject":"Re: [PATCH 1/8] gitk: fix file highlight when run in subdirectory","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-04-10T18:03:34Z","receivedAt":"2011-04-10T18:03:34Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Hi Paul,\n\nThanks for your input. I will work on a re-roll of this series based\non, at least,\nyour input on patch 8/8. However, I am going away for 4 weeks today and I will\nprobably not be able to do that until some time after I come back. If\nanyone else\ncares to do it before then, please go ahead.\n\nOn Sat, Apr 9, 2011 at 9:54 PM, Paul Mackerras <paulus@samba.org> wrote:\n> I have to admit I wasn't aware of GIT_WORK_TREE before I saw your\n> patches.  The patches look OK, but I wonder how many of the problems\n> would go away if gitk were simply to set GIT_WORK_TREE in the\n> environment for the programs it runs, if it is not already set.\n\nMost of the problems I have tried to fix in this series only appear when\nGIT_WORK_TREE _is_ set, so I don't think setting it would help. When\nit it not set,\nsetting it to the directory that git detected should have no effect as\nfar as I can see.\n\n\n/Martin\n"},{"id":"165642","messageId":"7vipukwqf2.fsf@alter.siamese.dyndns.org","threadId":"26992","inReplyTo":"20110410020318.GB25368@brick.ozlabs.ibm.com","subject":"Re: [PATCH 8/8] gitk: show modified files with separate work tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-11T19:15:13Z","receivedAt":"2011-04-11T19:15:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Mackerras <paulus@samba.org> writes:\n\n>> Why do git commands that need a work tree not work under .git?\n> ...\n> Don't know, ask Junio. :)\n\nWhatever the current behaviour is, I am reasonably sure that it is coming\nmore from \"meh -- who cares such a case?\" than \"it should work like this\nwhen you are in .git because of such and such reasons\".\n\nFor example, what does it mean to be able to do this?\n\n\t$ edit Makefile\n        $ git add Makefile\n        $ edit Makefile\n\t$ cd .git\n        $ git grep frotz Makefile\n\nPerhaps the last step needs to be\n\n\t$ git grep frotz ../Makefile\n\ninstead, but a more important point is, how would that be useful?\n\nIf you have both GIT_DIR and GIT_WORK_TREE set up to point at the correct\nplaces, I think it is sensible to make the above (the \"../Makefile\"\nversion, not the one without dot-dot) work as expected.\n\nI suspect (but would not bother to dig the history myself to find out)\nthat \"we require a working tree\" semantics that in fact often means \"we\nrequire you to be in the working tree\" was a misdesign that did not matter\nthat came from the days back when GIT_WORK_TREE was not either present or\nnot widely used.  Now more people seem to be using GIT_WORK_TREE for some\nreason, I don't have anything against a patch series that defines and\nimplements a more desirable behaviour clearly.\n\nThanks.\n"},{"id":"168567","messageId":"1306205048-9747-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"26992","inReplyTo":"1301969659-19703-9-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH v2 8/8] gitk: show modified files with separate work tree","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2011-05-24T02:44:08Z","receivedAt":"2011-05-24T02:44:08Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"\"git rev-parse --is-inside-work-tree\" is currently used to determine\nwhether to show modified files in gitk (the red and green fake\ncommits). This does not work if the current directory is not inside\nthe work tree, as can be the case e.g. if GIT_WORK_TREE is\nset. Instead, check if the repository is not bare and that we are not\ninside the .git directory.\n\nSigned-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>\n---\n\nThe only change since v1 is that the hasworktree procedure has been\nsimplified. I was conservative and left the recalculatation of\nhasworktree in updatecommits(). There are no changes to any of the\nother patches, so I didn't bother resending them. Sorry about the long\ndelay for such a trivial fixup.\n\n\n gitk-git/gitk |   17 +++++++++++------\n 1 files changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 10b2bca..01b63e5 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -9,6 +9,11 @@ exec wish \"$0\" -- \"$@\"\n \n package require Tk\n \n+proc hasworktree {} {\n+    return [expr {[exec git rev-parse --is-bare-repository] == \"false\" &&\n+\t\t  [exec git rev-parse --is-inside-git-dir] == \"false\"}]\n+}\n+\n # A simple scheduler for compute-intensive stuff.\n # The aim is to make sure that event handlers for GUI actions can\n # run at least every 50-100 ms.  Unfortunately fileevent handlers are\n@@ -459,11 +464,11 @@ proc updatecommits {} {\n     global viewactive viewcomplete tclencoding\n     global startmsecs showneartags showlocalchanges\n     global mainheadid viewmainheadid viewmainheadid_orig pending_select\n-    global isworktree\n+    global hasworktree\n     global varcid vposids vnegids vflags vrevs\n     global show_notes\n \n-    set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == \"true\"}]\n+    set hasworktree [hasworktree]\n     rereadrefs\n     set view $curview\n     if {$mainheadid ne $viewmainheadid_orig($view)} {\n@@ -5026,9 +5031,9 @@ proc dohidelocalchanges {} {\n # spawn off a process to do git diff-index --cached HEAD\n proc dodiffindex {} {\n     global lserial showlocalchanges vfilelimit curview\n-    global isworktree\n+    global hasworktree\n \n-    if {!$showlocalchanges || !$isworktree} return\n+    if {!$showlocalchanges || !$hasworktree} return\n     incr lserial\n     set cmd \"|git diff-index --cached HEAD\"\n     if {$vfilelimit($curview) ne {}} {\n@@ -11621,9 +11626,9 @@ set stopped 0\n set stuffsaved 0\n set patchnum 0\n set lserial 0\n-set isworktree [expr {[exec git rev-parse --is-inside-work-tree] == \"true\"}]\n+set hasworktree [hasworktree]\n set cdup {}\n-if {$isworktree} {\n+if {[expr {[exec git rev-parse --is-inside-work-tree] == \"true\"}]} {\n     set cdup [exec git rev-parse --show-cdup]\n }\n set worktree [exec git rev-parse --show-toplevel]\n-- \n1.7.4.79.gcbe20\n"}]}