git gui create desktop icon currently does not work

git gui create desktop icon currently does not work

[Heiko Voigt]

Hi,

I have a patch (further down) to this issue but I only tested this on
Windows 7 and my virtual machine (XP) did not like it for some reason (it
created the shortcut but the shortcut did not work). If anyone else can
confirm that this patch actually works we can include it.

Commit 3748b0 git-gui: update shortcut tools to use _gitworktree
from Guiseppe does break the creation of Desktop shortcuts on Windows.

I do not know whether this has worked with any tcl interpreter as it
attempts to execute $_gitworktree as a command.

Here is my fix:

>From 5c22c39d530ffb308826629d974048d30cd32e53 Mon Sep 17 00:00:00 2001
From: Heiko Voigt <hvoigt@hvoigt.net>
Date: Tue, 23 Feb 2010 10:40:14 +0100
Subject: [PATCH 1/2] git-gui: fix usage of _gitworktree when creating shortcut for windows

Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
 git-gui/lib/shortcut.tcl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl
index 79c1888..8cad0e2 100644
--- a/git-gui/lib/shortcut.tcl
+++ b/git-gui/lib/shortcut.tcl
@@ -16,7 +16,7 @@ proc do_windows_shortcut {} {
 					[info nameofexecutable] \
 					[file normalize $::argv0] \
 					] \
-					[file normalize [$_gitworktree]]
+					[file normalize $_gitworktree]
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
-- 
1.7.0.m5.rc2.15.g8ba4c

Re: git gui create desktop icon currently does not work

[Giuseppe Bilotta]

On Tue, Feb 23, 2010 at 11:49 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> Hi,
>
> I have a patch (further down) to this issue but I only tested this on
> Windows 7 and my virtual machine (XP) did not like it for some reason (it
> created the shortcut but the shortcut did not work). If anyone else can
> confirm that this patch actually works we can include it.
>
> Commit 3748b0 git-gui: update shortcut tools to use _gitworktree
> from Guiseppe does break the creation of Desktop shortcuts on Windows.
>
> I do not know whether this has worked with any tcl interpreter as it
> attempts to execute $_gitworktree as a command.
>
> Here is my fix:
>
> From 5c22c39d530ffb308826629d974048d30cd32e53 Mon Sep 17 00:00:00 2001
> From: Heiko Voigt <hvoigt@hvoigt.net>
> Date: Tue, 23 Feb 2010 10:40:14 +0100
> Subject: [PATCH 1/2] git-gui: fix usage of _gitworktree when creating shortcut for windows
>
> Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
> ---
>  git-gui/lib/shortcut.tcl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl
> index 79c1888..8cad0e2 100644
> --- a/git-gui/lib/shortcut.tcl
> +++ b/git-gui/lib/shortcut.tcl
> @@ -16,7 +16,7 @@ proc do_windows_shortcut {} {
>                                        [info nameofexecutable] \
>                                        [file normalize $::argv0] \
>                                        ] \
> -                                       [file normalize [$_gitworktree]]
> +                                       [file normalize $_gitworktree]
>                        } err]} {
>                        error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
>                }

Ouch. Yes, I think your patch is obviously correct.

