Re: [PATCH] Makefile: make mandir, htmldir and infodir absolute

2013-02-25 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 This matches the use of the variables with the same names in autotools,
 reducing the potential for user surprise.

 Using relative paths in these variables also causes issues if they are
 exported from the Makefile, as discussed in commit c09d62f (Makefile: do
 not export mandir/htmldir/infodir, 2013-02-12).

 Suggested-by: Junio C Hamano gits...@pobox.com
 Signed-off-by: John Keeping j...@keeping.me.uk
 ---
 On Tue, Feb 12, 2013 at 03:09:53PM -0800, Junio C Hamano wrote:
 A longer term fix is to introduce runtime_{man,html,info}dir variables
 to hold these funny values, and make {man,html,info}dir variables
 to have real paths whose default values begin with $(prefix), but
 as a first step, stop exporting them from the top-level Makefile.

 Here's an attempt at doing that.

 If this is sensible, should bindir_relative be calculated in the same
 way?

Thanks for taking the cue from existing bindir_relative, which I
overlooked.  Calling these *dir_relative, not runtime_*dir as I
hinted, makes a lot more sense and overall makes things consistent.

As most people would want to say bindir=/usr/local/bin and not
bindir_relative=bin from the command line (and I suspect that people
coming from ./configure would not even have a way to specify the
latter), I think your suggestion to make bindir the primary one and
derive bindir_relative from it makes sense.

What do Windows folks think?

[patch unsnipped for others' reference]

  Makefile | 36 +---
  1 file changed, 21 insertions(+), 15 deletions(-)

 diff --git a/Makefile b/Makefile
 index 7c75e3b..087eec4 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -360,33 +360,39 @@ STRIP ?= strip
  # Among the variables below, these:
  #   gitexecdir
  #   template_dir
 -#   mandir
 -#   infodir
 -#   htmldir
  #   sysconfdir
  # can be specified as a relative path some/where/else;
  # this is interpreted as relative to $(prefix) and git at
  # runtime figures out where they are based on the path to the executable.
 +# Additionally, the following will be treated as relative by git if they
 +# begin with $(prefix)/:
 +#   mandir
 +#   infodir
 +#   htmldir
  # This can help installing the suite in a relocatable way.
  
  prefix = $(HOME)
  bindir_relative = bin
  bindir = $(prefix)/$(bindir_relative)
 -mandir = share/man
 -infodir = share/info
 +mandir = $(prefix)/share/man
 +infodir = $(prefix)/share/info
  gitexecdir = libexec/git-core
  mergetoolsdir = $(gitexecdir)/mergetools
  sharedir = $(prefix)/share
  gitwebdir = $(sharedir)/gitweb
  localedir = $(sharedir)/locale
  template_dir = share/git-core/templates
 -htmldir = share/doc/git-doc
 +htmldir = $(prefix)/share/doc/git-doc
  ETC_GITCONFIG = $(sysconfdir)/gitconfig
  ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
  lib = lib
  # DESTDIR =
  pathsep = :
  
 +mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
 +infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
 +htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
 +
  export prefix bindir sharedir sysconfdir gitwebdir localedir
  
  CC = cc
 @@ -1548,12 +1554,12 @@ ETC_GITATTRIBUTES_SQ = $(subst 
 ','\'',$(ETC_GITATTRIBUTES))
  DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
  bindir_SQ = $(subst ','\'',$(bindir))
  bindir_relative_SQ = $(subst ','\'',$(bindir_relative))
 -mandir_SQ = $(subst ','\'',$(mandir))
 -infodir_SQ = $(subst ','\'',$(infodir))
 +mandir_relative_SQ = $(subst ','\'',$(mandir_relative))
 +infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
  localedir_SQ = $(subst ','\'',$(localedir))
  gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
  template_dir_SQ = $(subst ','\'',$(template_dir))
 -htmldir_SQ = $(subst ','\'',$(htmldir))
 +htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
  prefix_SQ = $(subst ','\'',$(prefix))
  gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
  
 @@ -1685,9 +1691,9 @@ strip: $(PROGRAMS) git$X
  
  git.sp git.s git.o: GIT-PREFIX
  git.sp git.s git.o: EXTRA_CPPFLAGS = \
 - '-DGIT_HTML_PATH=$(htmldir_SQ)' \
 - '-DGIT_MAN_PATH=$(mandir_SQ)' \
 - '-DGIT_INFO_PATH=$(infodir_SQ)'
 + '-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \
 + '-DGIT_MAN_PATH=$(mandir_relative_SQ)' \
 + '-DGIT_INFO_PATH=$(infodir_relative_SQ)'
  
  git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
   $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
 @@ -1697,9 +1703,9 @@ help.sp help.s help.o: common-cmds.h
  
  builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX
  builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 - '-DGIT_HTML_PATH=$(htmldir_SQ)' \
 - '-DGIT_MAN_PATH=$(mandir_SQ)' \
 - '-DGIT_INFO_PATH=$(infodir_SQ)'
 + '-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \
 + '-DGIT_MAN_PATH=$(mandir_relative_SQ)' \
 + '-DGIT_INFO_PATH=$(infodir_relative_SQ)'
  
  version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT
  version.sp version.s version.o: EXTRA_CPPFLAGS = \
--
To unsubscribe from 

[PATCH] Makefile: make mandir, htmldir and infodir absolute

2013-02-24 Thread John Keeping
This matches the use of the variables with the same names in autotools,
reducing the potential for user surprise.

Using relative paths in these variables also causes issues if they are
exported from the Makefile, as discussed in commit c09d62f (Makefile: do
not export mandir/htmldir/infodir, 2013-02-12).

Suggested-by: Junio C Hamano gits...@pobox.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
On Tue, Feb 12, 2013 at 03:09:53PM -0800, Junio C Hamano wrote:
 A longer term fix is to introduce runtime_{man,html,info}dir variables
 to hold these funny values, and make {man,html,info}dir variables
 to have real paths whose default values begin with $(prefix), but
 as a first step, stop exporting them from the top-level Makefile.

Here's an attempt at doing that.

If this is sensible, should bindir_relative be calculated in the same
way?

 Makefile | 36 +---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 7c75e3b..087eec4 100644
--- a/Makefile
+++ b/Makefile
@@ -360,33 +360,39 @@ STRIP ?= strip
 # Among the variables below, these:
 #   gitexecdir
 #   template_dir
-#   mandir
-#   infodir
-#   htmldir
 #   sysconfdir
 # can be specified as a relative path some/where/else;
 # this is interpreted as relative to $(prefix) and git at
 # runtime figures out where they are based on the path to the executable.
+# Additionally, the following will be treated as relative by git if they
+# begin with $(prefix)/:
+#   mandir
+#   infodir
+#   htmldir
 # This can help installing the suite in a relocatable way.
 
 prefix = $(HOME)
 bindir_relative = bin
 bindir = $(prefix)/$(bindir_relative)
-mandir = share/man
-infodir = share/info
+mandir = $(prefix)/share/man
+infodir = $(prefix)/share/info
 gitexecdir = libexec/git-core
 mergetoolsdir = $(gitexecdir)/mergetools
 sharedir = $(prefix)/share
 gitwebdir = $(sharedir)/gitweb
 localedir = $(sharedir)/locale
 template_dir = share/git-core/templates
-htmldir = share/doc/git-doc
+htmldir = $(prefix)/share/doc/git-doc
 ETC_GITCONFIG = $(sysconfdir)/gitconfig
 ETC_GITATTRIBUTES = $(sysconfdir)/gitattributes
 lib = lib
 # DESTDIR =
 pathsep = :
 
+mandir_relative = $(patsubst $(prefix)/%,%,$(mandir))
+infodir_relative = $(patsubst $(prefix)/%,%,$(infodir))
+htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir))
+
 export prefix bindir sharedir sysconfdir gitwebdir localedir
 
 CC = cc
