{"thread":{"id":"59909","subject":"[PATCH v0 1/4] git gui Makefile - remove Cygwin modiifications","startedAt":"2023-06-24T21:23:55Z","lastAt":"2023-08-29T16:19:52Z","messageCount":27,"participants":["Mark Levedahl","Junio C Hamano","Eric Sunshine","Johannes Schindelin","Pratyush Yadav"],"isPatch":true,"patchVersion":0,"patchTotal":4},"messages":[{"id":"478771","messageId":"20230624212347.179656-2-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230624212347.179656-1-mlevedahl@gmail.com","subject":"[PATCH v0 1/4] git gui Makefile - remove Cygwin modiifications","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-24T21:23:44Z","receivedAt":"2023-06-24T21:23:55Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries\ninto git-gui, destroying the ability to package git-gui on one machine and\ndistribute to others. The intent is to do this only if a non-Cygwin Tcl/Tk is\ninstalled, but the test for this is wrong with the unix/X11 Tcl/Tk shipped\nsince 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk.\n\nThe Cygwin git maintainer disables this code, so this code is definitely\nnot in use in the Cygwin distribution, and targets an untested /\nunsupportable configuration.\n\nThe simplest approach is to just delete the Cygwin specific code as\nstock Cygwin needs no special handling. Do so.\n\nSigned-off-by: Mark Levedahl \n---\n Makefile | 21 +++------------------\n 1 file changed, 3 insertions(+), 18 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex a0d5a4b..3f80435 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -138,25 +138,10 @@ GITGUI_SCRIPT := $$0\n GITGUI_RELATIVE :=\n GITGUI_MACOSXAPP :=\n \n-ifeq ($(uname_O),Cygwin)\n-\tGITGUI_SCRIPT := `cygpath --windows --absolute \"$(GITGUI_SCRIPT)\"`\n-\n-\t# Is this a Cygwin Tcl/Tk binary? If so it knows how to do\n-\t# POSIX path translation just like cygpath does and we must\n-\t# keep libdir in POSIX format so Cygwin packages of git-gui\n-\t# work no matter where the user installs them.\n-\t#\n-\tifeq ($(shell echo 'puts [file normalize /]' | '$(TCL_PATH_SQ)'),$(shell cygpath --mixed --absolute /))\n-\t\tgg_libdir_sed_in := $(gg_libdir)\n-\telse\n-\t\tgg_libdir_sed_in := $(shell cygpath --windows --absolute \"$(gg_libdir)\")\n-\tendif\n-else\n-\tifeq ($(exedir),$(gg_libdir))\n-\t\tGITGUI_RELATIVE := 1\n-\tendif\n-\tgg_libdir_sed_in := $(gg_libdir)\n+ifeq ($(exedir),$(gg_libdir))\n+\tGITGUI_RELATIVE := 1\n endif\n+gg_libdir_sed_in := $(gg_libdir)\n ifeq ($(uname_S),Darwin)\n \tifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)\n \t\tGITGUI_MACOSXAPP := YesPlease\n-- \n2.41.0.99.19\n\n"},{"id":"478772","messageId":"20230624212347.179656-1-mlevedahl@gmail.com","threadId":"59909","inReplyTo":null,"subject":"[PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-24T21:23:43Z","receivedAt":"2023-06-24T21:23:55Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"git-gui has many snippets of code guarded by an is_Cygwin test, all of\nwhich target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed in\nMarch 2012. That is when Cygwin switched to a well-supported unix/X11\nTcl/Tk package. 64-bit Cygwin was released later so has always had the\nunix/X11 package. git-gui runs as-is on more recent Cygwin, treating it\nas a Linux variant, though two functions are disabled.\n\nThe old Tcl/Tk understood Windows pathnames, so git-gui's Cygwin\nspecific code uses Windows pathnames. The unix/X11 code requires use of\nunix pathnames, so none of the Cygwin specific code is compatible, and\nall should be removed.\n\nFortunately, the is_Cygwin funcion in git-gui (on the git master branch)\nrelies upon the old Tcl/Tk and doesn't detect Cygwin. But, commit\nc5766eae6f2b002396b6cd4f85b62317b707174e on the git-gui master branch\n\"fixed\" is_Cygwin, enabling the incompatible code, so upstream git-gui\nis now broken on Cygwin.\n\nThere is Cygwin specific code in the Makefile, intended to allow a\ncompletely unsupported configuration with a Windows TclTk. This code\nmisdetects the configuration, creating a non-portable installation. The\nCygwin git maintainer comments this code out. The code should be\nremoved.\n\nThe existing code for file browsing and creating a desktop icon is\nshared with Git For Windows support, and uses Windows pathnames. This\ncode does not work on Cygwin, and needs replacement. These functions\nhave not worked since 2012.\n\npatch 1 removes the obsolete Makefile code\npatch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...\npatch 3 implements Cygwin specific file browsing support\npatch 4 implemetns Cygwin specific desktop icon support\n\nPatches 1/2 cause git-gui to function as it has for the last decade on\nCygwin, but without bugs masking bugs. Patches 3/4 restore functionality\nlost with the Tcl/Tk switch in 2012.\n\nAny argument for keeping the old Cygwin code must address who is going\nto test and maintain that code, on what platform, and who the target\naudience is. The old Tcl/Tk was only on 32-bit Cygwin and only supported\nfor the Insight debugger, 32-bit Cygwin is no longer supported, git-gui\nis not supported on 8.4.1 Tcl/Tk, and the Windows versions targeted by\n2012'ish 32-bit Cygwin are no longer supported.\n\nMark Levedahl (4):\n git gui Makefile - remove Cygwin modiifications\n git-gui - remove obsolete Cygwin specific code\n git-gui - use cygstart to browse on Cygwin\n git-gui - use mkshortcut on Cygwin\n\n Makefile | 21 +------\n git-gui.sh | 126 ++++----------------------------------\n lib/choose_repository.tcl | 27 +-------\n lib/shortcut.tcl | 31 +++++-----\n 4 files changed, 31 insertions(+), 174 deletions(-)\n\n-- \n2.41.0.99.19\n\n"},{"id":"478773","messageId":"20230624212347.179656-3-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230624212347.179656-1-mlevedahl@gmail.com","subject":"[PATCH v0 2/4] git-gui - remove obsolete Cygwin specific code","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-24T21:23:45Z","receivedAt":"2023-06-24T21:23:58Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"In the current git release, git-gui runs on Cygwin without enabling any\nof git-gui's Cygwin specific code. This happens as the Cygwin specific\ncode in git-gui was (mostly) written in 2007-2008 to work with Cygwin's\nthen supplied Tcl/Tk which was an incompletely ported variant of the\n8.4.1 Windows Tcl/Tk code. In March, 2012, that 8.4.1 package was\nreplaced with a full port based upon the upstream unix/X11 code,\nsince maintained up to date. The two Tcl/Tk packages are completely\nincompatible, and have different sygnatures.\n\nWhen Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer\ndetected Cygwin, so did not enable Cygwin specific code, and the POSIX\nenvironment provided by Cygwin since 2012 supported git-gui as a generic\nunix. Thus, no-one apparently noticed the existence of incompatible\nCygwin specific code.\n\nHowever, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in\nupstream git-gui, the is_Cygwin funcion does detect current Cygwin. The\nCygwin specific code is enabled, causing use of Windows rather than unix\npathnames, and enabling incorrect warnings about environment variables\nthat are not relevant for the fully functional unix/X11 Tcl/Tk. The end\nresult is that git-gui is now incommpatible with Cygwin.\n\nSo, delete all Cygwin specific code (code protected by \"if is_Cygwin\"),\nthus restoring the post-2012 behaviour. Note that Cygwin specific code\nis required to enable file browsing and shortcut creation (supported\nbefore 2012), but is not addressed in this patch.\n\nSigned-off-by: Mark Levedahl \n---\n git-gui.sh | 122 +++-----------------------------------\n lib/choose_repository.tcl | 27 +--------\n lib/shortcut.tcl | 41 -------------\n 3 files changed, 9 insertions(+), 181 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex cb92bba..b5dba80 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -84,14 +84,7 @@ proc _which {what args} {\n \tglobal env _search_exe _search_path\n \n \tif {$_search_path eq {}} {\n-\t\tif {[is_Cygwin] && [regexp {^(/|\\.:)} $env(PATH)]} {\n-\t\t\tset _search_path [split [exec cygpath \\\n-\t\t\t\t--windows \\\n-\t\t\t\t--path \\\n-\t\t\t\t--absolute \\\n-\t\t\t\t$env(PATH)] {;}]\n-\t\t\tset _search_exe .exe\n-\t\t} elseif {[is_Windows]} {\n+\t\tif {[is_Windows]} {\n \t\t\tset gitguidir [file dirname [info script]]\n \t\t\tregsub -all \";\" $gitguidir \"\\\\;\" gitguidir\n \t\t\tset env(PATH) \"$gitguidir;$env(PATH)\"\n@@ -342,14 +335,7 @@ proc gitexec {args} {\n \t\tif {[catch {set _gitexec [git --exec-path]} err]} {\n \t\t\terror \"Git not installed?\\n\\n$err\"\n \t\t}\n-\t\tif {[is_Cygwin]} {\n-\t\t\tset _gitexec [exec cygpath \\\n-\t\t\t\t--windows \\\n-\t\t\t\t--absolute \\\n-\t\t\t\t$_gitexec]\n-\t\t} else {\n-\t\t\tset _gitexec [file normalize $_gitexec]\n-\t\t}\n+\t\tset _gitexec [file normalize $_gitexec]\n \t}\n \tif {$args eq {}} {\n \t\treturn $_gitexec\n@@ -364,14 +350,7 @@ proc githtmldir {args} {\n \t\t\t# Git not installed or option not yet supported\n \t\t\treturn {}\n \t\t}\n-\t\tif {[is_Cygwin]} {\n-\t\t\tset _githtmldir [exec cygpath \\\n-\t\t\t\t--windows \\\n-\t\t\t\t--absolute \\\n-\t\t\t\t$_githtmldir]\n-\t\t} else {\n-\t\t\tset _githtmldir [file normalize $_githtmldir]\n-\t\t}\n+\t\tset _githtmldir [file normalize $_githtmldir]\n \t}\n \tif {$args eq {}} {\n \t\treturn $_githtmldir\n@@ -1318,9 +1297,6 @@ if {$_gitdir eq \".\"} {\n \tset _gitdir [pwd]\n }\n \n-if {![file isdirectory $_gitdir] && [is_Cygwin]} {\n-\tcatch {set _gitdir [exec cygpath --windows $_gitdir]}\n-}\n if {![file isdirectory $_gitdir]} {\n \tcatch {wm withdraw .}\n \terror_popup [strcat [mc \"Git directory not found:\"] \"\\n\\n$_gitdir\"]\n@@ -1332,11 +1308,7 @@ apply_config\n \n # v1.7.0 introduced --show-toplevel to return the canonical work-tree\n if {[package vcompare $_git_version 1.7.0] >= 0} {\n-\tif { [is_Cygwin] } {\n-\t\tcatch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}\n-\t} else {\n-\t\tset _gitworktree [git rev-parse --show-toplevel]\n-\t}\n+\tset _gitworktree [git rev-parse --show-toplevel]\n } else {\n \t# try to set work tree from environment, core.worktree or use\n \t# cdup to obtain a relative path to the top of the worktree. If\n@@ -1561,24 +1533,8 @@ proc rescan {after {honor_trustmtime 1}} {\n \t}\n }\n \n-if {[is_Cygwin]} {\n-\tset is_git_info_exclude {}\n-\tproc have_info_exclude {} {\n-\t\tglobal is_git_info_exclude\n-\n-\t\tif {$is_git_info_exclude eq {}} {\n-\t\t\tif {[catch {exec test -f [gitdir info exclude]}]} {\n-\t\t\t\tset is_git_info_exclude 0\n-\t\t\t} else {\n-\t\t\t\tset is_git_info_exclude 1\n-\t\t\t}\n-\t\t}\n-\t\treturn $is_git_info_exclude\n-\t}\n-} else {\n-\tproc have_info_exclude {} {\n-\t\treturn [file readable [gitdir info exclude]]\n-\t}\n+proc have_info_exclude {} {\n+\treturn [file readable [gitdir info exclude]]\n }\n \n proc rescan_stage2 {fd after} {\n@@ -2318,7 +2274,7 @@ proc do_git_gui {} {\n \n # Get the system-specific explorer app/command.\n proc get_explorer {} {\n-\tif {[is_Cygwin] || [is_Windows]} {\n+\tif {[is_Windows]} {\n \t\tset explorer \"explorer.exe\"\n \t} elseif {[is_MacOSX]} {\n \t\tset explorer \"open\"\n@@ -2874,11 +2830,7 @@ if {[is_enabled multicommit]} {\n \n \t.mbar.repository add separator\n \n-\tif {[is_Cygwin]} {\n-\t\t.mbar.repository add command \\\n-\t\t\t-label [mc \"Create Desktop Icon\"] \\\n-\t\t\t-command do_cygwin_shortcut\n-\t} elseif {[is_Windows]} {\n+\tif {[is_Windows]} {\n \t\t.mbar.repository add command \\\n \t\t\t-label [mc \"Create Desktop Icon\"] \\\n \t\t\t-command do_windows_shortcut\n@@ -3112,10 +3064,6 @@ if {[is_MacOSX]} {\n set doc_path [githtmldir]\n if {$doc_path ne {}} {\n \tset doc_path [file join $doc_path index.html]\n-\n-\tif {[is_Cygwin]} {\n-\t\tset doc_path [exec cygpath --mixed $doc_path]\n-\t}\n }\n \n if {[file isfile $doc_path]} {\n@@ -4087,60 +4035,6 @@ set file_lists($ui_workdir) [list]\n wm title . \"[appname] ([reponame]) [file normalize $_gitworktree]\"\n focus -force $ui_comm\n \n-# -- Warn the user about environmental problems. Cygwin's Tcl\n-# does *not* pass its env array onto any processes it spawns.\n-# This means that git processes get none of our environment.\n-#\n-if {[is_Cygwin]} {\n-\tset ignored_env 0\n-\tset suggest_user {}\n-\tset msg [mc \"Possible environment issues exist.\n-\n-The following environment variables are probably\n-going to be ignored by any Git subprocess run\n-by %s:\n-\n-\" [appname]]\n-\tforeach name [array names env] {\n-\t\tswitch -regexp -- $name {\n-\t\t{^GIT_INDEX_FILE$} -\n-\t\t{^GIT_OBJECT_DIRECTORY$} -\n-\t\t{^GIT_ALTERNATE_OBJECT_DIRECTORIES$} -\n-\t\t{^GIT_DIFF_OPTS$} -\n-\t\t{^GIT_EXTERNAL_DIFF$} -\n-\t\t{^GIT_PAGER$} -\n-\t\t{^GIT_TRACE$} -\n-\t\t{^GIT_CONFIG$} -\n-\t\t{^GIT_(AUTHOR|COMMITTER)_DATE$} {\n-\t\t\tappend msg \" - $name\\n\"\n-\t\t\tincr ignored_env\n-\t\t}\n-\t\t{^GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL)$} {\n-\t\t\tappend msg \" - $name\\n\"\n-\t\t\tincr ignored_env\n-\t\t\tset suggest_user $name\n-\t\t}\n-\t\t}\n-\t}\n-\tif {$ignored_env > 0} {\n-\t\tappend msg [mc \"\n-This is due to a known issue with the\n-Tcl binary distributed by Cygwin.\"]\n-\n-\t\tif {$suggest_user ne {}} {\n-\t\t\tappend msg [mc \"\n-\n-A good replacement for %s\n-is placing values for the user.name and\n-user.email settings into your personal\n-~/.gitconfig file.\n-\" $suggest_user]\n-\t\t}\n-\t\twarn_popup $msg\n-\t}\n-\tunset ignored_env msg suggest_user name\n-}\n-\n # -- Only initialize complex UI if we are going to stay running.\n #\n if {[is_enabled transport]} {\ndiff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\nindex af1fee7..d23abed 100644\n--- a/lib/choose_repository.tcl\n+++ b/lib/choose_repository.tcl\n@@ -174,9 +174,6 @@ constructor pick {} {\n \t\t\t-foreground blue \\\n \t\t\t-underline 1\n \t\tset home $::env(HOME)\n-\t\tif {[is_Cygwin]} {\n-\t\t\tset home [exec cygpath --windows --absolute $home]\n-\t\t}\n \t\tset home \"[file normalize $home]/\"\n \t\tset hlen [string length $home]\n \t\tforeach p $sorted_recent {\n@@ -374,18 +371,6 @@ proc _objdir {path} {\n \t\treturn $objdir\n \t}\n \n-\tif {[is_Cygwin]} {\n-\t\tset objdir [file join $path .git objects.lnk]\n-\t\tif {[file isfile $objdir]} {\n-\t\t\treturn [win32_read_lnk $objdir]\n-\t\t}\n-\n-\t\tset objdir [file join $path objects.lnk]\n-\t\tif {[file isfile $objdir]} {\n-\t\t\treturn [win32_read_lnk $objdir]\n-\t\t}\n-\t}\n-\n \treturn {}\n }\n \n@@ -623,12 +608,6 @@ method _do_clone2 {} {\n \t}\n \n \tset giturl $origin_url\n-\tif {[is_Cygwin] && [file isdirectory $giturl]} {\n-\t\tset giturl [exec cygpath --unix --absolute $giturl]\n-\t\tif {$clone_type eq {shared}} {\n-\t\t\tset objdir [exec cygpath --unix --absolute $objdir]\n-\t\t}\n-\t}\n \n \tif {[file exists $local_path]} {\n \t\terror_popup [mc \"Location %s already exists.\" $local_path]\n@@ -668,11 +647,7 @@ method _do_clone2 {} {\n \t\t\t\tfconfigure $f_cp -translation binary -encoding binary\n \t\t\t\tcd $objdir\n \t\t\t\twhile {[gets $f_in line] >= 0} {\n-\t\t\t\t\tif {[is_Cygwin]} {\n-\t\t\t\t\t\tputs $f_cp [exec cygpath --unix --absolute $line]\n-\t\t\t\t\t} else {\n-\t\t\t\t\t\tputs $f_cp [file normalize $line]\n-\t\t\t\t\t}\n+\t\t\t\t\tputs $f_cp [file normalize $line]\n \t\t\t\t}\n \t\t\t\tclose $f_in\n \t\t\t\tclose $f_cp\ndiff --git a/lib/shortcut.tcl b/lib/shortcut.tcl\nindex 97d1d7a..1d8374b 100644\n--- a/lib/shortcut.tcl\n+++ b/lib/shortcut.tcl\n@@ -26,47 +26,6 @@ proc do_windows_shortcut {} {\n \t}\n }\n \n-proc do_cygwin_shortcut {} {\n-\tglobal argv0 _gitworktree\n-\n-\tif {[catch {\n-\t\tset desktop [exec cygpath \\\n-\t\t\t--windows \\\n-\t\t\t--absolute \\\n-\t\t\t--long-name \\\n-\t\t\t--desktop]\n-\t\t}]} {\n-\t\t\tset desktop .\n-\t}\n-\tset fn [tk_getSaveFile \\\n-\t\t-parent . \\\n-\t\t-title [mc \"%s (%s): Create Desktop Icon\" [appname] [reponame]] \\\n-\t\t-initialdir $desktop \\\n-\t\t-initialfile \"Git [reponame].lnk\"]\n-\tif {$fn != {}} {\n-\t\tif {[file extension $fn] ne {.lnk}} {\n-\t\t\tset fn ${fn}.lnk\n-\t\t}\n-\t\tif {[catch {\n-\t\t\t\tset sh [exec cygpath \\\n-\t\t\t\t\t--windows \\\n-\t\t\t\t\t--absolute \\\n-\t\t\t\t\t/bin/sh.exe]\n-\t\t\t\tset me [exec cygpath \\\n-\t\t\t\t\t--unix \\\n-\t\t\t\t\t--absolute \\\n-\t\t\t\t\t$argv0]\n-\t\t\t\twin32_create_lnk $fn [list \\\n-\t\t\t\t\t$sh -c \\\n-\t\t\t\t\t\"CHERE_INVOKING=1 source /etc/profile;[sq $me] &\" \\\n-\t\t\t\t\t] \\\n-\t\t\t\t\t[file normalize $_gitworktree]\n-\t\t\t} err]} {\n-\t\t\terror_popup [strcat [mc \"Cannot write shortcut:\"] \"\\n\\n$err\"]\n-\t\t}\n-\t}\n-}\n-\n proc do_macosx_app {} {\n \tglobal argv0 env\n \n-- \n2.41.0.99.19\n\n"},{"id":"478774","messageId":"20230624212347.179656-4-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230624212347.179656-1-mlevedahl@gmail.com","subject":"[PATCH v0 3/4] git-gui - use cygstart to browse on Cygwin","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-24T21:23:46Z","receivedAt":"2023-06-24T21:24:01Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Pre-2012, git-gui enabled the \"Repository->Explore Working Copy\" menu on\nCygwin, offering open a Windows graphical file browser at the root\nworking directory. The old code relied upon internal use of Windows\npathnames, while git-gui must use unix pathnames on Cygwin since 2012,\nso was removed in a previous patch.\n\nA base install of Cygwin provides the /bin/cygstart utility that runs\narbtitrary Windows applications after translating unix pathnames to\nWindows. Adding the --explore option guarantees that the Windows file\nexplorer is opened, regardless of the supplied pathname's file type and\navoiding possibility of some other action being taken.\n\nSo, teach git-gui to use cygstart --explore on Cygwin, restoring the\npre-2012 behavior of opening a Windows file explorer for browsing.\n\nSigned-off-by: Mark Levedahl \n---\n git-gui.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex b5dba80..523770a 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2276,6 +2276,8 @@ proc do_git_gui {} {\n proc get_explorer {} {\n \tif {[is_Windows]} {\n \t\tset explorer \"explorer.exe\"\n+\t} elseif {[is_Cygwin]} {\n+\t\tset explorer \"/bin/cygstart.exe --explore\"\n \t} elseif {[is_MacOSX]} {\n \t\tset explorer \"open\"\n \t} else {\n-- \n2.41.0.99.19\n\n"},{"id":"478775","messageId":"20230624212347.179656-5-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230624212347.179656-1-mlevedahl@gmail.com","subject":"[PATCH v0 4/4] git-gui - use mkshortcut on Cygwin","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-24T21:23:47Z","receivedAt":"2023-06-24T21:24:01Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Prior to 2012, git-gui enabled the \"Repository->Create Desktop Icon\"\nitem on Cygwin, offering to create a shortcut that starts git-gui on a\nparticular repository. The original code for this in lib/win32.tcl,\nshared with Git for Windows support, requires Windows pathnames, while\ngit-gui must use unix pathnames with the unix/X11 Tcl/Tk since 2012. The\nability to use this from Cygwin was removed in a previous patch.\n\nCygwin's default installation provides /bin/mkshortcut for creating\ndesktop shortuts, this is compatible with exec under tcl, and understands\nCygwin's unix pathnames. So, teach git-gui to use mkshortcut on Cygwin,\nleaving lib/win32.tcl as Git for Windows specific support.\n\nNotes: \"CHERE_INVOKING=1\" is recognized by Cygwin's /etc/profile and\nprevents a \"chdir $HOME\", leaving the shell in the working directory\nspecified by the shortcut. That directory is written directly by\nmkshortcut eliminating any problems with shell escapes and quoting.\n\nThe pre-2012 code includes the full pathname of the git-gui creating the\nshortcut (rather than using the system git-gui), but that git-gui might\nnot be compatible with the git found after /etc/profile sets the path,\nand might have a pathname that defies encoding using shell escapes that\ncan survive the multiple incompatible interpreters involved in this\nchain. Instead, use \"git gui\", thus defaulting to the system git and\navoiding both issues.\n\nSigned-off-by: Mark Levedahl \n---\n git-gui.sh | 4 ++++\n lib/shortcut.tcl | 38 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 42 insertions(+)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 523770a..5c13521 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2836,6 +2836,10 @@ if {[is_enabled multicommit]} {\n \t\t.mbar.repository add command \\\n \t\t\t-label [mc \"Create Desktop Icon\"] \\\n \t\t\t-command do_windows_shortcut\n+\t} elseif {[is_Cygwin]} {\n+\t\t.mbar.repository add command \\\n+\t\t\t-label [mc \"Create Desktop Icon\"] \\\n+\t\t\t-command do_cygwin_shortcut\n \t} elseif {[is_MacOSX]} {\n \t\t.mbar.repository add command \\\n \t\t\t-label [mc \"Create Desktop Icon\"] \\\ndiff --git a/lib/shortcut.tcl b/lib/shortcut.tcl\nindex 1d8374b..6c2a99e 100644\n--- a/lib/shortcut.tcl\n+++ b/lib/shortcut.tcl\n@@ -26,6 +26,44 @@ proc do_windows_shortcut {} {\n \t}\n }\n \n+proc do_cygwin_shortcut {} {\n+\tglobal argv0 _gitworktree oguilib\n+\n+\tif {[catch {\n+\t\tset desktop [exec cygpath \\\n+\t\t\t--desktop]\n+\t\t}]} {\n+\t\t\tset desktop .\n+\t}\n+\tset fn [tk_getSaveFile \\\n+\t\t-parent . \\\n+\t\t-title [mc \"%s (%s): Create Desktop Icon\" [appname] [reponame]] \\\n+\t\t-initialdir $desktop \\\n+\t\t-initialfile \"Git [reponame].lnk\"]\n+\tif {$fn != {}} {\n+\t\tif {[file extension $fn] ne {.lnk}} {\n+\t\t\tset fn ${fn}.lnk\n+\t\t}\n+\t\tif {[catch {\n+\t\t\t\tset repodir [file normalize $_gitworktree]\n+\t\t\t\tset shargs {-c \\\n+\t\t\t\t\t\"CHERE_INVOKING=1 \\\n+\t\t\t\t\tsource /etc/profile; \\\n+\t\t\t\t\tgit gui\"}\n+\t\t\t\texec /bin/mkshortcut.exe \\\n+\t\t\t\t\t-a $shargs \\\n+\t\t\t\t\t-d \"git-gui on $repodir\" \\\n+\t\t\t\t\t-i $oguilib/git-gui.ico \\\n+\t\t\t\t\t-n $fn \\\n+\t\t\t\t\t-s min \\\n+\t\t\t\t\t-w $repodir \\\n+\t\t\t\t\t/bin/sh.exe\n+\t\t\t} err]} {\n+\t\t\terror_popup [strcat [mc \"Cannot write shortcut:\"] \"\\n\\n$err\"]\n+\t\t}\n+\t}\n+}\n+\n proc do_macosx_app {} {\n \tglobal argv0 env\n \n-- \n2.41.0.99.19\n\n"},{"id":"478777","messageId":"xmqq8rc8781p.fsf@gitster.g","threadId":"59909","inReplyTo":"20230624212347.179656-1-mlevedahl@gmail.com","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-24T23:30:10Z","receivedAt":"2023-06-24T23:31:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> git-gui has many snippets of code guarded by an is_Cygwin test, all of\n> which target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed in\n> March 2012. That is when Cygwin switched to a well-supported unix/X11\n> Tcl/Tk package. 64-bit Cygwin was released later so has always had the\n> unix/X11 package. git-gui runs as-is on more recent Cygwin, treating it\n> as a Linux variant, though two functions are disabled.\n>\n> The old Tcl/Tk understood Windows pathnames, so git-gui's Cygwin\n> specific code uses Windows pathnames. The unix/X11 code requires use of\n> unix pathnames, so none of the Cygwin specific code is compatible, and\n> all should be removed.\n>\n> Fortunately, the is_Cygwin funcion in git-gui (on the git master branch)\n> relies upon the old Tcl/Tk and doesn't detect Cygwin. But, commit\n> c5766eae6f2b002396b6cd4f85b62317b707174e on the git-gui master branch\n> \"fixed\" is_Cygwin, enabling the incompatible code, so upstream git-gui\n> is now broken on Cygwin.\n\nHere I presume \"upstream git-gui master\" refers to a5005ded (Merge\nbranch 'ab/makeflags', 2023-01-25) sitting at 'master' in Pratyush's\nhttps://github.com/prati0100/git-gui/ repository.\n\n> There is Cygwin specific code in the Makefile, intended to allow a\n> completely unsupported configuration with a Windows TclTk. This code\n> misdetects the configuration, creating a non-portable installation. The\n> Cygwin git maintainer comments this code out. The code should be\n> removed.\n>\n> ...\n>\n> patch 1 removes the obsolete Makefile code\n> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...\n\nAs it has been quite a while since I had access to any Windows box\nor Cygwin, but the earlier two patches look obviously correct to me.\n\n> The existing code for file browsing and creating a desktop icon is\n> shared with Git For Windows support, and uses Windows pathnames. This\n> code does not work on Cygwin, and needs replacement. These functions\n> have not worked since 2012.\n> ...\n> patch 3 implements Cygwin specific file browsing support\n> patch 4 implemetns Cygwin specific desktop icon support\n\nBoth of these two patches do\n\n\tif {[is_Windows]} {\n\t\t... do Windows thing ...\n+\t} elseif {[is_Cygwin]} {\n+\t\t... do Cygwin thing ...\n\t} elseif {[is_MacOSX]} {\n\t\t... do macOS thing ...\n\t} else {\n\t\t... do it for others ...\n\t}\n\nwhich I do not quite understand how the existing code meshes with\nyour \"is shared with Git For Windows support\", though. If \"is\nshared with GfW\" is to be trusted, on a modern Cygwin box,\n\"is_Windows\" would be yielding true (that is the only way the \"do\nWindows thing\" block will be entered on Cygwin box, sharing the\nsupport with GfW. But then, adding elseif _after_ we check for\nWindows would be pointless. Puzzled...\n\nThanks.\n\n"},{"id":"478778","messageId":"xmqq4jmw77sk.fsf@gitster.g","threadId":"59909","inReplyTo":"xmqq8rc8781p.fsf@gitster.g","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-24T23:35:39Z","receivedAt":"2023-06-24T23:35:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> patch 1 removes the obsolete Makefile code\n>> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...\n>\n> As it has been quite a while since I had access to any Windows box\n> or Cygwin, but the earlier two patches look obviously correct to me.\n\nEhh, in an early draft, I had \"I cannot comment on patches #3 and\n#4\" before that \"but\", but I ended up commenting on them anyway, and\nended up with such a garbled construction. I should have copyedited\nthe above to \"Even though it has been ... or Cygwin, the earlier...\".\n\nSorry for the noise.\n"},{"id":"478779","messageId":"CAPig+cTQcN9um=Pmtze9wyM_kBezpFQ4tJ-LsC-Jh37L=93Bpw@mail.gmail.com","threadId":"59909","inReplyTo":"20230624212347.179656-3-mlevedahl@gmail.com","subject":"Re: [PATCH v0 2/4] git-gui - remove obsolete Cygwin specific code","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2023-06-25T02:56:09Z","receivedAt":"2023-06-25T02:56:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Jun 24, 2023 at 5:35 PM Mark Levedahl wrote:\n> In the current git release, git-gui runs on Cygwin without enabling any\n> of git-gui's Cygwin specific code. This happens as the Cygwin specific\n> code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's\n> then supplied Tcl/Tk which was an incompletely ported variant of the\n> 8.4.1 Windows Tcl/Tk code. In March, 2012, that 8.4.1 package was\n> replaced with a full port based upon the upstream unix/X11 code,\n> since maintained up to date. The two Tcl/Tk packages are completely\n> incompatible, and have different sygnatures.\n\nGiven the context, an understandable typo perhaps: s/sygnatures/signatures/\n\n> When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer\n> detected Cygwin, so did not enable Cygwin specific code, and the POSIX\n> environment provided by Cygwin since 2012 supported git-gui as a generic\n> unix. Thus, no-one apparently noticed the existence of incompatible\n> Cygwin specific code.\n>\n> However, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in\n> upstream git-gui, the is_Cygwin funcion does detect current Cygwin. The\n> Cygwin specific code is enabled, causing use of Windows rather than unix\n> pathnames, and enabling incorrect warnings about environment variables\n> that are not relevant for the fully functional unix/X11 Tcl/Tk. The end\n> result is that git-gui is now incommpatible with Cygwin.\n\ns/incommpatible/incompatible/\n\n> So, delete all Cygwin specific code (code protected by \"if is_Cygwin\"),\n> thus restoring the post-2012 behaviour. Note that Cygwin specific code\n> is required to enable file browsing and shortcut creation (supported\n> before 2012), but is not addressed in this patch.\n>\n> Signed-off-by: Mark Levedahl \n"},{"id":"478780","messageId":"e04e28e2-2308-1db8-9462-5f81aeff1155@gmail.com","threadId":"59909","inReplyTo":"xmqq8rc8781p.fsf@gitster.g","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-25T11:26:11Z","receivedAt":"2023-06-25T11:26:19Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/24/23 19:30, Junio C Hamano wrote:\n> Mark Levedahl writes:\n>\n>> git-gui has many snippets of code guarded by an is_Cygwin test, all of\n>> which target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed in\n>> March 2012. That is when Cygwin switched to a well-supported unix/X11\n>> Tcl/Tk package. 64-bit Cygwin was released later so has always had the\n>> unix/X11 package. git-gui runs as-is on more recent Cygwin, treating it\n>> as a Linux variant, though two functions are disabled.\n>>\n>> The old Tcl/Tk understood Windows pathnames, so git-gui's Cygwin\n>> specific code uses Windows pathnames. The unix/X11 code requires use of\n>> unix pathnames, so none of the Cygwin specific code is compatible, and\n>> all should be removed.\n>>\n>> Fortunately, the is_Cygwin funcion in git-gui (on the git master branch)\n>> relies upon the old Tcl/Tk and doesn't detect Cygwin. But, commit\n>> c5766eae6f2b002396b6cd4f85b62317b707174e on the git-gui master branch\n>> \"fixed\" is_Cygwin, enabling the incompatible code, so upstream git-gui\n>> is now broken on Cygwin.\n> Here I presume \"upstream git-gui master\" refers to a5005ded (Merge\n> branch 'ab/makeflags', 2023-01-25) sitting at 'master' in Pratyush's\n> https://github.com/prati0100/git-gui/ repository.\nYes.\n>\n>> There is Cygwin specific code in the Makefile, intended to allow a\n>> completely unsupported configuration with a Windows TclTk. This code\n>> misdetects the configuration, creating a non-portable installation. The\n>> Cygwin git maintainer comments this code out. The code should be\n>> removed.\n>>\n>> ...\n>>\n>> patch 1 removes the obsolete Makefile code\n>> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...\n> As it has been quite a while since I had access to any Windows box\n> or Cygwin, but the earlier two patches look obviously correct to me.\n>\n>> The existing code for file browsing and creating a desktop icon is\n>> shared with Git For Windows support, and uses Windows pathnames. This\n>> code does not work on Cygwin, and needs replacement. These functions\n>> have not worked since 2012.\n>> ...\n>> patch 3 implements Cygwin specific file browsing support\n>> patch 4 implemetns Cygwin specific desktop icon support\n> Both of these two patches do\n>\n> \tif {[is_Windows]} {\n> \t\t... do Windows thing ...\n> +\t} elseif {[is_Cygwin]} {\n> +\t\t... do Cygwin thing ...\n> \t} elseif {[is_MacOSX]} {\n> \t\t... do macOS thing ...\n> \t} else {\n> \t\t... do it for others ...\n> \t}\n>\n> which I do not quite understand how the existing code meshes with\n> your \"is shared with Git For Windows support\", though. If \"is\n> shared with GfW\" is to be trusted, on a modern Cygwin box,\n> \"is_Windows\" would be yielding true (that is the only way the \"do\n> Windows thing\" block will be entered on Cygwin box, sharing the\n> support with GfW. But then, adding elseif _after_ we check for\n> Windows would be pointless. Puzzled...\n>\n> Thanks.\n>\ngit-gui has three independent functions (is_Cygwin, is_Windows, and \nis_MaxOSX), each determine if running on that platform, and \"generic \nUnix/Linux\" can be considered the result if all three functions return \nfalse. In Pratyush's tree, those three functions essentially are:\n\nis_Cygwin: $::tcl_platform(os) startswith(\"CYGWIN\")\n\nis_MaxOSX: [tk windowingsystem] == \"AQUA\"\n\nis_Windows: $::tcl_platform(platform) == \"Windows\"\n\nIt turns out, only one of the . is ever true, and none are true on \nLinux. So, the if/else tree above is not confused by Windows / Cygwin.\n\nBut, different Tcl/Tk signatures as platforms evolve could cause \nproblems. A better design might be to just have a $HOSTTYPE variable set \nonce, perhaps in startup, perhaps even by the makefile, to assure \nexactly one hosttype is ever active and make this clear to others. \nNormal configuration checking in the makefile could have uncovered this \nwhole problem in 2012. But, this is a possible cleanup topic for another \nday.\n\nSo, the code under the is_Windows and is_Cygwin branches of the if/else \ntrees are now completely independent, and the is_Windows branch is never \nentered on Cygwin.\n\n\nThank you,\n\nMark\n\n"},{"id":"478781","messageId":"b290eac3-e07b-aa0d-593e-c9f8abed8826@gmail.com","threadId":"59909","inReplyTo":"xmqq4jmw77sk.fsf@gitster.g","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-25T11:28:16Z","receivedAt":"2023-06-25T11:28:20Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/24/23 19:35, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>>> patch 1 removes the obsolete Makefile code\n>>> patch 2 removes all obsolete git-gui.sh code, wrapped in is_Cygwin...\n>> As it has been quite a while since I had access to any Windows box\n>> or Cygwin, but the earlier two patches look obviously correct to me.\n> Ehh, in an early draft, I had \"I cannot comment on patches #3 and\n> #4\" before that \"but\", but I ended up commenting on them anyway, and\n> ended up with such a garbled construction. I should have copyedited\n> the above to \"Even though it has been ... or Cygwin, the earlier...\".\n>\n> Sorry for the noise.\n\nNo problem, I appreciate the review. The last two patches absolutely \nneed the Cygwin / G4W folks to review.\n\n\nMark\n\n"},{"id":"478782","messageId":"dd3d50fa-6c48-459a-bf24-9e902696e2d0@gmail.com","threadId":"59909","inReplyTo":"CAPig+cTQcN9um=Pmtze9wyM_kBezpFQ4tJ-LsC-Jh37L=93Bpw@mail.gmail.com","subject":"Re: [PATCH v0 2/4] git-gui - remove obsolete Cygwin specific code","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-25T11:29:01Z","receivedAt":"2023-06-25T11:29:06Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/24/23 22:56, Eric Sunshine wrote:\n> On Sat, Jun 24, 2023 at 5:35 PM Mark Levedahl wrote:\n>> In the current git release, git-gui runs on Cygwin without enabling any\n>> of git-gui's Cygwin specific code. This happens as the Cygwin specific\n>> code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's\n>> then supplied Tcl/Tk which was an incompletely ported variant of the\n>> 8.4.1 Windows Tcl/Tk code. In March, 2012, that 8.4.1 package was\n>> replaced with a full port based upon the upstream unix/X11 code,\n>> since maintained up to date. The two Tcl/Tk packages are completely\n>> incompatible, and have different sygnatures.\n> Given the context, an understandable typo perhaps: s/sygnatures/signatures/\n>\n>> When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer\n>> detected Cygwin, so did not enable Cygwin specific code, and the POSIX\n>> environment provided by Cygwin since 2012 supported git-gui as a generic\n>> unix. Thus, no-one apparently noticed the existence of incompatible\n>> Cygwin specific code.\n>>\n>> However, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in\n>> upstream git-gui, the is_Cygwin funcion does detect current Cygwin. The\n>> Cygwin specific code is enabled, causing use of Windows rather than unix\n>> pathnames, and enabling incorrect warnings about environment variables\n>> that are not relevant for the fully functional unix/X11 Tcl/Tk. The end\n>> result is that git-gui is now incommpatible with Cygwin.\n> s/incommpatible/incompatible/\n>\n>> So, delete all Cygwin specific code (code protected by \"if is_Cygwin\"),\n>> thus restoring the post-2012 behaviour. Note that Cygwin specific code\n>> is required to enable file browsing and shortcut creation (supported\n>> before 2012), but is not addressed in this patch.\n>>\n>> Signed-off-by: Mark Levedahl \n\n\nThanks, will fix both (and a few other typos ...)\n\nMark\n\n"},{"id":"478783","messageId":"16959998-cb50-6f7d-370f-22c7293c89c2@gmail.com","threadId":"59909","inReplyTo":"e04e28e2-2308-1db8-9462-5f81aeff1155@gmail.com","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-25T12:10:42Z","receivedAt":"2023-06-25T12:10:49Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/25/23 07:26, Mark Levedahl wrote:\n>\n> On 6/24/23 19:30, Junio C Hamano wrote: git-gui has three independent \n> functions (is_Cygwin, is_Windows, and is_MaxOSX), each determine if \n> running on that platform, and \"generic Unix/Linux\" can be considered \n> the result if all three functions return false. In Pratyush's tree, \n> those three functions essentially are:\n>\n> is_Cygwin: $::tcl_platform(os) startswith(\"CYGWIN\")\n>\n> is_MaxOSX: [tk windowingsystem] == \"AQUA\"\n>\n> is_Windows: $::tcl_platform(platform) == \"Windows\"\n>\n> It turns out, only one of the . is ever true, and none are true on \n> Linux. So, the if/else tree above is not confused by Windows / Cygwin.\n>\n> But, different Tcl/Tk signatures as platforms evolve could cause \n> problems. A better design might be to just have a $HOSTTYPE variable \n> set once, perhaps in startup, perhaps even by the makefile, to assure \n> exactly one hosttype is ever active and make this clear to others. \n> Normal configuration checking in the makefile could have uncovered \n> this whole problem in 2012. But, this is a possible cleanup topic for \n> another day.\n>\n> So, the code under the is_Windows and is_Cygwin branches of the \n> if/else trees are now completely independent, and the is_Windows \n> branch is never entered on Cygwin.\n>\n>\n> Thank you,\n>\n> Mark\n>\nA follow up - I have Cygwin in a Windows VM on my laptop, no G4W, no Mac ...\n\n\nCygwin gives:   $::tcl_platform(os) = CYGWIN_NT-10.0-22621\n                 $::tcl_platform(platform) = unix\n                 tk windowingsystem = x11\n\nLinux gives     $::tcl_platform(os) = Linux\n                 $::tcl_platform(platform) = unix\n                 tk windowingsystem = x11\n\nSo, neither Cygwin nor Linux trigger the checks for is_Windows or is_MacOSX\n\n\n"},{"id":"478785","messageId":"xmqqwmzr5yul.fsf@gitster.g","threadId":"59909","inReplyTo":"e04e28e2-2308-1db8-9462-5f81aeff1155@gmail.com","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-25T15:46:26Z","receivedAt":"2023-06-25T15:46:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> So, the code under the is_Windows and is_Cygwin branches of the\n> if/else trees are now completely independent, and the is_Windows\n> branch is never entered on Cygwin.\n\nI missed this hunk in your updated get_explorer in [2/4]\n\n proc get_explorer {} {\n-\tif {[is_Cygwin] || [is_Windows]} {\n+\tif {[is_Windows]} {\n \t\tset explorer \"explorer.exe\"\n \t} elseif {[is_MacOSX]} {\n \t\tset explorer \"open\"\n\nand saw only this in [3/4]\n\n proc get_explorer {} {\n \tif {[is_Windows]} {\n \t\tset explorer \"explorer.exe\"\n+\t} elseif {[is_Cygwin]} {\n+\t\tset explorer \"/bin/cygstart.exe --explore\"\n \t} elseif {[is_MacOSX]} {\n \t\tset explorer \"open\"\n \t} else {\n\nAs I missed the earlier change, the latter one alone looked to me\nthat for get_explorer to be share with GfW, the only explanation was\nthat is_Windows yields true on Cygwin, in which case the new elseif\ndid not make sense.\n\nI think the hunk in [2/4] should be removed; it is confusing, it\ndoes not have anything to do with the theme of [2/4], which is to\n\"remove obsolete Cygwin specific code\". And instead [3/4] should\nbe updated to do\n\n+\tif {[is_Cygwin] || [is_Windows]} {\n-\tif {[is_Windows]} {\n\t\t... do windows thing ...\n+\t} elseif {[is_Cygwin]} {\n+\t\t... do Cygwin thing ...\n\t} elseif {[is_MacOSX]} {\n\t\t... do macOS thing ...\n\nThe earlier explanation in the cover letter says this:\n\n The existing code for file browsing and creating a desktop icon is\n shared with Git For Windows support, and uses Windows pathnames. This\n code does not work on Cygwin, and needs replacement. These functions\n have not worked since 2012.\n\nIf the change for get_explorer is updated to read like so, then \"was\nshared with GfW, now we have one that is for Cygwin\" starts making\nsense for the file browsing.\n\nBut I still do not understand the issue with desktop icon, though.\ndo_windows_shortcut and do_cygwin_shortcut were separate proc before\nthis series---while I fully believe that do_cygwin_shortcut did not\nwork on modern Cygwin if you say so, and \"uses Windows pathnames\"\nmay be what makes the original implementation not work on modern\nCygwin, I do not see how the existing code for the desktop icon \"is\nshared with GfW\".\n\nAh, this is again due to the suboptimal splitting of the patches.\n\nThe original does have do_cygwin_shortcut, but you remove it in step\n[2/4], together with its caller. The code before your series did\nhave its own do_cygwin_shortcut, but after [2/4] it and its caller\nare removed. The code may not have worked before step [2/4], so it\nis probably alright in the end, but it does make step [4/4] very\nconfusing. Since [4/4] does need to add Cygwin specific code,\nperhaps you should exclude the shortcut related change from [2/4]\nand keep it focused on removing Cygwin specific code that will not\nbe used in the end (instead of getting fixed to keep it alive).\n\nSo, earlier I said [2/4] made sense and obviously good. But not\nanymore. It does a bit too many things and then have later steps\ncompensate for it, which made reviewing the series harder than\nnecessary. It needs to be cleaned up a bit, I think.\n\nThanks.\n\n"},{"id":"478786","messageId":"b7181a2d-ba97-eae8-6bf4-4fc4b0db64c2@gmail.com","threadId":"59909","inReplyTo":"xmqqwmzr5yul.fsf@gitster.g","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-25T17:01:13Z","receivedAt":"2023-06-25T17:01:20Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/25/23 11:46, Junio C Hamano wrote:\n> Mark Levedahl writes:\n>\n>> So, the code under the is_Windows and is_Cygwin branches of the\n>> if/else trees are now completely independent, and the is_Windows\n>> branch is never entered on Cygwin.\n\n>\n> So, earlier I said [2/4] made sense and obviously good. But not\n> anymore. It does a bit too many things and then have later steps\n> compensate for it, which made reviewing the series harder than\n> necessary. It needs to be cleaned up a bit, I think.\n>\n> Thanks.\n>\n\nI had originally organized as you suggest, no problem doing so again. \nWhat gave me pause was this paragraph I originally wrote for the cover \nletter:\n\n\nPatches 1/2 cause git-gui to function as it has for the last decade on\nCygwin, but with Cygwin being detected. However, the browsing and\nshortcut creation menu items, removed in 2012 then re-added when is_Cygwin\nwas fixed, do not work, and shortcut creation will crash git-gui if used.\nThese are fixed in patches 3 / 4.\n\n\nSo, I'm just checking that the above situation is ok. I agree, this \nmakes the changes easier to follow.\n\n\nMark\n\n"},{"id":"478796","messageId":"xmqqsfae5igi.fsf@gitster.g","threadId":"59909","inReplyTo":"b7181a2d-ba97-eae8-6bf4-4fc4b0db64c2@gmail.com","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-26T15:52:45Z","receivedAt":"2023-06-26T15:53:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> I had originally organized as you suggest, no problem doing so\n> again. What gave me pause was this paragraph I originally wrote for\n> the cover letter:\n>\n> Patches 1/2 cause git-gui to function as it has for the last decade on\n> Cygwin, but with Cygwin being detected. However, the browsing and\n> shortcut creation menu items, removed in 2012 then re-added when is_Cygwin\n> was fixed, do not work, and shortcut creation will crash git-gui if used.\n> These are fixed in patches 3 / 4.\n\nAs you are removing (ancient) Cygwin specific code that did not work\nwith modern Cygwin at all in step [2/4], it is not so unexpected\nthat some stuff does still not work after that step. I am not sure\nwhat your reservation exactly is, but if you are wondering if code\nto disable browsing and shortcut creation on Cygwin temporarily\nneeds to be there in the same step (instead of crashing or not\nworking), it may make sense if and only if it is done with minimal\nchanges.\n\nThanks.\n\n"},{"id":"478826","messageId":"20230626165305.37488-2-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230626165305.37488-1-mlevedahl@gmail.com","subject":"[PATCH v1 1/4] git gui Makefile - remove Cygwin modifications","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-26T16:53:02Z","receivedAt":"2023-06-26T16:53:12Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries\ninto git-gui, destroying the ability to package git-gui on one machine and\ndistribute to others. The intent is to do this only if a non-Cygwin Tcl/Tk is\ninstalled, but the test for this is wrong with the unix/X11 Tcl/Tk shipped\nsince 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk.\n\nThe Cygwin git maintainer disables this code, so this code is definitely\nnot in use in the Cygwin distribution.\n\nThe simplest fix is to just delete the Cygwin specific code,\nallowing the Makefile to work out of the box on Cygwin. Do so.\n\nSigned-off-by: Mark Levedahl \n---\n Makefile | 21 +++------------------\n 1 file changed, 3 insertions(+), 18 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex a0d5a4b..3f80435 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -138,25 +138,10 @@ GITGUI_SCRIPT := $$0\n GITGUI_RELATIVE :=\n GITGUI_MACOSXAPP :=\n \n-ifeq ($(uname_O),Cygwin)\n-\tGITGUI_SCRIPT := `cygpath --windows --absolute \"$(GITGUI_SCRIPT)\"`\n-\n-\t# Is this a Cygwin Tcl/Tk binary? If so it knows how to do\n-\t# POSIX path translation just like cygpath does and we must\n-\t# keep libdir in POSIX format so Cygwin packages of git-gui\n-\t# work no matter where the user installs them.\n-\t#\n-\tifeq ($(shell echo 'puts [file normalize /]' | '$(TCL_PATH_SQ)'),$(shell cygpath --mixed --absolute /))\n-\t\tgg_libdir_sed_in := $(gg_libdir)\n-\telse\n-\t\tgg_libdir_sed_in := $(shell cygpath --windows --absolute \"$(gg_libdir)\")\n-\tendif\n-else\n-\tifeq ($(exedir),$(gg_libdir))\n-\t\tGITGUI_RELATIVE := 1\n-\tendif\n-\tgg_libdir_sed_in := $(gg_libdir)\n+ifeq ($(exedir),$(gg_libdir))\n+\tGITGUI_RELATIVE := 1\n endif\n+gg_libdir_sed_in := $(gg_libdir)\n ifeq ($(uname_S),Darwin)\n \tifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y)\n \t\tGITGUI_MACOSXAPP := YesPlease\n-- \n2.41.0.99.19\n\n"},{"id":"478827","messageId":"20230626165305.37488-1-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230624212347.179656-1-mlevedahl@gmail.com","subject":"[PATCH v1 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-26T16:53:01Z","receivedAt":"2023-06-26T16:53:13Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"=== This is an update, incorporating responses to Junio's and Eric's\ncomments:\n -- clarified what the \"upstream\" git-gui branch is\n -- Removed some changes from patch 2 as requested by Junio, reducing\n changes in patch 3 and patch 4\n All code is fixed only after applying patch 4\n Differences in patch 3 and 4 are minimimized\n -- updated comments to clarify G4w dedicated code.\n -- updated all comments to (hopefully) clarify points of confusion\n===\n\ngit-gui has many snippets of code guarded by an is_Cygwin test, all of\nwhich target a problematic hybrid Cygwin/Windows 8.4.1 Tcl/Tk removed\nfrom the Cygwin project in March 2012. That is when Cygwin switched to a\nwell-supported unix/X11 Tcl/Tk package. 64-bit Cygwin was released\nlater so has always had the unix/X11 package. git-gui runs as-is on\nmore recent Cygwin, treating it as a Linux variant, though two functions\nare disabled.\n\nThe old Tcl/Tk understands Windows pathnames but has incomplete support\nfor unix pathnames (for instance, all pathnames output by that Tcl are\nWindows, not unix). The Cygwin git executables all use unix pathnames\n(though like all Cygwin executables have some capability to accept\nWindows pathnames). git-gui's Cygwin specific code causes git-gui to use\nWindows pathnames everywhere. The unix/X11 Tcl/Tk requires use of unix\npathname, so none of the Cygwin specific code is compatible.\n\ngit-gui is maintained at https://github.com/prati0100/git-gui. The\ngit-gui in Junio's tree corresponds to commit c0698df057, behind the\ncurrent git-gui master which is a5005ded.\n\nFortunately, the git-gui/is_Cygwin function in Junio's tree relies upon\nthe old Tcl/Tk that outputs Windows pathnames. As this fails to detect\nCygwin, git-gui treats Cygwin as a unix variant with no platform\nspecific code enabled and git-gui currently runs on Cygwin.\n\nBut, commit c5766eae6f on the git-gui master branch fixes is_Cygwin to\nwork with the new Tcl/Tk's signature (which is not that of a Windows\nTcl/Tk). Thus, Cygwin is detected, the incompatible Cygwin code is\nenabled, and git-gui no longer runs on Cygwin.\n\nAlso, there is Cygwin specific code in the Makefile, intended to allow a\ncompletely unsupported configuration with a Windows Tcl/Tk. However,\nthe Makefile code mis-identifies the unix/X11 Tcl/Tk as Windows,\ntriggering insertion of a hardcoded Windows path to the library\ndirectory into git-gui making it non-portable. The Cygwin git maintainer\ncomments this code out, but the code should be removed as it is\ndemonstrated to be incompatible with Cygwin and targets a configuration\nCygwin does not support.\n\nThe existing code for file browsing and creating a desktop icon is\nshared with Git For Windows support, and supports only Windows\npathnames. This code does not work on Cygwin and needs replacement or\nupdate. The menu items for these functions are enabled by is_Cygwin, so\nappear only after is_Cygwin is fixed as discussed above.\n\npatch 1 removes the obsolete Makefile code\npatch 2 removes obsolete git-gui.sh code, wrapped in is_Cygwin\n except for code fixed in patches 3 and 4\npatch 3 implements Cygwin specific file browsing support\npatch 4 implements Cygwin specific desktop icon support\n\nThe end result is that git-gui on Cygwin is restored to the full\ncapabilities existing prior to the Tcl/Tk switch in 2012. Also, the\nremaining Cygwin specific code, updated in patches 3 and 4, no longer\noverlaps with Git For Windows support.\n\nAny argument for keeping the old Cygwin code must address who is going\nto test and maintain that code, on what platform, and who the target\naudience is. The old Tcl/Tk was only on 32-bit Cygwin and only supported\nfor the Insight debugger, 32-bit Cygwin is no longer supported, git-gui\nis not supported on 8.4.1 Tcl/Tk, and the Windows versions targeted by\n2012'ish 32-bit Cygwin are no longer supported.\n\nMark Levedahl (4):\n git gui Makefile - remove Cygwin modifications\n git-gui - remove obsolete Cygwin specific code\n git-gui - use cygstart to browse on Cygwin\n git-gui - use mkshortcut on Cygwin\n\n Makefile | 21 +------\n git-gui.sh | 118 +++-----------------------------------\n lib/choose_repository.tcl | 27 +--------\n lib/shortcut.tcl | 31 +++++-----\n 4 files changed, 27 insertions(+), 170 deletions(-)\n\n-- \n2.41.0.99.19\n\n"},{"id":"478828","messageId":"20230626165305.37488-3-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230626165305.37488-1-mlevedahl@gmail.com","subject":"[PATCH v1 2/4] git-gui - remove obsolete Cygwin specific code","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-26T16:53:03Z","receivedAt":"2023-06-26T16:53:16Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"In the current git release, git-gui runs on Cygwin without enabling any\nof git-gui's Cygwin specific code. This happens as the Cygwin specific\ncode in git-gui was (mostly) written in 2007-2008 to work with Cygwin's\nthen supplied Tcl/Tk which was an incompletely ported variant of the\n8.4.1 Windows Tcl/Tk code. In March, 2012, that 8.4.1 package was\nreplaced with a full port based upon the upstream unix/X11 code,\nsince maintained up to date. The two Tcl/Tk packages are completely\nincompatible, and have different signatures.\n\nWhen Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer\ndetected Cygwin, so did not enable Cygwin specific code, and the POSIX\nenvironment provided by Cygwin since 2012 supported git-gui as a generic\nunix. Thus, no-one apparently noticed the existence of incompatible\nCygwin specific code.\n\nHowever, since commit c5766eae6f in the git-gui source tree\n(https://github.com/prati0100/git-gui, master at a5005ded), and not yet\npulled into the git repository, the is_Cygwin function does detect\nCygwin using the unix/X11 Tcl/Tk. The Cygwin specific code is enabled,\ncausing use of Windows rather than unix pathnames, and enabling\nincorrect warnings about environment variables that were relevant only\nto the old Tcl/Tk. The end result is that (upstream) git-gui is now\nincompatible with Cygwin.\n\nSo, delete Cygwin specific code (code protected by \"if is_Cygwin\") that\nis not needed in any form to work with the unix/X11 Tcl/Tk.\n\nCygwin specific code required to enable file browsing and shortcut\ncreation is not addressed in this patch, does not currently work, and\ninvocation of those items may leave git-gui in a confused state.\n\nSigned-off-by: Mark Levedahl \n---\nchanges since v0\n -- do not touch any code fixed in patches 3/4, meaning the browsing\n and shortcut creating menu items do not work.\n\n git-gui.sh | 114 ++------------------------------------\n lib/choose_repository.tcl | 27 +--------\n 2 files changed, 7 insertions(+), 134 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex cb92bba..3f7c31e 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -84,14 +84,7 @@ proc _which {what args} {\n \tglobal env _search_exe _search_path\n \n \tif {$_search_path eq {}} {\n-\t\tif {[is_Cygwin] && [regexp {^(/|\\.:)} $env(PATH)]} {\n-\t\t\tset _search_path [split [exec cygpath \\\n-\t\t\t\t--windows \\\n-\t\t\t\t--path \\\n-\t\t\t\t--absolute \\\n-\t\t\t\t$env(PATH)] {;}]\n-\t\t\tset _search_exe .exe\n-\t\t} elseif {[is_Windows]} {\n+\t\tif {[is_Windows]} {\n \t\t\tset gitguidir [file dirname [info script]]\n \t\t\tregsub -all \";\" $gitguidir \"\\\\;\" gitguidir\n \t\t\tset env(PATH) \"$gitguidir;$env(PATH)\"\n@@ -342,14 +335,7 @@ proc gitexec {args} {\n \t\tif {[catch {set _gitexec [git --exec-path]} err]} {\n \t\t\terror \"Git not installed?\\n\\n$err\"\n \t\t}\n-\t\tif {[is_Cygwin]} {\n-\t\t\tset _gitexec [exec cygpath \\\n-\t\t\t\t--windows \\\n-\t\t\t\t--absolute \\\n-\t\t\t\t$_gitexec]\n-\t\t} else {\n-\t\t\tset _gitexec [file normalize $_gitexec]\n-\t\t}\n+\t\tset _gitexec [file normalize $_gitexec]\n \t}\n \tif {$args eq {}} {\n \t\treturn $_gitexec\n@@ -364,14 +350,7 @@ proc githtmldir {args} {\n \t\t\t# Git not installed or option not yet supported\n \t\t\treturn {}\n \t\t}\n-\t\tif {[is_Cygwin]} {\n-\t\t\tset _githtmldir [exec cygpath \\\n-\t\t\t\t--windows \\\n-\t\t\t\t--absolute \\\n-\t\t\t\t$_githtmldir]\n-\t\t} else {\n-\t\t\tset _githtmldir [file normalize $_githtmldir]\n-\t\t}\n+\t\tset _githtmldir [file normalize $_githtmldir]\n \t}\n \tif {$args eq {}} {\n \t\treturn $_githtmldir\n@@ -1318,9 +1297,6 @@ if {$_gitdir eq \".\"} {\n \tset _gitdir [pwd]\n }\n \n-if {![file isdirectory $_gitdir] && [is_Cygwin]} {\n-\tcatch {set _gitdir [exec cygpath --windows $_gitdir]}\n-}\n if {![file isdirectory $_gitdir]} {\n \tcatch {wm withdraw .}\n \terror_popup [strcat [mc \"Git directory not found:\"] \"\\n\\n$_gitdir\"]\n@@ -1332,11 +1308,7 @@ apply_config\n \n # v1.7.0 introduced --show-toplevel to return the canonical work-tree\n if {[package vcompare $_git_version 1.7.0] >= 0} {\n-\tif { [is_Cygwin] } {\n-\t\tcatch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]}\n-\t} else {\n-\t\tset _gitworktree [git rev-parse --show-toplevel]\n-\t}\n+\tset _gitworktree [git rev-parse --show-toplevel]\n } else {\n \t# try to set work tree from environment, core.worktree or use\n \t# cdup to obtain a relative path to the top of the worktree. If\n@@ -1561,24 +1533,8 @@ proc rescan {after {honor_trustmtime 1}} {\n \t}\n }\n \n-if {[is_Cygwin]} {\n-\tset is_git_info_exclude {}\n-\tproc have_info_exclude {} {\n-\t\tglobal is_git_info_exclude\n-\n-\t\tif {$is_git_info_exclude eq {}} {\n-\t\t\tif {[catch {exec test -f [gitdir info exclude]}]} {\n-\t\t\t\tset is_git_info_exclude 0\n-\t\t\t} else {\n-\t\t\t\tset is_git_info_exclude 1\n-\t\t\t}\n-\t\t}\n-\t\treturn $is_git_info_exclude\n-\t}\n-} else {\n-\tproc have_info_exclude {} {\n-\t\treturn [file readable [gitdir info exclude]]\n-\t}\n+proc have_info_exclude {} {\n+\treturn [file readable [gitdir info exclude]]\n }\n \n proc rescan_stage2 {fd after} {\n@@ -3112,10 +3068,6 @@ if {[is_MacOSX]} {\n set doc_path [githtmldir]\n if {$doc_path ne {}} {\n \tset doc_path [file join $doc_path index.html]\n-\n-\tif {[is_Cygwin]} {\n-\t\tset doc_path [exec cygpath --mixed $doc_path]\n-\t}\n }\n \n if {[file isfile $doc_path]} {\n@@ -4087,60 +4039,6 @@ set file_lists($ui_workdir) [list]\n wm title . \"[appname] ([reponame]) [file normalize $_gitworktree]\"\n focus -force $ui_comm\n \n-# -- Warn the user about environmental problems. Cygwin's Tcl\n-# does *not* pass its env array onto any processes it spawns.\n-# This means that git processes get none of our environment.\n-#\n-if {[is_Cygwin]} {\n-\tset ignored_env 0\n-\tset suggest_user {}\n-\tset msg [mc \"Possible environment issues exist.\n-\n-The following environment variables are probably\n-going to be ignored by any Git subprocess run\n-by %s:\n-\n-\" [appname]]\n-\tforeach name [array names env] {\n-\t\tswitch -regexp -- $name {\n-\t\t{^GIT_INDEX_FILE$} -\n-\t\t{^GIT_OBJECT_DIRECTORY$} -\n-\t\t{^GIT_ALTERNATE_OBJECT_DIRECTORIES$} -\n-\t\t{^GIT_DIFF_OPTS$} -\n-\t\t{^GIT_EXTERNAL_DIFF$} -\n-\t\t{^GIT_PAGER$} -\n-\t\t{^GIT_TRACE$} -\n-\t\t{^GIT_CONFIG$} -\n-\t\t{^GIT_(AUTHOR|COMMITTER)_DATE$} {\n-\t\t\tappend msg \" - $name\\n\"\n-\t\t\tincr ignored_env\n-\t\t}\n-\t\t{^GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL)$} {\n-\t\t\tappend msg \" - $name\\n\"\n-\t\t\tincr ignored_env\n-\t\t\tset suggest_user $name\n-\t\t}\n-\t\t}\n-\t}\n-\tif {$ignored_env > 0} {\n-\t\tappend msg [mc \"\n-This is due to a known issue with the\n-Tcl binary distributed by Cygwin.\"]\n-\n-\t\tif {$suggest_user ne {}} {\n-\t\t\tappend msg [mc \"\n-\n-A good replacement for %s\n-is placing values for the user.name and\n-user.email settings into your personal\n-~/.gitconfig file.\n-\" $suggest_user]\n-\t\t}\n-\t\twarn_popup $msg\n-\t}\n-\tunset ignored_env msg suggest_user name\n-}\n-\n # -- Only initialize complex UI if we are going to stay running.\n #\n if {[is_enabled transport]} {\ndiff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl\nindex af1fee7..d23abed 100644\n--- a/lib/choose_repository.tcl\n+++ b/lib/choose_repository.tcl\n@@ -174,9 +174,6 @@ constructor pick {} {\n \t\t\t-foreground blue \\\n \t\t\t-underline 1\n \t\tset home $::env(HOME)\n-\t\tif {[is_Cygwin]} {\n-\t\t\tset home [exec cygpath --windows --absolute $home]\n-\t\t}\n \t\tset home \"[file normalize $home]/\"\n \t\tset hlen [string length $home]\n \t\tforeach p $sorted_recent {\n@@ -374,18 +371,6 @@ proc _objdir {path} {\n \t\treturn $objdir\n \t}\n \n-\tif {[is_Cygwin]} {\n-\t\tset objdir [file join $path .git objects.lnk]\n-\t\tif {[file isfile $objdir]} {\n-\t\t\treturn [win32_read_lnk $objdir]\n-\t\t}\n-\n-\t\tset objdir [file join $path objects.lnk]\n-\t\tif {[file isfile $objdir]} {\n-\t\t\treturn [win32_read_lnk $objdir]\n-\t\t}\n-\t}\n-\n \treturn {}\n }\n \n@@ -623,12 +608,6 @@ method _do_clone2 {} {\n \t}\n \n \tset giturl $origin_url\n-\tif {[is_Cygwin] && [file isdirectory $giturl]} {\n-\t\tset giturl [exec cygpath --unix --absolute $giturl]\n-\t\tif {$clone_type eq {shared}} {\n-\t\t\tset objdir [exec cygpath --unix --absolute $objdir]\n-\t\t}\n-\t}\n \n \tif {[file exists $local_path]} {\n \t\terror_popup [mc \"Location %s already exists.\" $local_path]\n@@ -668,11 +647,7 @@ method _do_clone2 {} {\n \t\t\t\tfconfigure $f_cp -translation binary -encoding binary\n \t\t\t\tcd $objdir\n \t\t\t\twhile {[gets $f_in line] >= 0} {\n-\t\t\t\t\tif {[is_Cygwin]} {\n-\t\t\t\t\t\tputs $f_cp [exec cygpath --unix --absolute $line]\n-\t\t\t\t\t} else {\n-\t\t\t\t\t\tputs $f_cp [file normalize $line]\n-\t\t\t\t\t}\n+\t\t\t\t\tputs $f_cp [file normalize $line]\n \t\t\t\t}\n \t\t\t\tclose $f_in\n \t\t\t\tclose $f_cp\n-- \n2.41.0.99.19\n\n"},{"id":"478829","messageId":"20230626165305.37488-4-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230626165305.37488-1-mlevedahl@gmail.com","subject":"[PATCH v1 3/4] git-gui - use cygstart to browse on Cygwin","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-26T16:53:04Z","receivedAt":"2023-06-26T16:53:17Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"git-gui enables the \"Repository->Explore Working Copy\" menu on Cygwin,\noffering to open a Windows graphical file browser at the root of the\nworking directory. This code, shared with Git For Windows support,\ndepends upon use of Windows pathnames. However, git gui on Cygwin uses\nunix pathnames, so this shared code will not work on Cygwin.\n\nA base install of Cygwin provides the /bin/cygstart utility that runs\na registered Windows application based upon the file type, after\ntranslating unix pathnames to Windows. Adding the --explore option\nguarantees that the Windows file explorer is opened, regardless of the\nsupplied pathname's file type and avoiding possibility of some other\naction being taken.\n\nSo, teach git-gui to use cygstart --explore on Cygwin, restoring the\npre-2012 behavior of opening a Windows file explorer for browsing. This\nseparates the Git For Windows and Cygwin code paths. Note that\nis_Windows is never true on Cygwin, and is_Cygwin is never true on Git\nfor Windows, though this is not obvious by examining the code for those\nindependent functions.\n\nSigned-off-by: Mark Levedahl \n---\nchanges since v0\n -- assumes the if/else tree being modified is untouched by prior\n patches making the changes minimal and easier to review.\n\n git-gui.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 3f7c31e..8bc8892 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -2274,7 +2274,9 @@ proc do_git_gui {} {\n \n # Get the system-specific explorer app/command.\n proc get_explorer {} {\n-\tif {[is_Cygwin] || [is_Windows]} {\n+\tif {[is_Cygwin]} {\n+\t\tset explorer \"/bin/cygstart.exe --explore\"\n+\t} elseif {[is_Windows]} {\n \t\tset explorer \"explorer.exe\"\n \t} elseif {[is_MacOSX]} {\n \t\tset explorer \"open\"\n-- \n2.41.0.99.19\n\n"},{"id":"478830","messageId":"20230626165305.37488-5-mlevedahl@gmail.com","threadId":"59909","inReplyTo":"20230626165305.37488-1-mlevedahl@gmail.com","subject":"[PATCH v1 4/4] git-gui - use mkshortcut on Cygwin","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-26T16:53:05Z","receivedAt":"2023-06-26T16:53:26Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"git-gui enables the \"Repository->Create Desktop Icon\" item on Cygwin,\noffering to create a shortcut that starts git-gui on the current\nrepository. The code in do_cygwin_shortcut invokes function\nwin32_create_lnk to create the shortcut. This latter function is shared\nbetween Cygwin and Git For Windows and expects Windows rather than unix\npathnames, though do_cygwin_shortcut provides unix pathnames. Also, this\nfunction tries to invoke the Windows Script Host to run a javascript\nsnippet, but this fails under Cygwin's Tcl. So, win32_create_lnk just\ndoes not support Cygwin.\n\nHowever, Cygwin's default installation provides /bin/mkshortcut for\ncreating desktop shortcuts. This is compatible with exec under Cygwin's\nTcl, understands Cygwin's unix pathnames, and avoids the need for shell\nescapes to encode troublesome paths. So, teach git-gui to use mkshortcut\non Cygwin, leaving win32_create_lnk unchanged and for exclusive use by\nGit For Windows.\n\nNotes: \"CHERE_INVOKING=1\" is recognized by Cygwin's /etc/profile and\nprevents a \"chdir $HOME\", leaving the shell in the working directory\nspecified by the shortcut. That directory is written directly by\nmkshortcut eliminating any problems with shell escapes and quoting.\n\nThe code being replaced includes the full pathname of the git-gui\ncreating the shortcut, but that git-gui might not be compatible with the\ngit found after /etc/profile sets the path, and might have a pathname\nthat defies encoding using shell escapes that can survive the multiple\nincompatible interpreters involved in the chain of creating and using\nthis shortcut. The new code uses bare \"git gui\" as the command to\nexecute, thus using the system git to launch the system git-gui, and\navoiding both issues.\n\nSigned-off-by: Mark Levedahl \n---\nchanges since v0\n -- assumes no changes to shortcut creation code in prior patches,\n so changes in this patch are easier to review.\n -- changed to use long option names for mkshortcut, better conforming\n to practice in git-gui overall.\n\n lib/shortcut.tcl | 31 ++++++++++++++-----------------\n 1 file changed, 14 insertions(+), 17 deletions(-)\n\ndiff --git a/lib/shortcut.tcl b/lib/shortcut.tcl\nindex 97d1d7a..674a41f 100644\n--- a/lib/shortcut.tcl\n+++ b/lib/shortcut.tcl\n@@ -27,13 +27,10 @@ proc do_windows_shortcut {} {\n }\n \n proc do_cygwin_shortcut {} {\n-\tglobal argv0 _gitworktree\n+\tglobal argv0 _gitworktree oguilib\n \n \tif {[catch {\n \t\tset desktop [exec cygpath \\\n-\t\t\t--windows \\\n-\t\t\t--absolute \\\n-\t\t\t--long-name \\\n \t\t\t--desktop]\n \t\t}]} {\n \t\t\tset desktop .\n@@ -48,19 +45,19 @@ proc do_cygwin_shortcut {} {\n \t\t\tset fn ${fn}.lnk\n \t\t}\n \t\tif {[catch {\n-\t\t\t\tset sh [exec cygpath \\\n-\t\t\t\t\t--windows \\\n-\t\t\t\t\t--absolute \\\n-\t\t\t\t\t/bin/sh.exe]\n-\t\t\t\tset me [exec cygpath \\\n-\t\t\t\t\t--unix \\\n-\t\t\t\t\t--absolute \\\n-\t\t\t\t\t$argv0]\n-\t\t\t\twin32_create_lnk $fn [list \\\n-\t\t\t\t\t$sh -c \\\n-\t\t\t\t\t\"CHERE_INVOKING=1 source /etc/profile;[sq $me] &\" \\\n-\t\t\t\t\t] \\\n-\t\t\t\t\t[file normalize $_gitworktree]\n+\t\t\t\tset repodir [file normalize $_gitworktree]\n+\t\t\t\tset shargs {-c \\\n+\t\t\t\t\t\"CHERE_INVOKING=1 \\\n+\t\t\t\t\tsource /etc/profile; \\\n+\t\t\t\t\tgit gui\"}\n+\t\t\t\texec /bin/mkshortcut.exe \\\n+\t\t\t\t\t--arguments $shargs \\\n+\t\t\t\t\t--desc \"git-gui on $repodir\" \\\n+\t\t\t\t\t--icon $oguilib/git-gui.ico \\\n+\t\t\t\t\t--name $fn \\\n+\t\t\t\t\t--show min \\\n+\t\t\t\t\t--workingdir $repodir \\\n+\t\t\t\t\t/bin/sh.exe\n \t\t\t} err]} {\n \t\t\terror_popup [strcat [mc \"Cannot write shortcut:\"] \"\\n\\n$err\"]\n \t\t}\n-- \n2.41.0.99.19\n\n"},{"id":"478831","messageId":"7392e94c-ad72-95d1-6cb7-2112aa7bf29b@gmail.com","threadId":"59909","inReplyTo":"xmqqsfae5igi.fsf@gitster.g","subject":"Re: [PATCH v0 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-06-26T16:55:01Z","receivedAt":"2023-06-26T16:55:06Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/26/23 11:52, Junio C Hamano wrote:\n> Mark Levedahl writes:\n>\n>> I had originally organized as you suggest, no problem doing so\n>> again. What gave me pause was this paragraph I originally wrote for\n>> the cover letter:\n>>\n>> Patches 1/2 cause git-gui to function as it has for the last decade on\n>> Cygwin, but with Cygwin being detected. However, the browsing and\n>> shortcut creation menu items, removed in 2012 then re-added when is_Cygwin\n>> was fixed, do not work, and shortcut creation will crash git-gui if used.\n>> These are fixed in patches 3 / 4.\n> As you are removing (ancient) Cygwin specific code that did not work\n> with modern Cygwin at all in step [2/4], it is not so unexpected\n> that some stuff does still not work after that step. I am not sure\n> what your reservation exactly is, but if you are wondering if code\n> to disable browsing and shortcut creation on Cygwin temporarily\n> needs to be there in the same step (instead of crashing or not\n> working), it may make sense if and only if it is done with minimal\n> changes.\n>\n> Thanks.\n>\nTimely response ... yes, that was my concern. I resolved this by making \nthe cover letter and patch 2 commit message explicit that broken code \nremains.\nThank you,\nMark\n"},{"id":"478878","messageId":"5c1dc85f-f3a3-2e60-be85-08eefb633e5c@gmx.de","threadId":"59909","inReplyTo":"20230626165305.37488-1-mlevedahl@gmail.com","subject":"Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-06-27T11:51:15Z","receivedAt":"2023-06-27T11:51:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Mark,\n\nOn Mon, 26 Jun 2023, Mark Levedahl wrote:\n\n> === This is an update, incorporating responses to Junio's and Eric's\n> comments:\n> -- clarified what the \"upstream\" git-gui branch is\n> -- Removed some changes from patch 2 as requested by Junio, reducing\n> changes in patch 3 and patch 4\n> All code is fixed only after applying patch 4\n> Differences in patch 3 and 4 are minimimized\n> -- updated comments to clarify G4w dedicated code.\n> -- updated all comments to (hopefully) clarify points of confusion\n> ===\n\nAnd here is the range-diff:\n\n 1: 00000000000 ! 1: 00000000000 git gui Makefile - remove Cygwin modifications\n @@ Metadata\n Author: Mark Levedahl \n\n ## Commit message ##\n - git gui Makefile - remove Cygwin modiifications\n + git gui Makefile - remove Cygwin modifications\n\n git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries\n into git-gui, destroying the ability to package git-gui on one machine and\n @@ Commit message\n since 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk.\n\n The Cygwin git maintainer disables this code, so this code is definitely\n - not in use in the Cygwin distribution, and targets an untested /\n - unsupportable configuration.\n + not in use in the Cygwin distribution.\n\n - The simplest approach is to just delete the Cygwin specific code as\n - stock Cygwin needs no special handling. Do so.\n + The simplest fix is to just delete the Cygwin specific code,\n + allowing the Makefile to work out of the box on Cygwin. Do so.\n\n Signed-off-by: Mark Levedahl \n\n 2: 00000000000 ! 2: 00000000000 git-gui - remove obsolete Cygwin specific code\n @@ Commit message\n 8.4.1 Windows Tcl/Tk code. In March, 2012, that 8.4.1 package was\n replaced with a full port based upon the upstream unix/X11 code,\n since maintained up to date. The two Tcl/Tk packages are completely\n - incompatible, and have different sygnatures.\n + incompatible, and have different signatures.\n\n When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer\n detected Cygwin, so did not enable Cygwin specific code, and the POSIX\n @@ Commit message\n unix. Thus, no-one apparently noticed the existence of incompatible\n Cygwin specific code.\n\n - However, since commit c5766eae6f2b002396b6cd4f85b62317b707174e in\n - upstream git-gui, the is_Cygwin funcion does detect current Cygwin. The\n - Cygwin specific code is enabled, causing use of Windows rather than unix\n - pathnames, and enabling incorrect warnings about environment variables\n - that are not relevant for the fully functional unix/X11 Tcl/Tk. The end\n - result is that git-gui is now incommpatible with Cygwin.\n + However, since commit c5766eae6f in the git-gui source tree\n + (https://github.com/prati0100/git-gui, master at a5005ded), and not yet\n + pulled into the git repository, the is_Cygwin function does detect\n + Cygwin using the unix/X11 Tcl/Tk. The Cygwin specific code is enabled,\n + causing use of Windows rather than unix pathnames, and enabling\n + incorrect warnings about environment variables that were relevant only\n + to the old Tcl/Tk. The end result is that (upstream) git-gui is now\n + incompatible with Cygwin.\n\n - So, delete all Cygwin specific code (code protected by \"if is_Cygwin\"),\n - thus restoring the post-2012 behaviour. Note that Cygwin specific code\n - is required to enable file browsing and shortcut creation (supported\n - before 2012), but is not addressed in this patch.\n + So, delete Cygwin specific code (code protected by \"if is_Cygwin\") that\n + is not needed in any form to work with the unix/X11 Tcl/Tk.\n +\n + Cygwin specific code required to enable file browsing and shortcut\n + creation is not addressed in this patch, does not currently work, and\n + invocation of those items may leave git-gui in a confused state.\n\n Signed-off-by: Mark Levedahl \n\n @@ git-gui.sh: proc rescan {after {honor_trustmtime 1}} {\n }\n\n proc rescan_stage2 {fd after} {\n -@@ git-gui.sh: proc do_git_gui {} {\n -\n - # Get the system-specific explorer app/command.\n - proc get_explorer {} {\n --\tif {[is_Cygwin] || [is_Windows]} {\n -+\tif {[is_Windows]} {\n - \t\tset explorer \"explorer.exe\"\n - \t} elseif {[is_MacOSX]} {\n - \t\tset explorer \"open\"\n -@@ git-gui.sh: if {[is_enabled multicommit]} {\n -\n - \t.mbar.repository add separator\n -\n --\tif {[is_Cygwin]} {\n --\t\t.mbar.repository add command \\\n --\t\t\t-label [mc \"Create Desktop Icon\"] \\\n --\t\t\t-command do_cygwin_shortcut\n --\t} elseif {[is_Windows]} {\n -+\tif {[is_Windows]} {\n - \t\t.mbar.repository add command \\\n - \t\t\t-label [mc \"Create Desktop Icon\"] \\\n - \t\t\t-command do_windows_shortcut\n @@ git-gui.sh: if {[is_MacOSX]} {\n set doc_path [githtmldir]\n if {$doc_path ne {}} {\n @@ lib/choose_repository.tcl: method _do_clone2 {} {\n \t\t\t\t}\n \t\t\t\tclose $f_in\n \t\t\t\tclose $f_cp\n -\n - ## lib/shortcut.tcl ##\n -@@ lib/shortcut.tcl: proc do_windows_shortcut {} {\n - \t}\n - }\n -\n --proc do_cygwin_shortcut {} {\n --\tglobal argv0 _gitworktree\n --\n --\tif {[catch {\n --\t\tset desktop [exec cygpath \\\n --\t\t\t--windows \\\n --\t\t\t--absolute \\\n --\t\t\t--long-name \\\n --\t\t\t--desktop]\n --\t\t}]} {\n --\t\t\tset desktop .\n --\t}\n --\tset fn [tk_getSaveFile \\\n --\t\t-parent . \\\n --\t\t-title [mc \"%s (%s): Create Desktop Icon\" [appname] [reponame]] \\\n --\t\t-initialdir $desktop \\\n --\t\t-initialfile \"Git [reponame].lnk\"]\n --\tif {$fn != {}} {\n --\t\tif {[file extension $fn] ne {.lnk}} {\n --\t\t\tset fn ${fn}.lnk\n --\t\t}\n --\t\tif {[catch {\n --\t\t\t\tset sh [exec cygpath \\\n --\t\t\t\t\t--windows \\\n --\t\t\t\t\t--absolute \\\n --\t\t\t\t\t/bin/sh.exe]\n --\t\t\t\tset me [exec cygpath \\\n --\t\t\t\t\t--unix \\\n --\t\t\t\t\t--absolute \\\n --\t\t\t\t\t$argv0]\n --\t\t\t\twin32_create_lnk $fn [list \\\n --\t\t\t\t\t$sh -c \\\n --\t\t\t\t\t\"CHERE_INVOKING=1 source /etc/profile;[sq $me] &\" \\\n --\t\t\t\t\t] \\\n --\t\t\t\t\t[file normalize $_gitworktree]\n --\t\t\t} err]} {\n --\t\t\terror_popup [strcat [mc \"Cannot write shortcut:\"] \"\\n\\n$err\"]\n --\t\t}\n --\t}\n --}\n --\n - proc do_macosx_app {} {\n - \tglobal argv0 env\n -\n 3: 00000000000 ! 3: 00000000000 git-gui - use cygstart to browse on Cygwin\n @@ Metadata\n ## Commit message ##\n git-gui - use cygstart to browse on Cygwin\n\n - Pre-2012, git-gui enabled the \"Repository->Explore Working Copy\" menu on\n - Cygwin, offering open a Windows graphical file browser at the root\n - working directory. The old code relied upon internal use of Windows\n - pathnames, while git-gui must use unix pathnames on Cygwin since 2012,\n - so was removed in a previous patch.\n + git-gui enables the \"Repository->Explore Working Copy\" menu on Cygwin,\n + offering to open a Windows graphical file browser at the root of the\n + working directory. This code, shared with Git For Windows support,\n + depends upon use of Windows pathnames. However, git gui on Cygwin uses\n + unix pathnames, so this shared code will not work on Cygwin.\n\n A base install of Cygwin provides the /bin/cygstart utility that runs\n - arbtitrary Windows applications after translating unix pathnames to\n - Windows. Adding the --explore option guarantees that the Windows file\n - explorer is opened, regardless of the supplied pathname's file type and\n - avoiding possibility of some other action being taken.\n + a registered Windows application based upon the file type, after\n + translating unix pathnames to Windows. Adding the --explore option\n + guarantees that the Windows file explorer is opened, regardless of the\n + supplied pathname's file type and avoiding possibility of some other\n + action being taken.\n\n So, teach git-gui to use cygstart --explore on Cygwin, restoring the\n - pre-2012 behavior of opening a Windows file explorer for browsing.\n + pre-2012 behavior of opening a Windows file explorer for browsing. This\n + separates the Git For Windows and Cygwin code paths. Note that\n + is_Windows is never true on Cygwin, and is_Cygwin is never true on Git\n + for Windows, though this is not obvious by examining the code for those\n + independent functions.\n\n Signed-off-by: Mark Levedahl \n\n ## git-gui.sh ##\n @@ git-gui.sh: proc do_git_gui {} {\n +\n + # Get the system-specific explorer app/command.\n proc get_explorer {} {\n - \tif {[is_Windows]} {\n - \t\tset explorer \"explorer.exe\"\n -+\t} elseif {[is_Cygwin]} {\n +-\tif {[is_Cygwin] || [is_Windows]} {\n ++\tif {[is_Cygwin]} {\n +\t\tset explorer \"/bin/cygstart.exe --explore\"\n ++\t} elseif {[is_Windows]} {\n + \t\tset explorer \"explorer.exe\"\n \t} elseif {[is_MacOSX]} {\n \t\tset explorer \"open\"\n - \t} else {\n 4: 00000000000 ! 4: 00000000000 git-gui - use mkshortcut on Cygwin\n @@ Metadata\n ## Commit message ##\n git-gui - use mkshortcut on Cygwin\n\n - Prior to 2012, git-gui enabled the \"Repository->Create Desktop Icon\"\n - item on Cygwin, offering to create a shortcut that starts git-gui on a\n - particular repository. The original code for this in lib/win32.tcl,\n - shared with Git for Windows support, requires Windows pathnames, while\n - git-gui must use unix pathnames with the unix/X11 Tcl/Tk since 2012. The\n - ability to use this from Cygwin was removed in a previous patch.\n + git-gui enables the \"Repository->Create Desktop Icon\" item on Cygwin,\n + offering to create a shortcut that starts git-gui on the current\n + repository. The code in do_cygwin_shortcut invokes function\n + win32_create_lnk to create the shortcut. This latter function is shared\n + between Cygwin and Git For Windows and expects Windows rather than unix\n + pathnames, though do_cygwin_shortcut provides unix pathnames. Also, this\n + function tries to invoke the Windows Script Host to run a javascript\n + snippet, but this fails under Cygwin's Tcl. So, win32_create_lnk just\n + does not support Cygwin.\n\n - Cygwin's default installation provides /bin/mkshortcut for creating\n - desktop shortuts, this is compatible with exec under tcl, and understands\n - Cygwin's unix pathnames. So, teach git-gui to use mkshortcut on Cygwin,\n - leaving lib/win32.tcl as Git for Windows specific support.\n + However, Cygwin's default installation provides /bin/mkshortcut for\n + creating desktop shortcuts. This is compatible with exec under Cygwin's\n + Tcl, understands Cygwin's unix pathnames, and avoids the need for shell\n + escapes to encode troublesome paths. So, teach git-gui to use mkshortcut\n + on Cygwin, leaving win32_create_lnk unchanged and for exclusive use by\n + Git For Windows.\n\n Notes: \"CHERE_INVOKING=1\" is recognized by Cygwin's /etc/profile and\n prevents a \"chdir $HOME\", leaving the shell in the working directory\n specified by the shortcut. That directory is written directly by\n mkshortcut eliminating any problems with shell escapes and quoting.\n\n - The pre-2012 code includes the full pathname of the git-gui creating the\n - shortcut (rather than using the system git-gui), but that git-gui might\n - not be compatible with the git found after /etc/profile sets the path,\n - and might have a pathname that defies encoding using shell escapes that\n - can survive the multiple incompatible interpreters involved in this\n - chain. Instead, use \"git gui\", thus defaulting to the system git and\n + The code being replaced includes the full pathname of the git-gui\n + creating the shortcut, but that git-gui might not be compatible with the\n + git found after /etc/profile sets the path, and might have a pathname\n + that defies encoding using shell escapes that can survive the multiple\n + incompatible interpreters involved in the chain of creating and using\n + this shortcut. The new code uses bare \"git gui\" as the command to\n + execute, thus using the system git to launch the system git-gui, and\n avoiding both issues.\n\n Signed-off-by: Mark Levedahl \n\n - ## git-gui.sh ##\n -@@ git-gui.sh: if {[is_enabled multicommit]} {\n - \t\t.mbar.repository add command \\\n - \t\t\t-label [mc \"Create Desktop Icon\"] \\\n - \t\t\t-command do_windows_shortcut\n -+\t} elseif {[is_Cygwin]} {\n -+\t\t.mbar.repository add command \\\n -+\t\t\t-label [mc \"Create Desktop Icon\"] \\\n -+\t\t\t-command do_cygwin_shortcut\n - \t} elseif {[is_MacOSX]} {\n - \t\t.mbar.repository add command \\\n - \t\t\t-label [mc \"Create Desktop Icon\"] \\\n -\n ## lib/shortcut.tcl ##\n @@ lib/shortcut.tcl: proc do_windows_shortcut {} {\n - \t}\n }\n\n -+proc do_cygwin_shortcut {} {\n + proc do_cygwin_shortcut {} {\n +-\tglobal argv0 _gitworktree\n +\tglobal argv0 _gitworktree oguilib\n -+\n -+\tif {[catch {\n -+\t\tset desktop [exec cygpath \\\n -+\t\t\t--desktop]\n -+\t\t}]} {\n -+\t\t\tset desktop .\n -+\t}\n -+\tset fn [tk_getSaveFile \\\n -+\t\t-parent . \\\n -+\t\t-title [mc \"%s (%s): Create Desktop Icon\" [appname] [reponame]] \\\n -+\t\t-initialdir $desktop \\\n -+\t\t-initialfile \"Git [reponame].lnk\"]\n -+\tif {$fn != {}} {\n -+\t\tif {[file extension $fn] ne {.lnk}} {\n -+\t\t\tset fn ${fn}.lnk\n -+\t\t}\n -+\t\tif {[catch {\n +\n + \tif {[catch {\n + \t\tset desktop [exec cygpath \\\n +-\t\t\t--windows \\\n +-\t\t\t--absolute \\\n +-\t\t\t--long-name \\\n + \t\t\t--desktop]\n + \t\t}]} {\n + \t\t\tset desktop .\n +@@ lib/shortcut.tcl: proc do_cygwin_shortcut {} {\n + \t\t\tset fn ${fn}.lnk\n + \t\t}\n + \t\tif {[catch {\n +-\t\t\t\tset sh [exec cygpath \\\n +-\t\t\t\t\t--windows \\\n +-\t\t\t\t\t--absolute \\\n +-\t\t\t\t\t/bin/sh.exe]\n +-\t\t\t\tset me [exec cygpath \\\n +-\t\t\t\t\t--unix \\\n +-\t\t\t\t\t--absolute \\\n +-\t\t\t\t\t$argv0]\n +-\t\t\t\twin32_create_lnk $fn [list \\\n +-\t\t\t\t\t$sh -c \\\n +-\t\t\t\t\t\"CHERE_INVOKING=1 source /etc/profile;[sq $me] &\" \\\n +-\t\t\t\t\t] \\\n +-\t\t\t\t\t[file normalize $_gitworktree]\n +\t\t\t\tset repodir [file normalize $_gitworktree]\n +\t\t\t\tset shargs {-c \\\n +\t\t\t\t\t\"CHERE_INVOKING=1 \\\n +\t\t\t\t\tsource /etc/profile; \\\n +\t\t\t\t\tgit gui\"}\n +\t\t\t\texec /bin/mkshortcut.exe \\\n -+\t\t\t\t\t-a $shargs \\\n -+\t\t\t\t\t-d \"git-gui on $repodir\" \\\n -+\t\t\t\t\t-i $oguilib/git-gui.ico \\\n -+\t\t\t\t\t-n $fn \\\n -+\t\t\t\t\t-s min \\\n -+\t\t\t\t\t-w $repodir \\\n ++\t\t\t\t\t--arguments $shargs \\\n ++\t\t\t\t\t--desc \"git-gui on $repodir\" \\\n ++\t\t\t\t\t--icon $oguilib/git-gui.ico \\\n ++\t\t\t\t\t--name $fn \\\n ++\t\t\t\t\t--show min \\\n ++\t\t\t\t\t--workingdir $repodir \\\n +\t\t\t\t\t/bin/sh.exe\n -+\t\t\t} err]} {\n -+\t\t\terror_popup [strcat [mc \"Cannot write shortcut:\"] \"\\n\\n$err\"]\n -+\t\t}\n -+\t}\n -+}\n -+\n - proc do_macosx_app {} {\n - \tglobal argv0 env\n -\n + \t\t\t} err]} {\n + \t\t\terror_popup [strcat [mc \"Cannot write shortcut:\"] \"\\n\\n$err\"]\n + \t\t}\n\nFWIW even v1 looked good to me (I don't care about typos as long as they\ndon't change meaning).\n\nI tested the changes on top of Git for Windows and everything works as\nexpected (if you want to cross check my work, look no further than the\n`git-gui-cygwin` branch at https://github.com/dscho/git).\n\nIf you want, please feel free to add\n\n\tAcked-by: Johannes Schindelin \n\nThank you!\nJohannes\n"},{"id":"478912","messageId":"xmqq4jmsiyhw.fsf@gitster.g","threadId":"59909","inReplyTo":"20230626165305.37488-1-mlevedahl@gmail.com","subject":"Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-27T17:52:27Z","receivedAt":"2023-06-27T17:52:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> === This is an update, incorporating responses to Junio's and Eric's\n> comments:\n> -- clarified what the \"upstream\" git-gui branch is\n> -- Removed some changes from patch 2 as requested by Junio, reducing\n> changes in patch 3 and patch 4\n> All code is fixed only after applying patch 4\n> Differences in patch 3 and 4 are minimimized\n> -- updated comments to clarify G4w dedicated code.\n> -- updated all comments to (hopefully) clarify points of confusion\n> ===\n> ...\n> Mark Levedahl (4):\n> git gui Makefile - remove Cygwin modifications\n> git-gui - remove obsolete Cygwin specific code\n> git-gui - use cygstart to browse on Cygwin\n> git-gui - use mkshortcut on Cygwin\n>\n> Makefile | 21 +------\n> git-gui.sh | 118 +++-----------------------------------\n> lib/choose_repository.tcl | 27 +--------\n> lib/shortcut.tcl | 31 +++++-----\n> 4 files changed, 27 insertions(+), 170 deletions(-)\n\nOK, Dscho says v1 looks good, and I have no further comments.\n\nPratyush, can I expect that you take further comments and usher\nthese patches to your tree, and eventually tell me to pull from your\nrepository?\n\nThanks, all.\n"},{"id":"480195","messageId":"07677f17-be9b-dc46-d204-6fe46d46ebc0@gmail.com","threadId":"59909","inReplyTo":"xmqq4jmsiyhw.fsf@gitster.g","subject":"Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-08-05T14:47:57Z","receivedAt":"2023-08-05T14:48:04Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/27/23 13:52, Junio C Hamano wrote:\n> Mark Levedahl writes:\n>\n>> === This is an update, incorporating responses to Junio's and Eric's\n>> comments:\n>> -- clarified what the \"upstream\" git-gui branch is\n>> -- Removed some changes from patch 2 as requested by Junio, reducing\n>> changes in patch 3 and patch 4\n>> All code is fixed only after applying patch 4\n>> Differences in patch 3 and 4 are minimimized\n>> -- updated comments to clarify G4w dedicated code.\n>> -- updated all comments to (hopefully) clarify points of confusion\n>> ===\n>> ...\n>> Mark Levedahl (4):\n>> git gui Makefile - remove Cygwin modifications\n>> git-gui - remove obsolete Cygwin specific code\n>> git-gui - use cygstart to browse on Cygwin\n>> git-gui - use mkshortcut on Cygwin\n>>\n>> Makefile | 21 +------\n>> git-gui.sh | 118 +++-----------------------------------\n>> lib/choose_repository.tcl | 27 +--------\n>> lib/shortcut.tcl | 31 +++++-----\n>> 4 files changed, 27 insertions(+), 170 deletions(-)\n> OK, Dscho says v1 looks good, and I have no further comments.\n>\n> Pratyush, can I expect that you take further comments and usher\n> these patches to your tree, and eventually tell me to pull from your\n> repository?\n>\n> Thanks, all.\n\nJunio,\n\nThank you and Dscho for the detailed reviews. But, there is no response \nfrom Pratyush in over a month, is there a different maintainer then who \nshould take this?\n\nMark\n\n"},{"id":"480997","messageId":"mafs01qfse8re.fsf@amazon.de","threadId":"59909","inReplyTo":"07677f17-be9b-dc46-d204-6fe46d46ebc0@gmail.com","subject":"Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Pratyush Yadav","fromEmail":"me@yadavpratyush.com","sentAt":"2023-08-24T15:54:13Z","receivedAt":"2023-08-24T15:55:15Z","isPatch":true,"sender":{"key":"me@yadavpratyush.com","avatar":"https://avatars.githubusercontent.com/u/8817931?v=4"},"body":"On Sat, Aug 05 2023, Mark Levedahl wrote:\n\n> On 6/27/23 13:52, Junio C Hamano wrote:\n>> Mark Levedahl writes:\n>>\n>>> === This is an update, incorporating responses to Junio's and Eric's\n>>> comments:\n>>> -- clarified what the \"upstream\" git-gui branch is\n>>> -- Removed some changes from patch 2 as requested by Junio, reducing\n>>> changes in patch 3 and patch 4\n>>> All code is fixed only after applying patch 4\n>>> Differences in patch 3 and 4 are minimimized\n>>> -- updated comments to clarify G4w dedicated code.\n>>> -- updated all comments to (hopefully) clarify points of confusion\n>>> ===\n>>> ...\n>>> Mark Levedahl (4):\n>>> git gui Makefile - remove Cygwin modifications\n>>> git-gui - remove obsolete Cygwin specific code\n>>> git-gui - use cygstart to browse on Cygwin\n>>> git-gui - use mkshortcut on Cygwin\n>>>\n>>> Makefile | 21 +------\n>>> git-gui.sh | 118 +++-----------------------------------\n>>> lib/choose_repository.tcl | 27 +--------\n>>> lib/shortcut.tcl | 31 +++++-----\n>>> 4 files changed, 27 insertions(+), 170 deletions(-)\n>> OK, Dscho says v1 looks good, and I have no further comments.\n>>\n>> Pratyush, can I expect that you take further comments and usher\n>> these patches to your tree, and eventually tell me to pull from your\n>> repository?\n>>\n>> Thanks, all.\n>\n> Junio,\n>\n> Thank you and Dscho for the detailed reviews. But, there is no response from\n> Pratyush in over a month, is there a different maintainer then who should take\n> this?\n\nAlmost 2 months now... I'm sorry. I just do not find enough time or\nenergy for git-gui these days. More on that later.\n\nFor now, I took a brief look at the patches. They look good to me. I\nappreciate the detailed commit messages. I did not test them since I do\nnot have a Windows setup currently, but I believe Johannes did so it's\nall good for me.\n\nApplied to git-gui/master. Will send a pull request soon.\n\n-- \nRegards,\nPratyush Yadav\n"},{"id":"481116","messageId":"a1e256cd-072d-a3a0-cdbe-ed65ed21bfd3@gmail.com","threadId":"59909","inReplyTo":"xmqq4jmsiyhw.fsf@gitster.g","subject":"Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Mark Levedahl","fromEmail":"mlevedahl@gmail.com","sentAt":"2023-08-29T16:03:35Z","receivedAt":"2023-08-29T16:04:19Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"\nOn 6/27/23 13:52, Junio C Hamano wrote:\n> OK, Dscho says v1 looks good, and I have no further comments.\n>\n> Pratyush, can I expect that you take further comments and usher\n> these patches to your tree, and eventually tell me to pull from your\n> repository?\n>\n> Thanks, all.\n\nJunio,\n\nI see you merged latest git-gui from Pratyush onto next. As git-gui has \nno test facility I merged your py/git-gui-updates (a7935203) into your \nmaster (5dc72c0f), built on both Linux and Cygwin. git-gui works as \nexpected on both. Looks good to me.\n\nMark\n\n"},{"id":"481117","messageId":"xmqq8r9tg6u3.fsf@gitster.g","threadId":"59909","inReplyTo":"a1e256cd-072d-a3a0-cdbe-ed65ed21bfd3@gmail.com","subject":"Re: [PATCH v1 0/4] Remove obsolete Cygwin support from git-gui","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-08-29T16:18:44Z","receivedAt":"2023-08-29T16:19:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> On 6/27/23 13:52, Junio C Hamano wrote:\n>> OK, Dscho says v1 looks good, and I have no further comments.\n>>\n>> Pratyush, can I expect that you take further comments and usher\n>> these patches to your tree, and eventually tell me to pull from your\n>> repository?\n>>\n>> Thanks, all.\n>\n> Junio,\n>\n> I see you merged latest git-gui from Pratyush onto next. As git-gui\n> has no test facility I merged your py/git-gui-updates (a7935203) into\n> your master (5dc72c0f), built on both Linux and Cygwin. git-gui works\n> as expected on both. Looks good to me.\n\nThanks!\n"}]}