(I'll go sit in a corner with my 'TCL Dunce' cap on for a while ...)

-- 
Giuseppe "Oblomov" Bilotta

Re: Re: git gui create desktop icon currently does not work

[Heiko Voigt]

On Tue, Feb 23, 2010 at 11:54:40PM +0100, Giuseppe Bilotta wrote:
> On Tue, Feb 23, 2010 at 11:49 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
[...]
> > From 5c22c39d530ffb308826629d974048d30cd32e53 Mon Sep 17 00:00:00 2001
> > From: Heiko Voigt <hvoigt@hvoigt.net>
> > Date: Tue, 23 Feb 2010 10:40:14 +0100
> > Subject: [PATCH 1/2] git-gui: fix usage of _gitworktree when creating shortcut for windows
> >
> > Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
> > ---
> >  git-gui/lib/shortcut.tcl |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl
> > index 79c1888..8cad0e2 100644
> > --- a/git-gui/lib/shortcut.tcl
> > +++ b/git-gui/lib/shortcut.tcl
> > @@ -16,7 +16,7 @@ proc do_windows_shortcut {} {
> >                                        [info nameofexecutable] \
> >                                        [file normalize $::argv0] \
> >                                        ] \
> > -                                       [file normalize [$_gitworktree]]
> > +                                       [file normalize $_gitworktree]
> >                        } err]} {
> >                        error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
> >                }
> 
> Ouch. Yes, I think your patch is obviously correct.
> 
> (I'll go sit in a corner with my 'TCL Dunce' cap on for a while ...)

No problem obviously nobody else saw this, but please test your patches
in the future. That could have avoided this one.

One thing which is missing from my patch is the cygwin part which, as I
saw from your original patch, has the same problem.

cheers Heiko

Re: Re: git gui create desktop icon currently does not work

[Giuseppe Bilotta]

On Wed, Feb 24, 2010 at 12:02 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> On Tue, Feb 23, 2010 at 11:54:40PM +0100, Giuseppe Bilotta wrote:
>> On Tue, Feb 23, 2010 at 11:49 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> [...]
>> > From 5c22c39d530ffb308826629d974048d30cd32e53 Mon Sep 17 00:00:00 2001
>> > From: Heiko Voigt <hvoigt@hvoigt.net>
>> > Date: Tue, 23 Feb 2010 10:40:14 +0100
>> > Subject: [PATCH 1/2] git-gui: fix usage of _gitworktree when creating shortcut for windows
>> >
>> > Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
>> > ---
>> >  git-gui/lib/shortcut.tcl |    2 +-
>> >  1 files changed, 1 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl
>> > index 79c1888..8cad0e2 100644
>> > --- a/git-gui/lib/shortcut.tcl
>> > +++ b/git-gui/lib/shortcut.tcl
>> > @@ -16,7 +16,7 @@ proc do_windows_shortcut {} {
>> >                                        [info nameofexecutable] \
>> >                                        [file normalize $::argv0] \
>> >                                        ] \
>> > -                                       [file normalize [$_gitworktree]]
>> > +                                       [file normalize $_gitworktree]
>> >                        } err]} {
>> >                        error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
>> >                }
>>
>> Ouch. Yes, I think your patch is obviously correct.
>>
>> (I'll go sit in a corner with my 'TCL Dunce' cap on for a while ...)
>
> No problem obviously nobody else saw this, but please test your patches
> in the future. That could have avoided this one.

You're right, although finding a Windows installation on which to test
git is nontrivial for me. (I did mention this particular part was
untested when I submitted it.)

> One thing which is missing from my patch is the cygwin part which, as I
> saw from your original patch, has the same problem.

Also, I'm wondering: does the Windows part actually need a  [file
nativename $_gitworktree], or is this automatically handled by the
shortcut creation code?


-- 
Giuseppe "Oblomov" Bilotta

Re: Re: Re: git gui create desktop icon currently does not work

[Heiko Voigt]

On Thu, Feb 25, 2010 at 08:53:57AM +0100, Giuseppe Bilotta wrote:
> On Wed, Feb 24, 2010 at 12:02 AM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > On Tue, Feb 23, 2010 at 11:54:40PM +0100, Giuseppe Bilotta wrote:
> >> On Tue, Feb 23, 2010 at 11:49 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
[...]
> >> > -                                       [file normalize [$_gitworktree]]
> >> > +                                       [file normalize $_gitworktree]
> >> >                        } err]} {
> >> >                        error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
> >> >                }
> >>
> >> Ouch. Yes, I think your patch is obviously correct.
> >>
> >> (I'll go sit in a corner with my 'TCL Dunce' cap on for a while ...)
> >
> > No problem obviously nobody else saw this, but please test your patches
> > in the future. That could have avoided this one.
> 
> You're right, although finding a Windows installation on which to test
> git is nontrivial for me. (I did mention this particular part was
> untested when I submitted it.)

Ok then we can not blame you. Maybe send such a patch to the msysgit
mailinglist and me CC then we can test early.

> > One thing which is missing from my patch is the cygwin part which, as I
> > saw from your original patch, has the same problem.
> 
> Also, I'm wondering: does the Windows part actually need a  [file
> nativename $_gitworktree], or is this automatically handled by the
> shortcut creation code?

I investigated further and it seems that the cygwin part needs it. I
prepared a patchseries about this and some more issues I will send out
shortly.

cheers Heiko

[PATCH 1/3] git-gui: fix usage of _gitworktree when creating shortcut for windows

[Heiko Voigt]

Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
---
 git-gui/lib/shortcut.tcl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/shortcut.tcl b/git-gui/lib/shortcut.tcl
index 79c1888..78878ef 100644
--- a/git-gui/lib/shortcut.tcl
+++ b/git-gui/lib/shortcut.tcl
@@ -16,7 +16,7 @@ proc do_windows_shortcut {} {
 					[info nameofexecutable] \
 					[file normalize $::argv0] \
 					] \
