[PATCHv4 0/5] git-gui: more robust handling of fancy repos

[PATCHv4 0/5] git-gui: more robust handling of fancy repos

[Giuseppe Bilotta]

A re-issue of the patchest to make git-gui more robust towards
non-standard repository setup, with two additional fixes.

The first three patches are unchanged from the previous iteration (still
waiting for review, actually).

The 4th sets GIT_DIR and GIT_WORK_TREE in git-gui to ensure that all
external tool invocation work in the same setup.

The last one also updates the shortcut library to make use of
_gitworktree instead of the old assumption that the worktree is the
updir of the git dir. I can't really test this patch though because
I don't have Windows.


Giuseppe Bilotta (5):
  git-gui: handle non-standard worktree locations
  git-gui: handle bare repos correctly
  git-gui: work from the .git dir
  git-gui: set GIT_DIR and GIT_WORK_TREE after setup
  git-gui: update shortcut tools to use _gitworktree

 git-gui/git-gui.sh       |  101 ++++++++++++++++++++++++++++++++++------------
 git-gui/lib/shortcut.tcl |    7 ++-
 2 files changed, 79 insertions(+), 29 deletions(-)

[PATCHv4 1/5] git-gui: handle non-standard worktree locations

[Giuseppe Bilotta]

Don't rely on the git worktree being the updir of the gitdir, since it
might not be. Instead, define (and use) a new _gitworktree global
variable, setting it to $GIT_WORK_TREE if present, falling back to
core.worktree if defined, and finally to whatever we guess the correct
worktree is. Getting core.worktree requires the config from the alleged
git dir _gitdir to be loaded early.

Supporting non-standard worktree locations also breaks the git-gui
assumption (made when calling gitk) that the worktree was the dirname of
$_gitdir and that, by consequence, the git dir could be set to the tail
of $_gitdir once we changed to the worktree root directory. Therefore,
we need to export a GIT_DIR environment variable set to the full,
normalized path of $_gitdir instead. We also skip changing to the worktree
directory if it's empty (i.e. if we're working on a bare repository).

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-gui/git-gui.sh |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 718277a..7e633f4 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -121,6 +121,7 @@ unset oguimsg
 
 set _appname {Git Gui}
 set _gitdir {}
+set _gitworktree {}
 set _gitexec {}
 set _githtmldir {}
 set _reponame {}
@@ -1090,13 +1091,25 @@ if {![file isdirectory $_gitdir]} {
 	error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
 	exit 1
 }
