From 8cf36407cab4065d210d36aee03f58cd18397db8 Mon Sep 17 00:00:00 2001 From: Vipul Kumar Date: Fri, 5 Nov 2021 11:53:33 +0530 Subject: [PATCH 01/11] git-gui: Fix a typo in README Add a missing punctuation. Signed-off-by: Vipul Kumar Signed-off-by: Pratyush Yadav --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5ce2122fbc6183..b460b649a8ccfe 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ that you first use `git-format-patch` to generate the emails, audit them, and then send them via `git-send-email`. A pretty good guide to configuring and using `git-send-email` can be found -[here](https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/) +[here](https://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/). ### Using your email client From 8f23432b38d9b122be8179295a56688391dc8ad6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 23 Nov 2022 09:12:49 +0100 Subject: [PATCH 02/11] windows: ignore empty `PATH` elements When looking up an executable via the `_which` function, Git GUI imitates the `execlp()` strategy where the environment variable `PATH` is interpreted as a list of paths in which to search. For historical reasons, stemming from the olden times when it was uncommon to download a lot of files from the internet into the current directory, empty elements in this list are treated as if the current directory had been specified. Nowadays, of course, this treatment is highly dangerous as the current directory often contains files that have just been downloaded and not yet been inspected by the user. Unix/Linux users are essentially expected to be very, very careful to simply not add empty `PATH` elements, i.e. not to make use of that feature. On Windows, however, it is quite common for `PATH` to contain empty elements by mistake, e.g. as an unintended left-over entry when an application was installed from the Windows Store and then uninstalled manually. While it would probably make most sense to safe-guard not only Windows users, it seems to be common practice to ignore these empty `PATH` elements _only_ on Windows, but not on other platforms. Sadly, this practice is followed inconsistently between different software projects, where projects with few, if any, Windows-based contributors tend to be less consistent or even "blissful" about it. Here is a non-exhaustive list: Cygwin: It specifically "eats" empty paths when converting path lists to POSIX: https://github.com/cygwin/cygwin/commit/753702223c7d I.e. it follows the common practice. PowerShell: It specifically ignores empty paths when searching the `PATH`. The reason for this is apparently so self-evident that it is not even mentioned here: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_environment_variables#path-information I.e. it follows the common practice. CMD: Oh my, CMD. Let's just forget about it, nobody in their right (security) mind takes CMD as inspiration. It is so unsafe by default that we even planned on dropping `Git CMD` from Git for Windows altogether, and only walked back on that plan when we found a super ugly hack, just to keep Git's users secure by default: https://github.com/git-for-windows/MINGW-packages/commit/82172388bb51 So CMD chooses to hide behind the battle cry "Works as Designed!" that all too often leaves users vulnerable. CMD is probably the most prominent project whose lead you want to avoid following in matters of security. Win32 API (`CreateProcess()`) Just like CMD, `CreateProcess()` adheres to the original design of the path lookup in the name of backward compatibility (see https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw for details): If the file name does not contain a directory path, the system searches for the executable file in the following sequence: 1. The directory from which the application loaded. 2. The current directory for the parent process. [...] I.e. the Win32 API itself chooses backwards compatibility over users' safety. Git LFS: There have been not one, not two, but three security advisories about Git LFS executing executables from the current directory by mistake. As part of one of them, a change was introduced to stop treating empty `PATH` elements as equivalent to `.`: https://github.com/git-lfs/git-lfs/commit/7cd7bb0a1f0d I.e. it follows the common practice. Go: Go does not follow the common practice, and you can think about that what you want: https://github.com/golang/go/blob/go1.19.3/src/os/exec/lp_windows.go#L114-L135 https://github.com/golang/go/blob/go1.19.3/src/path/filepath/path_windows.go#L108-L137 Git Credential Manager: It tries to imitate Git LFS, but unfortunately misses the empty `PATH` element handling. As of time of writing, this is in the process of being fixed: https://github.com/GitCredentialManager/git-credential-manager/pull/968 So now that we have established that it is a common practice to ignore empty `PATH` elements on Windows, let's assess this commit's change using Schneier's Five-Step Process (https://www.schneier.com/crypto-gram/archives/2002/0415.html#1): Step 1: What problem does it solve? It prevents an entire class of Remote Code Execution exploits via Git GUI's `Clone` functionality. Step 2: How well does it solve that problem? Very well. It prevents the attack vector of luring an unsuspecting victim into cloning an executable into the worktree root directory that Git GUI immediately executes. Step 3: What other security problems does it cause? Maybe non-security problems: If a project (ab-)uses the unsafe `PATH` lookup. That would not only be unsafe, though, but fragile in the first place because it would break when running in a subdirectory. Therefore I would consider this a scenario not worth keeping working. Step 4: What are the costs of this measure? Almost nil, except for the time writing up this commit message ;-) Step 5: Given the answers to steps two through four, is the security measure worth the costs? Yes. Keeping Git's users Secure By Default is worth it. It's a tiny price to pay compared to the damages even a single successful exploit can cost. So let's follow that common practice in Git GUI, too. Signed-off-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- git-gui.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index 201524c34edac0..0cf625ca01ca35 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -464,6 +464,9 @@ proc _which {what args} { regsub -all ";" $gitguidir "\\;" gitguidir set env(PATH) "$gitguidir;$env(PATH)" set _search_path [split $env(PATH) {;}] + # Skip empty `PATH` elements + set _search_path [lsearch -all -inline -not -exact \ + $_search_path ""] set _search_exe .exe } else { set _search_path [split $env(PATH) :] From c5766eae6f2b002396b6cd4f85b62317b707174e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 4 Dec 2022 22:56:08 +0100 Subject: [PATCH 03/11] is_Cygwin: avoid `exec`ing anything The `is_Cygwin` function is used, among other things, to determine how executables are discovered in the `PATH` list by the `_which` function. We are about to change the behavior of the `_which` function on Windows (but not Cygwin): On Windows, we want it to ignore empty elements of the `PATH` instead of treating them as referring to the current directory (which is a "legacy feature" according to https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03, but apparently not explicitly deprecated, the POSIX documentation is quite unclear on that even if the Cygwin project itself considers it to be deprecated: https://github.com/cygwin/cygwin/commit/fc74dbf22f5c). This is important because on Windows, `exec` does something very unsafe by default (unless we're running a Cygwin version of Tcl, which follows Unix semantics). However, we try to `exec` something _inside_ `is_Cygwin` to determine whether we're running within Cygwin or not, i.e. before we determined whether we need to handle `PATH` specially or not. That's a Catch-22. Therefore, and because it is much cleaner anyway, use the `$::tcl_platform(os)` value which is guaranteed to start with `CYGWIN_` when running a Cygwin variant of Tcl/Tk, instead of executing `cygpath --windir`. Signed-off-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- git-gui.sh | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 0cf625ca01ca35..0fe60f80cc256f 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -269,16 +269,8 @@ proc is_Windows {} { proc is_Cygwin {} { global _iscygwin if {$_iscygwin eq {}} { - if {$::tcl_platform(platform) eq {windows}} { - if {[catch {set p [exec cygpath --windir]} err]} { - set _iscygwin 0 - } else { - set _iscygwin 1 - # Handle MSys2 which is only cygwin when MSYSTEM is MSYS. - if {[info exists ::env(MSYSTEM)] && $::env(MSYSTEM) ne "MSYS"} { - set _iscygwin 0 - } - } + if {[string match "CYGWIN_*" $::tcl_platform(os)]} { + set _iscygwin 1 } else { set _iscygwin 0 } From e0539b4b25310e242d30b91a2c86f5e7a30aade6 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 16 Dec 2022 20:41:28 +0100 Subject: [PATCH 04/11] Move is_ functions to the beginning We need these in `_which` and they should be defined before that function's definition. This commit is best viewed with `--color-moved`. Signed-off-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- git-gui.sh | 58 +++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index 0fe60f80cc256f..f779fc92684856 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -44,6 +44,37 @@ if {[catch {package require Tcl 8.5} err] catch {rename send {}} ; # What an evil concept... +###################################################################### +## +## Enabling platform-specific code paths + +proc is_MacOSX {} { + if {[tk windowingsystem] eq {aqua}} { + return 1 + } + return 0 +} + +proc is_Windows {} { + if {$::tcl_platform(platform) eq {windows}} { + return 1 + } + return 0 +} + +set _iscygwin {} +proc is_Cygwin {} { + global _iscygwin + if {$_iscygwin eq {}} { + if {[string match "CYGWIN_*" $::tcl_platform(os)]} { + set _iscygwin 1 + } else { + set _iscygwin 0 + } + } + return $_iscygwin +} + ###################################################################### ## ## locate our library @@ -163,7 +194,6 @@ set _isbare {} set _gitexec {} set _githtmldir {} set _reponame {} -set _iscygwin {} set _search_path {} set _shellpath {@@SHELL_PATH@@} @@ -252,32 +282,6 @@ proc reponame {} { return $::_reponame } -proc is_MacOSX {} { - if {[tk windowingsystem] eq {aqua}} { - return 1 - } - return 0 -} - -proc is_Windows {} { - if {$::tcl_platform(platform) eq {windows}} { - return 1 - } - return 0 -} - -proc is_Cygwin {} { - global _iscygwin - if {$_iscygwin eq {}} { - if {[string match "CYGWIN_*" $::tcl_platform(os)]} { - set _iscygwin 1 - } else { - set _iscygwin 0 - } - } - return $_iscygwin -} - proc is_enabled {option} { global enabled_options if {[catch {set on $enabled_options($option)}]} {return 0} From fd477a1d3bab580c2fcdc435f551dca3094286ae Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 5 Dec 2022 14:37:41 +0100 Subject: [PATCH 05/11] Move the `_which` function (almost) to the top We are about to make use of the `_which` function to address CVE-2022-41953 by overriding Tcl/Tk's unsafe PATH lookup on Windows. In preparation for that, let's move it close to the top of the file to make sure that even early `exec` calls that happen during the start-up of Git GUI benefit from the fix. This commit is best viewed with `--color-moved`. Signed-off-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- git-gui.sh | 88 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index f779fc92684856..b0eb5a6ae48c00 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -75,6 +75,52 @@ proc is_Cygwin {} { return $_iscygwin } +###################################################################### +## +## PATH lookup + +set _search_path {} +proc _which {what args} { + global env _search_exe _search_path + + if {$_search_path eq {}} { + if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} { + set _search_path [split [exec cygpath \ + --windows \ + --path \ + --absolute \ + $env(PATH)] {;}] + set _search_exe .exe + } elseif {[is_Windows]} { + set gitguidir [file dirname [info script]] + regsub -all ";" $gitguidir "\\;" gitguidir + set env(PATH) "$gitguidir;$env(PATH)" + set _search_path [split $env(PATH) {;}] + # Skip empty `PATH` elements + set _search_path [lsearch -all -inline -not -exact \ + $_search_path ""] + set _search_exe .exe + } else { + set _search_path [split $env(PATH) :] + set _search_exe {} + } + } + + if {[is_Windows] && [lsearch -exact $args -script] >= 0} { + set suffix {} + } else { + set suffix $_search_exe + } + + foreach p $_search_path { + set p [file join $p $what$suffix] + if {[file exists $p]} { + return [file normalize $p] + } + } + return {} +} + ###################################################################### ## ## locate our library @@ -194,7 +240,6 @@ set _isbare {} set _gitexec {} set _githtmldir {} set _reponame {} -set _search_path {} set _shellpath {@@SHELL_PATH@@} set _trace [lsearch -exact $argv --trace] @@ -444,47 +489,6 @@ proc _git_cmd {name} { return $v } -proc _which {what args} { - global env _search_exe _search_path - - if {$_search_path eq {}} { - if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} { - set _search_path [split [exec cygpath \ - --windows \ - --path \ - --absolute \ - $env(PATH)] {;}] - set _search_exe .exe - } elseif {[is_Windows]} { - set gitguidir [file dirname [info script]] - regsub -all ";" $gitguidir "\\;" gitguidir - set env(PATH) "$gitguidir;$env(PATH)" - set _search_path [split $env(PATH) {;}] - # Skip empty `PATH` elements - set _search_path [lsearch -all -inline -not -exact \ - $_search_path ""] - set _search_exe .exe - } else { - set _search_path [split $env(PATH) :] - set _search_exe {} - } - } - - if {[is_Windows] && [lsearch -exact $args -script] >= 0} { - set suffix {} - } else { - set suffix $_search_exe - } - - foreach p $_search_path { - set p [file join $p $what$suffix] - if {[file exists $p]} { - return [file normalize $p] - } - } - return {} -} - # Test a file for a hashbang to identify executable scripts on Windows. proc is_shellscript {filename} { if {![file exists $filename]} {return 0} From aae9560a355d4ab91385e49eae62fade2ddd27ef Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 23 Nov 2022 09:31:06 +0100 Subject: [PATCH 06/11] Work around Tcl's default `PATH` lookup As per https://www.tcl.tk/man/tcl8.6/TclCmd/exec.html#M23, Tcl's `exec` function goes out of its way to imitate the highly dangerous path lookup of `cmd.exe`, but _of course_ only on Windows: If a directory name was not specified as part of the application name, the following directories are automatically searched in order when attempting to locate the application: The directory from which the Tcl executable was loaded. The current directory. The Windows 32-bit system directory. The Windows home directory. The directories listed in the path. The dangerous part is the second item, of course: `exec` _prefers_ executables in the current directory to those that are actually in the `PATH`. It is almost as if people wanted to Windows users vulnerable, specifically. To avoid that, Git GUI already has the `_which` function that does not imitate that dangerous practice when looking up executables in the search path. However, Git GUI currently fails to use that function e.g. when trying to execute `aspell` for spell checking. That is not only dangerous but combined with Tcl's unfortunate default behavior and with the fact that Git GUI tries to spell-check a repository just after cloning, leads to a critical Remote Code Execution vulnerability. Let's override both `exec` and `open` to always use `_which` instead of letting Tcl perform the path lookup, to prevent this attack vector. This addresses CVE-2022-41953. For more details, see https://github.com/git-for-windows/git/security/advisories/GHSA-v4px-mx59-w99c Signed-off-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- git-gui.sh | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/git-gui.sh b/git-gui.sh index b0eb5a6ae48c00..cb92bba1c40dc9 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -121,6 +121,62 @@ proc _which {what args} { return {} } +proc sanitize_command_line {command_line from_index} { + set i $from_index + while {$i < [llength $command_line]} { + set cmd [lindex $command_line $i] + if {[file pathtype $cmd] ne "absolute"} { + set fullpath [_which $cmd] + if {$fullpath eq ""} { + throw {NOT-FOUND} "$cmd not found in PATH" + } + lset command_line $i $fullpath + } + + # handle piped commands, e.g. `exec A | B` + for {incr i} {$i < [llength $command_line]} {incr i} { + if {[lindex $command_line $i] eq "|"} { + incr i + break + } + } + } + return $command_line +} + +# Override `exec` to avoid unsafe PATH lookup + +rename exec real_exec + +proc exec {args} { + # skip options + for {set i 0} {$i < [llength $args]} {incr i} { + set arg [lindex $args $i] + if {$arg eq "--"} { + incr i + break + } + if {[string range $arg 0 0] ne "-"} { + break + } + } + set args [sanitize_command_line $args $i] + uplevel 1 real_exec $args +} + +# Override `open` to avoid unsafe PATH lookup + +rename open real_open + +proc open {args} { + set arg0 [lindex $args 0] + if {[string range $arg0 0 0] eq "|"} { + set command_line [string trim [string range $arg0 1 end]] + lset args 0 "| [sanitize_command_line $command_line 0]" + } + uplevel 1 real_open $args +} + ###################################################################### ## ## locate our library From e69547b7b6484328c3f334934d8737f9b28d6a7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Wed, 30 Nov 2022 09:23:49 +0100 Subject: [PATCH 07/11] Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since GNU make 4.4 the semantics of the $(MAKEFLAGS) variable has changed in a backward-incompatible way, as its "NEWS" file notes: Previously only simple (one-letter) options were added to the MAKEFLAGS variable that was visible while parsing makefiles. Now, all options are available in MAKEFLAGS. If you want to check MAKEFLAGS for a one-letter option, expanding "$(firstword -$(MAKEFLAGS))" is a reliable way to return the set of one-letter options which can be examined via findstring, etc. This means that $(MAKEFLAGS) now contains long options like "--jobserver-auth=fifo:" and we have to adapt to that. Note that the "-" in "-$(MAKEFLAGS)" is critical here, as the variable will always contain leading whitespace if there are no short options, but long options are present. This is a partial backport of 67b36879fc0 (Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4, 2022-11-30), which had been applied directly to git/git. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano Signed-off-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 56c85a85c1e493..a0d5a4b28e1715 100644 --- a/Makefile +++ b/Makefile @@ -116,7 +116,7 @@ ifeq ($(uname_S),Darwin) TKEXECUTABLE = $(shell basename "$(TKFRAMEWORK)" .app) endif -ifeq ($(findstring $(MAKEFLAGS),s),s) +ifeq ($(findstring $(firstword -$(MAKEFLAGS)),s),s) QUIET_GEN = endif From ae49066982fff5cab5f29422dc12421803cacfe2 Mon Sep 17 00:00:00 2001 From: Mark Levedahl Date: Mon, 26 Jun 2023 12:53:02 -0400 Subject: [PATCH 08/11] git gui Makefile - remove Cygwin modifications git-gui's Makefile hardcodes the absolute Windows path of git-gui's libraries into git-gui, destroying the ability to package git-gui on one machine and distribute to others. The intent is to do this only if a non-Cygwin Tcl/Tk is installed, but the test for this is wrong with the unix/X11 Tcl/Tk shipped since 2012. Also, Cygwin does not support a non-Cygwin Tcl/Tk. The Cygwin git maintainer disables this code, so this code is definitely not in use in the Cygwin distribution. The simplest fix is to just delete the Cygwin specific code, allowing the Makefile to work out of the box on Cygwin. Do so. Signed-off-by: Mark Levedahl Acked-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- Makefile | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index a0d5a4b28e1715..3f80435436c11d 100644 --- a/Makefile +++ b/Makefile @@ -138,25 +138,10 @@ GITGUI_SCRIPT := $$0 GITGUI_RELATIVE := GITGUI_MACOSXAPP := -ifeq ($(uname_O),Cygwin) - GITGUI_SCRIPT := `cygpath --windows --absolute "$(GITGUI_SCRIPT)"` - - # Is this a Cygwin Tcl/Tk binary? If so it knows how to do - # POSIX path translation just like cygpath does and we must - # keep libdir in POSIX format so Cygwin packages of git-gui - # work no matter where the user installs them. - # - ifeq ($(shell echo 'puts [file normalize /]' | '$(TCL_PATH_SQ)'),$(shell cygpath --mixed --absolute /)) - gg_libdir_sed_in := $(gg_libdir) - else - gg_libdir_sed_in := $(shell cygpath --windows --absolute "$(gg_libdir)") - endif -else - ifeq ($(exedir),$(gg_libdir)) - GITGUI_RELATIVE := 1 - endif - gg_libdir_sed_in := $(gg_libdir) +ifeq ($(exedir),$(gg_libdir)) + GITGUI_RELATIVE := 1 endif +gg_libdir_sed_in := $(gg_libdir) ifeq ($(uname_S),Darwin) ifeq ($(shell test -d $(TKFRAMEWORK) && echo y),y) GITGUI_MACOSXAPP := YesPlease From 7145c654fffecd1f3d4a2b8bf05755ce262903e8 Mon Sep 17 00:00:00 2001 From: Mark Levedahl Date: Mon, 26 Jun 2023 12:53:03 -0400 Subject: [PATCH 09/11] git-gui - remove obsolete Cygwin specific code In the current git release, git-gui runs on Cygwin without enabling any of git-gui's Cygwin specific code. This happens as the Cygwin specific code in git-gui was (mostly) written in 2007-2008 to work with Cygwin's then supplied Tcl/Tk which was an incompletely ported variant of the 8.4.1 Windows Tcl/Tk code. In March, 2012, that 8.4.1 package was replaced with a full port based upon the upstream unix/X11 code, since maintained up to date. The two Tcl/Tk packages are completely incompatible, and have different signatures. When Cygwin's Tcl/Tk signature changed in 2012, git-gui no longer detected Cygwin, so did not enable Cygwin specific code, and the POSIX environment provided by Cygwin since 2012 supported git-gui as a generic unix. Thus, no-one apparently noticed the existence of incompatible Cygwin specific code. However, since commit c5766eae6f in the git-gui source tree (https://github.com/prati0100/git-gui, master at a5005ded), and not yet pulled into the git repository, the is_Cygwin function does detect Cygwin using the unix/X11 Tcl/Tk. The Cygwin specific code is enabled, causing use of Windows rather than unix pathnames, and enabling incorrect warnings about environment variables that were relevant only to the old Tcl/Tk. The end result is that (upstream) git-gui is now incompatible with Cygwin. So, delete Cygwin specific code (code protected by "if is_Cygwin") that is not needed in any form to work with the unix/X11 Tcl/Tk. Cygwin specific code required to enable file browsing and shortcut creation is not addressed in this patch, does not currently work, and invocation of those items may leave git-gui in a confused state. Signed-off-by: Mark Levedahl Acked-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- git-gui.sh | 114 ++------------------------------------ lib/choose_repository.tcl | 27 +-------- 2 files changed, 7 insertions(+), 134 deletions(-) diff --git a/git-gui.sh b/git-gui.sh index cb92bba1c40dc9..3f7c31e4c4b295 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -84,14 +84,7 @@ proc _which {what args} { global env _search_exe _search_path if {$_search_path eq {}} { - if {[is_Cygwin] && [regexp {^(/|\.:)} $env(PATH)]} { - set _search_path [split [exec cygpath \ - --windows \ - --path \ - --absolute \ - $env(PATH)] {;}] - set _search_exe .exe - } elseif {[is_Windows]} { + if {[is_Windows]} { set gitguidir [file dirname [info script]] regsub -all ";" $gitguidir "\\;" gitguidir set env(PATH) "$gitguidir;$env(PATH)" @@ -342,14 +335,7 @@ proc gitexec {args} { if {[catch {set _gitexec [git --exec-path]} err]} { error "Git not installed?\n\n$err" } - if {[is_Cygwin]} { - set _gitexec [exec cygpath \ - --windows \ - --absolute \ - $_gitexec] - } else { - set _gitexec [file normalize $_gitexec] - } + set _gitexec [file normalize $_gitexec] } if {$args eq {}} { return $_gitexec @@ -364,14 +350,7 @@ proc githtmldir {args} { # Git not installed or option not yet supported return {} } - if {[is_Cygwin]} { - set _githtmldir [exec cygpath \ - --windows \ - --absolute \ - $_githtmldir] - } else { - set _githtmldir [file normalize $_githtmldir] - } + set _githtmldir [file normalize $_githtmldir] } if {$args eq {}} { return $_githtmldir @@ -1318,9 +1297,6 @@ if {$_gitdir eq "."} { set _gitdir [pwd] } -if {![file isdirectory $_gitdir] && [is_Cygwin]} { - catch {set _gitdir [exec cygpath --windows $_gitdir]} -} if {![file isdirectory $_gitdir]} { catch {wm withdraw .} error_popup [strcat [mc "Git directory not found:"] "\n\n$_gitdir"] @@ -1332,11 +1308,7 @@ apply_config # v1.7.0 introduced --show-toplevel to return the canonical work-tree if {[package vcompare $_git_version 1.7.0] >= 0} { - if { [is_Cygwin] } { - catch {set _gitworktree [exec cygpath --windows [git rev-parse --show-toplevel]]} - } else { - set _gitworktree [git rev-parse --show-toplevel] - } + set _gitworktree [git rev-parse --show-toplevel] } else { # try to set work tree from environment, core.worktree or use # cdup to obtain a relative path to the top of the worktree. If @@ -1561,24 +1533,8 @@ proc rescan {after {honor_trustmtime 1}} { } } -if {[is_Cygwin]} { - set is_git_info_exclude {} - proc have_info_exclude {} { - global is_git_info_exclude - - if {$is_git_info_exclude eq {}} { - if {[catch {exec test -f [gitdir info exclude]}]} { - set is_git_info_exclude 0 - } else { - set is_git_info_exclude 1 - } - } - return $is_git_info_exclude - } -} else { - proc have_info_exclude {} { - return [file readable [gitdir info exclude]] - } +proc have_info_exclude {} { + return [file readable [gitdir info exclude]] } proc rescan_stage2 {fd after} { @@ -3112,10 +3068,6 @@ if {[is_MacOSX]} { set doc_path [githtmldir] if {$doc_path ne {}} { set doc_path [file join $doc_path index.html] - - if {[is_Cygwin]} { - set doc_path [exec cygpath --mixed $doc_path] - } } if {[file isfile $doc_path]} { @@ -4087,60 +4039,6 @@ set file_lists($ui_workdir) [list] wm title . "[appname] ([reponame]) [file normalize $_gitworktree]" focus -force $ui_comm -# -- Warn the user about environmental problems. Cygwin's Tcl -# does *not* pass its env array onto any processes it spawns. -# This means that git processes get none of our environment. -# -if {[is_Cygwin]} { - set ignored_env 0 - set suggest_user {} - set msg [mc "Possible environment issues exist. - -The following environment variables are probably -going to be ignored by any Git subprocess run -by %s: - -" [appname]] - foreach name [array names env] { - switch -regexp -- $name { - {^GIT_INDEX_FILE$} - - {^GIT_OBJECT_DIRECTORY$} - - {^GIT_ALTERNATE_OBJECT_DIRECTORIES$} - - {^GIT_DIFF_OPTS$} - - {^GIT_EXTERNAL_DIFF$} - - {^GIT_PAGER$} - - {^GIT_TRACE$} - - {^GIT_CONFIG$} - - {^GIT_(AUTHOR|COMMITTER)_DATE$} { - append msg " - $name\n" - incr ignored_env - } - {^GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL)$} { - append msg " - $name\n" - incr ignored_env - set suggest_user $name - } - } - } - if {$ignored_env > 0} { - append msg [mc " -This is due to a known issue with the -Tcl binary distributed by Cygwin."] - - if {$suggest_user ne {}} { - append msg [mc " - -A good replacement for %s -is placing values for the user.name and -user.email settings into your personal -~/.gitconfig file. -" $suggest_user] - } - warn_popup $msg - } - unset ignored_env msg suggest_user name -} - # -- Only initialize complex UI if we are going to stay running. # if {[is_enabled transport]} { diff --git a/lib/choose_repository.tcl b/lib/choose_repository.tcl index af1fee7c751dc6..d23abedcb36fd9 100644 --- a/lib/choose_repository.tcl +++ b/lib/choose_repository.tcl @@ -174,9 +174,6 @@ constructor pick {} { -foreground blue \ -underline 1 set home $::env(HOME) - if {[is_Cygwin]} { - set home [exec cygpath --windows --absolute $home] - } set home "[file normalize $home]/" set hlen [string length $home] foreach p $sorted_recent { @@ -374,18 +371,6 @@ proc _objdir {path} { return $objdir } - if {[is_Cygwin]} { - set objdir [file join $path .git objects.lnk] - if {[file isfile $objdir]} { - return [win32_read_lnk $objdir] - } - - set objdir [file join $path objects.lnk] - if {[file isfile $objdir]} { - return [win32_read_lnk $objdir] - } - } - return {} } @@ -623,12 +608,6 @@ method _do_clone2 {} { } set giturl $origin_url - if {[is_Cygwin] && [file isdirectory $giturl]} { - set giturl [exec cygpath --unix --absolute $giturl] - if {$clone_type eq {shared}} { - set objdir [exec cygpath --unix --absolute $objdir] - } - } if {[file exists $local_path]} { error_popup [mc "Location %s already exists." $local_path] @@ -668,11 +647,7 @@ method _do_clone2 {} { fconfigure $f_cp -translation binary -encoding binary cd $objdir while {[gets $f_in line] >= 0} { - if {[is_Cygwin]} { - puts $f_cp [exec cygpath --unix --absolute $line] - } else { - puts $f_cp [file normalize $line] - } + puts $f_cp [file normalize $line] } close $f_in close $f_cp From 4ed23c3c928bdafb2efb9912d6f9e3039ca1ea03 Mon Sep 17 00:00:00 2001 From: Mark Levedahl Date: Mon, 26 Jun 2023 12:53:04 -0400 Subject: [PATCH 10/11] git-gui - use cygstart to browse on Cygwin git-gui enables the "Repository->Explore Working Copy" menu on Cygwin, offering to open a Windows graphical file browser at the root of the working directory. This code, shared with Git For Windows support, depends upon use of Windows pathnames. However, git gui on Cygwin uses unix pathnames, so this shared code will not work on Cygwin. A base install of Cygwin provides the /bin/cygstart utility that runs a registered Windows application based upon the file type, after translating unix pathnames to Windows. Adding the --explore option guarantees that the Windows file explorer is opened, regardless of the supplied pathname's file type and avoiding possibility of some other action being taken. So, teach git-gui to use cygstart --explore on Cygwin, restoring the pre-2012 behavior of opening a Windows file explorer for browsing. This separates the Git For Windows and Cygwin code paths. Note that is_Windows is never true on Cygwin, and is_Cygwin is never true on Git for Windows, though this is not obvious by examining the code for those independent functions. Signed-off-by: Mark Levedahl Acked-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- git-gui.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-gui.sh b/git-gui.sh index 3f7c31e4c4b295..8bc8892c400a08 100755 --- a/git-gui.sh +++ b/git-gui.sh @@ -2274,7 +2274,9 @@ proc do_git_gui {} { # Get the system-specific explorer app/command. proc get_explorer {} { - if {[is_Cygwin] || [is_Windows]} { + if {[is_Cygwin]} { + set explorer "/bin/cygstart.exe --explore" + } elseif {[is_Windows]} { set explorer "explorer.exe" } elseif {[is_MacOSX]} { set explorer "open" From b85c5a4ec66f18fdaa550fdf7801f48ebbc4292f Mon Sep 17 00:00:00 2001 From: Mark Levedahl Date: Mon, 26 Jun 2023 12:53:05 -0400 Subject: [PATCH 11/11] git-gui - use mkshortcut on Cygwin git-gui enables the "Repository->Create Desktop Icon" item on Cygwin, offering to create a shortcut that starts git-gui on the current repository. The code in do_cygwin_shortcut invokes function win32_create_lnk to create the shortcut. This latter function is shared between Cygwin and Git For Windows and expects Windows rather than unix pathnames, though do_cygwin_shortcut provides unix pathnames. Also, this function tries to invoke the Windows Script Host to run a javascript snippet, but this fails under Cygwin's Tcl. So, win32_create_lnk just does not support Cygwin. However, Cygwin's default installation provides /bin/mkshortcut for creating desktop shortcuts. This is compatible with exec under Cygwin's Tcl, understands Cygwin's unix pathnames, and avoids the need for shell escapes to encode troublesome paths. So, teach git-gui to use mkshortcut on Cygwin, leaving win32_create_lnk unchanged and for exclusive use by Git For Windows. Notes: "CHERE_INVOKING=1" is recognized by Cygwin's /etc/profile and prevents a "chdir $HOME", leaving the shell in the working directory specified by the shortcut. That directory is written directly by mkshortcut eliminating any problems with shell escapes and quoting. The code being replaced includes the full pathname of the git-gui creating the shortcut, but that git-gui might not be compatible with the git found after /etc/profile sets the path, and might have a pathname that defies encoding using shell escapes that can survive the multiple incompatible interpreters involved in the chain of creating and using this shortcut. The new code uses bare "git gui" as the command to execute, thus using the system git to launch the system git-gui, and avoiding both issues. Signed-off-by: Mark Levedahl Acked-by: Johannes Schindelin Signed-off-by: Pratyush Yadav --- lib/shortcut.tcl | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/shortcut.tcl b/lib/shortcut.tcl index 97d1d7aa026866..674a41f5e0c868 100644 --- a/lib/shortcut.tcl +++ b/lib/shortcut.tcl @@ -27,13 +27,10 @@ proc do_windows_shortcut {} { } proc do_cygwin_shortcut {} { - global argv0 _gitworktree + global argv0 _gitworktree oguilib if {[catch { set desktop [exec cygpath \ - --windows \ - --absolute \ - --long-name \ --desktop] }]} { set desktop . @@ -48,19 +45,19 @@ proc do_cygwin_shortcut {} { set fn ${fn}.lnk } if {[catch { - set sh [exec cygpath \ - --windows \ - --absolute \ - /bin/sh.exe] - set me [exec cygpath \ - --unix \ - --absolute \ - $argv0] - win32_create_lnk $fn [list \ - $sh -c \ - "CHERE_INVOKING=1 source /etc/profile;[sq $me] &" \ - ] \ - [file normalize $_gitworktree] + set repodir [file normalize $_gitworktree] + set shargs {-c \ + "CHERE_INVOKING=1 \ + source /etc/profile; \ + git gui"} + exec /bin/mkshortcut.exe \ + --arguments $shargs \ + --desc "git-gui on $repodir" \ + --icon $oguilib/git-gui.ico \ + --name $fn \ + --show min \ + --workingdir $repodir \ + /bin/sh.exe } err]} { error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"] }