-					[file normalize [$_gitworktree]]
+					[file normalize $_gitworktree]
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
@@ -57,7 +57,7 @@ proc do_cygwin_shortcut {} {
 					$sh -c \
 					"CHERE_INVOKING=1 source /etc/profile;[sq $me] &" \
 					] \
-					[file normalize [$_gitworktree]]
+					[file normalize $_gitworktree]
 			} err]} {
 			error_popup [strcat [mc "Cannot write shortcut:"] "\n\n$err"]
 		}
-- 
1.7.0.m5.rc3.5.g38df2

[PATCH 2/3] git-gui: fix PATH environment for mingw development environment

[Heiko Voigt]

When creating a desktop shortcut from the gui the shortcut directly
starts wish with the git-gui script. In the msysgit development
environment some dll's reside in the mingw/bin directory which causes
that git can not start because libiconv2.dll is not found.

When using such a link the error is even more cryptic stating:
"child killed: unknown signal"

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
I am not sure whether this is the most elegant solution, but making this
work would definitely ease the development workflow.

 git-gui/windows/git-gui.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-gui/windows/git-gui.sh b/git-gui/windows/git-gui.sh
index 66bbb2f..b1845c5 100644
--- a/git-gui/windows/git-gui.sh
+++ b/git-gui/windows/git-gui.sh
@@ -13,10 +13,11 @@ if { $argc >=2 && [lindex $argv 0] == "--working-dir" } {
 	incr argc -2
 }
 
