git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH 0/3] git-gui (Windows): Adapt to new execdir 'libexec/git-core'
@ 2008-07-27 16:49 Steffen Prohaska
  2008-07-27 16:49 ` [PATCH 1/3] git-gui: Adapt discovery of oguilib to " Steffen Prohaska
  0 siblings, 1 reply; 13+ messages in thread
From: Steffen Prohaska @ 2008-07-27 16:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt

After moving the dashed programs, including git-gui, to execdir
'libexec/git-core', we have problems running git-gui from
msysgit.

The following patch series solves the problems.  I am not sure
however if it is the less intrusive solution.  Maybe the series
needs further discussion before applying it.  Personally, I think
the patches are good enough.

Note that you also need the branch 'spr/installer' in msysgit to
build an installer.

 [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
 [PATCH 2/3] git-gui (Windows): Switch to relative discovery of oguilib
 [PATCH 3/3] git-gui (Windows): Change wrapper to execdir 'libexec/git-core'

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

* [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-07-27 16:49 [PATCH 0/3] git-gui (Windows): Adapt to new execdir 'libexec/git-core' Steffen Prohaska
@ 2008-07-27 16:49 ` Steffen Prohaska
  2008-07-27 16:49   ` [PATCH 2/3] git-gui (Windows): Switch to relative discovery of oguilib Steffen Prohaska
  2008-07-27 21:24   ` [PATCH 1/3] git-gui: Adapt discovery of oguilib " Shawn O. Pearce
  0 siblings, 2 replies; 13+ messages in thread
From: Steffen Prohaska @ 2008-07-27 16:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt, Steffen Prohaska

