git-gui/Makefile: Replace libdir with gitgui_libdir

PATCH In review Dmitry V. Levin 2 participants 5 messages Jun 15, 2016

Share & tools

Dmitry V. Levin

author

2016-06-15 22:43 UTC #0 lore raw

PATCH 1/3 git-gui/Makefile: Replace libdir with gitgui_libdir

On GNU/Linux, libdir is used to mean "/usr/lib or /usr/lib64" depending on architecture. Different libdir meaning breaks idiomatic expressions like rpm specfile "make libdir=%_libdir".

Signed-off-by Dmitry V. Levin <[email protected]>

---
 git-gui/Makefile |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

git-gui/Makefile +8 -8

index 1bac6fe..f143b2c 100644
@@ -76,8 +76,8 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 TCL_PATH_SQ = $(subst ','\'',$(TCL_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 
-libdir   ?= $(sharedir)/git-gui/lib
-libdir_SQ = $(subst ','\'',$(libdir))
+gitgui_libdir   ?= $(sharedir)/git-gui/lib
+gitgui_libdir_SQ = $(subst ','\'',$(gitgui_libdir))
 
 exedir    = $(dir $(gitexecdir))share/git-gui/lib
 exedir_SQ = $(subst ','\'',$(exedir))

@@ -85,7 +85,7 @@ exedir_SQ = $(subst ','\'',$(exedir))
 $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 	$(QUIET_GEN)rm -f $@ $@+ && \
 	GITGUI_RELATIVE= && \
-	if test '$(exedir_SQ)' = '$(libdir_SQ)'; then \
+	if test '$(exedir_SQ)' = '$(gitgui_libdir_SQ)'; then \
 		if test "$(uname_O)" = Cygwin; \
 		then GITGUI_RELATIVE= ; \
 		else GITGUI_RELATIVE=1; \

@@ -95,7 +95,7 @@ $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh
 		-e 's|^ exec wish "$$0"| exec $(subst |,'\|',$(TCLTK_PATH_SQ)) "$$0"|' \
 		-e 's/@@GITGUI_VERSION@@/$(GITGUI_VERSION)/g' \
 		-e 's|@@GITGUI_RELATIVE@@|'$$GITGUI_RELATIVE'|' \
-		-e $$GITGUI_RELATIVE's|@@GITGUI_LIBDIR@@|$(libdir_SQ)|' \
+		-e $$GITGUI_RELATIVE's|@@GITGUI_LIBDIR@@|$(gitgui_libdir_SQ)|' \
 		[email protected] >$@+ && \
 	chmod +x $@+ && \
 	mv $@+ $@

@@ -126,7 +126,7 @@ TRACK_VARS = \
 	$(subst ','\'',TCL_PATH='$(TCL_PATH_SQ)') \
 	$(subst ','\'',TCLTK_PATH='$(TCLTK_PATH_SQ)') \
 	$(subst ','\'',gitexecdir='$(gitexecdir_SQ)') \
-	$(subst ','\'',libdir='$(libdir_SQ)') \
+	$(subst ','\'',gitgui_libdir='$(gitgui_libdir_SQ)') \
 #end TRACK_VARS
 
 GIT-GUI-VARS: .FORCE-GIT-GUI-VARS

@@ -142,9 +142,9 @@ install: all
 	$(QUIET)$(INSTALL_D0)'$(DESTDIR_SQ)$(gitexecdir_SQ)' $(INSTALL_D1)
 	$(QUIET)$(INSTALL_X0)git-gui $(INSTALL_X1) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
 	$(QUIET)$(foreach p,$(GITGUI_BUILT_INS), $(INSTALL_L0)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L1)'$(DESTDIR_SQ)$(gitexecdir_SQ)/git-gui' $(INSTALL_L2)'$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' $(INSTALL_L3) &&) true
-	$(QUIET)$(INSTALL_D0)'$(DESTDIR_SQ)$(libdir_SQ)' $(INSTALL_D1)
-	$(QUIET)$(INSTALL_R0)lib/tclIndex $(INSTALL_R1) '$(DESTDIR_SQ)$(libdir_SQ)'
-	$(QUIET)$(foreach p,$(ALL_LIBFILES), $(INSTALL_R0)$p $(INSTALL_R1) '$(DESTDIR_SQ)$(libdir_SQ)' &&) true
+	$(QUIET)$(INSTALL_D0)'$(DESTDIR_SQ)$(gitgui_libdir_SQ)' $(INSTALL_D1)
+	$(QUIET)$(INSTALL_R0)lib/tclIndex $(INSTALL_R1) '$(DESTDIR_SQ)$(gitgui_libdir_SQ)'
+	$(QUIET)$(foreach p,$(ALL_LIBFILES), $(INSTALL_R0)$p $(INSTALL_R1) '$(DESTDIR_SQ)$(gitgui_libdir_SQ)' &&) true
 
 dist-version:
 	@mkdir -p $(TARDIR)

Signature · 1 line

ldv

Shawn O. Pearce

2016-06-15 22:43 UTC #1 lore raw

Quoted 8 lines · Dmitry V. Levin

On GNU/Linux, libdir is used to mean "/usr/lib or /usr/lib64" depending on architecture. Different libdir meaning breaks idiomatic expressions like rpm specfile "make libdir=%_libdir".
Signed-off-by Dmitry V. Levin <[email protected]>
---
 git-gui/Makefile |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

Although I could apply this with am -3 I'm NACK'ing this right now because...

git-gui/Makefile SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) · line 80