-set bindir [file dirname \
+set basedir [file dirname \
             [file dirname \
              [file dirname [info script]]]]
-set bindir [file join $bindir bin]
+set bindir [file join $basedir bin]
+set bindir "$bindir;[file join $basedir mingw bin]"
 regsub -all ";" $bindir "\\;" bindir
 set env(PATH) "$bindir;$env(PATH)"
 unset bindir
-- 
1.7.0.m5.rc3.5.g38df2

Re: [msysGit] [PATCH 2/3] git-gui: fix PATH environment for mingw development environment

[Pat Thoyts]

On 27 February 2010 20:47, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> When creating a desktop shortcut from the gui the shortcut directly
> starts wish with the git-gui script. In the msysgit development
> environment some dll's reside in the mingw/bin directory which causes
> that git can not start because libiconv2.dll is not found.
>
> When using such a link the error is even more cryptic stating:
> "child killed: unknown signal"
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> ---
> I am not sure whether this is the most elegant solution, but making this
> work would definitely ease the development workflow.
>
>  git-gui/windows/git-gui.sh |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/git-gui/windows/git-gui.sh b/git-gui/windows/git-gui.sh
> index 66bbb2f..b1845c5 100644
> --- a/git-gui/windows/git-gui.sh
> +++ b/git-gui/windows/git-gui.sh
> @@ -13,10 +13,11 @@ if { $argc >=2 && [lindex $argv 0] == "--working-dir" } {
>        incr argc -2
>  }
>
> -set bindir [file dirname \
> +set basedir [file dirname \
>             [file dirname \
>              [file dirname [info script]]]]
> -set bindir [file join $bindir bin]
> +set bindir [file join $basedir bin]
> +set bindir "$bindir;[file join $basedir mingw bin]"
>  regsub -all ";" $bindir "\\;" bindir
>  set env(PATH) "$bindir;$env(PATH)"
>  unset bindir
> --
> 1.7.0.m5.rc3.5.g38df2

I have not tried to test this - I use cmd shell all the time so I
always run this using 'git gui' which runs the git.cmd script in
ProgramFiles\Git\cmd or in the development environment in
msysgit\cmd\. It makes me wonder if the Windows shortcut shouldn;t be
pointing to the cmd script rather than to a tcl script like this as
the cmd script does some quite extensive environment setup. It assigns
HOME if it is not set, extends the PATH to include the git
installation locations, sets the PLINK_PROTOCOL and the codepages for
the console. Probably only the PATH and HOME fixups are necessary for
git-gui and in fact tcl's configuration of "~" in the path as HOME
probably deals with that too.
This is just for the development setup right? An installation of
msysGit doesn't have a mingw\bin directory.
Pat Thoyts

Re: Re: [PATCH 2/3] git-gui: fix PATH environment for mingw development environment

[Heiko Voigt]

On Sun, Feb 28, 2010 at 12:57:14AM +0000, Pat Thoyts wrote:
> On 27 February 2010 20:47, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > When creating a desktop shortcut from the gui the shortcut directly
> > starts wish with the git-gui script. In the msysgit development
> > environment some dll's reside in the mingw/bin directory which causes
> > that git can not start because libiconv2.dll is not found.
> >
> > When using such a link the error is even more cryptic stating:
> > "child killed: unknown signal"
> >
> > Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
> > ---
> > I am not sure whether this is the most elegant solution, but making this
> > work would definitely ease the development workflow.
[...]
> > -set bindir [file dirname \
> > +set basedir [file dirname \
> >             [file dirname \
> >              [file dirname [info script]]]]
> > -set bindir [file join $bindir bin]
> > +set bindir [file join $basedir bin]
> > +set bindir "$bindir;[file join $basedir mingw bin]"
[...]

> I have not tried to test this - I use cmd shell all the time so I
> always run this using 'git gui' which runs the git.cmd script in
> ProgramFiles\Git\cmd or in the development environment in
> msysgit\cmd\. It makes me wonder if the Windows shortcut shouldn;t be
> pointing to the cmd script rather than to a tcl script like this as
> the cmd script does some quite extensive environment setup. It assigns
> HOME if it is not set, extends the PATH to include the git
> installation locations, sets the PLINK_PROTOCOL and the codepages for
> the console.

Problem here is that AFAIK the cmd script is part of msysgit and not
upstream git which would break the desktop icon on cygwin. So this
solution seems to me the most unintrusive one because on cygwin (and
installed msysgit) you just end up having one obsolete path in PATH.
And the current heuristic does not always work. Depending on where you
put your prefix variable in your make call you have to manually add to
your path. For example if you, like me, usually install git in your home
directory.

Of course if we had a way to tell where the dll's of git are stored that
would be a way nicer solution.

> Probably only the PATH and HOME fixups are necessary for
> git-gui and in fact tcl's configuration of "~" in the path as HOME
> probably deals with that too.
> This is just for the development setup right? An installation of
> msysGit doesn't have a mingw\bin directory.

Yes it is for development only. But we have this paths in other locations
as well: For example I noticed that even the installation of msysgit has
/mingw/bin in PATH (from etc/profile probably) when starting git bash.

We need something for the development setup. Otherwise testing bugfixes
in this area becomes very tedious. And as stated above the error message
you get is way worse than my patch ;)

cheers Heiko

[PATCH 3/3] git-gui: fix shortcut creation on cygwin

[Heiko Voigt]

When the user tried to create a desktop icon with git gui on cygwin
wscript was complaining about an unknown option and displaying the
non-native path as such.

Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
 git-gui/lib/win32.tcl |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-gui/lib/win32.tcl b/git-gui/lib/win32.tcl
index d7f93d0..db91ab8 100644
--- a/git-gui/lib/win32.tcl
+++ b/git-gui/lib/win32.tcl
@@ -18,9 +18,9 @@ proc win32_create_lnk {lnk_path lnk_exec lnk_dir} {
 	eval [list exec wscript.exe \
 		/E:jscript \
 		/nologo \
-		[file join $oguilib win32_shortcut.js] \
+		[file nativename [file join $oguilib win32_shortcut.js]] \
 		$lnk_path \
-		[file join $oguilib git-gui.ico] \
+		[file nativename [file join $oguilib git-gui.ico]] \
 		$lnk_dir \
 		$lnk_exec] $lnk_args
 }
-- 
1.7.0.m5.rc3.5.g38df2

Re: [PATCH 1/3] git-gui: fix usage of _gitworktree when creating shortcut for windows

[Johannes Schindelin]

Hi,

On Sat, 27 Feb 2010, Heiko Voigt wrote:

> Signed-off-by: Heiko Voigt <heiko.voigt@mahr.de>
> ---

I applied these three patches to 4msysgit.git (I was pretty surprised that 
the faulty patch made it into 1.7.0.2, but then, even my comment about the 
commit message style was met with unwillingness to change).

Thanks Heiko, for pre-emptively fixing msysGit issue 425!

Ciao,
Dscho

Re: Re: Re: git gui create desktop icon currently does not work

[Giuseppe Bilotta]

On Sat, Feb 27, 2010 at 9:41 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> On Thu, Feb 25, 2010 at 08:53:57AM +0100, Giuseppe Bilotta wrote:
>> You're right, although finding a Windows installation on which to test
>> git is nontrivial for me. (I did mention this particular part was
>> untested when I submitted it.)
>
> Ok then we can not blame you. Maybe send such a patch to the msysgit
> mailinglist and me CC then we can test early.

I should have thought about posting to the msysgit mailing list
myself. I'll remember next time 8-)

-- 
Giuseppe "Oblomov" Bilotta