From: John Keeping <hidden> Date: 2016-06-15 22:56:08
When using the top-level install-doc target the html, info and man
target directories are inherited from the top-level Makefile by the
documentation Makefile as relative paths, which is not expected and
results in the files being installed in an unexpected location.
The first two patches are simple style fixes. The third one fixes the
issue described above.
John Keeping (3):
Documentation/Makefile: fix spaces around assignments
Documentation/Makefile: move infodir to be with other '*dir's
Documentation/Makefile: fix inherited {html,info,man}dir
Documentation/Makefile | 88 ++++++++++++++++++++++++++++++++------------------
1 file changed, 56 insertions(+), 32 deletions(-)
--
1.8.1.2
From: John Keeping <hidden> Date: 2016-06-15 22:56:08
Commit e14421b (Allow INSTALL, bindir, mandir to be set in main Makefile
- 2006-06-29) changed Documentation/Makefile to inherit the value of
mandir from the top-level Makefile when invoked as "make install-doc" at
the top-level. This was inherited by infodir and htmldir when they were
added.
This was broken by commit 026fa0d (Move computation of absolute paths
from Makefile to runtime (in preparation for RUNTIME_PREFIX) -
2009-01-18) which changed these variables to have relative paths in the
top-level Makefile, causing the documentation to be installed into the
path without $(prefix) prepended.
Fix this by changing the defaults to be paths relative to $(prefix) and
introducing new variables {html,info,man}_instdir which contain the full
installation paths.
Signed-off-by: John Keeping <redacted>
---
I'm not sure if this is the best approach - the alternative would be to
change the top-level Makefile to use {html,info,man}dir_relative and
derive the {html,info,man}dir variables from that.
The top-level Makefile is inconsistent in the approach it takes - bindir
is derived from bindir_relative but gitexecdir and template_dir have
gitexec_instdir and template_instdir derived from them.
Documentation/Makefile | 56 +++++++++++++++++++++++++++++++++++---------------
1 file changed, 40 insertions(+), 16 deletions(-)
@@ -78,15 +78,21 @@ DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT))DOC_MAN5=$(patsubst%.txt,%.5,$(MAN5_TXT))DOC_MAN7=$(patsubst%.txt,%.7,$(MAN7_TXT))+# The following variables can be relative paths due to the way they can be+# inherited from the top-level Makefile:+# htmldir+# infodir+# mandir+# Note that pdfdir is an exception to this since it is not used by git-help.prefix?=$(HOME)bindir?=$(prefix)/bin-htmldir?=$(prefix)/share/doc/git-doc-infodir?=$(prefix)/share/infopdfdir?=$(prefix)/share/doc/git-doc-mandir?=$(prefix)/share/man-man1dir=$(mandir)/man1-man5dir=$(mandir)/man5-man7dir=$(mandir)/man7+htmldir?=share/doc/git-doc+infodir?=share/info+mandir?=share/man+man1dir=$(man_instdir)/man1+man5dir=$(man_instdir)/man5+man7dir=$(man_instdir)/man7# DESTDIR =ASCIIDOC=asciidoc
@@ -110,6 +116,24 @@ endif-include ../config.mak.autogen-include ../config.mak+ifneq ($(filter /%,$(firstword $(htmldir))),)+html_instdir=$(htmldir)+else+html_instdir=$(prefix)/$(htmldir)+endif++ifneq ($(filter /%,$(firstword $(infodir))),)+info_instdir=$(infodir)+else+info_instdir=$(prefix)/$(infodir)+endif++ifneq ($(filter /%,$(firstword $(mandir))),)+man_instdir=$(mandir)+else+man_instdir=$(prefix)/$(mandir)+endif+## For docbook-xsl ...# -1.68.1, no extra settings are needed?
@@ -144,7 +168,7 @@ endif# Distros may want to use MAN_BASE_URL=file:///path/to/git/docs/# or similar.ifndef MAN_BASE_URL-MAN_BASE_URL=file://$(htmldir)/+MAN_BASE_URL=file://$(html_instdir)/endifXMLTO_EXTRA+=-mmanpage-base-url.xsl
@@ -220,13 +244,13 @@ install-man: man$(INSTALL)-m644$(DOC_MAN7)$(DESTDIR)$(man7dir)install-info:info-$(INSTALL)-d-m755$(DESTDIR)$(infodir)-$(INSTALL)-m644git.infogitman.info$(DESTDIR)$(infodir)- if test -r $(DESTDIR)$(infodir)/dir; then \-$(INSTALL_INFO)--info-dir=$(DESTDIR)$(infodir)git.info;\-$(INSTALL_INFO)--info-dir=$(DESTDIR)$(infodir)gitman.info;\+$(INSTALL)-d-m755$(DESTDIR)$(info_instdir)+$(INSTALL)-m644git.infogitman.info$(DESTDIR)$(info_instdir)+ if test -r $(DESTDIR)$(info_instdir)/dir; then \+$(INSTALL_INFO)--info-dir=$(DESTDIR)$(info_instdir)git.info;\+$(INSTALL_INFO)--info-dir=$(DESTDIR)$(info_instdir)gitman.info;\ else \-echo"No directory found in $(DESTDIR)$(infodir)">&2;\+echo"No directory found in $(DESTDIR)$(info_instdir)">&2;\fiinstall-pdf:pdf
@@ -402,14 +426,14 @@ require-manrepo::thenecho"git-manpages repository must exist at $(MAN_REPO)";exit1;fiquick-install-man:require-manrepo-'$(SHELL_PATH_SQ)'./install-doc-quick.sh$(MAN_REPO)$(DESTDIR)$(mandir)+'$(SHELL_PATH_SQ)'./install-doc-quick.sh$(MAN_REPO)$(DESTDIR)$(man_instdir)require-htmlrepo::@iftest!-d$(HTML_REPO);\thenecho"git-htmldocs repository must exist at $(HTML_REPO)";exit1;fiquick-install-html:require-htmlrepo-'$(SHELL_PATH_SQ)'./install-doc-quick.sh$(HTML_REPO)$(DESTDIR)$(htmldir)+'$(SHELL_PATH_SQ)'./install-doc-quick.sh$(HTML_REPO)$(DESTDIR)$(html_instdir)print-man1:@foriin$(MAN1_TXT);doecho$$i;done
From: Jonathan Nieder <hidden> Date: 2016-06-15 22:56:09
Hi,
John Keeping wrote:
[Subject: [PATCH 1/3] Documentation/Makefile: fix spaces around assignments]
It's not so much "fix spaces" as "use consistent spacing", no?
Aside from that nit, looks like a sensible no-op to me, so
Reviewed-by: Jonathan Nieder <redacted>
Thanks.