Re: [PATCH 1/3] git-gui: Adapt discovery of oguilib to execdir 'libexec/git-core'
On Aug 3, 2008, at 11:35 AM, Johannes Sixt wrote:
Show 89 quoted lines
> 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.
Show 9 quoted lines
> 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.
Show 5 quoted lines
> 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
Steffen Prohaska