@@ -1548,12 +1554,12 @@ ETC_GITATTRIBUTES_SQ = $(subst 
','\'',$(ETC_GITATTRIBUTES))
 DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
 bindir_SQ = $(subst ','\'',$(bindir))
 bindir_relative_SQ = $(subst ','\'',$(bindir_relative))
-mandir_SQ = $(subst ','\'',$(mandir))
-infodir_SQ = $(subst ','\'',$(infodir))
+mandir_relative_SQ = $(subst ','\'',$(mandir_relative))
+infodir_relative_SQ = $(subst ','\'',$(infodir_relative))
 localedir_SQ = $(subst ','\'',$(localedir))
 gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
 template_dir_SQ = $(subst ','\'',$(template_dir))
-htmldir_SQ = $(subst ','\'',$(htmldir))
+htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative))
 prefix_SQ = $(subst ','\'',$(prefix))
 gitwebdir_SQ = $(subst ','\'',$(gitwebdir))
 
@@ -1685,9 +1691,9 @@ strip: $(PROGRAMS) git$X
 
 git.sp git.s git.o: GIT-PREFIX
 git.sp git.s git.o: EXTRA_CPPFLAGS = \
-   '-DGIT_HTML_PATH=$(htmldir_SQ)' \
-   '-DGIT_MAN_PATH=$(mandir_SQ)' \
-   '-DGIT_INFO_PATH=$(infodir_SQ)'
+   '-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \
+   '-DGIT_MAN_PATH=$(mandir_relative_SQ)' \
+   '-DGIT_INFO_PATH=$(infodir_relative_SQ)'
 
 git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ git.o \
@@ -1697,9 +1703,9 @@ help.sp help.s help.o: common-cmds.h
 
 builtin/help.sp builtin/help.s builtin/help.o: common-cmds.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
-   '-DGIT_HTML_PATH=$(htmldir_SQ)' \
-   '-DGIT_MAN_PATH=$(mandir_SQ)' \
-   '-DGIT_INFO_PATH=$(infodir_SQ)'
+   '-DGIT_HTML_PATH=$(htmldir_relative_SQ)' \
+   '-DGIT_MAN_PATH=$(mandir_relative_SQ)' \
+   '-DGIT_INFO_PATH=$(infodir_relative_SQ)'
 
 version.sp version.s version.o: GIT-VERSION-FILE GIT-USER-AGENT
 version.sp version.s version.o: EXTRA_CPPFLAGS = \
-- 
1.8.2.rc0.248.g2dab8ff

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html