The new execdir has is two levels below the root directory, while
the old execdir 'bin' was only one level below.  This commit
adapts the discovery of oguilib that uses relative paths
accordingly.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 git-gui/git-gui.sh |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 940677c..baccd57 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -52,7 +52,9 @@ catch {rename send {}} ; # What an evil concept...
 set oguilib {@@GITGUI_LIBDIR@@}
 set oguirel {@@GITGUI_RELATIVE@@}
 if {$oguirel eq {1}} {
-	set oguilib [file dirname [file dirname [file normalize $argv0]]]
+	set oguilib [file dirname \
+	             [file dirname \
+	              [file dirname [file normalize $argv0]]]]
 	set oguilib [file join $oguilib share git-gui lib]
 	set oguimsg [file join $oguilib msgs]
 } elseif {[string match @@* $oguirel]} {
-- 
1.6.0.rc0.79.gb0320

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

* [PATCH 2/3] git-gui (Windows): Switch to relative discovery of oguilib
  2008-07-27 16:49 ` [PATCH 1/3] git-gui: Adapt discovery of oguilib to " Steffen Prohaska
@ 2008-07-27 16:49   ` Steffen Prohaska
  2008-07-27 16:49     ` [PATCH 3/3] git-gui (Windows): Change wrapper to execdir 'libexec/git-core' Steffen Prohaska
  2008-07-27 21:24   ` [PATCH 1/3] git-gui: Adapt discovery of oguilib " Shawn O. Pearce
  1 sibling, 1 reply; 13+ messages in thread
From: Steffen Prohaska @ 2008-07-27 16:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt, Steffen Prohaska

Instead of using an absolute path, git-gui can discover its
gui library using a relative path from execdir.  We want to
use the relative path discovery on MinGW to avoid issues
with translation of absolute paths.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 git-gui/Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-gui/Makefile b/git-gui/Makefile
index b19fb2d..ec4b33e 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -156,6 +156,7 @@ endif
 ifneq (,$(findstring MINGW,$(uname_S)))
 	NO_MSGFMT=1
 	GITGUI_WINDOWS_WRAPPER := YesPlease
+	GITGUI_RELATIVE := 1
 endif
 
 ifdef GITGUI_MACOSXAPP
-- 
1.6.0.rc0.79.gb0320

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

* [PATCH 3/3] git-gui (Windows): Change wrapper to execdir 'libexec/git-core'
  2008-07-27 16:49   ` [PATCH 2/3] git-gui (Windows): Switch to relative discovery of oguilib Steffen Prohaska
@ 2008-07-27 16:49     ` Steffen Prohaska
  0 siblings, 0 replies; 13+ messages in thread
From: Steffen Prohaska @ 2008-07-27 16:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt, Steffen Prohaska

git-gui needs bindir in PATH to be able to run 'git'.  bindir
however is not necessarily in PATH if started directly through a
Windows shortcut.  Therefore, we used to add the directory
git-gui is located in.  But with the new 'libexec/git-core'
layout this directory is no longer identical to bindir.

This commit modifies the wrapper script to discover the bindir
and add it to PATH.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 git-gui/windows/git-gui.sh |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/git-gui/windows/git-gui.sh b/git-gui/windows/git-gui.sh
index 98f32c0..53c3a94 100644
--- a/git-gui/windows/git-gui.sh
+++ b/git-gui/windows/git-gui.sh
@@ -8,9 +8,12 @@ if { $argc >=2 && [lindex $argv 0] == "--working-dir" } {
 	incr argc -2
 }
 
-set gitguidir [file dirname [info script]]
-regsub -all ";" $gitguidir "\\;" gitguidir
-set env(PATH) "$gitguidir;$env(PATH)"
-unset gitguidir
+set bindir [file dirname \
+            [file dirname \
+             [file dirname [info script]]]]
+set bindir [file join $bindir bin]
+regsub -all ";" $bindir "\\;" bindir
+set env(PATH) "$bindir;$env(PATH)"
+unset bindir
 
 source [file join [file dirname [info script]] git-gui.tcl]
-- 
1.6.0.rc0.79.gb0320

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

* Re: [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-07-27 16:49 ` [PATCH 1/3] git-gui: Adapt discovery of oguilib to " Steffen Prohaska
  2008-07-27 16:49   ` [PATCH 2/3] git-gui (Windows): Switch to relative discovery of oguilib Steffen Prohaska
@ 2008-07-27 21:24   ` Shawn O. Pearce
  2008-07-28  5:01     ` Steffen Prohaska
  1 sibling, 1 reply; 13+ messages in thread
From: Shawn O. Pearce @ 2008-07-27 21:24 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Johannes Sixt

Steffen Prohaska <prohaska@zib.de> wrote:
> The new execdir has is two levels below the root directory, while
> the old execdir 'bin' was only one level below.  This commit
> adapts the discovery of oguilib that uses relative paths
> accordingly.
...
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index 940677c..baccd57 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -52,7 +52,9 @@ catch {rename send {}} ; # What an evil concept...
>  set oguilib {@@GITGUI_LIBDIR@@}
>  set oguirel {@@GITGUI_RELATIVE@@}
>  if {$oguirel eq {1}} {
> -	set oguilib [file dirname [file dirname [file normalize $argv0]]]
> +	set oguilib [file dirname \
> +	             [file dirname \
> +	              [file dirname [file normalize $argv0]]]]
>  	set oguilib [file join $oguilib share git-gui lib]

Hmmph.  This actually comes up incorrectly on my system.  The issue
appears to be `git --exec-path` gives me $prefix/libexec/git-core,
and git-gui installs its library into $prefix/libexec/share, which
is wrong.  It should have gone to $prefix/share.

I wonder if this is better.  Your other two patches seem fine.

--8<--
[PATCH] git-gui: Correct installation of library to be $prefix/share

We always wanted the library for git-gui to install into the
$prefix/share directory, not $prefix/libexec/share.  All of
the files in our library are platform independent and may
be reused across systems, like any other content stored in
the share directory.

Our computation of where our library should install to was broken
when git itself started installing to $prefix/libexec/git-core,
which was one level down from where we expected it to be.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Makefile |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index b19fb2d..f72ab6c 100644
--- a/Makefile
+++ b/Makefile
@@ -32,6 +32,9 @@ endif
 ifndef gitexecdir
 	gitexecdir := $(shell git --exec-path)
 endif
+ifeq (git-core,$(notdir $(gitexecdir)))
+	gitexecdir := $(patsubst %/,%,$(dir $(gitexecdir)))
+endif
 
 ifndef sharedir
 	sharedir := $(dir $(gitexecdir))share
-- 
1.6.0.rc0.182.gb96c7


-- 
Shawn.

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

* Re: [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-07-27 21:24   ` [PATCH 1/3] git-gui: Adapt discovery of oguilib " Shawn O. Pearce
@ 2008-07-28  5:01     ` Steffen Prohaska
  2008-07-30  5:25       ` Shawn O. Pearce
  2008-08-03  9:35       ` Johannes Sixt
  0 siblings, 2 replies; 13+ messages in thread
From: Steffen Prohaska @ 2008-07-28  5:01 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt


On Jul 27, 2008, at 11:24 PM, Shawn O. Pearce wrote:

> Steffen Prohaska <prohaska@zib.de> wrote:
>> The new execdir has is two levels below the root directory, while
>> the old execdir 'bin' was only one level below.  This commit
>> adapts the discovery of oguilib that uses relative paths
>> accordingly.
> ...
>> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
>> index 940677c..baccd57 100755
>> --- a/git-gui/git-gui.sh
>> +++ b/git-gui/git-gui.sh
>> @@ -52,7 +52,9 @@ catch {rename send {}} ; # What an evil concept...
>> set oguilib {@@GITGUI_LIBDIR@@}
>> set oguirel {@@GITGUI_RELATIVE@@}
>> if {$oguirel eq {1}} {
>> -	set oguilib [file dirname [file dirname [file normalize $argv0]]]
>> +	set oguilib [file dirname \
>> +	             [file dirname \
>> +	              [file dirname [file normalize $argv0]]]]
>> 	set oguilib [file join $oguilib share git-gui lib]
>
> Hmmph.  This actually comes up incorrectly on my system.  The issue
> appears to be `git --exec-path` gives me $prefix/libexec/git-core,
> and git-gui installs its library into $prefix/libexec/share, which
> is wrong.  It should have gone to $prefix/share.

I am not seeing this problem because I am installing using the
toplevel makefile, which sets and exports sharedir to $prefix/share.


> I wonder if this is better.  Your other two patches seem fine.
>
> --8<--
> [PATCH] git-gui: Correct installation of library to be $prefix/share
>
> We always wanted the library for git-gui to install into the
> $prefix/share directory, not $prefix/libexec/share.  All of
> the files in our library are platform independent and may
> be reused across systems, like any other content stored in
> the share directory.
>
> Our computation of where our library should install to was broken
> when git itself started installing to $prefix/libexec/git-core,
> which was one level down from where we expected it to be.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> Makefile |    3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b19fb2d..f72ab6c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -32,6 +32,9 @@ endif
> ifndef gitexecdir
> 	gitexecdir := $(shell git --exec-path)
> endif
> +ifeq (git-core,$(notdir $(gitexecdir)))
> +	gitexecdir := $(patsubst %/,%,$(dir $(gitexecdir)))
> +endif

But gitexecdir has the correct value, no?  gitexecdir is used
at several places in the makefile.  It seems wrong to strip
'git-core' from gitexecdir.  But I must admit that I do not
understand all the details of git-gui's Makefile.  So maybe
you know better.

Isn't only the computation of sharedir based on gitexecdir wrong?

> ifndef sharedir
> 	sharedir := $(dir $(gitexecdir))share


and could be replaced with this (instead of your patch):

  ifndef sharedir
+ifeq (git-core,$(notdir $(gitexecdir)))
+       sharedir := $(dir $(patsubst %/,%,$(dir $(gitexecdir))))share
+else
         sharedir := $(dir $(gitexecdir))share
  endif
+endif

	Steffen

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

* Re: [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-07-28  5:01     ` Steffen Prohaska
@ 2008-07-30  5:25       ` Shawn O. Pearce
  2008-07-30  5:39         ` Steffen Prohaska
  2008-08-03  9:35       ` Johannes Sixt
  1 sibling, 1 reply; 13+ messages in thread
From: Shawn O. Pearce @ 2008-07-30  5:25 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git, Johannes Sixt

Steffen Prohaska <prohaska@zib.de> wrote:
> Isn't only the computation of sharedir based on gitexecdir wrong?
>
>> ifndef sharedir
>> 	sharedir := $(dir $(gitexecdir))share
>
> and could be replaced with this (instead of your patch):
>
>  ifndef sharedir
> +ifeq (git-core,$(notdir $(gitexecdir)))
> +       sharedir := $(dir $(patsubst %/,%,$(dir $(gitexecdir))))share
> +else
>         sharedir := $(dir $(gitexecdir))share
>  endif
> +endif

Oh, damn good catch.  Thanks.

How about this then?  Its your patch above, my message, and me
forging your SOB...

--8<--
From: Steffen Prohaska <prohaska@zib.de>
Subject: git-gui: Correct installation of library to be $prefix/share

We always wanted the library for git-gui to install into the
$prefix/share directory, not $prefix/libexec/share.  All of
the files in our library are platform independent and may
be reused across systems, like any other content stored in
the share directory.

Our computation of where our library should install to was broken
when git itself started installing to $prefix/libexec/git-core,
which was one level down from where we expected it to be.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 Makefile |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index b19fb2d..c9d67fe 100644
--- a/Makefile
+++ b/Makefile
@@ -34,8 +34,12 @@ ifndef gitexecdir
 endif
 
 ifndef sharedir
+ifeq (git-core,$(notdir $(gitexecdir)))
+	sharedir := $(dir $(patsubst %/,%,$(dir $(gitexecdir))))share
+else
 	sharedir := $(dir $(gitexecdir))share
 endif
+endif
 
 ifndef INSTALL
 	INSTALL = install
-- 
1.6.0.rc1.166.gbbfa8


-- 
Shawn.

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

* Re: [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-07-30  5:25       ` Shawn O. Pearce
@ 2008-07-30  5:39         ` Steffen Prohaska
  0 siblings, 0 replies; 13+ messages in thread
From: Steffen Prohaska @ 2008-07-30  5:39 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Johannes Sixt


On Jul 30, 2008, at 7:25 AM, Shawn O. Pearce wrote:

> Steffen Prohaska <prohaska@zib.de> wrote:
>> Isn't only the computation of sharedir based on gitexecdir wrong?
>>
>>> ifndef sharedir
>>> 	sharedir := $(dir $(gitexecdir))share
>>
>> and could be replaced with this (instead of your patch):
>>
>> ifndef sharedir
>> +ifeq (git-core,$(notdir $(gitexecdir)))
>> +       sharedir := $(dir $(patsubst %/,%,$(dir $(gitexecdir))))share
>> +else
>>        sharedir := $(dir $(gitexecdir))share
>> endif
>> +endif
>
> Oh, damn good catch.  Thanks.
>
> How about this then?  Its your patch above, my message, and me
> forging your SOB...

looks good.  SOB ok.

Thanks,
Steffen

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

* Re: [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-07-28  5:01     ` Steffen Prohaska
  2008-07-30  5:25       ` Shawn O. Pearce
@ 2008-08-03  9:35       ` Johannes Sixt
  2008-08-03 10:09         ` Steffen Prohaska
  1 sibling, 1 reply; 13+ messages in thread
From: Johannes Sixt @ 2008-08-03  9:35 UTC (permalink / raw)
  To: Steffen Prohaska, Shawn O. Pearce; +Cc: git

Zitat von Steffen Prohaska <prohaska@zib.de>:
>
> On Jul 27, 2008, at 11:24 PM, Shawn O. Pearce wrote:
>
> > Steffen Prohaska <prohaska@zib.de> wrote:
> >> The new execdir has is two levels below the root directory, while
> >> the old execdir 'bin' was only one level below.  This commit
> >> adapts the discovery of oguilib that uses relative paths
> >> accordingly.
> > ...
> >> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> >> index 940677c..baccd57 100755
> >> --- a/git-gui/git-gui.sh
> >> +++ b/git-gui/git-gui.sh
> >> @@ -52,7 +52,9 @@ catch {rename send {}} ; # What an evil concept...
> >> set oguilib {@@GITGUI_LIBDIR@@}
> >> set oguirel {@@GITGUI_RELATIVE@@}
> >> if {$oguirel eq {1}} {
> >> -	set oguilib [file dirname [file dirname [file normalize $argv0]]]
> >> +	set oguilib [file dirname \
> >> +	             [file dirname \
> >> +	              [file dirname [file normalize $argv0]]]]
> >> 	set oguilib [file join $oguilib share git-gui lib]
> >
> > Hmmph.  This actually comes up incorrectly on my system.  The issue
> > appears to be `git --exec-path` gives me $prefix/libexec/git-core,
> > and git-gui installs its library into $prefix/libexec/share, which
> > is wrong.  It should have gone to $prefix/share.
>
> I am not seeing this problem because I am installing using the
> toplevel makefile, which sets and exports sharedir to $prefix/share.
>
>
> > I wonder if this is better.  Your other two patches seem fine.
> >
> > --8<--
> > [PATCH] git-gui: Correct installation of library to be $prefix/share
> >
> > We always wanted the library for git-gui to install into the
> > $prefix/share directory, not $prefix/libexec/share.  All of
> > the files in our library are platform independent and may
> > be reused across systems, like any other content stored in
> > the share directory.
> >
> > Our computation of where our library should install to was broken
> > when git itself started installing to $prefix/libexec/git-core,
> > which was one level down from where we expected it to be.
> >
> > Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> > ---
> > Makefile |    3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index b19fb2d..f72ab6c 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -32,6 +32,9 @@ endif
> > ifndef gitexecdir
> > 	gitexecdir := $(shell git --exec-path)
> > endif
> > +ifeq (git-core,$(notdir $(gitexecdir)))
> > +	gitexecdir := $(patsubst %/,%,$(dir $(gitexecdir)))
> > +endif
>
> But gitexecdir has the correct value, no?  gitexecdir is used
> at several places in the makefile.  It seems wrong to strip
> 'git-core' from gitexecdir.  But I must admit that I do not
> understand all the details of git-gui's Makefile.  So maybe
> you know better.
>
> Isn't only the computation of sharedir based on gitexecdir wrong?
>
> > ifndef sharedir
> > 	sharedir := $(dir $(gitexecdir))share
>
>
> and could be replaced with this (instead of your patch):
>
>   ifndef sharedir
> +ifeq (git-core,$(notdir $(gitexecdir)))
> +       sharedir := $(dir $(patsubst %/,%,$(dir $(gitexecdir))))share
> +else
>          sharedir := $(dir $(gitexecdir))share
>   endif
> +endif

This is not good enough in my environment. I run git-gui effectivly with

   wish $prefix/libexec/git-core/git-gui

(and I have $PATH set up to contain $bindir, but not $gitexecdir), and this
needs the original hunk with the three [file dirname ... ], because $argv0
points to $prefix/libexec/git-core/git-gui.

I thought I understood what's going on, but I don't anymore.

Mybe the relative discovery of oguilib must be conditional on the "git-core"
part as well, just like you discover sharedir?

-- Hannes

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

* Re: [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-08-03  9:35       ` Johannes Sixt
@ 2008-08-03 10:09         ` Steffen Prohaska
  2008-08-04 19:58           ` [PATCH] " Johannes Sixt
  0 siblings, 1 reply; 13+ messages in thread
From: Steffen Prohaska @ 2008-08-03 10:09 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git Mailing List, Shawn O. Pearce


On Aug 3, 2008, at 11:35 AM, Johannes Sixt wrote:

> Zitat von Steffen Prohaska <prohaska@zib.de>:
>>
>> On Jul 27, 2008, at 11:24 PM, Shawn O. Pearce wrote:
>>
>>> Steffen Prohaska <prohaska@zib.de> wrote:
>>>> The new execdir has is two levels below the root directory, while
>>>> the old execdir 'bin' was only one level below.  This commit
>>>> adapts the discovery of oguilib that uses relative paths
>>>> accordingly.
>>> ...
>>>> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
>>>> index 940677c..baccd57 100755
>>>> --- a/git-gui/git-gui.sh
>>>> +++ b/git-gui/git-gui.sh
>>>> @@ -52,7 +52,9 @@ catch {rename send {}} ; # What an evil  
>>>> concept...
>>>> set oguilib {@@GITGUI_LIBDIR@@}
>>>> set oguirel {@@GITGUI_RELATIVE@@}
>>>> if {$oguirel eq {1}} {
>>>> -	set oguilib [file dirname [file dirname [file normalize $argv0]]]
>>>> +	set oguilib [file dirname \
>>>> +	             [file dirname \
>>>> +	              [file dirname [file normalize $argv0]]]]
>>>> 	set oguilib [file join $oguilib share git-gui lib]
>>>
>>> Hmmph.  This actually comes up incorrectly on my system.  The issue
>>> appears to be `git --exec-path` gives me $prefix/libexec/git-core,
>>> and git-gui installs its library into $prefix/libexec/share, which
>>> is wrong.  It should have gone to $prefix/share.
>>
>> I am not seeing this problem because I am installing using the
>> toplevel makefile, which sets and exports sharedir to $prefix/share.
>>
>>
>>> I wonder if this is better.  Your other two patches seem fine.
>>>
>>> --8<--
>>> [PATCH] git-gui: Correct installation of library to be $prefix/share
>>>
>>> We always wanted the library for git-gui to install into the
>>> $prefix/share directory, not $prefix/libexec/share.  All of
>>> the files in our library are platform independent and may
>>> be reused across systems, like any other content stored in
>>> the share directory.
>>>
>>> Our computation of where our library should install to was broken
>>> when git itself started installing to $prefix/libexec/git-core,
>>> which was one level down from where we expected it to be.
>>>
>>> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
>>> ---
>>> Makefile |    3 +++
>>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Makefile b/Makefile
>>> index b19fb2d..f72ab6c 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -32,6 +32,9 @@ endif
>>> ifndef gitexecdir
>>> 	gitexecdir := $(shell git --exec-path)
>>> endif
>>> +ifeq (git-core,$(notdir $(gitexecdir)))
>>> +	gitexecdir := $(patsubst %/,%,$(dir $(gitexecdir)))
>>> +endif
>>
>> But gitexecdir has the correct value, no?  gitexecdir is used
>> at several places in the makefile.  It seems wrong to strip
>> 'git-core' from gitexecdir.  But I must admit that I do not
>> understand all the details of git-gui's Makefile.  So maybe
>> you know better.
>>
>> Isn't only the computation of sharedir based on gitexecdir wrong?
>>
>>> ifndef sharedir
>>> 	sharedir := $(dir $(gitexecdir))share
>>
>>
>> and could be replaced with this (instead of your patch):
>>
>>  ifndef sharedir
>> +ifeq (git-core,$(notdir $(gitexecdir)))
>> +       sharedir := $(dir $(patsubst %/,%,$(dir $(gitexecdir))))share
>> +else
>>         sharedir := $(dir $(gitexecdir))share
>>  endif
>> +endif
>
> This is not good enough in my environment.

This only fixes the installation directory.


> I run git-gui effectivly with
>
>   wish $prefix/libexec/git-core/git-gui
>
> (and I have $PATH set up to contain $bindir, but not $gitexecdir),  
> and this
> needs the original hunk with the three [file dirname ... ], because  
> $argv0
> points to $prefix/libexec/git-core/git-gui.

The original hunk fixes the discovery of oguilib, i.e.
from $prefix/libexec/git-core/git-gui to $prefix/share/git-gui/lib

I didn't recognize that the the 'three [file dinames ...]' have
not been applied because I had this change already in 4msysgit
(and still have).  Apologies for not checking this more carefully.


> I thought I understood what's going on, but I don't anymore.
>
> Mybe the relative discovery of oguilib must be conditional on the  
> "git-core"
> part as well, just like you discover sharedir?

Hmm... you are right.  If we want to maintain compatibility
with *both* directory layouts, obviously all computations
that depend on the layout need to be conditional on it.

	Steffen

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

* [PATCH] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-08-03 10:09         ` Steffen Prohaska
@ 2008-08-04 19:58           ` Johannes Sixt
  2008-08-04 20:00             ` Shawn O. Pearce
  0 siblings, 1 reply; 13+ messages in thread
From: Johannes Sixt @ 2008-08-04 19:58 UTC (permalink / raw)
  To: Steffen Prohaska, Shawn O. Pearce; +Cc: Git Mailing List

The new execdir has is two levels below the root directory, while
the old execdir 'bin' was only one level below.  This commit
adapts the discovery of oguilib that uses relative paths
accordingly. We determine whether we have the extra level in the same
way in which the Makefile defines sharedir, i.e. whether the last
directory part is 'git-core'.

Inspired-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---

Zitat von Steffen Prohaska <prohaska@zib.de>:
>
> On Aug 3, 2008, at 11:35 AM, Johannes Sixt wrote:
> > I run git-gui effectivly with
> >
> >   wish $prefix/libexec/git-core/git-gui
> >
> > (and I have $PATH set up to contain $bindir, but not $gitexecdir),
> > and this
> > needs the original hunk with the three [file dirname ... ], because
> > $argv0
> > points to $prefix/libexec/git-core/git-gui.
>
> The original hunk fixes the discovery of oguilib, i.e.
> from $prefix/libexec/git-core/git-gui to $prefix/share/git-gui/lib
>
> I didn't recognize that the the 'three [file dinames ...]' have
> not been applied because I had this change already in 4msysgit
> (and still have).  Apologies for not checking this more carefully.
>
>
> > I thought I understood what's going on, but I don't anymore.
> >
> > Mybe the relative discovery of oguilib must be conditional on the
> > "git-core"
> > part as well, just like you discover sharedir?
>
> Hmm... you are right.  If we want to maintain compatibility
> with *both* directory layouts, obviously all computations
> that depend on the layout need to be conditional on it.

Here is a patch that does just this.

-- Hannes

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

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index fd3875a..fa08d49 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -52,7 +52,11 @@ catch {rename send {}} ; # What an evil concept...
 set oguilib {@@GITGUI_LIBDIR@@}
 set oguirel {@@GITGUI_RELATIVE@@}
 if {$oguirel eq {1}} {
-	set oguilib [file dirname [file dirname [file normalize $argv0]]]
+	set oguilib [file dirname [file normalize $argv0]]
+	if {[lindex [file split $oguilib] end] eq {git-core}} {
+		set oguilib [file dirname $oguilib]
+	}
+	set oguilib [file dirname $oguilib]
 	set oguilib [file join $oguilib share git-gui lib]
 	set oguimsg [file join $oguilib msgs]
 } elseif {[string match @@* $oguirel]} {
-- 
1.6.0.rc1.958.gce1ed

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

* Re: [PATCH] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-08-04 19:58           ` [PATCH] " Johannes Sixt
@ 2008-08-04 20:00             ` Shawn O. Pearce
  2008-08-04 20:09               ` [PATCH v2] " Johannes Sixt
  0 siblings, 1 reply; 13+ messages in thread
From: Shawn O. Pearce @ 2008-08-04 20:00 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Steffen Prohaska, Git Mailing List

Johannes Sixt <johannes.sixt@telecom.at> wrote:
> diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
> index fd3875a..fa08d49 100755
> --- a/git-gui/git-gui.sh
> +++ b/git-gui/git-gui.sh
> @@ -52,7 +52,11 @@ catch {rename send {}} ; # What an evil concept...
>  set oguilib {@@GITGUI_LIBDIR@@}
>  set oguirel {@@GITGUI_RELATIVE@@}
>  if {$oguirel eq {1}} {
> -	set oguilib [file dirname [file dirname [file normalize $argv0]]]
> +	set oguilib [file dirname [file normalize $argv0]]
> +	if {[lindex [file split $oguilib] end] eq {git-core}} {
> +		set oguilib [file dirname $oguilib]

Isn't that what [file tail] does?

	if {[file tail $oguilib] eq {git-core}} {

But otherwise this looks good to me.  I'll apply it later tonight,
unless you agree that file tail is better here.

-- 
Shawn.

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

* [PATCH v2] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
  2008-08-04 20:00             ` Shawn O. Pearce
@ 2008-08-04 20:09               ` Johannes Sixt
  0 siblings, 0 replies; 13+ messages in thread
From: Johannes Sixt @ 2008-08-04 20:09 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Steffen Prohaska, Git Mailing List

The new execdir has is two levels below the root directory, while
the old execdir 'bin' was only one level below.  This commit
adapts the discovery of oguilib that uses relative paths
accordingly. We determine whether we have the extra level in the same
way in which the Makefile defines sharedir, i.e. whether the last
directory part is 'git-core'.

Inspired-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---

Zitat von "Shawn O. Pearce" <spearce@spearce.org>:
> Johannes Sixt <johannes.sixt@telecom.at> wrote:
> > +	if {[lindex [file split $oguilib] end] eq {git-core}} {
>
> Isn't that what [file tail] does?

Yes, it is. I missed it in the myriad of file's options. Here's the updated
version of the patch.

-- Hannes

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

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index fd3875a..60f376d 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh
@@ -52,7 +52,11 @@ catch {rename send {}} ; # What an evil concept...
 set oguilib {@@GITGUI_LIBDIR@@}
 set oguirel {@@GITGUI_RELATIVE@@}
 if {$oguirel eq {1}} {
-	set oguilib [file dirname [file dirname [file normalize $argv0]]]
+	set oguilib [file dirname [file normalize $argv0]]
+	if {[file tail $oguilib] eq {git-core}} {
+		set oguilib [file dirname $oguilib]
+	}
+	set oguilib [file dirname $oguilib]
 	set oguilib [file join $oguilib share git-gui lib]
 	set oguimsg [file join $oguilib msgs]
 } elseif {[string match @@* $oguirel]} {
-- 
1.6.0.rc1.958.gb3893

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

end of thread, other threads:[~2008-08-04 20:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-07-27 16:49 [PATCH 0/3] git-gui (Windows): Adapt to new execdir 'libexec/git-core' Steffen Prohaska
2008-07-27 16:49 ` [PATCH 1/3] git-gui: Adapt discovery of oguilib to " Steffen Prohaska
2008-07-27 16:49   ` [PATCH 2/3] git-gui (Windows): Switch to relative discovery of oguilib Steffen Prohaska
2008-07-27 16:49     ` [PATCH 3/3] git-gui (Windows): Change wrapper to execdir 'libexec/git-core' Steffen Prohaska
2008-07-27 21:24   ` [PATCH 1/3] git-gui: Adapt discovery of oguilib " Shawn O. Pearce
2008-07-28  5:01     ` Steffen Prohaska
2008-07-30  5:25       ` Shawn O. Pearce
2008-07-30  5:39         ` Steffen Prohaska
2008-08-03  9:35       ` Johannes Sixt
2008-08-03 10:09         ` Steffen Prohaska
2008-08-04 19:58           ` [PATCH] " Johannes Sixt
2008-08-04 20:00             ` Shawn O. Pearce
2008-08-04 20:09               ` [PATCH v2] " Johannes Sixt

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).