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:

Zitat von Steffen Prohaska [off-list ref]:

quoted

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

quoted

Steffen Prohaska [off-list ref] wrote:

quoted

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.

...

quoted

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.

quoted

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 <redacted>
---
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?

quoted

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