[PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

git-gui has many snippets of code guarded by an is_Cygwin test, all of
which target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed in
March 2012. That is when Cygwin switched to a well-supported unix/X11
Tcl/Tk package.  64-bit Cygwin was released later so has always had the
unix/X11 package. git-gui runs as-is on more recent Cygwin, treating it
as a Linux variant, though two functions are disabled.

The old Tcl/Tk understood Windows pathnames, so git-gui's Cygwin
specific code uses Windows pathnames. The unix/X11 code requires use of
unix pathnames, so none of the Cygwin specific code is compatible, and
all should be removed.

Fortunately, the is_Cygwin funcion in git-gui (on the git master branch)
relies upon the old Tcl/Tk and doesn't detect Cygwin. But, commit
c5766eae6f2b002396b6cd4f85b62317b707174e on the git-gui master branch
"fixed" is_Cygwin, enabling the incompatible code, so upstream git-gui
is now broken on Cygwin.

There is Cygwin specific code in the Makefile, intended to allow a
completely unsupported configuration with a Windows TclTk.  This code
misdetects the configuration, creating a non-portable installation. The
Cygwin git maintainer comments this code out. The code should be
removed.

The existing code for file browsing and creating a desktop icon is
shared with Git For Windows support, and uses Windows pathnames. This
code does not work on Cygwin, and needs replacement.  These functions
have not worked since 2012.

patch 1 removes the obsolete Makefile code
patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...
patch 3 implements Cygwin specific file browsing support
patch 4 implemetns Cygwin specific desktop icon support

Patches 1/2 cause git-gui to function as it has for the last decade on
Cygwin, but without bugs masking bugs. Patches 3/4 restore functionality
lost with the Tcl/Tk switch in 2012.

Any argument for keeping the old Cygwin code must address who is going
to test and maintain that code, on what platform, and who the target
audience is. The old Tcl/Tk was only on 32-bit Cygwin and only supported
for the Insight debugger, 32-bit Cygwin is no longer supported, git-gui
is not supported on 8.4.1 Tcl/Tk, and the Windows versions targeted by
2012'ish 32-bit Cygwin are no longer supported.

Mark Levedahl (4):
  git gui Makefile - remove Cygwin modiifications
  git-gui - remove obsolete Cygwin specific code
  git-gui - use cygstart to browse on Cygwin
  git-gui - use mkshortcut on Cygwin

 Makefile                  |  21 +------
 git-gui.sh                | 126 ++++----------------------------------
 lib/choose_repository.tcl |  27 +-------
 lib/shortcut.tcl          |  31 +++++-----
 4 files changed, 31 insertions(+), 174 deletions(-)

-- 
2.41.0.99.19

[PATCH v0 1/4] git gui Makefile - remove Cygwin modiifications

[Mark Levedahl]

git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries
into git-gui, destroying the ability to package git-gui on one machine and
distribute to others. The intent is to do this only if a non-Cygwin Tcl/Tk is
installed, but the test for this is wrong with the unix/X11 Tcl/Tk shipped
since 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk.

The Cygwin git maintainer disables this code, so this code is definitely
not in use in the Cygwin distribution, and targets an untested /
unsupportable configuration.

The simplest approach is to just delete the Cygwin specific code as
stock Cygwin needs no special handling. Do so.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Makefile | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index a0d5a4b..3f80435 100644
--- a/Makefile
+++ b/Makefile
@@ -138,25 +138,10 @@ GITGUI_SCRIPT   := $$0
 GITGUI_RELATIVE :=
 GITGUI_MACOSXAPP :=
 
-ifeq ($(uname_O),Cygwin)
-	GITGUI_SCRIPT := `cygpath --windows --absolute "$(GITGUI_SCRIPT)"`
-
-	# Is this a Cygwin Tcl/Tk binary?  If so it knows how to do
-	# POSIX path translation just like cygpath does and we must
-	# keep libdir in POSIX format so Cygwin packages of git-gui
-	# work no matter where the user installs them.
-	#
-	ifeq ($(shell echo 'puts [file normalize /]' | '$(TCL_PATH_SQ)'),$(shell cygpath --mixed --absolute /))
-		gg_libdir_sed_in := $(gg_libdir)
-	else
-		gg_libdir_sed_in := $(shell cygpath --windows --absolute "$(gg_libdir)")
-	endif
-else
-	ifeq ($(exedir),$(gg_libdir))
-		GITGUI_RELATIVE := 1
-	endif
-	gg_libdir_sed_in := $(gg_libdir)
+ifeq ($(exedir),$(gg_libdir))
+	GITGUI_RELATIVE := 1
 endif
+gg_libdir_sed_in := $(gg_libdir)
 ifeq ($(uname_S),Darwin)
 	ifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)
 		GITGUI_MACOSXAPP := YesPlease
-- 
2.41.0.99.19

[PATCH v0 2/4] git-gui - remove obsolete Cygwin specific code

[Mark Levedahl]

In the current git release, git-gui runs on Cygwin without enabling any
of git-gui's Cygwin specific code.  This happens as the Cygwin specific
code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's
then supplied Tcl/Tk which was an incompletely ported variant of the
8.4.1 Windows Tcl/Tk code.  In March, 2012, that 8.4.1 package was
replaced with a full port based upon the upstream unix/X11 code,
since maintained up to date. The two Tcl/Tk packages are completely
incompatible, and have different sygnatures.

When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer
detected Cygwin, so did not enable Cygwin specific code, and the POSIX
environment provided by Cygwin since 2012 supported git-gui as a generic
unix. Thus, no-one apparently noticed the existence of incompatible
Cygwin specific code.

However, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in
upstream git-gui, the is_Cygwin funcion does detect current Cygwin.  The
Cygwin specific code is enabled, causing use of Windows rather than unix
pathnames, and enabling incorrect warnings about environment variables
that are not relevant for the fully functional unix/X11 Tcl/Tk. The end
result is that git-gui is now incommpatible with Cygwin.

