{"thread":{"id":"17666","subject":"[PATCH 1/3] git-gui: properly check for a bare repo","startedAt":"2009-02-09T02:00:47Z","lastAt":"2009-02-18T21:53:12Z","messageCount":9,"participants":["Giuseppe Bilotta","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"103804","messageId":"1234144850-2903-1-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17666","inReplyTo":null,"subject":"[PATCH 0/3] git-gui: generic and robust worktree/gitdir support","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-09T02:00:47Z","receivedAt":"2009-02-09T02:00:47Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"A-hem.\n\n<intro class=\"shameless plug\">\nI've recently restarted using (and thus working on) my Zit project, the\nGit-based single-file content tracker ( http://git.oblomov.eu/zit ),\nadding support for importing RCS history (via my rcs-fast-export\nproject, http://git.oblomov.eu/rcs-fast-export ), and even managed to\nrun a successful rebase (which turned out to be somewhat clumsier due to\nthe inevitably longer command lines, but perfectly feasible). My next\nstep was looking for ways to support the graphical utilities, in\nparticular git-gui and gitk\n</intro>\n\nIt turns out that while gitk works pretty fine out of the box, git-gui\nmakes a lot of guesses and assumptions about the nature and relation\nbetween the git dir and its worktree. The following three patches try to\nremedy the problems I came across with some simple testing.\n\nGiuseppe Bilotta (3):\n  git-gui: properly check for a bare repo\n  git-gui: use the actual worktree\n  git-gui: define correct GIT_DIR for gitk\n"},{"id":"103803","messageId":"1234144850-2903-2-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17666","inReplyTo":"1234144850-2903-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 1/3] git-gui: properly check for a bare repo","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-09T02:00:48Z","receivedAt":"2009-02-09T02:00:48Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"When bare repository handling is not enabled, check for a bare\nrepository looking at the core.bare config option rather than refusing\nto operate with a git directory ending with .git.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\nI know I should have probably used something like\ngit rev-parse --is-bare-repository instead, but I didn't feel like\nadding another git call. Is the config approach robust enough?\n\n git-gui/git-gui.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex e018e07..658a728 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -1071,7 +1071,7 @@ if {$_prefix ne {}} {\n \t}\n \tunset cdup\n } elseif {![is_enabled bare]} {\n-\tif {[lindex [file split $_gitdir] end] ne {.git}} {\n+\tif {[is_config_true core.bare]} {\n \t\tcatch {wm withdraw .}\n \t\terror_popup [strcat [mc \"Cannot use funny .git directory:\"] \"\\n\\n$_gitdir\"]\n \t\texit 1\n-- \n1.6.2.rc0.173.g5e148\n"},{"id":"103805","messageId":"1234144850-2903-3-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17666","inReplyTo":"1234144850-2903-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 2/3] git-gui: use the actual worktree","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-09T02:00:49Z","receivedAt":"2009-02-09T02:00:49Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Don't rely on the git worktree being the updir of the gitdir, since it\nmight not be. Instead, define (and use) a new _gitworktree global\nvariable, setting it to $GIT_WORK_TREE if present, or to whatever we\nguess the correct worktree is.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n git-gui/git-gui.sh |   25 +++++++++++++++++++------\n 1 files changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 658a728..94317c7 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -121,6 +121,7 @@ unset oguimsg\n \n set _appname {Git Gui}\n set _gitdir {}\n+set _gitworktree {}\n set _gitexec {}\n set _reponame {}\n set _iscygwin {}\n@@ -1062,13 +1063,19 @@ if {![file isdirectory $_gitdir]} {\n \terror_popup [strcat [mc \"Git directory not found:\"] \"\\n\\n$_gitdir\"]\n \texit 1\n }\n+set _gitworktree $env(GIT_WORK_TREE)\n if {$_prefix ne {}} {\n-\tregsub -all {[^/]+/} $_prefix ../ cdup\n+\tif {$_gitworktree eq {}} {\n+\t\tregsub -all {[^/]+/} $_prefix ../ cdup\n+\t} else {\n+\t\tset cdup $_gitworktree\n+\t}\n \tif {[catch {cd $cdup} err]} {\n \t\tcatch {wm withdraw .}\n \t\terror_popup [strcat [mc \"Cannot move to top of working directory:\"] \"\\n\\n$err\"]\n \t\texit 1\n \t}\n+\tset _gitworktree [pwd]\n \tunset cdup\n } elseif {![is_enabled bare]} {\n \tif {[is_config_true core.bare]} {\n@@ -1076,11 +1083,15 @@ if {$_prefix ne {}} {\n \t\terror_popup [strcat [mc \"Cannot use funny .git directory:\"] \"\\n\\n$_gitdir\"]\n \t\texit 1\n \t}\n-\tif {[catch {cd [file dirname $_gitdir]} err]} {\n+\tif {$_gitworktree eq {}} {\n+\t\tset _gitworktree [file dirname $_gitdir]\n+\t}\n+\tif {[catch {cd $_gitworktree} err]} {\n \t\tcatch {wm withdraw .}\n-\t\terror_popup [strcat [mc \"No working directory\"] \" [file dirname $_gitdir]:\\n\\n$err\"]\n+\t\terror_popup [strcat [mc \"No working directory\"] \" $_gitworktree:\\n\\n$err\"]\n \t\texit 1\n \t}\n+\tset _gitworktree [pwd]\n }\n set _reponame [file split [file normalize $_gitdir]]\n if {[lindex $_reponame end] eq {.git}} {\n@@ -1878,6 +1889,7 @@ proc incr_font_size {font {amt 1}} {\n set starting_gitk_msg [mc \"Starting gitk... please wait...\"]\n \n proc do_gitk {revs} {\n+\tglobal _gitworktree\n \t# -- Always start gitk through whatever we were loaded with.  This\n \t#    lets us bypass using shell process on Windows systems.\n \t#\n@@ -1895,7 +1907,7 @@ proc do_gitk {revs} {\n \t\t}\n \n \t\tset pwd [pwd]\n-\t\tcd [file dirname [gitdir]]\n+\t\tcd $_gitworktree\n \t\tset env(GIT_DIR) [file tail [gitdir]]\n \n \t\teval exec $cmd $revs &\n@@ -1915,6 +1927,7 @@ proc do_gitk {revs} {\n }\n \n proc do_explore {} {\n+\tglobal _gitworktree\n \tset explorer {}\n \tif {[is_Cygwin] || [is_Windows]} {\n \t\tset explorer \"explorer.exe\"\n@@ -1924,7 +1937,7 @@ proc do_explore {} {\n \t\t# freedesktop.org-conforming system is our best shot\n \t\tset explorer \"xdg-open\"\n \t}\n-\teval exec $explorer [file dirname [gitdir]] &\n+\teval exec $explorer $_gitworktree &\n }\n \n set is_quitting 0\n@@ -3302,7 +3315,7 @@ unset i\n set file_lists($ui_index) [list]\n set file_lists($ui_workdir) [list]\n \n-wm title . \"[appname] ([reponame]) [file normalize [file dirname [gitdir]]]\"\n+wm title . \"[appname] ([reponame]) [file normalize $_gitworktree]\"\n focus -force $ui_comm\n \n # -- Warn the user about environmental problems.  Cygwin's Tcl\n-- \n1.6.2.rc0.173.g5e148\n"},{"id":"103806","messageId":"1234144850-2903-4-git-send-email-giuseppe.bilotta@gmail.com","threadId":"17666","inReplyTo":"1234144850-2903-1-git-send-email-giuseppe.bilotta@gmail.com","subject":"[PATCH 3/3] git-gui: define correct GIT_DIR for gitk","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-09T02:00:50Z","receivedAt":"2009-02-09T02:00:50Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"When invoking gitk, git-gui assumed that the worktree was the dirname of\n$_gitdir and that, by consequence, the git dir could be set to the tail of\n$_gitdir once we changed to the worktree root directory.\n\nThe assumption is no longer true since the previous commit introduced\nsupport for more generic worktree locations, so export a GIT_DIR\nenvironment variable set to the full, normalized path of $_gitdir\ninstead.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n git-gui/git-gui.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\nindex 94317c7..de3f29c 100755\n--- a/git-gui/git-gui.sh\n+++ b/git-gui/git-gui.sh\n@@ -1908,7 +1908,7 @@ proc do_gitk {revs} {\n \n \t\tset pwd [pwd]\n \t\tcd $_gitworktree\n-\t\tset env(GIT_DIR) [file tail [gitdir]]\n+\t\tset env(GIT_DIR) [file normalize [gitdir]]\n \n \t\teval exec $cmd $revs &\n \n-- \n1.6.2.rc0.173.g5e148\n"},{"id":"105326","messageId":"20090218171639.GE22848@spearce.org","threadId":"17666","inReplyTo":"1234144850-2903-2-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 1/3] git-gui: properly check for a bare repo","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-18T17:16:39Z","receivedAt":"2009-02-18T17:16:39Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:\n> When bare repository handling is not enabled, check for a bare\n> repository looking at the core.bare config option rather than refusing\n> to operate with a git directory ending with .git.\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n> I know I should have probably used something like\n> git rev-parse --is-bare-repository instead, but I didn't feel like\n> adding another git call. Is the config approach robust enough?\n> \n>  git-gui/git-gui.sh |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index e018e07..658a728 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -1071,7 +1071,7 @@ if {$_prefix ne {}} {\n>  \t}\n>  \tunset cdup\n>  } elseif {![is_enabled bare]} {\n> -\tif {[lindex [file split $_gitdir] end] ne {.git}} {\n> +\tif {[is_config_true core.bare]} {\n\nThis doesn't work as you expect.  Its a chicken-and-egg problem.\nWe haven't read the config yet because we aren't sure that the\n$_gitdir really is a git directory.  Consequently, core.bare is\nalways false.\n\nHowever, yes, in modern versions of git the core.bare setting in\n.git/config is *usually* accurate.\n\nThe only time it isn't is when its not present (really old repository\nwhich hasn't been re-init'd in a long time), or when the user does\n\"mv .git ../foo.git\" to create a bare repository from a non-bare one.\n\n;-)\n\n-- \nShawn.\n"},{"id":"105327","messageId":"20090218172219.GF22848@spearce.org","threadId":"17666","inReplyTo":"1234144850-2903-3-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 2/3] git-gui: use the actual worktree","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-18T17:22:19Z","receivedAt":"2009-02-18T17:22:19Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:\n> Don't rely on the git worktree being the updir of the gitdir, since it\n> might not be. Instead, define (and use) a new _gitworktree global\n> variable, setting it to $GIT_WORK_TREE if present, or to whatever we\n> guess the correct worktree is.\n> \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n>  git-gui/git-gui.sh |   25 +++++++++++++++++++------\n>  1 files changed, 19 insertions(+), 6 deletions(-)\n> \n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 658a728..94317c7 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -1062,13 +1063,19 @@ if {![file isdirectory $_gitdir]} {\n>  \terror_popup [strcat [mc \"Git directory not found:\"] \"\\n\\n$_gitdir\"]\n>  \texit 1\n>  }\n> +set _gitworktree $env(GIT_WORK_TREE)\n\nIn TCL it is an error if an environment variable is undefined\nwhen accessed.  You need to wrap this up in a catch block to handle\nthe error:\n\nError in startup script: can't read \"env(GIT_WORK_TREE)\": no such variable\n    while executing\n\"set _gitworktree $env(GIT_WORK_TREE)\"\n    (file \"./git-gui.sh\" line 1066)\n\nAlso, what about honoring core.worktree as a setting, in\naddition to $env(GIT_WORK_TREE) like the git wrapper does?\n\n-- \nShawn.\n"},{"id":"105328","messageId":"20090218172252.GG22848@spearce.org","threadId":"17666","inReplyTo":"1234144850-2903-4-git-send-email-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 3/3] git-gui: define correct GIT_DIR for gitk","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-18T17:22:52Z","receivedAt":"2009-02-18T17:22:52Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:\n> When invoking gitk, git-gui assumed that the worktree was the dirname of\n> $_gitdir and that, by consequence, the git dir could be set to the tail of\n> $_gitdir once we changed to the worktree root directory.\n> \n> The assumption is no longer true since the previous commit introduced\n> support for more generic worktree locations, so export a GIT_DIR\n> environment variable set to the full, normalized path of $_gitdir\n> instead.\n\nSince this is a one line patch to fix a bug introduced by the prior\npatch, I'd rather just see this squash into the prior patch.\n \n> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n> ---\n>  git-gui/git-gui.sh |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n> index 94317c7..de3f29c 100755\n> --- a/git-gui/git-gui.sh\n> +++ b/git-gui/git-gui.sh\n> @@ -1908,7 +1908,7 @@ proc do_gitk {revs} {\n>  \n>  \t\tset pwd [pwd]\n>  \t\tcd $_gitworktree\n> -\t\tset env(GIT_DIR) [file tail [gitdir]]\n> +\t\tset env(GIT_DIR) [file normalize [gitdir]]\n>  \n>  \t\teval exec $cmd $revs &\n>  \n> -- \n> 1.6.2.rc0.173.g5e148\n> \n\n-- \nShawn.\n"},{"id":"105346","messageId":"cb7bb73a0902181347k6ae479b4v84e2ea8a9f6639dc@mail.gmail.com","threadId":"17666","inReplyTo":"20090218171639.GE22848@spearce.org","subject":"Re: [PATCH 1/3] git-gui: properly check for a bare repo","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2009-02-18T21:47:02Z","receivedAt":"2009-02-18T21:47:02Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Wed, Feb 18, 2009 at 6:16 PM, Shawn O. Pearce <spearce@spearce.org> wrote:\n> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:\n>> When bare repository handling is not enabled, check for a bare\n>> repository looking at the core.bare config option rather than refusing\n>> to operate with a git directory ending with .git.\n>>\n>> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n>> ---\n>> I know I should have probably used something like\n>> git rev-parse --is-bare-repository instead, but I didn't feel like\n>> adding another git call. Is the config approach robust enough?\n>>\n>>  git-gui/git-gui.sh |    2 +-\n>>  1 files changed, 1 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh\n>> index e018e07..658a728 100755\n>> --- a/git-gui/git-gui.sh\n>> +++ b/git-gui/git-gui.sh\n>> @@ -1071,7 +1071,7 @@ if {$_prefix ne {}} {\n>>       }\n>>       unset cdup\n>>  } elseif {![is_enabled bare]} {\n>> -     if {[lindex [file split $_gitdir] end] ne {.git}} {\n>> +     if {[is_config_true core.bare]} {\n>\n> This doesn't work as you expect.  Its a chicken-and-egg problem.\n> We haven't read the config yet because we aren't sure that the\n> $_gitdir really is a git directory.  Consequently, core.bare is\n> always false.\n\nIsn't the config loaded on line 1053?\n\nBetter ways to work around the issue?\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"105348","messageId":"20090218215312.GO22848@spearce.org","threadId":"17666","inReplyTo":"cb7bb73a0902181347k6ae479b4v84e2ea8a9f6639dc@mail.gmail.com","subject":"Re: [PATCH 1/3] git-gui: properly check for a bare repo","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-02-18T21:53:12Z","receivedAt":"2009-02-18T21:53:12Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:\n> On Wed, Feb 18, 2009 at 6:16 PM, Shawn O. Pearce <spearce@spearce.org> wrote:\n> >\n> > This doesn't work as you expect.  Its a chicken-and-egg problem.\n> > We haven't read the config yet because we aren't sure that the\n> > $_gitdir really is a git directory.  Consequently, core.bare is\n> > always false.\n> \n> Isn't the config loaded on line 1053?\n\nNo, that line is only run if there is no git repository here and\nwe are opening the repository selection wizard.  The load_config\nis to pull in the user's ~/.gitconfig.\n \n> Better ways to work around the issue?\n\nBreak down and run the fork command here.  We already ran two\ngit-rev-parse calls at 1049,1050.  One more here during startup\nisn't going to kill the world.\n\nThe only problem is, you need to watch out for the git version.\nAFAIK git-gui still runs on git 1.5.0.  Any option added since should\nbe used only if its known to exist, or if there is a fallback that\nworks almost as good.\n\nOr, we need to up our minimum version around line 842, and in the\nmessages above in 756, 787, 789, 791, and their translations...\n\n-- \nShawn.\n"}]}