git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Git Gui bug calling gitk
@ 2008-07-24 13:01 Murphy, John
  2008-07-24 13:28 ` [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core Abhijit Menon-Sen
  0 siblings, 1 reply; 6+ messages in thread
From: Murphy, John @ 2008-07-24 13:01 UTC (permalink / raw)
  To: git

I have recently installed git: v1.5.6.1-167-gbf270ab and git gui:
gitgui-0.10.2-8-g2add5cb

>From Git Gui we try to run Visualize Branch History and get the
following error:

Unable to start gitk:
c:/cygwin/usr/local/git/libexec/git-core/gitk does not exist

I presume this was just an oversight when git- commands were removed
from the bin directory.

-John Murphy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core
  2008-07-24 13:01 Git Gui bug calling gitk Murphy, John
@ 2008-07-24 13:28 ` Abhijit Menon-Sen
  2008-07-25 22:05   ` Shawn O. Pearce
  0 siblings, 1 reply; 6+ messages in thread
From: Abhijit Menon-Sen @ 2008-07-24 13:28 UTC (permalink / raw)
  To: Murphy, John; +Cc: git

Signed-off-by: Abhijit Menon-Sen <ams@toroid.org>
---

At 2008-07-24 09:01:48 -0400, john.murphy@bankofamerica.com wrote:
>
> I presume this was just an oversight when git- commands were removed
> from the bin directory.

Looks like it. The following patch fixes it for me. Does it work for
you on Windows?

-- ams

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

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 940677c..a70fa67 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -1670,10 +1670,10 @@ proc do_gitk {revs} {
 	# -- Always start gitk through whatever we were loaded with.  This
 	#    lets us bypass using shell process on Windows systems.
 	#
-	set exe [file join [file dirname $::_git] gitk]
+	set exe [_which gitk]
 	set cmd [list [info nameofexecutable] $exe]
-	if {! [file exists $exe]} {
-		error_popup [mc "Unable to start gitk:\n\n%s does not exist" $exe]
+	if {$exe eq {}} {
+		error_popup [mc "Couldn't find gitk in PATH"]
 	} else {
 		global env
 
-- 
1.5.6.GIT

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core
  2008-07-24 13:28 ` [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core Abhijit Menon-Sen
@ 2008-07-25 22:05   ` Shawn O. Pearce
  2008-07-29 12:40     ` Murphy, John
  0 siblings, 1 reply; 6+ messages in thread
From: Shawn O. Pearce @ 2008-07-25 22:05 UTC (permalink / raw)
  To: Abhijit Menon-Sen; +Cc: Murphy, John, git

Abhijit Menon-Sen <ams@toroid.org> wrote:
> At 2008-07-24 09:01:48 -0400, john.murphy@bankofamerica.com wrote:
> >
> > I presume this was just an oversight when git- commands were removed
> > from the bin directory.
> 
> Looks like it. The following patch fixes it for me. Does it work for
> you on Windows?

Thanks.

This patch is obviously correct on any platform except Mac OS X.
On Mac OS X it gets confusing because [info nameofexecutable] is
a wrapper .app which loads its own main script.  That main script
needs to use the gitk location it receives from git-gui, and not
the location it computes from gitexecdir.  So I'm following up
your patch with this:

diff --git a/macosx/AppMain.tcl b/macosx/AppMain.tcl
index 41ca08e..ddbe633 100644
--- a/macosx/AppMain.tcl
+++ b/macosx/AppMain.tcl
@@ -7,7 +7,7 @@ if {[string first -psn [lindex $argv 0]] == 0} {
 }
 
 if {[file tail [lindex $argv 0]] eq {gitk}} {
-	set argv0 [file join $gitexecdir gitk]
+	set argv0 [lindex $argv 0]
 	set AppMain_source $argv0
 } else {
 	set argv0 [file join $gitexecdir [file tail [lindex $argv 0]]]
 
-- 
Shawn.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core
  2008-07-25 22:05   ` Shawn O. Pearce
@ 2008-07-29 12:40     ` Murphy, John
  2008-07-29 16:48       ` Shawn O. Pearce
  0 siblings, 1 reply; 6+ messages in thread
From: Murphy, John @ 2008-07-29 12:40 UTC (permalink / raw)
  To: spearce, Abhijit Menon-Sen; +Cc: git

I have rebuilt git-gui with version 0.10.2.18.gc629 it is still not finding gitk.
I have done some debugging in proc _which
I have found the issue is with the following line:

set p [file join $p $what$_search_exe]

The variable $p = gitk.exe

And there is no such animal

When I copy gitk to gitk.exe in /usr/local/git/bin, it works fine.

So, not sure if this is a git issue or a git-gui issue.

-John 
-----Original Message-----
From: spearce@spearce.org [mailto:spearce@spearce.org] 
Sent: Friday, July 25, 2008 6:06 PM
To: Abhijit Menon-Sen
Cc: Murphy, John; git@vger.kernel.org
Subject: Re: [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core

Abhijit Menon-Sen <ams@toroid.org> wrote:
> At 2008-07-24 09:01:48 -0400, john.murphy@bankofamerica.com wrote:
> >
> > I presume this was just an oversight when git- commands were removed
> > from the bin directory.
> 
> Looks like it. The following patch fixes it for me. Does it work for
> you on Windows?

Thanks.

This patch is obviously correct on any platform except Mac OS X.
On Mac OS X it gets confusing because [info nameofexecutable] is
a wrapper .app which loads its own main script.  That main script
needs to use the gitk location it receives from git-gui, and not
the location it computes from gitexecdir.  So I'm following up
your patch with this:

diff --git a/macosx/AppMain.tcl b/macosx/AppMain.tcl
index 41ca08e..ddbe633 100644
--- a/macosx/AppMain.tcl
+++ b/macosx/AppMain.tcl
@@ -7,7 +7,7 @@ if {[string first -psn [lindex $argv 0]] == 0} {
 }
 
 if {[file tail [lindex $argv 0]] eq {gitk}} {
-	set argv0 [file join $gitexecdir gitk]
+	set argv0 [lindex $argv 0]
 	set AppMain_source $argv0
 } else {
 	set argv0 [file join $gitexecdir [file tail [lindex $argv 0]]]
 
-- 
Shawn.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core
  2008-07-29 12:40     ` Murphy, John
@ 2008-07-29 16:48       ` Shawn O. Pearce
  2008-07-30  5:42         ` Shawn O. Pearce
  0 siblings, 1 reply; 6+ messages in thread
From: Shawn O. Pearce @ 2008-07-29 16:48 UTC (permalink / raw)
  To: Murphy, John; +Cc: Abhijit Menon-Sen, git

"Murphy, John" <john.murphy@bankofamerica.com> wrote:
> I have rebuilt git-gui with version 0.10.2.18.gc629 it is still not finding gitk.
> I have done some debugging in proc _which
> I have found the issue is with the following line:
> 
> set p [file join $p $what$_search_exe]
> 
> The variable $p = gitk.exe
> 
> And there is no such animal
> 
> When I copy gitk to gitk.exe in /usr/local/git/bin, it works fine.
> 
> So, not sure if this is a git issue or a git-gui issue.

Since you are new to the list, I will politely point out we do
not top-post.  (http://www.html-faq.com/etiquette/?toppost) and
much prefer inline posting.  (Yea, I know, its Outlook.  Switch to
text only message format and then edit inline in the reply.)

This is definately a git-gui bug.  _which should search not just
.exe extensions, but also the base name itself in case it is a
shell script or perl script.  git-gui already has special cases
to handle executing those through their interpreters even on a
pure Win32 Tcl/Tk process.

Its wrong to install gitk as gitk.exe, it is not a Win32 executable
process and the Windows kernel cannot load it into memory on its own.

 
> From: spearce@spearce.org [mailto:spearce@spearce.org] 
> Abhijit Menon-Sen <ams@toroid.org> wrote:
> > At 2008-07-24 09:01:48 -0400, john.murphy@bankofamerica.com wrote:
> > >
> > > I presume this was just an oversight when git- commands were removed
> > > from the bin directory.
> > 
> > Looks like it. The following patch fixes it for me. Does it work for
> > you on Windows?
> 
> Thanks.
> 
> This patch is obviously correct on any platform except Mac OS X.
> On Mac OS X it gets confusing because [info nameofexecutable] is
> a wrapper .app which loads its own main script.  That main script
> needs to use the gitk location it receives from git-gui, and not
> the location it computes from gitexecdir.  So I'm following up
> your patch with this:
> 
> diff --git a/macosx/AppMain.tcl b/macosx/AppMain.tcl
> index 41ca08e..ddbe633 100644
> --- a/macosx/AppMain.tcl
> +++ b/macosx/AppMain.tcl
> @@ -7,7 +7,7 @@ if {[string first -psn [lindex $argv 0]] == 0} {
>  }
>  
>  if {[file tail [lindex $argv 0]] eq {gitk}} {
> -	set argv0 [file join $gitexecdir gitk]
> +	set argv0 [lindex $argv 0]
>  	set AppMain_source $argv0
>  } else {
>  	set argv0 [file join $gitexecdir [file tail [lindex $argv 0]]]

-- 
Shawn.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core
  2008-07-29 16:48       ` Shawn O. Pearce
@ 2008-07-30  5:42         ` Shawn O. Pearce
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn O. Pearce @ 2008-07-30  5:42 UTC (permalink / raw)
  To: Murphy, John; +Cc: Abhijit Menon-Sen, git

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> "Murphy, John" <john.murphy@bankofamerica.com> wrote:
> > I have rebuilt git-gui with version 0.10.2.18.gc629 it is still not finding gitk.
> > I have done some debugging in proc _which
> > I have found the issue is with the following line:
> > 
> > set p [file join $p $what$_search_exe]
> > 
> > The variable $p = gitk.exe
> > 
> > And there is no such animal
> > 
> > When I copy gitk to gitk.exe in /usr/local/git/bin, it works fine.
> 
> This is definately a git-gui bug.

And this should fix it.

--8<--
git-gui: Fix gitk search in $PATH to work on Windows

Back in 15430be5a1 ("Look for gitk in $PATH, not $LIBEXEC/git-core")
git-gui learned to use [_which gitk] to locate where gitk's script
is as Git 1.6 will install gitk to $prefix/bin (in $PATH) and all
of the other tools are in $gitexecdir.

This failed on Windows because _which adds the ".exe" suffix as it
searches for the program on $PATH, under the assumption that we can
only execute something from Tcl if it is a proper Windows executable.

When scanning for gitk on Windows we need to omit the ".exe" suffix.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 git-gui.sh |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/git-gui.sh b/git-gui.sh
index ce941ad..14b2d9a 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -317,7 +317,7 @@ proc _git_cmd {name} {
 	return $v
 }
 
-proc _which {what} {
+proc _which {what args} {
 	global env _search_exe _search_path
 
 	if {$_search_path eq {}} {
@@ -340,8 +340,14 @@ proc _which {what} {
 		}
 	}
 
+	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$_search_exe]
+		set p [file join $p $what$suffix]
 		if {[file exists $p]} {
 			return [file normalize $p]
 		}
@@ -1686,7 +1692,7 @@ proc do_gitk {revs} {
 	# -- Always start gitk through whatever we were loaded with.  This
 	#    lets us bypass using shell process on Windows systems.
 	#
-	set exe [_which gitk]
+	set exe [_which gitk -script]
 	set cmd [list [info nameofexecutable] $exe]
 	if {$exe eq {}} {
 		error_popup [mc "Couldn't find gitk in PATH"]
-- 
1.6.0.rc1.166.gbbfa8


-- 
Shawn.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-07-30  5:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-24 13:01 Git Gui bug calling gitk Murphy, John
2008-07-24 13:28 ` [PATCH] git-gui: Look for gitk in $PATH, not $LIBEXEC/git-core Abhijit Menon-Sen
2008-07-25 22:05   ` Shawn O. Pearce
2008-07-29 12:40     ` Murphy, John
2008-07-29 16:48       ` Shawn O. Pearce
2008-07-30  5:42         ` Shawn O. Pearce

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).