Quoted 12 lines

git-gui/Makefile +2 -2

index 1bac6fe..f143b2c 100644
@@ -76,8 +76,8 @@ SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
 TCL_PATH_SQ = $(subst ','\'',$(TCL_PATH))
 TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH))
 
-libdir   ?= $(sharedir)/git-gui/lib
-libdir_SQ = $(subst ','\'',$(libdir))
+gitgui_libdir   ?= $(sharedir)/git-gui/lib
+gitgui_libdir_SQ = $(subst ','\'',$(gitgui_libdir))

git-gui is its own project with its own Makefile. Junio includes it in git.git to help widen its audience, and because it is quite
portable and easy for him to include. In the future git-gui will
become a proper subproject of git.git.

If you want to define libdir in git's toplevel Makefile *and* that definition is being exported down into git-gui's Makefile then I think git's toplevel Makefile should arrange for it to not export libdir down into git-gui.

Of course one has to wonder if that is even possible in GNU make. I'm not a GNU make expert so I don't really know.

Signature · 1 line

Shawn.

Dmitry V. Levin

author

2016-06-15 22:43 UTC #2 lore raw

Quoted 12 lines · Shawn O. Pearce

Quoted 8 lines · Dmitry V. Levin
On GNU/Linux, libdir is used to mean "/usr/lib or /usr/lib64" depending on architecture. Different libdir meaning breaks idiomatic expressions like rpm specfile "make libdir=%_libdir".
Signed-off-by Dmitry V. Levin <[email protected]>
---
 git-gui/Makefile |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
Although I could apply this with am -3 I'm NACK'ing this right now because...

[...]

Quoted 4 lines

git-gui is its own project with its own Makefile. Junio includes it in git.git to help widen its audience, and because it is quite
portable and easy for him to include. In the future git-gui will
become a proper subproject of git.git.

The idea is that git-gui's libdir is not a traditional arch-dependent libdir's subdirectory, but rather arch-independent datadir's subdirectory. That is, I see no reason to call it libdir even in standalone project.

Quoted 2 lines

If you want to define libdir in git's toplevel Makefile *and* that definition is being exported down into git-gui's Makefile

Yes, that was my first motivation, but the issue appears to be a bit more complex.

Signature · 1 line

ldv

Shawn O. Pearce

2016-06-15 22:43 UTC #3 lore raw

Quoted 8 lines · Dmitry V. Levin

Quoted 4 lines
Quoted 3 lines · Dmitry V. Levin
On GNU/Linux, libdir is used to mean "/usr/lib or /usr/lib64" depending on architecture. Different libdir meaning breaks idiomatic expressions like rpm specfile "make libdir=%_libdir".
The idea is that git-gui's libdir is not a traditional arch-dependent libdir's subdirectory, but rather arch-independent datadir's subdirectory. That is, I see no reason to call it libdir even in standalone project.

Call it datadir then? I see you point, and now agree with you. Mind resending a patch to call it 'datadir'?

Signature · 1 line

Shawn.

Dmitry V. Levin

author

2016-06-15 22:43 UTC #4 lore raw

Quoted 11 lines · Shawn O. Pearce

Quoted 8 lines · Dmitry V. Levin
Quoted 4 lines
Quoted 3 lines · Dmitry V. Levin
On GNU/Linux, libdir is used to mean "/usr/lib or /usr/lib64" depending on architecture. Different libdir meaning breaks idiomatic expressions like rpm specfile "make libdir=%_libdir".
The idea is that git-gui's libdir is not a traditional arch-dependent libdir's subdirectory, but rather arch-independent datadir's subdirectory. That is, I see no reason to call it libdir even in standalone project.
Call it datadir then? I see you point, and now agree with you.

I'm not sure that "datadir" is better choice than current "libdir". First, from git-gui point of view, files placed in git-gui/lib/ are rather library files than data files.
Second, if top-level makefile will define "datadir", original issue will raise again.
That's why I suggested to add some git-gui specific component to git-gui's libdir variable name.

Signature · 1 line

ldv