+# _gitdir exists, so try loading the config
+load_config 0
+apply_config
+# try to set work tree from environment, falling back to core.worktree
+if {[catch { set _gitworktree $env(GIT_WORK_TREE) }]} {
+	set _gitworktree [get_config core.worktree]
+}
 if {$_prefix ne {}} {
-	regsub -all {[^/]+/} $_prefix ../ cdup
+	if {$_gitworktree eq {}} {
+		regsub -all {[^/]+/} $_prefix ../ cdup
+	} else {
+		set cdup $_gitworktree
+	}
 	if {[catch {cd $cdup} err]} {
 		catch {wm withdraw .}
 		error_popup [strcat [mc "Cannot move to top of working directory:"] "\n\n$err"]
 		exit 1
 	}
+	set _gitworktree [pwd]
 	unset cdup
 } elseif {![is_enabled bare]} {
 	if {[lindex [file split $_gitdir] end] ne {.git}} {
@@ -1104,11 +1117,15 @@ if {$_prefix ne {}} {
 		error_popup [strcat [mc "Cannot use funny .git directory:"] "\n\n$_gitdir"]
 		exit 1
 	}
-	if {[catch {cd [file dirname $_gitdir]} err]} {
+	if {$_gitworktree eq {}} {
+		set _gitworktree [file dirname $_gitdir]
+	}
+	if {[catch {cd $_gitworktree} err]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "No working directory"] " [file dirname $_gitdir]:\n\n$err"]
+		error_popup [strcat [mc "No working directory"] " $_gitworktree:\n\n$err"]
 		exit 1
 	}
+	set _gitworktree [pwd]
 }
 set _reponame [file split [file normalize $_gitdir]]
 if {[lindex $_reponame end] eq {.git}} {
@@ -1921,6 +1938,7 @@ proc incr_font_size {font {amt 1}} {
 set starting_gitk_msg [mc "Starting gitk... please wait..."]
 
 proc do_gitk {revs} {
+	global _gitworktree
 	# -- Always start gitk through whatever we were loaded with.  This
 	#    lets us bypass using shell process on Windows systems.
 	#
@@ -1938,8 +1956,10 @@ proc do_gitk {revs} {
 		}
 
 		set pwd [pwd]
-		cd [file dirname [gitdir]]
-		set env(GIT_DIR) [file tail [gitdir]]
+		if { $_gitworktree ne {} } {
+			cd $_gitworktree
+		}
+		set env(GIT_DIR) [file normalize [gitdir]]
 
 		eval exec $cmd $revs &
 
@@ -1958,6 +1978,7 @@ proc do_gitk {revs} {
 }
 
 proc do_explore {} {
+	global _gitworktree
 	set explorer {}
 	if {[is_Cygwin] || [is_Windows]} {
 		set explorer "explorer.exe"
@@ -1967,7 +1988,7 @@ proc do_explore {} {
 		# freedesktop.org-conforming system is our best shot
 		set explorer "xdg-open"
 	}
-	eval exec $explorer [list [file nativename [file dirname [gitdir]]]] &
+	eval exec $explorer $_gitworktree &
 }
 
 set is_quitting 0
@@ -2331,8 +2352,6 @@ proc show_less_context {} {
 ##
 ## ui construction
 
-load_config 0
-apply_config
 set ui_comm {}
 
 # -- Menu Bar
@@ -3370,7 +3389,7 @@ unset i
 set file_lists($ui_index) [list]
 set file_lists($ui_workdir) [list]
 
-wm title . "[appname] ([reponame]) [file normalize [file dirname [gitdir]]]"
+wm title . "[appname] ([reponame]) [file normalize $_gitworktree]"
 focus -force $ui_comm
 
 # -- Warn the user about environmental problems.  Cygwin's Tcl
-- 
1.6.6.rc1.295.g3a4e71

[PATCHv4 2/5] git-gui: handle bare repos correctly

[Giuseppe Bilotta]

Refactor checking for a bare repository into its own proc, that relies
on git rev-parse --is-bare-repository if possible. For older versions of
git we fall back to a logic such that the repository is considered bare
if:
 * either the core.bare setting is true
 * or the worktree is not set and the directory name ends with .git
The error message for the case of an unhandled bare repository is also
updated to reflect the fact that the problem is not the funny name but
the bareness.

The new refactored proc is also used to disable the menu entry to
explore the working copy, and to skip changing to the worktree before
the gitk invocation.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-gui/git-gui.sh |   43 ++++++++++++++++++++++++++++++++++++-------
 1 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 7e633f4..d0dff40 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -122,6 +122,7 @@ unset oguimsg
 set _appname {Git Gui}
 set _gitdir {}
 set _gitworktree {}
+set _isbare {}
 set _gitexec {}
 set _githtmldir {}
 set _reponame {}
@@ -277,6 +278,32 @@ proc get_config {name} {
 	}
 }
 
+proc is_bare {} {
+	global _isbare
+	global _gitdir
+	global _gitworktree
+
+	if {$_isbare eq {}} {
+		if {[catch {
+			set _bare [git rev-parse --is-bare-repository]
+			switch  -- $_bare {
+			true { set _isbare 1 }
+			false { set _isbare 0}
+			default { throw }
+			}
+		}]} {
+			if {[is_config_true core.bare]
+				|| ($_gitworktree eq {}
+					&& [lindex [file split $_gitdir] end] ne {.git})} {
+				set _isbare 1
+			} else {
+				set _isbare 0
+			}
+		}
+	}
+	return $_isbare
+}
+
 ######################################################################
 ##
 ## handy utils
@@ -1112,9 +1139,9 @@ if {$_prefix ne {}} {
 	set _gitworktree [pwd]
 	unset cdup
 } elseif {![is_enabled bare]} {
-	if {[lindex [file split $_gitdir] end] ne {.git}} {
+	if {[is_bare]} {
 		catch {wm withdraw .}
-		error_popup [strcat [mc "Cannot use funny .git directory:"] "\n\n$_gitdir"]
+		error_popup [strcat [mc "Cannot use bare repository:"] "\n\n$_gitdir"]
 		exit 1
 	}
 	if {$_gitworktree eq {}} {
@@ -1956,7 +1983,7 @@ proc do_gitk {revs} {
 		}
 
 		set pwd [pwd]
-		if { $_gitworktree ne {} } {
+		if { ![is_bare] } {
 			cd $_gitworktree
 		}
 		set env(GIT_DIR) [file normalize [gitdir]]
@@ -2383,10 +2410,12 @@ if {[is_enabled multicommit] || [is_enabled singlecommit]} {
 #
 menu .mbar.repository
 
-.mbar.repository add command \
-	-label [mc "Explore Working Copy"] \
-	-command {do_explore}
-.mbar.repository add separator
+if {![is_bare]} {
+	.mbar.repository add command \
+		-label [mc "Explore Working Copy"] \
+		-command {do_explore}
+	.mbar.repository add separator
+}
 
 .mbar.repository add command \
 	-label [mc "Browse Current Branch's Files"] \
-- 
1.6.6.rc1.295.g3a4e71

[PATCHv4 3/5] git-gui: work from the .git dir

[Giuseppe Bilotta]

When git-gui is run from a .git dir, _gitdir would be set to "." by
rev-parse, something that confuses the worktree detection.

Fix by expanding the value of _gitdir to pwd in this special case.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-gui/git-gui.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index d0dff40..c5c787d 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1102,6 +1102,8 @@ if {[catch {
 		set _prefix {}
 		}]
 	&& [catch {
+		# beware that from the .git dir this sets _gitdir to .
+		# and _prefix to the empty string
 		set _gitdir [git rev-parse --git-dir]
 		set _prefix [git rev-parse --show-prefix]
 	} err]} {
@@ -1110,6 +1112,14 @@ if {[catch {
 	choose_repository::pick
 	set picked 1
 }
+
+# we expand the _gitdir when it's just a single dot (i.e. when we're being
+# run from the .git dir itself) lest the routines to find the worktree
+# get confused
+if {$_gitdir eq "."} {
+	set _gitdir [pwd]
+}
+
 if {![file isdirectory $_gitdir] && [is_Cygwin]} {
 	catch {set _gitdir [exec cygpath --windows $_gitdir]}
 }
-- 
1.6.6.rc1.295.g3a4e71

[PATCHv4 4/5] git-gui: set GIT_DIR and GIT_WORK_TREE after setup

[Giuseppe Bilotta]

Rather than juggling $GIT_DIR around the invocation of gitk, set it
(and $GIT_WORK_TREE) after finishing setup, ensuring that any external
tool works with the setup we're running with.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-gui/git-gui.sh |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index c5c787d..aff0644 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1171,6 +1171,9 @@ if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end]
 }
 
+set env(GIT_DIR) $_gitdir
+set env(GIT_WORK_TREE) $_gitworktree
+
 ######################################################################
 ##
 ## global init
@@ -1986,25 +1989,13 @@ proc do_gitk {revs} {
 	} else {
 		global env
 
-		if {[info exists env(GIT_DIR)]} {
-			set old_GIT_DIR $env(GIT_DIR)
-		} else {
-			set old_GIT_DIR {}
-		}
-
 		set pwd [pwd]
 		if { ![is_bare] } {
 			cd $_gitworktree
 		}
-		set env(GIT_DIR) [file normalize [gitdir]]
 
 		eval exec $cmd $revs &
 
-		if {$old_GIT_DIR eq {}} {
-			unset env(GIT_DIR)
-		} else {
-			set env(GIT_DIR) $old_GIT_DIR
-		}
 		cd $pwd
 
 		ui_status $::starting_gitk_msg
-- 
1.6.6.rc1.295.g3a4e71

[PATCHv4 5/5] git-gui: update shortcut tools to use _gitworktree

[Giuseppe Bilotta]

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-gui/lib/shortcut.tcl |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl
index 2f20eb3..79c1888 100644
--- a/git-gui/lib/shortcut.tcl
+++ b/git-gui/lib/shortcut.tcl
@@ -2,6 +2,7 @@
 # Copyright (C) 2006, 2007 Shawn Pearce
 
 proc do_windows_shortcut {} {
+	global _gitworktree
 	set fn [tk_getSaveFile \
 		-parent . \
 		-title [append "[appname] ([reponame]): " [mc "Create Desktop Icon"]] \
@@ -15,7 +16,7 @@ proc do_windows_shortcut {} {
 					[info nameofexecutable] \
 					[file normalize $::argv0] \
 					] \
-					[file dirname [file normalize [gitdir]]]
+					[file normalize [$_gitworktree]]
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
@@ -23,7 +24,7 @@ proc do_windows_shortcut {} {
 }
 
 proc do_cygwin_shortcut {} {
-	global argv0
+	global argv0 _gitworktree
 
 	if {[catch {
 		set desktop [exec cygpath \
@@ -56,7 +57,7 @@ proc do_cygwin_shortcut {} {
 					$sh -c \
 					"CHERE_INVOKING=1 source /etc/profile;[sq $me] &" \
 					] \
-					[file dirname [file normalize [gitdir]]]
+					[file normalize [$_gitworktree]]
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
-- 
1.6.6.rc1.295.g3a4e71

Re: [PATCHv4 4/5] git-gui: set GIT_DIR and GIT_WORK_TREE after setup

[Shawn O. Pearce]

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:
> Rather than juggling $GIT_DIR around the invocation of gitk, set it
> (and $GIT_WORK_TREE) after finishing setup, ensuring that any external
> tool works with the setup we're running with.
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  git-gui/git-gui.sh |   15 +++------------
>  1 files changed, 3 insertions(+), 12 deletions(-)

This patch has merge conflicts with changes from Jens Lehmann
related to starting gitk inside of a submodule.  Since I merged
his menu code first, I would appreciate it if you could rebase this
one patch on top of my latest master:

  git://git.spearce.org/git-gui.git master


The other 4 patches in this series are already applied, thanks.

-- 
Shawn.

[PATCH] git-gui: set GIT_DIR and GIT_WORK_TREE after setup

[Giuseppe Bilotta]

Rather than juggling with the env var GIT_DIR around the invocation of
gitk, set it and GIT_WORK_TREE after finishing setup, ensuring that any
external tool works with the setup we're running with.

This also allows us to remove a couple of conditionals when running gitk
or git gui in a submodule, as we know that the variables are present and
have to be unset and reset before and after the invocation.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 git-gui.sh |   43 ++++++++++++++++++++-----------------------
 1 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index 5d39955..81143c7 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -1171,6 +1171,9 @@ if {[lindex $_reponame end] eq {.git}} {
 	set _reponame [lindex $_reponame end]
 }
 
+set env(GIT_DIR) $_gitdir
+set env(GIT_WORK_TREE) $_gitworktree
+
 ######################################################################
 ##
 ## global init
@@ -1979,7 +1982,7 @@ set starting_gitk_msg [mc "Starting gitk... please wait..."]
 
 proc do_gitk {revs {is_submodule false}} {
 	global current_diff_path file_states current_diff_side ui_index
-	global _gitworktree
+	global _gitdir _gitworktree
 
 	# -- Always start gitk through whatever we were loaded with.  This
 	#    lets us bypass using shell process on Windows systems.
@@ -1991,19 +1994,12 @@ proc do_gitk {revs {is_submodule false}} {
 	} else {
 		global env
 
-		if {[info exists env(GIT_DIR)]} {
-			set old_GIT_DIR $env(GIT_DIR)
-		} else {
-			set old_GIT_DIR {}
-		}
-
 		set pwd [pwd]
 
 		if {!$is_submodule} {
 			if {![is_bare]} {
 				cd $_gitworktree
 			}
-			set env(GIT_DIR) [file normalize [gitdir]]
 		} else {
 			cd $current_diff_path
 			if {$revs eq {--}} {
@@ -2024,15 +2020,18 @@ proc do_gitk {revs {is_submodule false}} {
 				}
 				set revs $old_sha1...$new_sha1
 			}
-			if {[info exists env(GIT_DIR)]} {
-				unset env(GIT_DIR)
-			}
+			# GIT_DIR and GIT_WORK_TREE for the submodule are not the ones
+			# we've been using for the main repository, so unset them.
+			# TODO we could make life easier (start up faster?) for gitk
+			# by setting these to the appropriate values to allow gitk
+			# to skip the heuristics to find their proper value
+			unset env(GIT_DIR)
+			unset env(GIT_WORK_TREE)
 		}
 		eval exec $cmd $revs "--" "--" &
 
-		if {$old_GIT_DIR ne {}} {
-			set env(GIT_DIR) $old_GIT_DIR
-		}
+		set env(GIT_DIR) $_gitdir
+		set env(GIT_WORK_TREE) $_gitworktree
 		cd $pwd
 
 		ui_status $::starting_gitk_msg
@@ -2053,22 +2052,20 @@ proc do_git_gui {} {
 		error_popup [mc "Couldn't find git gui in PATH"]
 	} else {
 		global env
+		global _gitdir _gitworktree
 
-		if {[info exists env(GIT_DIR)]} {
-			set old_GIT_DIR $env(GIT_DIR)
-			unset env(GIT_DIR)
-		} else {
-			set old_GIT_DIR {}
-		}
+		# see note in do_gitk about unsetting these vars when
+		# running tools in a submodule
+		unset env(GIT_DIR)
+		unset env(GIT_WORK_TREE)
 
 		set pwd [pwd]
 		cd $current_diff_path
 
 		eval exec $exe gui &
 
-		if {$old_GIT_DIR ne {}} {
-			set env(GIT_DIR) $old_GIT_DIR
-		}
+		set env(GIT_DIR) $_gitdir
+		set env(GIT_WORK_TREE) $_gitworktree
 		cd $pwd
 
 		ui_status $::starting_gitk_msg
-- 
1.6.6.rc1.295.g3a4e71

Re: [PATCH] git-gui: set GIT_DIR and GIT_WORK_TREE after setup

[Shawn O. Pearce]

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:
> Rather than juggling with the env var GIT_DIR around the invocation of
> gitk, set it and GIT_WORK_TREE after finishing setup, ensuring that any
> external tool works with the setup we're running with.
> 
> This also allows us to remove a couple of conditionals when running gitk
> or git gui in a submodule, as we know that the variables are present and
> have to be unset and reset before and after the invocation.
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  git-gui.sh |   43 ++++++++++++++++++++-----------------------
>  1 files changed, 20 insertions(+), 23 deletions(-)

Thanks, applied.

-- 
Shawn.