git-gui: msys2 compatibility patches
PATCH 1/1 git-gui: msys2 compatibility patches
From: Konstantin Podsvirov <[email protected]>
Allow using git gui command via MSYS2's MINGW32/64 subsystems (apropriate shells).
Just install apropriate tk package:
```bash user@host MINGW32 ~ pacman -S mingw-w64-i686-tk ```
or
```bash user@host MINGW64 ~ pacman -S mingw-w64-x86_64-tk ```
For more info see: https://github.com/msys2/MSYS2-packages/pull/1912
---
git-gui: msys2 compatibility patches
Allow using git gui command via MSYS2's MINGW32/64 subsystems (apropriate shells).
Just install apropriate tk package:
user@host MINGW32 ~
pacman -S mingw-w64-i686-tk
or
user@host MINGW64 ~
pacman -S mingw-w64-x86_64-tk
For more info see: https://github.com/msys2/MSYS2-packages/pull/1912
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-612%2Fpodsvirov%2Fgit-gui%2Fmsys2-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-612/podsvirov/git-gui/msys2-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/612
git-gui.sh | 52 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 22 deletions(-)
git-gui.sh +30 -22
@@ -44,6 +44,28 @@ if {[catch {package require Tcl 8.5} err] catch {rename send {}} ; # What an evil concept... +###################################################################### +## +## platform detection + +set _iscygwin {} + +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 + } + } else { + set _iscygwin 0 + } + } + return $_iscygwin +} + ###################################################################### ## ## locate our library
@@ -51,7 +73,14 @@ catch {rename send {}} ; # What an evil concept... if { [info exists ::env(GIT_GUI_LIB_DIR) ] } { set oguilib $::env(GIT_GUI_LIB_DIR) } else { - set oguilib {@@GITGUI_LIBDIR@@} + if {[is_Cygwin]} { + set oguilib [exec cygpath \ + --windows \ + --absolute \ + @@GITGUI_LIBDIR@@] + } else { + set oguilib {@@GITGUI_LIBDIR@@} + } } set oguirel {@@GITGUI_RELATIVE@@} if {$oguirel eq {1}} {
@@ -163,7 +192,6 @@ set _isbare {} set _gitexec {} set _githtmldir {} set _reponame {} -set _iscygwin {} set _search_path {} set _shellpath {@@SHELL_PATH@@}
@@ -266,26 +294,6 @@ proc is_Windows {} { return 0 } -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 - } - } - } else { - set _iscygwin 0 - } - } - return $_iscygwin -} - proc is_enabled {option} { global enabled_options if {[catch {set on $enabled_options($option)}]} {return 0}
base-commit: a5728022e07c53e5ac91db0960870518e243b7c1
Signature · 1 line
gitgitgadget
Hi Konstantin,
Thanks for the patch, and sorry for the late reply.
Quoted 3 lines · Konstantin Podsvirov via GitGitGadget
From: Konstantin Podsvirov <[email protected]>
Allow using
git guicommand via MSYS2's MINGW32/64 subsystems (apropriate shells).
I think this should be the commit subject, instead of "msys2 compatibility patches".
Quoted 7 lines
Just install apropriate
tkpackage:```bash user@host MINGW32 ~ pacman -S mingw-w64-i686-tk ```
Quoted 8 lines
or
```bash user@host MINGW64 ~ pacman -S mingw-w64-x86_64-tk ```For more info see: https://github.com/msys2/MSYS2-packages/pull/1912
Please don't just link to an external website. Put the explanation there in the commit message. Explain what the problem was, and how this patch fixes it.
Quoted 41 lines
Signed-off-by Konstantin Podsvirov <[email protected]>--- git-gui.sh | 52 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 22 deletions(-)git-gui.sh +22
-0@@ -44,6 +44,28 @@ if {[catch {package require Tcl 8.5} err] catch {rename send {}} ; # What an evil concept... +###################################################################### +## +## platform detection + +set _iscygwin {} + +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 + } + } else { + set _iscygwin 0 + } + } + return $_iscygwin +} + ###################################################################### ## ## locate our library@@ -51,7 +73,14 @@ catch {rename send {}} ; # What an evil concept... if { [info exists ::env(GIT_GUI_LIB_DIR) ] } { set oguilib $::env(GIT_GUI_LIB_DIR)
git-gui.sh catch {rename send {}} ; # What an evil concept... · line 83
Quoted 10 lines
git-gui.sh +8
-1} else { - set oguilib {@@GITGUI_LIBDIR@@} + if {[is_Cygwin]} { + set oguilib [exec cygpath \ + --windows \ + --absolute \ + @@GITGUI_LIBDIR@@] + } else { + set oguilib {@@GITGUI_LIBDIR@@} + }
This would convert the Windows style path to a Unix style path if we are running in Cygwin, right? This is what I assume the heart of the problem is.
Style nitpick: something like this would probably be better:
set oguilib {@@GITGUI_LIBDIR@@}
if {[is_Cygwin]} {
...}
This makes it clear that Cygwin is the exception. For all other cases, we want to use @@GITGUI_LIBDIR@@ directly.
git-gui.sh set _isbare {} · line 166
Quoted 8 lines
git-gui.sh +0
-1} set oguirel {@@GITGUI_RELATIVE@@} if {$oguirel eq {1}} {@@ -163,7 +192,6 @@ set _isbare {} set _gitexec {} set _githtmldir {} set _reponame {} -set _iscygwin {}
Why move the initialization?
Quoted 9 lines
git-gui.sh +0
-2set _search_path {} set _shellpath {@@SHELL_PATH@@}@@ -266,26 +294,6 @@ proc is_Windows {} { return 0 } -proc is_Cygwin {} { - global _iscygwin
git-gui.sh proc is_Windows {} { · line 280
Quoted 10 lines
git-gui.sh +0
-10- 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 - }
I'm afraid I don't understand this hunk. I don't use Windows, and don't completely understand the difference between cygwin, msys, etc. Could you please explain further why this check is removed? Are there any negative side-effects?
git-gui.sh proc is_Windows {} { · line 288
Quoted 8 lines
git-gui.sh +0
-8- } - } else { - set _iscygwin 0 - } - } - return $_iscygwin -} -
Why move the function? Can't this and the _iscygwin initialization just stay in their place? It will make the diff much easier to read.
Quoted 3 lines
git-gui.sh +0
-0proc is_enabled {option} { global enabled_options if {[catch {set on $enabled_options($option)}]} {return 0}
Signature · 2 lines
Regards, Pratyush Yadav
27.04.2020, 22:48, "Pratyush Yadav" <[email protected]>:
Quoted 11 lines
Hi Konstantin,
Thanks for the patch, and sorry for the late reply.
Quoted 3 lines · Konstantin Podsvirov via GitGitGadget
From: Konstantin Podsvirov <[email protected]>
Allow using
git guicommand via MSYS2's MINGW32/64 subsystems (apropriate shells).I think this should be the commit subject, instead of "msys2 compatibility patches".
I do not mind.
Quoted 14 lines
Quoted 14 lines
Just install apropriate
tkpackage:```bash user@host MINGW32 ~ pacman -S mingw-w64-i686-tk ```or
```bash user@host MINGW64 ~ pacman -S mingw-w64-x86_64-tk ```
Quoted 5 lines
Quoted 1 line
For more info see: https://github.com/msys2/MSYS2-packages/pull/1912
Please don't just link to an external website. Put the explanation there in the commit message. Explain what the problem was, and how this patch fixes it.
The startup script everywhere operates on Unix paths, but on Windows they are incomplete, and the interpreter expects full native paths.
Quoted 52 lines
Quoted 43 lines
Signed-off-by Konstantin Podsvirov <[email protected]>---
git-gui.sh | 52 ++++++++++++++++++++++++++++++----------------------1 file changed, 30 insertions(+), 22 deletions(-)diff --git
a/git-gui.shb/git-gui.sh
index4610e4ca72a..512f4f121aa100755
---a/git-gui.sh
+++b/git-gui.sh
@@ -44,6 +44,28 @@ if {[catch {package require Tcl 8.5} err]catch {rename send {}} ; # What an evil concept...
+######################################################################
+##
+## platform detection
+
+set_iscygwin{}
+
+procis_Cygwin{} {
+ global_iscygwin
+ if {$_iscygwin eq {}} {
+ if {$::tcl_platform(platform) eq {windows}} {
+ if {[catch {set p [exec cygpath --windir]} err]} {
+ set_iscygwin0
+ } else {
+ set_iscygwin1
+ }
+ } else {
+ set_iscygwin0
+ }
+ }
+ return $_iscygwin
+}
+
######################################################################
##
## locate our library
@@ -51,7 +73,14 @@ catch {rename send {}} ; # What an evil concept... if { [info exists ::env(GIT_GUI_LIB_DIR) ] } {
set oguilib $::env(GIT_GUI_LIB_DIR)
} else {
- set oguilib {@@GITGUI_LIBDIR@@}Quoted 8 lines
+ if {[
is_Cygwin]} {
+ set oguilib [exec cygpath \
+ --windows \
+ --absolute \
+ @@GITGUI_LIBDIR@@]
+ } else {
+ set oguilib {@@GITGUI_LIBDIR@@}
+ }
Quoted 3 lines
This would convert the Windows style path to a Unix style path if we are running in Cygwin, right? This is what I assume the heart of the problem is.
It is true exactly the opposite.
Quoted 6 lines
Style nitpick: something like this would probably be better:
set oguilib {@@GITGUI_LIBDIR@@}
if {[is_Cygwin]} {
...
}
Looks good.
Quoted 2 lines
This makes it clear that Cygwin is the exception. For all other cases, we want to use @@GITGUI_LIBDIR@@ directly.
Yes.
Quoted 10 lines
Quoted 8 lines
}
set oguirel {@@GITGUI_RELATIVE@@}
if {$oguirel eq {1}} {
@@ -163,7 +192,6 @@ set_isbare{}
set_gitexec{}
set_githtmldir{}
set_reponame{}
-set_iscygwin{}Why move the initialization?
To use this above when setting oguilib variable.
Quoted 20 lines
Quoted 7 lines
set
_search_path{}
set_shellpath{@@SHELL_PATH@@}@@ -266,26 +294,6 @@ proc
is_Windows{} {
return 0
}Quoted 12 lines
-proc
is_Cygwin{} {
- global_iscygwin
- if {$_iscygwin eq {}} {
- if {$::tcl_platform(platform) eq {windows}} {
- if {[catch {set p [exec cygpath --windir]} err]} {
- set_iscygwin0
- } else {
- set_iscygwin1
- # Handle MSys2 which is only cygwin when MSYSTEM is MSYS.
- if {[info exists ::env(MSYSTEM)] && $::env(MSYSTEM) ne "MSYS"} {
- set_iscygwin0
- }
Quoted 4 lines
I'm afraid I don't understand this hunk. I don't use Windows, and don't completely understand the difference between cygwin, msys, etc. Could you please explain further why this check is removed? Are there any negative side-effects?
To use git gui we need tk (wish), but tk (wish) can be available only when MSYSTEM is equal to MINGW32 or MINGW64.
Quoted 11 lines
Quoted 8 lines
- }
- } else {
- set_iscygwin0
- }
- }
- return $_iscygwin
-}
-Why move the function? Can't this and the
_iscygwininitialization just stay in their place? It will make the diff much easier to read.
To use this above when setting oguilib variable.
Quoted 7 lines
Quoted 3 lines
proc
is_enabled{option} {
globalenabled_options
if {[catch {set on $enabled_options($option)}]} {return 0}--
Regards,
Pratyush Yadav
What is the further course of action?
--
Regards,
Konstantin Podsvirov
Hi Konstantin,
Quoted 31 lines · Konstantin Podsvirov
27.04.2020, 22:48, "Pratyush Yadav" <[email protected]>:
Quoted 11 lines
Hi Konstantin,
Thanks for the patch, and sorry for the late reply.
Quoted 3 lines · Konstantin Podsvirov via GitGitGadget
From: Konstantin Podsvirov <[email protected]>
Allow using
git guicommand via MSYS2's MINGW32/64 subsystems (apropriate shells).I think this should be the commit subject, instead of "msys2 compatibility patches".
I do not mind.
Quoted 14 lines
Quoted 14 lines
Just install apropriate
tkpackage:```bash user@host MINGW32 ~ pacman -S mingw-w64-i686-tk ```or
```bash user@host MINGW64 ~ pacman -S mingw-w64-x86_64-tk ```
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
For more info see: https://github.com/msys2/MSYS2-packages/pull/1912
Please don't just link to an external website. Put the explanation there in the commit message. Explain what the problem was, and how this patch fixes it.
The startup script everywhere operates on Unix paths, but on Windows they are incomplete, and the interpreter expects full native paths.
Ok. Thanks for the explanation.
Quoted 52 lines
Quoted 52 lines
Quoted 51 lines
Signed-off-by Konstantin Podsvirov <[email protected]>---
git-gui.sh | 52 ++++++++++++++++++++++++++++++----------------------1 file changed, 30 insertions(+), 22 deletions(-)diff --git
a/git-gui.shb/git-gui.sh
index4610e4ca72a..512f4f121aa100755
---a/git-gui.sh
+++b/git-gui.sh
@@ -44,6 +44,28 @@ if {[catch {package require Tcl 8.5} err]catch {rename send {}} ; # What an evil concept...
+######################################################################
+##
+## platform detection
+
+set_iscygwin{}
+
+procis_Cygwin{} {
+ global_iscygwin
+ if {$_iscygwin eq {}} {
+ if {$::tcl_platform(platform) eq {windows}} {
+ if {[catch {set p [exec cygpath --windir]} err]} {
+ set_iscygwin0
+ } else {
+ set_iscygwin1
+ }
+ } else {
+ set_iscygwin0
+ }
+ }
+ return $_iscygwin
+}
+
######################################################################
##
## locate our library
@@ -51,7 +73,14 @@ catch {rename send {}} ; # What an evil concept... if { [info exists ::env(GIT_GUI_LIB_DIR) ] } {
set oguilib $::env(GIT_GUI_LIB_DIR)
} else {
- set oguilib {@@GITGUI_LIBDIR@@}
+ if {[is_Cygwin]} {
+ set oguilib [exec cygpath \
+ --windows \
+ --absolute \
+ @@GITGUI_LIBDIR@@]
+ } else {
+ set oguilib {@@GITGUI_LIBDIR@@}
+ }
Quoted 5 lines
Quoted 3 lines
This would convert the Windows style path to a Unix style path if we are running in Cygwin, right? This is what I assume the heart of the problem is.
It is true exactly the opposite.
Got it.
Quoted 23 lines
Quoted 6 lines
Style nitpick: something like this would probably be better:
set oguilib {@@GITGUI_LIBDIR@@}
if {[is_Cygwin]} {
...
}Looks good.
Quoted 2 lines
This makes it clear that Cygwin is the exception. For all other cases, we want to use @@GITGUI_LIBDIR@@ directly.
Yes.
Quoted 9 lines
Quoted 8 lines
}
set oguirel {@@GITGUI_RELATIVE@@}
if {$oguirel eq {1}} {
@@ -163,7 +192,6 @@ set_isbare{}
set_gitexec{}
set_githtmldir{}
set_reponame{}
-set_iscygwin{}
Quoted 3 lines
Quoted 1 line
Why move the initialization?
To use this above when setting
oguilibvariable.
Ok. Though I wonder if we need this variable at all. It is only used with the function is_Cygwin, and seems like a performance optimization so we don't need to perform the check for every call to the function. But since it only is a couple of if statements, I'm not sure if it will make a significant performance difference.
Quoted 20 lines
Quoted 20 lines
Quoted 19 lines
set
_search_path{}
set_shellpath{@@SHELL_PATH@@}@@ -266,26 +294,6 @@ proc
is_Windows{} {
return 0
}-proc
is_Cygwin{} {
- global_iscygwin
- if {$_iscygwin eq {}} {
- if {$::tcl_platform(platform) eq {windows}} {
- if {[catch {set p [exec cygpath --windir]} err]} {
- set_iscygwin0
- } else {
- set_iscygwin1
- # Handle MSys2 which is only cygwin when MSYSTEM is MSYS.
- if {[info exists ::env(MSYSTEM)] && $::env(MSYSTEM) ne "MSYS"} {
- set_iscygwin0
- }
Quoted 6 lines
Quoted 4 lines
I'm afraid I don't understand this hunk. I don't use Windows, and don't completely understand the difference between cygwin, msys, etc. Could you please explain further why this check is removed? Are there any negative side-effects?
To use
git guiwe needtk(wish), buttk(wish) can be available only when MSYSTEM is equal to MINGW32 or MINGW64.
So if MYSYTEM is something other than these two, would it be possible to run git-gui at all?
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
- }
- } else {
- set_iscygwin0
- }
- }
- return $_iscygwin
-}
-
Quoted 4 lines
Quoted 2 lines
Why move the function? Can't this and the
_iscygwininitialization just stay in their place? It will make the diff much easier to read.To use this above when setting
oguilibvariable.
Ok.
Quoted 9 lines
Quoted 7 lines
Quoted 3 lines
proc
is_enabled{option} {
globalenabled_options
if {[catch {set on $enabled_options($option)}]} {return 0}--
Regards,
Pratyush YadavWhat is the further course of action?
You should send another version of this series with the suggestions implemented. Since you are using GitGitGadget, you can amend the commit, force push it, and then issue another /submit command, and GGG will take care of the rest. The GGG welcome message would have all the details as well.
Signature · 2 lines
Regards, Pratyush Yadav