So, delete all Cygwin specific code (code protected by "if is_Cygwin"),
thus restoring the post-2012 behaviour. Note that Cygwin specific code
is required to enable file browsing and shortcut creation (supported
before 2012), but is not addressed in this patch.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh                | 122 +++-----------------------------------
 lib/choose_repository.tcl |  27 +--------
 lib/shortcut.tcl          |  41 -------------
 3 files changed, 9 insertions(+), 181 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index cb92bba..b5dba80 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -84,14 +84,7 @@ proc _which {what args} {
 	global env _search_exe _search_path
 
 	if {$_search_path eq {}} {
-		if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} {
-			set _search_path [split [exec cygpath \
-				--windows \
-				--path \
-				--absolute \
-				$env(PATH)] {;}]
-			set _search_exe .exe
-		} elseif {[is_Windows]} {
+		if {[is_Windows]} {
 			set gitguidir [file dirname [info script]]
 			regsub -all ";" $gitguidir "\\;" gitguidir
 			set env(PATH) "$gitguidir;$env(PATH)"
@@ -342,14 +335,7 @@ proc gitexec {args} {
 		if {[catch {set _gitexec [git --exec-path]} err]} {
 			error "Git not installed?\n\n$err"
 		}
-		if {[is_Cygwin]} {
-			set _gitexec [exec cygpath \
-				--windows \
-				--absolute \
-				$_gitexec]
-		} else {
-			set _gitexec [file normalize $_gitexec]
-		}
+		set _gitexec [file normalize $_gitexec]
 	}
 	if {$args eq {}} {
 		return $_gitexec
@@ -364,14 +350,7 @@ proc githtmldir {args} {
 			# Git not installed or option not yet supported
 			return {}
 		}
-		if {[is_Cygwin]} {
-			set _githtmldir [exec cygpath \
-				--windows \
-				--absolute \
-				$_githtmldir]
-		} else {
-			set _githtmldir [file normalize $_githtmldir]
-		}
+		set _githtmldir [file normalize $_githtmldir]
 	}
 	if {$args eq {}} {
 		return $_githtmldir
@@ -1318,9 +1297,6 @@ if {$_gitdir eq "."} {
 	set _gitdir [pwd]
 }
 
-if {![file isdirectory $_gitdir] && [is_Cygwin]} {
-	catch {set _gitdir [exec cygpath --windows $_gitdir]}
-}
 if {![file isdirectory $_gitdir]} {
 	catch {wm withdraw .}
 	error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
@@ -1332,11 +1308,7 @@ apply_config
 
 # v1.7.0 introduced --show-toplevel to return the canonical work-tree
 if {[package vcompare $_git_version 1.7.0] >= 0} {
-	if { [is_Cygwin] } {
-		catch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}
-	} else {
-		set _gitworktree [git rev-parse --show-toplevel]
-	}
+	set _gitworktree [git rev-parse --show-toplevel]
 } else {
 	# try to set work tree from environment, core.worktree or use
 	# cdup to obtain a relative path to the top of the worktree. If
@@ -1561,24 +1533,8 @@ proc rescan {after {honor_trustmtime 1}} {
 	}
 }
 
-if {[is_Cygwin]} {
-	set is_git_info_exclude {}
-	proc have_info_exclude {} {
-		global is_git_info_exclude
-
-		if {$is_git_info_exclude eq {}} {
-			if {[catch {exec test -f [gitdir info exclude]}]} {
-				set is_git_info_exclude 0
-			} else {
-				set is_git_info_exclude 1
-			}
-		}
-		return $is_git_info_exclude
-	}
-} else {
-	proc have_info_exclude {} {
-		return [file readable [gitdir info exclude]]
-	}
+proc have_info_exclude {} {
+	return [file readable [gitdir info exclude]]
 }
 
 proc rescan_stage2 {fd after} {
@@ -2318,7 +2274,7 @@ proc do_git_gui {} {
 
 # Get the system-specific explorer app/command.
 proc get_explorer {} {
-	if {[is_Cygwin] || [is_Windows]} {
+	if {[is_Windows]} {
 		set explorer "explorer.exe"
 	} elseif {[is_MacOSX]} {
 		set explorer "open"
@@ -2874,11 +2830,7 @@ if {[is_enabled multicommit]} {
 
 	.mbar.repository add separator
 
-	if {[is_Cygwin]} {
-		.mbar.repository add command \
-			-label [mc "Create Desktop Icon"] \
-			-command do_cygwin_shortcut
-	} elseif {[is_Windows]} {
+	if {[is_Windows]} {
 		.mbar.repository add command \
 			-label [mc "Create Desktop Icon"] \
 			-command do_windows_shortcut
@@ -3112,10 +3064,6 @@ if {[is_MacOSX]} {
 set doc_path [githtmldir]
 if {$doc_path ne {}} {
 	set doc_path [file join $doc_path index.html]
-
-	if {[is_Cygwin]} {
-		set doc_path [exec cygpath --mixed $doc_path]
-	}
 }
 
 if {[file isfile $doc_path]} {
@@ -4087,60 +4035,6 @@ set file_lists($ui_workdir) [list]
 wm title . "[appname] ([reponame]) [file normalize $_gitworktree]"
 focus -force $ui_comm
 
-# -- Warn the user about environmental problems.  Cygwin's Tcl
-#    does *not* pass its env array onto any processes it spawns.
-#    This means that git processes get none of our environment.
-#
-if {[is_Cygwin]} {
-	set ignored_env 0
-	set suggest_user {}
-	set msg [mc "Possible environment issues exist.
-
-The following environment variables are probably
-going to be ignored by any Git subprocess run
-by %s:
-
-" [appname]]
-	foreach name [array names env] {
-		switch -regexp -- $name {
-		{^GIT_INDEX_FILE$} -
-		{^GIT_OBJECT_DIRECTORY$} -
-		{^GIT_ALTERNATE_OBJECT_DIRECTORIES$} -
-		{^GIT_DIFF_OPTS$} -
-		{^GIT_EXTERNAL_DIFF$} -
-		{^GIT_PAGER$} -
-		{^GIT_TRACE$} -
-		{^GIT_CONFIG$} -
-		{^GIT_(AUTHOR|COMMITTER)_DATE$} {
-			append msg " - $name\n"
-			incr ignored_env
-		}
-		{^GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL)$} {
-			append msg " - $name\n"
-			incr ignored_env
-			set suggest_user $name
-		}
-		}
-	}
-	if {$ignored_env > 0} {
-		append msg [mc "
-This is due to a known issue with the
-Tcl binary distributed by Cygwin."]
-
-		if {$suggest_user ne {}} {
-			append msg [mc "
-
-A good replacement for %s
-is placing values for the user.name and
-user.email settings into your personal
-~/.gitconfig file.
-" $suggest_user]
-		}
-		warn_popup $msg
-	}
-	unset ignored_env msg suggest_user name
-}
-
 # -- Only initialize complex UI if we are going to stay running.
 #
 if {[is_enabled transport]} {
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index af1fee7..d23abed 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -174,9 +174,6 @@ constructor pick {} {
 			-foreground blue \
 			-underline 1
 		set home $::env(HOME)
-		if {[is_Cygwin]} {
-			set home [exec cygpath --windows --absolute $home]
-		}
 		set home "[file normalize $home]/"
 		set hlen [string length $home]
 		foreach p $sorted_recent {
@@ -374,18 +371,6 @@ proc _objdir {path} {
 		return $objdir
 	}
 
-	if {[is_Cygwin]} {
-		set objdir [file join $path .git objects.lnk]
-		if {[file isfile $objdir]} {
-			return [win32_read_lnk $objdir]
-		}
-
-		set objdir [file join $path objects.lnk]
-		if {[file isfile $objdir]} {
-			return [win32_read_lnk $objdir]
-		}
-	}
-
 	return {}
 }
 
@@ -623,12 +608,6 @@ method _do_clone2 {} {
 	}
 
 	set giturl $origin_url
-	if {[is_Cygwin] && [file isdirectory $giturl]} {
-		set giturl [exec cygpath --unix --absolute $giturl]
-		if {$clone_type eq {shared}} {
-			set objdir [exec cygpath --unix --absolute $objdir]
-		}
-	}
 
 	if {[file exists $local_path]} {
 		error_popup [mc "Location %s already exists." $local_path]
@@ -668,11 +647,7 @@ method _do_clone2 {} {
 				fconfigure $f_cp -translation binary -encoding binary
 				cd $objdir
 				while {[gets $f_in line] >= 0} {
-					if {[is_Cygwin]} {
-						puts $f_cp [exec cygpath --unix --absolute $line]
-					} else {
-						puts $f_cp [file normalize $line]
-					}
+					puts $f_cp [file normalize $line]
 				}
 				close $f_in
 				close $f_cp
diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl
index 97d1d7a..1d8374b 100644
--- a/lib/shortcut.tcl
+++ b/lib/shortcut.tcl
@@ -26,47 +26,6 @@ proc do_windows_shortcut {} {
 	}
 }
 
-proc do_cygwin_shortcut {} {
-	global argv0 _gitworktree
-
-	if {[catch {
-		set desktop [exec cygpath \
-			--windows \
-			--absolute \
-			--long-name \
-			--desktop]
-		}]} {
-			set desktop .
-	}
-	set fn [tk_getSaveFile \
-		-parent . \
-		-title [mc "%s (%s): Create Desktop Icon" [appname] [reponame]] \
-		-initialdir $desktop \
-		-initialfile "Git [reponame].lnk"]
-	if {$fn != {}} {
-		if {[file extension $fn] ne {.lnk}} {
-			set fn ${fn}.lnk
-		}
-		if {[catch {
-				set sh [exec cygpath \
-					--windows \
-					--absolute \
-					/bin/sh.exe]
-				set me [exec cygpath \
-					--unix \
-					--absolute \
-					$argv0]
-				win32_create_lnk $fn [list \
-					$sh -c \
-					"CHERE_INVOKING=1 source /etc/profile;[sq $me] &" \
-					] \
-					[file normalize $_gitworktree]
-			} err]} {
-			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
-		}
-	}
-}
-
 proc do_macosx_app {} {
 	global argv0 env
 
-- 
2.41.0.99.19

[PATCH v0 3/4] git-gui - use cygstart to browse on Cygwin

[Mark Levedahl]

Pre-2012, git-gui enabled the "Repository->Explore Working Copy" menu on
Cygwin, offering open a Windows graphical file browser at the root
working directory. The old code relied upon internal use of Windows
pathnames, while git-gui must use unix pathnames on Cygwin since 2012,
so was removed in a previous patch.

A base install of Cygwin provides the /bin/cygstart utility that runs
arbtitrary Windows applications after translating unix pathnames to
Windows.  Adding the --explore option guarantees that the Windows file
explorer is opened, regardless of the supplied pathname's file type and
avoiding possibility of some other action being taken.

So, teach git-gui to use cygstart --explore on Cygwin, restoring the
pre-2012 behavior of opening a Windows file explorer for browsing.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/git-gui.sh b/git-gui.sh
index b5dba80..523770a 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2276,6 +2276,8 @@ proc do_git_gui {} {
 proc get_explorer {} {
 	if {[is_Windows]} {
 		set explorer "explorer.exe"
+	} elseif {[is_Cygwin]} {
+		set explorer "/bin/cygstart.exe --explore"
 	} elseif {[is_MacOSX]} {
 		set explorer "open"
 	} else {
-- 
2.41.0.99.19

[PATCH v0 4/4] git-gui - use mkshortcut on Cygwin

[Mark Levedahl]

Prior to 2012, git-gui enabled the "Repository->Create Desktop Icon"
item on Cygwin, offering to create a shortcut that starts git-gui on a
particular repository. The original code for this in lib/win32.tcl,
shared with Git for Windows support, requires Windows pathnames, while
git-gui must use unix pathnames with the unix/X11 Tcl/Tk since 2012. The
ability to use this from Cygwin was removed in a previous patch.

Cygwin's default installation provides /bin/mkshortcut for creating
desktop shortuts, this is compatible with exec under tcl, and understands
Cygwin's unix pathnames. So, teach git-gui to use mkshortcut on Cygwin,
leaving lib/win32.tcl as Git for Windows specific support.

Notes: "CHERE_INVOKING=1" is recognized by Cygwin's /etc/profile and
prevents a "chdir $HOME", leaving the shell in the working directory
specified by the shortcut. That directory is written directly by
mkshortcut eliminating any problems with shell escapes and quoting.

The pre-2012 code includes the full pathname of the git-gui creating the
shortcut (rather than using the system git-gui), but that git-gui might
not be compatible with the git found after /etc/profile sets the path,
and might have a pathname that defies encoding using shell escapes that
can survive the multiple incompatible interpreters involved in this
chain. Instead, use "git gui", thus defaulting to the system git and
avoiding both issues.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 git-gui.sh       |  4 ++++
 lib/shortcut.tcl | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/git-gui.sh b/git-gui.sh
index 523770a..5c13521 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2836,6 +2836,10 @@ if {[is_enabled multicommit]} {
 		.mbar.repository add command \
 			-label [mc "Create Desktop Icon"] \
 			-command do_windows_shortcut
+	} elseif {[is_Cygwin]} {
+		.mbar.repository add command \
+			-label [mc "Create Desktop Icon"] \
+			-command do_cygwin_shortcut
 	} elseif {[is_MacOSX]} {
 		.mbar.repository add command \
 			-label [mc "Create Desktop Icon"] \
diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl
index 1d8374b..6c2a99e 100644
--- a/lib/shortcut.tcl
+++ b/lib/shortcut.tcl
@@ -26,6 +26,44 @@ proc do_windows_shortcut {} {
 	}
 }
 
+proc do_cygwin_shortcut {} {
+	global argv0 _gitworktree oguilib
+
+	if {[catch {
+		set desktop [exec cygpath \
+			--desktop]
+		}]} {
+			set desktop .
+	}
+	set fn [tk_getSaveFile \
+		-parent . \
+		-title [mc "%s (%s): Create Desktop Icon" [appname] [reponame]] \
+		-initialdir $desktop \
+		-initialfile "Git [reponame].lnk"]
+	if {$fn != {}} {
+		if {[file extension $fn] ne {.lnk}} {
+			set fn ${fn}.lnk
+		}
+		if {[catch {
+				set repodir [file normalize $_gitworktree]
+				set shargs {-c \
+					"CHERE_INVOKING=1 \
+					source /etc/profile; \
+					git gui"}
+				exec /bin/mkshortcut.exe \
+					-a $shargs \
+					-d "git-gui on $repodir" \
+					-i $oguilib/git-gui.ico \
+					-n $fn \
+					-s min \
+					-w $repodir \
+					/bin/sh.exe
+			} err]} {
+			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
+		}
+	}
+}
+
 proc do_macosx_app {} {
 	global argv0 env
 
-- 
2.41.0.99.19

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> git-gui has many snippets of code guarded by an is_Cygwin test, all of
> which target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed in
> March 2012. That is when Cygwin switched to a well-supported unix/X11
> Tcl/Tk package.  64-bit Cygwin was released later so has always had the
> unix/X11 package. git-gui runs as-is on more recent Cygwin, treating it
> as a Linux variant, though two functions are disabled.
>
> The old Tcl/Tk understood Windows pathnames, so git-gui's Cygwin
> specific code uses Windows pathnames. The unix/X11 code requires use of
> unix pathnames, so none of the Cygwin specific code is compatible, and
> all should be removed.
>
> Fortunately, the is_Cygwin funcion in git-gui (on the git master branch)
> relies upon the old Tcl/Tk and doesn't detect Cygwin. But, commit
> c5766eae6f2b002396b6cd4f85b62317b707174e on the git-gui master branch
> "fixed" is_Cygwin, enabling the incompatible code, so upstream git-gui
> is now broken on Cygwin.

Here I presume "upstream git-gui master" refers to a5005ded (Merge
branch 'ab/makeflags', 2023-01-25) sitting at 'master' in Pratyush's
https://github.com/prati0100/git-gui/ repository.

> There is Cygwin specific code in the Makefile, intended to allow a
> completely unsupported configuration with a Windows TclTk.  This code
> misdetects the configuration, creating a non-portable installation. The
> Cygwin git maintainer comments this code out. The code should be
> removed.
>
> ...
>
> patch 1 removes the obsolete Makefile code
> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...

As it has been quite a while since I had access to any Windows box
or Cygwin, but the earlier two patches look obviously correct to me.

> The existing code for file browsing and creating a desktop icon is
> shared with Git For Windows support, and uses Windows pathnames. This
> code does not work on Cygwin, and needs replacement.  These functions
> have not worked since 2012.
> ...
> patch 3 implements Cygwin specific file browsing support
> patch 4 implemetns Cygwin specific desktop icon support

Both of these two patches do

	if {[is_Windows]} {
		... do Windows thing ...
+	} elseif {[is_Cygwin]} {
+		... do Cygwin thing ...
	} elseif {[is_MacOSX]} {
		... do macOS thing ...
	} else {
		... do it for others ...
	}

which I do not quite understand how the existing code meshes with
your "is shared with Git For Windows support", though.  If "is
shared with GfW" is to be trusted, on a modern Cygwin box,
"is_Windows" would be yielding true (that is the only way the "do
Windows thing" block will be entered on Cygwin box, sharing the
support with GfW.  But then, adding elseif _after_ we check for
Windows would be pointless.  Puzzled...

Thanks.

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>> patch 1 removes the obsolete Makefile code
>> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...
>
> As it has been quite a while since I had access to any Windows box
> or Cygwin, but the earlier two patches look obviously correct to me.

Ehh, in an early draft, I had "I cannot comment on patches #3 and
#4" before that "but", but I ended up commenting on them anyway, and
ended up with such a garbled construction.  I should have copyedited
the above to "Even though it has been ... or Cygwin, the earlier...".

Sorry for the noise.

Re: [PATCH v0 2/4] git-gui - remove obsolete Cygwin specific code

[Eric Sunshine]

On Sat, Jun 24, 2023 at 5:35 PM Mark Levedahl <mlevedahl@gmail.com> wrote:
> In the current git release, git-gui runs on Cygwin without enabling any
> of git-gui's Cygwin specific code.  This happens as the Cygwin specific
> code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's
> then supplied Tcl/Tk which was an incompletely ported variant of the
> 8.4.1 Windows Tcl/Tk code.  In March, 2012, that 8.4.1 package was
> replaced with a full port based upon the upstream unix/X11 code,
> since maintained up to date. The two Tcl/Tk packages are completely
> incompatible, and have different sygnatures.

Given the context, an understandable typo perhaps: s/sygnatures/signatures/

> When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer
> detected Cygwin, so did not enable Cygwin specific code, and the POSIX
> environment provided by Cygwin since 2012 supported git-gui as a generic
> unix. Thus, no-one apparently noticed the existence of incompatible
> Cygwin specific code.
>
> However, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in
> upstream git-gui, the is_Cygwin funcion does detect current Cygwin.  The
> Cygwin specific code is enabled, causing use of Windows rather than unix
> pathnames, and enabling incorrect warnings about environment variables
> that are not relevant for the fully functional unix/X11 Tcl/Tk. The end
> result is that git-gui is now incommpatible with Cygwin.

s/incommpatible/incompatible/

> So, delete all Cygwin specific code (code protected by "if is_Cygwin"),
> thus restoring the post-2012 behaviour. Note that Cygwin specific code
> is required to enable file browsing and shortcut creation (supported
> before 2012), but is not addressed in this patch.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

On 6/24/23 19:30, Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>> git-gui has many snippets of code guarded by an is_Cygwin test, all of
>> which target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed in
>> March 2012. That is when Cygwin switched to a well-supported unix/X11
>> Tcl/Tk package.  64-bit Cygwin was released later so has always had the
>> unix/X11 package. git-gui runs as-is on more recent Cygwin, treating it
>> as a Linux variant, though two functions are disabled.
>>
>> The old Tcl/Tk understood Windows pathnames, so git-gui's Cygwin
>> specific code uses Windows pathnames. The unix/X11 code requires use of
>> unix pathnames, so none of the Cygwin specific code is compatible, and
>> all should be removed.
>>
>> Fortunately, the is_Cygwin funcion in git-gui (on the git master branch)
>> relies upon the old Tcl/Tk and doesn't detect Cygwin. But, commit
>> c5766eae6f2b002396b6cd4f85b62317b707174e on the git-gui master branch
>> "fixed" is_Cygwin, enabling the incompatible code, so upstream git-gui
>> is now broken on Cygwin.
> Here I presume "upstream git-gui master" refers to a5005ded (Merge
> branch 'ab/makeflags', 2023-01-25) sitting at 'master' in Pratyush's
> https://github.com/prati0100/git-gui/ repository.
Yes.
>
>> There is Cygwin specific code in the Makefile, intended to allow a
>> completely unsupported configuration with a Windows TclTk.  This code
>> misdetects the configuration, creating a non-portable installation. The
>> Cygwin git maintainer comments this code out. The code should be
>> removed.
>>
>> ...
>>
>> patch 1 removes the obsolete Makefile code
>> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...
> As it has been quite a while since I had access to any Windows box
> or Cygwin, but the earlier two patches look obviously correct to me.
>
>> The existing code for file browsing and creating a desktop icon is
>> shared with Git For Windows support, and uses Windows pathnames. This
>> code does not work on Cygwin, and needs replacement.  These functions
>> have not worked since 2012.
>> ...
>> patch 3 implements Cygwin specific file browsing support
>> patch 4 implemetns Cygwin specific desktop icon support
> Both of these two patches do
>
> 	if {[is_Windows]} {
> 		... do Windows thing ...
> +	} elseif {[is_Cygwin]} {
> +		... do Cygwin thing ...
> 	} elseif {[is_MacOSX]} {
> 		... do macOS thing ...
> 	} else {
> 		... do it for others ...
> 	}
>
> which I do not quite understand how the existing code meshes with
> your "is shared with Git For Windows support", though.  If "is
> shared with GfW" is to be trusted, on a modern Cygwin box,
> "is_Windows" would be yielding true (that is the only way the "do
> Windows thing" block will be entered on Cygwin box, sharing the
> support with GfW.  But then, adding elseif _after_ we check for
> Windows would be pointless.  Puzzled...
>
> Thanks.
>
git-gui has three independent functions (is_Cygwin, is_Windows, and 
is_MaxOSX), each determine if running on that platform, and "generic 
Unix/Linux" can be considered the result if all three functions return 
false. In Pratyush's tree, those three functions essentially are:

is_Cygwin: $::tcl_platform(os) startswith("CYGWIN")

is_MaxOSX: [tk windowingsystem] == "AQUA"

is_Windows: $::tcl_platform(platform) == "Windows"

It turns out, only one of the . is ever true, and none are true on 
Linux. So, the if/else tree above is not confused by Windows / Cygwin.

But, different Tcl/Tk signatures as platforms evolve could cause 
problems. A better design might be to just have a $HOSTTYPE variable set 
once, perhaps in startup, perhaps even by the makefile, to assure 
exactly one hosttype is ever active and make this clear to others. 
Normal configuration checking in the makefile could have uncovered this 
whole problem in 2012. But, this is a possible cleanup topic for another 
day.

So, the code under the is_Windows and is_Cygwin branches of the if/else 
trees are now completely independent, and the is_Windows branch is never 
entered on Cygwin.


Thank you,

Mark

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

On 6/24/23 19:35, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> patch 1 removes the obsolete Makefile code
>>> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...
>> As it has been quite a while since I had access to any Windows box
>> or Cygwin, but the earlier two patches look obviously correct to me.
> Ehh, in an early draft, I had "I cannot comment on patches #3 and
> #4" before that "but", but I ended up commenting on them anyway, and
> ended up with such a garbled construction.  I should have copyedited
> the above to "Even though it has been ... or Cygwin, the earlier...".
>
> Sorry for the noise.

No problem, I appreciate the review. The last two patches absolutely 
need the Cygwin / G4W folks to review.


Mark

Re: [PATCH v0 2/4] git-gui - remove obsolete Cygwin specific code

[Mark Levedahl]

On 6/24/23 22:56, Eric Sunshine wrote:
> On Sat, Jun 24, 2023 at 5:35 PM Mark Levedahl <mlevedahl@gmail.com> wrote:
>> In the current git release, git-gui runs on Cygwin without enabling any
>> of git-gui's Cygwin specific code.  This happens as the Cygwin specific
>> code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's
>> then supplied Tcl/Tk which was an incompletely ported variant of the
>> 8.4.1 Windows Tcl/Tk code.  In March, 2012, that 8.4.1 package was
>> replaced with a full port based upon the upstream unix/X11 code,
>> since maintained up to date. The two Tcl/Tk packages are completely
>> incompatible, and have different sygnatures.
> Given the context, an understandable typo perhaps: s/sygnatures/signatures/
>
>> When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer
>> detected Cygwin, so did not enable Cygwin specific code, and the POSIX
>> environment provided by Cygwin since 2012 supported git-gui as a generic
>> unix. Thus, no-one apparently noticed the existence of incompatible
>> Cygwin specific code.
>>
>> However, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in
>> upstream git-gui, the is_Cygwin funcion does detect current Cygwin.  The
>> Cygwin specific code is enabled, causing use of Windows rather than unix
>> pathnames, and enabling incorrect warnings about environment variables
>> that are not relevant for the fully functional unix/X11 Tcl/Tk. The end
>> result is that git-gui is now incommpatible with Cygwin.
> s/incommpatible/incompatible/
>
>> So, delete all Cygwin specific code (code protected by "if is_Cygwin"),
>> thus restoring the post-2012 behaviour. Note that Cygwin specific code
>> is required to enable file browsing and shortcut creation (supported
>> before 2012), but is not addressed in this patch.
>>
>> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>


Thanks, will fix both (and a few other typos ...)

Mark

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

On 6/25/23 07:26, Mark Levedahl wrote:
>
> On 6/24/23 19:30, Junio C Hamano wrote: git-gui has three independent 
> functions (is_Cygwin, is_Windows, and is_MaxOSX), each determine if 
> running on that platform, and "generic Unix/Linux" can be considered 
> the result if all three functions return false. In Pratyush's tree, 
> those three functions essentially are:
>
> is_Cygwin: $::tcl_platform(os) startswith("CYGWIN")
>
> is_MaxOSX: [tk windowingsystem] == "AQUA"
>
> is_Windows: $::tcl_platform(platform) == "Windows"
>
> It turns out, only one of the . is ever true, and none are true on 
> Linux. So, the if/else tree above is not confused by Windows / Cygwin.
>
> But, different Tcl/Tk signatures as platforms evolve could cause 
> problems. A better design might be to just have a $HOSTTYPE variable 
> set once, perhaps in startup, perhaps even by the makefile, to assure 
> exactly one hosttype is ever active and make this clear to others. 
> Normal configuration checking in the makefile could have uncovered 
> this whole problem in 2012. But, this is a possible cleanup topic for 
> another day.
>
> So, the code under the is_Windows and is_Cygwin branches of the 
> if/else trees are now completely independent, and the is_Windows 
> branch is never entered on Cygwin.
>
>
> Thank you,
>
> Mark
>
A follow up - I have Cygwin in a Windows VM on my laptop, no G4W, no Mac ...


Cygwin gives:   $::tcl_platform(os) = CYGWIN_NT-10.0-22621
                 $::tcl_platform(platform) = unix
                 tk windowingsystem = x11

Linux gives     $::tcl_platform(os) = Linux
                 $::tcl_platform(platform) = unix
                 tk windowingsystem = x11

So, neither Cygwin nor Linux trigger the checks for is_Windows or is_MacOSX

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> So, the code under the is_Windows and is_Cygwin branches of the
> if/else trees are now completely independent, and the is_Windows
> branch is never entered on Cygwin.

I missed this hunk in your updated get_explorer in [2/4]

 proc get_explorer {} {
-	if {[is_Cygwin] || [is_Windows]} {
+	if {[is_Windows]} {
 		set explorer "explorer.exe"
 	} elseif {[is_MacOSX]} {
 		set explorer "open"

and saw only this in [3/4]

 proc get_explorer {} {
 	if {[is_Windows]} {
 		set explorer "explorer.exe"
+	} elseif {[is_Cygwin]} {
+		set explorer "/bin/cygstart.exe --explore"
 	} elseif {[is_MacOSX]} {
 		set explorer "open"
 	} else {

As I missed the earlier change, the latter one alone looked to me
that for get_explorer to be share with GfW, the only explanation was
that is_Windows yields true on Cygwin, in which case the new elseif
did not make sense.

I think the hunk in [2/4] should be removed; it is confusing, it
does not have anything to do with the theme of [2/4], which is to
"remove obsolete Cygwin specific code".  And instead [3/4] should
be updated to do

+	if {[is_Cygwin] || [is_Windows]} {
-	if {[is_Windows]} {
		... do windows thing ...
+	} elseif {[is_Cygwin]} {
+		... do Cygwin thing ...
	} elseif {[is_MacOSX]} {
		... do macOS thing ...

The earlier explanation in the cover letter says this:

    The existing code for file browsing and creating a desktop icon is
    shared with Git For Windows support, and uses Windows pathnames. This
    code does not work on Cygwin, and needs replacement.  These functions
    have not worked since 2012.

If the change for get_explorer is updated to read like so, then "was
shared with GfW, now we have one that is for Cygwin" starts making
sense for the file browsing.

But I still do not understand the issue with desktop icon, though.
do_windows_shortcut and do_cygwin_shortcut were separate proc before
this series---while I fully believe that do_cygwin_shortcut did not
work on modern Cygwin if you say so, and "uses Windows pathnames"
may be what makes the original implementation not work on modern
Cygwin, I do not see how the existing code for the desktop icon "is
shared with GfW".

Ah, this is again due to the suboptimal splitting of the patches.

The original does have do_cygwin_shortcut, but you remove it in step
[2/4], together with its caller.  The code before your series did
have its own do_cygwin_shortcut, but after [2/4] it and its caller
are removed. The code may not have worked before step [2/4], so it
is probably alright in the end, but it does make step [4/4] very
confusing.  Since [4/4] does need to add Cygwin specific code,
perhaps you should exclude the shortcut related change from [2/4]
and keep it focused on removing Cygwin specific code that will not
be used in the end (instead of getting fixed to keep it alive).

So, earlier I said [2/4] made sense and obviously good.  But not
anymore.  It does a bit too many things and then have later steps
compensate for it, which made reviewing the series harder than
necessary.  It needs to be cleaned up a bit, I think.

Thanks.

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

On 6/25/23 11:46, Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>> So, the code under the is_Windows and is_Cygwin branches of the
>> if/else trees are now completely independent, and the is_Windows
>> branch is never entered on Cygwin.

>
> So, earlier I said [2/4] made sense and obviously good.  But not
> anymore.  It does a bit too many things and then have later steps
> compensate for it, which made reviewing the series harder than
> necessary.  It needs to be cleaned up a bit, I think.
>
> Thanks.
>

I had originally organized as you suggest, no problem doing so again. 
What gave me pause was this paragraph I originally wrote for the cover 
letter:


Patches 1/2 cause git-gui to function as it has for the last decade on
Cygwin, but with Cygwin being detected. However, the browsing and
shortcut creation menu items, removed in 2012 then re-added when is_Cygwin
was fixed, do not work, and shortcut creation will crash git-gui if used.
These are fixed in patches 3 / 4.


So, I'm just checking that the above situation is ok. I agree, this 
makes the changes easier to follow.


Mark

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> I had originally organized as you suggest, no problem doing so
> again. What gave me pause was this paragraph I originally wrote for
> the cover letter:
>
> Patches 1/2 cause git-gui to function as it has for the last decade on
> Cygwin, but with Cygwin being detected. However, the browsing and
> shortcut creation menu items, removed in 2012 then re-added when is_Cygwin
> was fixed, do not work, and shortcut creation will crash git-gui if used.
> These are fixed in patches 3 / 4.

As you are removing (ancient) Cygwin specific code that did not work
with modern Cygwin at all in step [2/4], it is not so unexpected
that some stuff does still not work after that step.  I am not sure
what your reservation exactly is, but if you are wondering if code
to disable browsing and shortcut creation on Cygwin temporarily
needs to be there in the same step (instead of crashing or not
working), it may make sense if and only if it is done with minimal
changes.

Thanks.

[PATCH v1 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

=== This is an update, incorporating responses to Junio's and Eric's
comments:
  -- clarified what the "upstream" git-gui branch is
  -- Removed some changes from patch 2 as requested by Junio, reducing
     changes in patch 3 and patch 4
       All code is fixed only after applying patch 4
       Differences in patch 3 and 4 are minimimized
   -- updated comments to clarify G4w dedicated code.
   -- updated all comments to (hopefully) clarify points of confusion
===

git-gui has many snippets of code guarded by an is_Cygwin test, all of
which target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed
from the Cygwin project in March 2012. That is when Cygwin switched to a
well-supported unix/X11 Tcl/Tk package.  64-bit Cygwin was released
later so has always had the unix/X11 package.  git-gui runs as-is on
more recent Cygwin, treating it as a Linux variant, though two functions
are disabled.

The old Tcl/Tk understands Windows pathnames but has incomplete support
for unix pathnames (for instance, all pathnames output by that Tcl are
Windows, not unix). The Cygwin git executables all use unix pathnames
(though like all Cygwin executables have some capability to accept
Windows pathnames). git-gui's Cygwin specific code causes git-gui to use
Windows pathnames everywhere.  The unix/X11 Tcl/Tk requires use of unix
pathname, so none of the Cygwin specific code is compatible.

git-gui is maintained at https://github.com/prati0100/git-gui. The
git-gui in Junio's tree corresponds to commit c0698df057, behind the
current git-gui master which is a5005ded.

Fortunately, the git-gui/is_Cygwin function in Junio's tree relies upon
the old Tcl/Tk that outputs Windows pathnames.  As this fails to detect
Cygwin, git-gui treats Cygwin as a unix variant with no platform
specific code enabled and git-gui currently runs on Cygwin.

But, commit c5766eae6f on the git-gui master branch fixes is_Cygwin to
work with the new Tcl/Tk's signature (which is not that of a Windows
Tcl/Tk). Thus, Cygwin is detected, the incompatible Cygwin code is
enabled, and git-gui no longer runs on Cygwin.

Also, there is Cygwin specific code in the Makefile, intended to allow a
completely unsupported configuration with a Windows Tcl/Tk.  However,
the Makefile code mis-identifies the unix/X11 Tcl/Tk as Windows,
triggering insertion of a hardcoded Windows path to the library
directory into git-gui making it non-portable. The Cygwin git maintainer
comments this code out, but the code should be removed as it is
demonstrated to be incompatible with Cygwin and targets a configuration
Cygwin does not support.

The existing code for file browsing and creating a desktop icon is
shared with Git For Windows support, and supports only Windows
pathnames. This code does not work on Cygwin and needs replacement or
update.  The menu items for these functions are enabled by is_Cygwin, so
appear only after is_Cygwin is fixed as discussed above.

patch 1 removes the obsolete Makefile code
patch 2 removes obsolete git-gui.sh code, wrapped in is_Cygwin
     except for code fixed in patches 3 and 4
patch 3 implements Cygwin specific file browsing support
patch 4 implements Cygwin specific desktop icon support

The end result is that git-gui on Cygwin is restored to the full
capabilities existing prior to the Tcl/Tk switch in 2012. Also, the
remaining Cygwin specific code, updated in patches 3 and 4, no longer
overlaps with Git For Windows support.

Any argument for keeping the old Cygwin code must address who is going
to test and maintain that code, on what platform, and who the target
audience is. The old Tcl/Tk was only on 32-bit Cygwin and only supported
for the Insight debugger, 32-bit Cygwin is no longer supported, git-gui
is not supported on 8.4.1 Tcl/Tk, and the Windows versions targeted by
2012'ish 32-bit Cygwin are no longer supported.

Mark Levedahl (4):
  git gui Makefile - remove Cygwin modifications
  git-gui - remove obsolete Cygwin specific code
  git-gui - use cygstart to browse on Cygwin
  git-gui - use mkshortcut on Cygwin

 Makefile                  |  21 +------
 git-gui.sh                | 118 +++-----------------------------------
 lib/choose_repository.tcl |  27 +--------
 lib/shortcut.tcl          |  31 +++++-----
 4 files changed, 27 insertions(+), 170 deletions(-)

-- 
2.41.0.99.19

[PATCH v1 1/4] git gui Makefile - remove Cygwin modifications

[Mark Levedahl]

git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries
into git-gui, destroying the ability to package git-gui on one machine and
distribute to others. The intent is to do this only if a non-Cygwin Tcl/Tk is
installed, but the test for this is wrong with the unix/X11 Tcl/Tk shipped
since 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk.

The Cygwin git maintainer disables this code, so this code is definitely
not in use in the Cygwin distribution.

The simplest fix is to just delete the Cygwin specific code,
allowing the Makefile to work out of the box on Cygwin. Do so.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 Makefile | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index a0d5a4b..3f80435 100644
--- a/Makefile
+++ b/Makefile
@@ -138,25 +138,10 @@ GITGUI_SCRIPT   := $$0
 GITGUI_RELATIVE :=
 GITGUI_MACOSXAPP :=
 
-ifeq ($(uname_O),Cygwin)
-	GITGUI_SCRIPT := `cygpath --windows --absolute "$(GITGUI_SCRIPT)"`
-
-	# Is this a Cygwin Tcl/Tk binary?  If so it knows how to do
-	# POSIX path translation just like cygpath does and we must
-	# keep libdir in POSIX format so Cygwin packages of git-gui
-	# work no matter where the user installs them.
-	#
-	ifeq ($(shell echo 'puts [file normalize /]' | '$(TCL_PATH_SQ)'),$(shell cygpath --mixed --absolute /))
-		gg_libdir_sed_in := $(gg_libdir)
-	else
-		gg_libdir_sed_in := $(shell cygpath --windows --absolute "$(gg_libdir)")
-	endif
-else
-	ifeq ($(exedir),$(gg_libdir))
-		GITGUI_RELATIVE := 1
-	endif
-	gg_libdir_sed_in := $(gg_libdir)
+ifeq ($(exedir),$(gg_libdir))
+	GITGUI_RELATIVE := 1
 endif
+gg_libdir_sed_in := $(gg_libdir)
 ifeq ($(uname_S),Darwin)
 	ifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)
 		GITGUI_MACOSXAPP := YesPlease
-- 
2.41.0.99.19

[PATCH v1 2/4] git-gui - remove obsolete Cygwin specific code

[Mark Levedahl]

In the current git release, git-gui runs on Cygwin without enabling any
of git-gui's Cygwin specific code.  This happens as the Cygwin specific
code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's
then supplied Tcl/Tk which was an incompletely ported variant of the
8.4.1 Windows Tcl/Tk code.  In March, 2012, that 8.4.1 package was
replaced with a full port based upon the upstream unix/X11 code,
since maintained up to date. The two Tcl/Tk packages are completely
incompatible, and have different signatures.

When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer
detected Cygwin, so did not enable Cygwin specific code, and the POSIX
environment provided by Cygwin since 2012 supported git-gui as a generic
unix. Thus, no-one apparently noticed the existence of incompatible
Cygwin specific code.

However, since commit c5766eae6f in the git-gui source tree
(https://github.com/prati0100/git-gui, master at a5005ded), and not yet
pulled into the git repository, the is_Cygwin function does detect
Cygwin using the unix/X11 Tcl/Tk.  The Cygwin specific code is enabled,
causing use of Windows rather than unix pathnames, and enabling
incorrect warnings about environment variables that were relevant only
to the old Tcl/Tk.  The end result is that (upstream) git-gui is now
incompatible with Cygwin.

So, delete Cygwin specific code (code protected by "if is_Cygwin") that
is not needed in any form to work with the unix/X11 Tcl/Tk.

Cygwin specific code required to enable file browsing and shortcut
creation is not addressed in this patch, does not currently work, and
invocation of those items may leave git-gui in a confused state.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
changes since v0
 -- do not touch any code fixed in patches 3/4, meaning the browsing
    and shortcut creating menu items do not work.

 git-gui.sh                | 114 ++------------------------------------
 lib/choose_repository.tcl |  27 +--------
 2 files changed, 7 insertions(+), 134 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index cb92bba..3f7c31e 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -84,14 +84,7 @@ proc _which {what args} {
 	global env _search_exe _search_path
 
 	if {$_search_path eq {}} {
-		if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} {
-			set _search_path [split [exec cygpath \
-				--windows \
-				--path \
-				--absolute \
-				$env(PATH)] {;}]
-			set _search_exe .exe
-		} elseif {[is_Windows]} {
+		if {[is_Windows]} {
 			set gitguidir [file dirname [info script]]
 			regsub -all ";" $gitguidir "\\;" gitguidir
 			set env(PATH) "$gitguidir;$env(PATH)"
@@ -342,14 +335,7 @@ proc gitexec {args} {
 		if {[catch {set _gitexec [git --exec-path]} err]} {
 			error "Git not installed?\n\n$err"
 		}
-		if {[is_Cygwin]} {
-			set _gitexec [exec cygpath \
-				--windows \
-				--absolute \
-				$_gitexec]
-		} else {
-			set _gitexec [file normalize $_gitexec]
-		}
+		set _gitexec [file normalize $_gitexec]
 	}
 	if {$args eq {}} {
 		return $_gitexec
@@ -364,14 +350,7 @@ proc githtmldir {args} {
 			# Git not installed or option not yet supported
 			return {}
 		}
-		if {[is_Cygwin]} {
-			set _githtmldir [exec cygpath \
-				--windows \
-				--absolute \
-				$_githtmldir]
-		} else {
-			set _githtmldir [file normalize $_githtmldir]
-		}
+		set _githtmldir [file normalize $_githtmldir]
 	}
 	if {$args eq {}} {
 		return $_githtmldir
@@ -1318,9 +1297,6 @@ if {$_gitdir eq "."} {
 	set _gitdir [pwd]
 }
 
-if {![file isdirectory $_gitdir] && [is_Cygwin]} {
-	catch {set _gitdir [exec cygpath --windows $_gitdir]}
-}
 if {![file isdirectory $_gitdir]} {
 	catch {wm withdraw .}
 	error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"]
@@ -1332,11 +1308,7 @@ apply_config
 
 # v1.7.0 introduced --show-toplevel to return the canonical work-tree
 if {[package vcompare $_git_version 1.7.0] >= 0} {
-	if { [is_Cygwin] } {
-		catch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}
-	} else {
-		set _gitworktree [git rev-parse --show-toplevel]
-	}
+	set _gitworktree [git rev-parse --show-toplevel]
 } else {
 	# try to set work tree from environment, core.worktree or use
 	# cdup to obtain a relative path to the top of the worktree. If
@@ -1561,24 +1533,8 @@ proc rescan {after {honor_trustmtime 1}} {
 	}
 }
 
-if {[is_Cygwin]} {
-	set is_git_info_exclude {}
-	proc have_info_exclude {} {
-		global is_git_info_exclude
-
-		if {$is_git_info_exclude eq {}} {
-			if {[catch {exec test -f [gitdir info exclude]}]} {
-				set is_git_info_exclude 0
-			} else {
-				set is_git_info_exclude 1
-			}
-		}
-		return $is_git_info_exclude
-	}
-} else {
-	proc have_info_exclude {} {
-		return [file readable [gitdir info exclude]]
-	}
+proc have_info_exclude {} {
+	return [file readable [gitdir info exclude]]
 }
 
 proc rescan_stage2 {fd after} {
@@ -3112,10 +3068,6 @@ if {[is_MacOSX]} {
 set doc_path [githtmldir]
 if {$doc_path ne {}} {
 	set doc_path [file join $doc_path index.html]
-
-	if {[is_Cygwin]} {
-		set doc_path [exec cygpath --mixed $doc_path]
-	}
 }
 
 if {[file isfile $doc_path]} {
@@ -4087,60 +4039,6 @@ set file_lists($ui_workdir) [list]
 wm title . "[appname] ([reponame]) [file normalize $_gitworktree]"
 focus -force $ui_comm
 
-# -- Warn the user about environmental problems.  Cygwin's Tcl
-#    does *not* pass its env array onto any processes it spawns.
-#    This means that git processes get none of our environment.
-#
-if {[is_Cygwin]} {
-	set ignored_env 0
-	set suggest_user {}
-	set msg [mc "Possible environment issues exist.
-
-The following environment variables are probably
-going to be ignored by any Git subprocess run
-by %s:
-
-" [appname]]
-	foreach name [array names env] {
-		switch -regexp -- $name {
-		{^GIT_INDEX_FILE$} -
-		{^GIT_OBJECT_DIRECTORY$} -
-		{^GIT_ALTERNATE_OBJECT_DIRECTORIES$} -
-		{^GIT_DIFF_OPTS$} -
-		{^GIT_EXTERNAL_DIFF$} -
-		{^GIT_PAGER$} -
-		{^GIT_TRACE$} -
-		{^GIT_CONFIG$} -
-		{^GIT_(AUTHOR|COMMITTER)_DATE$} {
-			append msg " - $name\n"
-			incr ignored_env
-		}
-		{^GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL)$} {
-			append msg " - $name\n"
-			incr ignored_env
-			set suggest_user $name
-		}
-		}
-	}
-	if {$ignored_env > 0} {
-		append msg [mc "
-This is due to a known issue with the
-Tcl binary distributed by Cygwin."]
-
-		if {$suggest_user ne {}} {
-			append msg [mc "
-
-A good replacement for %s
-is placing values for the user.name and
-user.email settings into your personal
-~/.gitconfig file.
-" $suggest_user]
-		}
-		warn_popup $msg
-	}
-	unset ignored_env msg suggest_user name
-}
-
 # -- Only initialize complex UI if we are going to stay running.
 #
 if {[is_enabled transport]} {
diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl
index af1fee7..d23abed 100644
--- a/lib/choose_repository.tcl
+++ b/lib/choose_repository.tcl
@@ -174,9 +174,6 @@ constructor pick {} {
 			-foreground blue \
 			-underline 1
 		set home $::env(HOME)
-		if {[is_Cygwin]} {
-			set home [exec cygpath --windows --absolute $home]
-		}
 		set home "[file normalize $home]/"
 		set hlen [string length $home]
 		foreach p $sorted_recent {
@@ -374,18 +371,6 @@ proc _objdir {path} {
 		return $objdir
 	}
 
-	if {[is_Cygwin]} {
-		set objdir [file join $path .git objects.lnk]
-		if {[file isfile $objdir]} {
-			return [win32_read_lnk $objdir]
-		}
-
-		set objdir [file join $path objects.lnk]
-		if {[file isfile $objdir]} {
-			return [win32_read_lnk $objdir]
-		}
-	}
-
 	return {}
 }
 
@@ -623,12 +608,6 @@ method _do_clone2 {} {
 	}
 
 	set giturl $origin_url
-	if {[is_Cygwin] && [file isdirectory $giturl]} {
-		set giturl [exec cygpath --unix --absolute $giturl]
-		if {$clone_type eq {shared}} {
-			set objdir [exec cygpath --unix --absolute $objdir]
-		}
-	}
 
 	if {[file exists $local_path]} {
 		error_popup [mc "Location %s already exists." $local_path]
@@ -668,11 +647,7 @@ method _do_clone2 {} {
 				fconfigure $f_cp -translation binary -encoding binary
 				cd $objdir
 				while {[gets $f_in line] >= 0} {
-					if {[is_Cygwin]} {
-						puts $f_cp [exec cygpath --unix --absolute $line]
-					} else {
-						puts $f_cp [file normalize $line]
-					}
+					puts $f_cp [file normalize $line]
 				}
 				close $f_in
 				close $f_cp
-- 
2.41.0.99.19

[PATCH v1 3/4] git-gui - use cygstart to browse on Cygwin

[Mark Levedahl]

git-gui enables the "Repository->Explore Working Copy" menu on Cygwin,
offering to open a Windows graphical file browser at the root of the
working directory. This code, shared with Git For Windows support,
depends upon use of Windows pathnames. However, git gui on Cygwin uses
unix pathnames, so this shared code will not work on Cygwin.

A base install of Cygwin provides the /bin/cygstart utility that runs
a registered Windows application based upon the file type, after
translating unix pathnames to Windows.  Adding the --explore option
guarantees that the Windows file explorer is opened, regardless of the
supplied pathname's file type and avoiding possibility of some other
action being taken.

So, teach git-gui to use cygstart --explore on Cygwin, restoring the
pre-2012 behavior of opening a Windows file explorer for browsing. This
separates the Git For Windows and Cygwin code paths. Note that
is_Windows is never true on Cygwin, and is_Cygwin is never true on Git
for Windows, though this is not obvious by examining the code for those
independent functions.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
changes since v0
 -- assumes the if/else tree being modified is untouched by prior
    patches making the changes minimal and easier to review.

 git-gui.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/git-gui.sh b/git-gui.sh
index 3f7c31e..8bc8892 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2274,7 +2274,9 @@ proc do_git_gui {} {
 
 # Get the system-specific explorer app/command.
 proc get_explorer {} {
-	if {[is_Cygwin] || [is_Windows]} {
+	if {[is_Cygwin]} {
+		set explorer "/bin/cygstart.exe --explore"
+	} elseif {[is_Windows]} {
 		set explorer "explorer.exe"
 	} elseif {[is_MacOSX]} {
 		set explorer "open"
-- 
2.41.0.99.19

[PATCH v1 4/4] git-gui - use mkshortcut on Cygwin

[Mark Levedahl]

git-gui enables the "Repository->Create Desktop Icon" item on Cygwin,
offering to create a shortcut that starts git-gui on the current
repository. The code in do_cygwin_shortcut invokes function
win32_create_lnk to create the shortcut. This latter function is shared
between Cygwin and Git For Windows and expects Windows rather than unix
pathnames, though do_cygwin_shortcut provides unix pathnames. Also, this
function tries to invoke the Windows Script Host to run a javascript
snippet, but this fails under Cygwin's Tcl. So, win32_create_lnk just
does not support Cygwin.

However, Cygwin's default installation provides /bin/mkshortcut for
creating desktop shortcuts. This is compatible with exec under Cygwin's
Tcl, understands Cygwin's unix pathnames, and avoids the need for shell
escapes to encode troublesome paths. So, teach git-gui to use mkshortcut
on Cygwin, leaving win32_create_lnk unchanged and for exclusive use by
Git For Windows.

Notes: "CHERE_INVOKING=1" is recognized by Cygwin's /etc/profile and
prevents a "chdir $HOME", leaving the shell in the working directory
specified by the shortcut. That directory is written directly by
mkshortcut eliminating any problems with shell escapes and quoting.

The code being replaced includes the full pathname of the git-gui
creating the shortcut, but that git-gui might not be compatible with the
git found after /etc/profile sets the path, and might have a pathname
that defies encoding using shell escapes that can survive the multiple
incompatible interpreters involved in the chain of creating and using
this shortcut.  The new code uses bare "git gui" as the command to
execute, thus using the system git to launch the system git-gui, and
avoiding both issues.

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
changes since v0
 -- assumes no changes to shortcut creation code in prior patches,
    so changes in this patch are easier to review.
 -- changed to use long option names for mkshortcut, better conforming
    to practice in git-gui overall.

 lib/shortcut.tcl | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl
index 97d1d7a..674a41f 100644
--- a/lib/shortcut.tcl
+++ b/lib/shortcut.tcl
@@ -27,13 +27,10 @@ proc do_windows_shortcut {} {
 }
 
 proc do_cygwin_shortcut {} {
-	global argv0 _gitworktree
+	global argv0 _gitworktree oguilib
 
 	if {[catch {
 		set desktop [exec cygpath \
-			--windows \
-			--absolute \
-			--long-name \
 			--desktop]
 		}]} {
 			set desktop .
@@ -48,19 +45,19 @@ proc do_cygwin_shortcut {} {
 			set fn ${fn}.lnk
 		}
 		if {[catch {
-				set sh [exec cygpath \
-					--windows \
-					--absolute \
-					/bin/sh.exe]
-				set me [exec cygpath \
-					--unix \
-					--absolute \
-					$argv0]
-				win32_create_lnk $fn [list \
-					$sh -c \
-					"CHERE_INVOKING=1 source /etc/profile;[sq $me] &" \
-					] \
-					[file normalize $_gitworktree]
+				set repodir [file normalize $_gitworktree]
+				set shargs {-c \
+					"CHERE_INVOKING=1 \
+					source /etc/profile; \
+					git gui"}
+				exec /bin/mkshortcut.exe \
+					--arguments $shargs \
+					--desc "git-gui on $repodir" \
+					--icon $oguilib/git-gui.ico \
+					--name $fn \
+					--show min \
+					--workingdir $repodir \
+					/bin/sh.exe
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
-- 
2.41.0.99.19

Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

On 6/26/23 11:52, Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>> I had originally organized as you suggest, no problem doing so
>> again. What gave me pause was this paragraph I originally wrote for
>> the cover letter:
>>
>> Patches 1/2 cause git-gui to function as it has for the last decade on
>> Cygwin, but with Cygwin being detected. However, the browsing and
>> shortcut creation menu items, removed in 2012 then re-added when is_Cygwin
>> was fixed, do not work, and shortcut creation will crash git-gui if used.
>> These are fixed in patches 3 / 4.
> As you are removing (ancient) Cygwin specific code that did not work
> with modern Cygwin at all in step [2/4], it is not so unexpected
> that some stuff does still not work after that step.  I am not sure
> what your reservation exactly is, but if you are wondering if code
> to disable browsing and shortcut creation on Cygwin temporarily
> needs to be there in the same step (instead of crashing or not
> working), it may make sense if and only if it is done with minimal
> changes.
>
> Thanks.
>
Timely response ... yes, that was my concern. I resolved this by making 
the cover letter and patch 2 commit message explicit that broken code 
remains.
Thank you,
Mark

Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui

[Johannes Schindelin]

Hi Mark,

On Mon, 26 Jun 2023, Mark Levedahl wrote:

> === This is an update, incorporating responses to Junio's and Eric's
> comments:
>   -- clarified what the "upstream" git-gui branch is
>   -- Removed some changes from patch 2 as requested by Junio, reducing
>      changes in patch 3 and patch 4
>        All code is fixed only after applying patch 4
>        Differences in patch 3 and 4 are minimimized
>    -- updated comments to clarify G4w dedicated code.
>    -- updated all comments to (hopefully) clarify points of confusion
> ===

And here is the range-diff:

    1:  00000000000 ! 1:  00000000000     git gui Makefile - remove Cygwin modifications
        @@ Metadata
         Author: Mark Levedahl <mlevedahl@gmail.com>

          ## Commit message ##
        -    git gui Makefile - remove Cygwin modiifications
        +    git gui Makefile - remove Cygwin modifications

             git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries
             into git-gui, destroying the ability to package git-gui on one machine and
        @@ Commit message
             since 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk.

             The Cygwin git maintainer disables this code, so this code is definitely
        -    not in use in the Cygwin distribution, and targets an untested /
        -    unsupportable configuration.
        +    not in use in the Cygwin distribution.

        -    The simplest approach is to just delete the Cygwin specific code as
        -    stock Cygwin needs no special handling. Do so.
        +    The simplest fix is to just delete the Cygwin specific code,
        +    allowing the Makefile to work out of the box on Cygwin. Do so.

             Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>

    2:  00000000000 ! 2:  00000000000     git-gui - remove obsolete Cygwin specific code
        @@ Commit message
             8.4.1 Windows Tcl/Tk code.  In March, 2012, that 8.4.1 package was
             replaced with a full port based upon the upstream unix/X11 code,
             since maintained up to date. The two Tcl/Tk packages are completely
        -    incompatible, and have different sygnatures.
        +    incompatible, and have different signatures.

             When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer
             detected Cygwin, so did not enable Cygwin specific code, and the POSIX
        @@ Commit message
             unix. Thus, no-one apparently noticed the existence of incompatible
             Cygwin specific code.

        -    However, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in
        -    upstream git-gui, the is_Cygwin funcion does detect current Cygwin.  The
        -    Cygwin specific code is enabled, causing use of Windows rather than unix
        -    pathnames, and enabling incorrect warnings about environment variables
        -    that are not relevant for the fully functional unix/X11 Tcl/Tk. The end
        -    result is that git-gui is now incommpatible with Cygwin.
        +    However, since commit c5766eae6f in the git-gui source tree
        +    (https://github.com/prati0100/git-gui, master at a5005ded), and not yet
        +    pulled into the git repository, the is_Cygwin function does detect
        +    Cygwin using the unix/X11 Tcl/Tk.  The Cygwin specific code is enabled,
        +    causing use of Windows rather than unix pathnames, and enabling
        +    incorrect warnings about environment variables that were relevant only
        +    to the old Tcl/Tk.  The end result is that (upstream) git-gui is now
        +    incompatible with Cygwin.

        -    So, delete all Cygwin specific code (code protected by "if is_Cygwin"),
        -    thus restoring the post-2012 behaviour. Note that Cygwin specific code
        -    is required to enable file browsing and shortcut creation (supported
        -    before 2012), but is not addressed in this patch.
        +    So, delete Cygwin specific code (code protected by "if is_Cygwin") that
        +    is not needed in any form to work with the unix/X11 Tcl/Tk.
        +
        +    Cygwin specific code required to enable file browsing and shortcut
        +    creation is not addressed in this patch, does not currently work, and
        +    invocation of those items may leave git-gui in a confused state.

             Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>

        @@ git-gui.sh: proc rescan {after {honor_trustmtime 1}} {
          }

          proc rescan_stage2 {fd after} {
        -@@ git-gui.sh: proc do_git_gui {} {
        -
        - # Get the system-specific explorer app/command.
        - proc get_explorer {} {
        --	if {[is_Cygwin] || [is_Windows]} {
        -+	if {[is_Windows]} {
        - 		set explorer "explorer.exe"
        - 	} elseif {[is_MacOSX]} {
        - 		set explorer "open"
        -@@ git-gui.sh: if {[is_enabled multicommit]} {
        -
        - 	.mbar.repository add separator
        -
        --	if {[is_Cygwin]} {
        --		.mbar.repository add command \
        --			-label [mc "Create Desktop Icon"] \
        --			-command do_cygwin_shortcut
        --	} elseif {[is_Windows]} {
        -+	if {[is_Windows]} {
        - 		.mbar.repository add command \
        - 			-label [mc "Create Desktop Icon"] \
        - 			-command do_windows_shortcut
         @@ git-gui.sh: if {[is_MacOSX]} {
          set doc_path [githtmldir]
          if {$doc_path ne {}} {
        @@ lib/choose_repository.tcl: method _do_clone2 {} {
          				}
          				close $f_in
          				close $f_cp
        -
        - ## lib/shortcut.tcl ##
        -@@ lib/shortcut.tcl: proc do_windows_shortcut {} {
        - 	}
        - }
        -
        --proc do_cygwin_shortcut {} {
        --	global argv0 _gitworktree
        --
        --	if {[catch {
        --		set desktop [exec cygpath \
        --			--windows \
        --			--absolute \
        --			--long-name \
        --			--desktop]
        --		}]} {
        --			set desktop .
        --	}
        --	set fn [tk_getSaveFile \
        --		-parent . \
        --		-title [mc "%s (%s): Create Desktop Icon" [appname] [reponame]] \
        --		-initialdir $desktop \
        --		-initialfile "Git [reponame].lnk"]
        --	if {$fn != {}} {
        --		if {[file extension $fn] ne {.lnk}} {
        --			set fn ${fn}.lnk
        --		}
        --		if {[catch {
        --				set sh [exec cygpath \
        --					--windows \
        --					--absolute \
        --					/bin/sh.exe]
        --				set me [exec cygpath \
        --					--unix \
        --					--absolute \
        --					$argv0]
        --				win32_create_lnk $fn [list \
        --					$sh -c \
        --					"CHERE_INVOKING=1 source /etc/profile;[sq $me] &" \
        --					] \
        --					[file normalize $_gitworktree]
        --			} err]} {
        --			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
        --		}
        --	}
        --}
        --
        - proc do_macosx_app {} {
        - 	global argv0 env
        -
    3:  00000000000 ! 3:  00000000000     git-gui - use cygstart to browse on Cygwin
        @@ Metadata
          ## Commit message ##
             git-gui - use cygstart to browse on Cygwin

        -    Pre-2012, git-gui enabled the "Repository->Explore Working Copy" menu on
        -    Cygwin, offering open a Windows graphical file browser at the root
        -    working directory. The old code relied upon internal use of Windows
        -    pathnames, while git-gui must use unix pathnames on Cygwin since 2012,
        -    so was removed in a previous patch.
        +    git-gui enables the "Repository->Explore Working Copy" menu on Cygwin,
        +    offering to open a Windows graphical file browser at the root of the
        +    working directory. This code, shared with Git For Windows support,
        +    depends upon use of Windows pathnames. However, git gui on Cygwin uses
        +    unix pathnames, so this shared code will not work on Cygwin.

             A base install of Cygwin provides the /bin/cygstart utility that runs
        -    arbtitrary Windows applications after translating unix pathnames to
        -    Windows.  Adding the --explore option guarantees that the Windows file
        -    explorer is opened, regardless of the supplied pathname's file type and
        -    avoiding possibility of some other action being taken.
        +    a registered Windows application based upon the file type, after
        +    translating unix pathnames to Windows.  Adding the --explore option
        +    guarantees that the Windows file explorer is opened, regardless of the
        +    supplied pathname's file type and avoiding possibility of some other
        +    action being taken.

             So, teach git-gui to use cygstart --explore on Cygwin, restoring the
        -    pre-2012 behavior of opening a Windows file explorer for browsing.
        +    pre-2012 behavior of opening a Windows file explorer for browsing. This
        +    separates the Git For Windows and Cygwin code paths. Note that
        +    is_Windows is never true on Cygwin, and is_Cygwin is never true on Git
        +    for Windows, though this is not obvious by examining the code for those
        +    independent functions.

             Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>

          ## git-gui.sh ##
         @@ git-gui.sh: proc do_git_gui {} {
        +
        + # Get the system-specific explorer app/command.
          proc get_explorer {} {
        - 	if {[is_Windows]} {
        - 		set explorer "explorer.exe"
        -+	} elseif {[is_Cygwin]} {
        +-	if {[is_Cygwin] || [is_Windows]} {
        ++	if {[is_Cygwin]} {
         +		set explorer "/bin/cygstart.exe --explore"
        ++	} elseif {[is_Windows]} {
        + 		set explorer "explorer.exe"
          	} elseif {[is_MacOSX]} {
          		set explorer "open"
        - 	} else {
    4:  00000000000 ! 4:  00000000000     git-gui - use mkshortcut on Cygwin
        @@ Metadata
          ## Commit message ##
             git-gui - use mkshortcut on Cygwin

        -    Prior to 2012, git-gui enabled the "Repository->Create Desktop Icon"
        -    item on Cygwin, offering to create a shortcut that starts git-gui on a
        -    particular repository. The original code for this in lib/win32.tcl,
        -    shared with Git for Windows support, requires Windows pathnames, while
        -    git-gui must use unix pathnames with the unix/X11 Tcl/Tk since 2012. The
        -    ability to use this from Cygwin was removed in a previous patch.
        +    git-gui enables the "Repository->Create Desktop Icon" item on Cygwin,
        +    offering to create a shortcut that starts git-gui on the current
        +    repository. The code in do_cygwin_shortcut invokes function
        +    win32_create_lnk to create the shortcut. This latter function is shared
        +    between Cygwin and Git For Windows and expects Windows rather than unix
        +    pathnames, though do_cygwin_shortcut provides unix pathnames. Also, this
        +    function tries to invoke the Windows Script Host to run a javascript
        +    snippet, but this fails under Cygwin's Tcl. So, win32_create_lnk just
        +    does not support Cygwin.

        -    Cygwin's default installation provides /bin/mkshortcut for creating
        -    desktop shortuts, this is compatible with exec under tcl, and understands
        -    Cygwin's unix pathnames. So, teach git-gui to use mkshortcut on Cygwin,
        -    leaving lib/win32.tcl as Git for Windows specific support.
        +    However, Cygwin's default installation provides /bin/mkshortcut for
        +    creating desktop shortcuts. This is compatible with exec under Cygwin's
        +    Tcl, understands Cygwin's unix pathnames, and avoids the need for shell
        +    escapes to encode troublesome paths. So, teach git-gui to use mkshortcut
        +    on Cygwin, leaving win32_create_lnk unchanged and for exclusive use by
        +    Git For Windows.

             Notes: "CHERE_INVOKING=1" is recognized by Cygwin's /etc/profile and
             prevents a "chdir $HOME", leaving the shell in the working directory
             specified by the shortcut. That directory is written directly by
             mkshortcut eliminating any problems with shell escapes and quoting.

        -    The pre-2012 code includes the full pathname of the git-gui creating the
        -    shortcut (rather than using the system git-gui), but that git-gui might
        -    not be compatible with the git found after /etc/profile sets the path,
        -    and might have a pathname that defies encoding using shell escapes that
        -    can survive the multiple incompatible interpreters involved in this
        -    chain. Instead, use "git gui", thus defaulting to the system git and
        +    The code being replaced includes the full pathname of the git-gui
        +    creating the shortcut, but that git-gui might not be compatible with the
        +    git found after /etc/profile sets the path, and might have a pathname
        +    that defies encoding using shell escapes that can survive the multiple
        +    incompatible interpreters involved in the chain of creating and using
        +    this shortcut.  The new code uses bare "git gui" as the command to
        +    execute, thus using the system git to launch the system git-gui, and
             avoiding both issues.

             Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>

        - ## git-gui.sh ##
        -@@ git-gui.sh: if {[is_enabled multicommit]} {
        - 		.mbar.repository add command \
        - 			-label [mc "Create Desktop Icon"] \
        - 			-command do_windows_shortcut
        -+	} elseif {[is_Cygwin]} {
        -+		.mbar.repository add command \
        -+			-label [mc "Create Desktop Icon"] \
        -+			-command do_cygwin_shortcut
        - 	} elseif {[is_MacOSX]} {
        - 		.mbar.repository add command \
        - 			-label [mc "Create Desktop Icon"] \
        -
          ## lib/shortcut.tcl ##
         @@ lib/shortcut.tcl: proc do_windows_shortcut {} {
        - 	}
          }

        -+proc do_cygwin_shortcut {} {
        + proc do_cygwin_shortcut {} {
        +-	global argv0 _gitworktree
         +	global argv0 _gitworktree oguilib
        -+
        -+	if {[catch {
        -+		set desktop [exec cygpath \
        -+			--desktop]
        -+		}]} {
        -+			set desktop .
        -+	}
        -+	set fn [tk_getSaveFile \
        -+		-parent . \
        -+		-title [mc "%s (%s): Create Desktop Icon" [appname] [reponame]] \
        -+		-initialdir $desktop \
        -+		-initialfile "Git [reponame].lnk"]
        -+	if {$fn != {}} {
        -+		if {[file extension $fn] ne {.lnk}} {
        -+			set fn ${fn}.lnk
        -+		}
        -+		if {[catch {
        +
        + 	if {[catch {
        + 		set desktop [exec cygpath \
        +-			--windows \
        +-			--absolute \
        +-			--long-name \
        + 			--desktop]
        + 		}]} {
        + 			set desktop .
        +@@ lib/shortcut.tcl: proc do_cygwin_shortcut {} {
        + 			set fn ${fn}.lnk
        + 		}
        + 		if {[catch {
        +-				set sh [exec cygpath \
        +-					--windows \
        +-					--absolute \
        +-					/bin/sh.exe]
        +-				set me [exec cygpath \
        +-					--unix \
        +-					--absolute \
        +-					$argv0]
        +-				win32_create_lnk $fn [list \
        +-					$sh -c \
        +-					"CHERE_INVOKING=1 source /etc/profile;[sq $me] &" \
        +-					] \
        +-					[file normalize $_gitworktree]
         +				set repodir [file normalize $_gitworktree]
         +				set shargs {-c \
         +					"CHERE_INVOKING=1 \
         +					source /etc/profile; \
         +					git gui"}
         +				exec /bin/mkshortcut.exe \
        -+					-a $shargs \
        -+					-d "git-gui on $repodir" \
        -+					-i $oguilib/git-gui.ico \
        -+					-n $fn \
        -+					-s min \
        -+					-w $repodir \
        ++					--arguments $shargs \
        ++					--desc "git-gui on $repodir" \
        ++					--icon $oguilib/git-gui.ico \
        ++					--name $fn \
        ++					--show min \
        ++					--workingdir $repodir \
         +					/bin/sh.exe
        -+			} err]} {
        -+			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
        -+		}
        -+	}
        -+}
        -+
        - proc do_macosx_app {} {
        - 	global argv0 env
        -
        + 			} err]} {
        + 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
        + 		}

FWIW even v1 looked good to me (I don't care about typos as long as they
don't change meaning).

I tested the changes on top of Git for Windows and everything works as
expected (if you want to cross check my work, look no further than the
`git-gui-cygwin` branch at https://github.com/dscho/git).

If you want, please feel free to add

	Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Thank you!
Johannes

Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> === This is an update, incorporating responses to Junio's and Eric's
> comments:
>   -- clarified what the "upstream" git-gui branch is
>   -- Removed some changes from patch 2 as requested by Junio, reducing
>      changes in patch 3 and patch 4
>        All code is fixed only after applying patch 4
>        Differences in patch 3 and 4 are minimimized
>    -- updated comments to clarify G4w dedicated code.
>    -- updated all comments to (hopefully) clarify points of confusion
> ===
> ...
> Mark Levedahl (4):
>   git gui Makefile - remove Cygwin modifications
>   git-gui - remove obsolete Cygwin specific code
>   git-gui - use cygstart to browse on Cygwin
>   git-gui - use mkshortcut on Cygwin
>
>  Makefile                  |  21 +------
>  git-gui.sh                | 118 +++-----------------------------------
>  lib/choose_repository.tcl |  27 +--------
>  lib/shortcut.tcl          |  31 +++++-----
>  4 files changed, 27 insertions(+), 170 deletions(-)

OK, Dscho says v1 looks good, and I have no further comments.

Pratyush, can I expect that you take further comments and usher
these patches to your tree, and eventually tell me to pull from your
repository?

Thanks, all.

Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

On 6/27/23 13:52, Junio C Hamano wrote:
> Mark Levedahl <mlevedahl@gmail.com> writes:
>
>> === This is an update, incorporating responses to Junio's and Eric's
>> comments:
>>    -- clarified what the "upstream" git-gui branch is
>>    -- Removed some changes from patch 2 as requested by Junio, reducing
>>       changes in patch 3 and patch 4
>>         All code is fixed only after applying patch 4
>>         Differences in patch 3 and 4 are minimimized
>>     -- updated comments to clarify G4w dedicated code.
>>     -- updated all comments to (hopefully) clarify points of confusion
>> ===
>> ...
>> Mark Levedahl (4):
>>    git gui Makefile - remove Cygwin modifications
>>    git-gui - remove obsolete Cygwin specific code
>>    git-gui - use cygstart to browse on Cygwin
>>    git-gui - use mkshortcut on Cygwin
>>
>>   Makefile                  |  21 +------
>>   git-gui.sh                | 118 +++-----------------------------------
>>   lib/choose_repository.tcl |  27 +--------
>>   lib/shortcut.tcl          |  31 +++++-----
>>   4 files changed, 27 insertions(+), 170 deletions(-)
> OK, Dscho says v1 looks good, and I have no further comments.
>
> Pratyush, can I expect that you take further comments and usher
> these patches to your tree, and eventually tell me to pull from your
> repository?
>
> Thanks, all.

Junio,

Thank you and Dscho for the detailed reviews. But, there is no response 
from Pratyush in over a month, is there a different maintainer then who 
should take this?

Mark

Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui

[Pratyush Yadav]

On Sat, Aug 05 2023, Mark Levedahl wrote:

> On 6/27/23 13:52, Junio C Hamano wrote:
>> Mark Levedahl <mlevedahl@gmail.com> writes:
>>
>>> === This is an update, incorporating responses to Junio's and Eric's
>>> comments:
>>>    -- clarified what the "upstream" git-gui branch is
>>>    -- Removed some changes from patch 2 as requested by Junio, reducing
>>>       changes in patch 3 and patch 4
>>>         All code is fixed only after applying patch 4
>>>         Differences in patch 3 and 4 are minimimized
>>>     -- updated comments to clarify G4w dedicated code.
>>>     -- updated all comments to (hopefully) clarify points of confusion
>>> ===
>>> ...
>>> Mark Levedahl (4):
>>>    git gui Makefile - remove Cygwin modifications
>>>    git-gui - remove obsolete Cygwin specific code
>>>    git-gui - use cygstart to browse on Cygwin
>>>    git-gui - use mkshortcut on Cygwin
>>>
>>>   Makefile                  |  21 +------
>>>   git-gui.sh                | 118 +++-----------------------------------
>>>   lib/choose_repository.tcl |  27 +--------
>>>   lib/shortcut.tcl          |  31 +++++-----
>>>   4 files changed, 27 insertions(+), 170 deletions(-)
>> OK, Dscho says v1 looks good, and I have no further comments.
>>
>> Pratyush, can I expect that you take further comments and usher
>> these patches to your tree, and eventually tell me to pull from your
>> repository?
>>
>> Thanks, all.
>
> Junio,
>
> Thank you and Dscho for the detailed reviews. But, there is no response from
> Pratyush in over a month, is there a different maintainer then who should take
> this?

Almost 2 months now... I'm sorry. I just do not find enough time or
energy for git-gui these days. More on that later.

For now, I took a brief look at the patches. They look good to me. I
appreciate the detailed commit messages. I did not test them since I do
not have a Windows setup currently, but I believe Johannes did so it's
all good for me.

Applied to git-gui/master. Will send a pull request soon.

-- 
Regards,
Pratyush Yadav

Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui

[Mark Levedahl]

On 6/27/23 13:52, Junio C Hamano wrote:
> OK, Dscho says v1 looks good, and I have no further comments.
>
> Pratyush, can I expect that you take further comments and usher
> these patches to your tree, and eventually tell me to pull from your
> repository?
>
> Thanks, all.

Junio,

I see you merged latest git-gui from Pratyush onto next. As git-gui has 
no test facility I merged your py/git-gui-updates (a7935203) into your 
master (5dc72c0f), built on both Linux and Cygwin. git-gui works as 
expected on both. Looks good to me.

Mark

Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui

[Junio C Hamano]

Mark Levedahl <mlevedahl@gmail.com> writes:

> On 6/27/23 13:52, Junio C Hamano wrote:
>> OK, Dscho says v1 looks good, and I have no further comments.
>>
>> Pratyush, can I expect that you take further comments and usher
>> these patches to your tree, and eventually tell me to pull from your
>> repository?
>>
>> Thanks, all.
>
> Junio,
>
> I see you merged latest git-gui from Pratyush onto next. As git-gui
> has no test facility I merged your py/git-gui-updates (a7935203) into
> your master (5dc72c0f), built on both Linux and Cygwin. git-gui works
> as expected on both. Looks good to me.

Thanks!