[PATCH 0/3] Fix installation paths with "make install-doc"

[PATCH 0/3] Fix installation paths with "make install-doc"

[John Keeping]

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

[PATCH 1/3] Documentation/Makefile: fix spaces around assignments

[John Keeping]

A simple style fix; no functional change.

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 Documentation/Makefile | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 62dbd9a..af3d8a4 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -31,11 +31,11 @@ MAN7_TXT += gittutorial.txt
 MAN7_TXT += gitworkflows.txt
 
 MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
-MAN_XML=$(patsubst %.txt,%.xml,$(MAN_TXT))
-MAN_HTML=$(patsubst %.txt,%.html,$(MAN_TXT))
+MAN_XML = $(patsubst %.txt,%.xml,$(MAN_TXT))
+MAN_HTML = $(patsubst %.txt,%.html,$(MAN_TXT))
 
 OBSOLETE_HTML = git-remote-helpers.html
-DOC_HTML=$(MAN_HTML) $(OBSOLETE_HTML)
+DOC_HTML = $(MAN_HTML) $(OBSOLETE_HTML)
 
 ARTICLES = howto-index
 ARTICLES += everyday
@@ -74,35 +74,35 @@ SP_ARTICLES += technical/api-index
 
 DOC_HTML += $(patsubst %,%.html,$(ARTICLES) $(SP_ARTICLES))
 
-DOC_MAN1=$(patsubst %.txt,%.1,$(MAN1_TXT))
-DOC_MAN5=$(patsubst %.txt,%.5,$(MAN5_TXT))
-DOC_MAN7=$(patsubst %.txt,%.7,$(MAN7_TXT))
+DOC_MAN1 = $(patsubst %.txt,%.1,$(MAN1_TXT))
+DOC_MAN5 = $(patsubst %.txt,%.5,$(MAN5_TXT))
+DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
 
-prefix?=$(HOME)
-bindir?=$(prefix)/bin
-htmldir?=$(prefix)/share/doc/git-doc
-pdfdir?=$(prefix)/share/doc/git-doc
-mandir?=$(prefix)/share/man
-man1dir=$(mandir)/man1
-man5dir=$(mandir)/man5
-man7dir=$(mandir)/man7
-# DESTDIR=
+prefix ?= $(HOME)
+bindir ?= $(prefix)/bin
+htmldir ?= $(prefix)/share/doc/git-doc
+pdfdir ?= $(prefix)/share/doc/git-doc
+mandir ?= $(prefix)/share/man
+man1dir = $(mandir)/man1
+man5dir = $(mandir)/man5
+man7dir = $(mandir)/man7
+# DESTDIR =
 
 ASCIIDOC = asciidoc
 ASCIIDOC_EXTRA =
 MANPAGE_XSL = manpage-normal.xsl
 XMLTO = xmlto
 XMLTO_EXTRA =
-INSTALL?=install
+INSTALL ?= install
 RM ?= rm -f
 MAN_REPO = ../../git-manpages
 HTML_REPO = ../../git-htmldocs
 
-infodir?=$(prefix)/share/info
-MAKEINFO=makeinfo
-INSTALL_INFO=install-info
-DOCBOOK2X_TEXI=docbook2x-texi
-DBLATEX=dblatex
+infodir ?= $(prefix)/share/info
+MAKEINFO = makeinfo
+INSTALL_INFO = install-info
+DOCBOOK2X_TEXI = docbook2x-texi
+DBLATEX = dblatex
 ifndef PERL_PATH
 	PERL_PATH = /usr/bin/perl
 endif
-- 
1.8.1.2

[PATCH 2/3] Documentation/Makefile: move infodir to be with other '*dir's

[John Keeping]

Signed-off-by: John Keeping <john@keeping.me.uk>
---
 Documentation/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index af3d8a4..0cfdc36 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
 prefix ?= $(HOME)
 bindir ?= $(prefix)/bin
 htmldir ?= $(prefix)/share/doc/git-doc
+infodir ?= $(prefix)/share/info
 pdfdir ?= $(prefix)/share/doc/git-doc
 mandir ?= $(prefix)/share/man
 man1dir = $(mandir)/man1
@@ -98,7 +99,6 @@ RM ?= rm -f
 MAN_REPO = ../../git-manpages
 HTML_REPO = ../../git-htmldocs
 
-infodir ?= $(prefix)/share/info
 MAKEINFO = makeinfo
 INSTALL_INFO = install-info
 DOCBOOK2X_TEXI = docbook2x-texi
-- 
1.8.1.2

[PATCH 3/3] Documentation/Makefile: fix inherited {html,info,man}dir

[John Keeping]

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 <john@keeping.me.uk>
---
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(-)

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 0cfdc36..34cd9f2 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -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/info
 pdfdir ?= $(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)/
 endif
 XMLTO_EXTRA += -m manpage-base-url.xsl
 
@@ -220,13 +244,13 @@ install-man: man
 	$(INSTALL) -m 644 $(DOC_MAN7) $(DESTDIR)$(man7dir)
 
 install-info: info
-	$(INSTALL) -d -m 755 $(DESTDIR)$(infodir)
-	$(INSTALL) -m 644 git.info gitman.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 -m 755 $(DESTDIR)$(info_instdir)
+	$(INSTALL) -m 644 git.info gitman.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 ; \
 	fi
 
 install-pdf: pdf
@@ -234,7 +258,7 @@ install-pdf: pdf
 	$(INSTALL) -m 644 user-manual.pdf $(DESTDIR)$(pdfdir)
 
 install-html: html
-	'$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(htmldir)
+	'$(SHELL_PATH_SQ)' ./install-webdoc.sh $(DESTDIR)$(html_instdir)
 
 ../GIT-VERSION-FILE: FORCE
 	$(QUIET_SUBDIR0)../ $(QUIET_SUBDIR1) GIT-VERSION-FILE
@@ -402,14 +426,14 @@ require-manrepo::
 	then echo "git-manpages repository must exist at $(MAN_REPO)"; exit 1; fi
 
 quick-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::
 	@if test ! -d $(HTML_REPO); \
 	then echo "git-htmldocs repository must exist at $(HTML_REPO)"; exit 1; fi
 
 quick-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:
 	@for i in $(MAN1_TXT); do echo $$i; done
-- 
1.8.1.2

Re: [PATCH 1/3] Documentation/Makefile: fix spaces around assignments

[Jonathan Nieder]

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 <jrnieder@gmail.com>

Thanks.

Re: [PATCH 2/3] Documentation/Makefile: move infodir to be with other '*dir's

[Jonathan Nieder]

John Keeping wrote:

> Signed-off-by: John Keeping <john@keeping.me.uk>
[...]
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
>  prefix ?= $(HOME)
>  bindir ?= $(prefix)/bin
>  htmldir ?= $(prefix)/share/doc/git-doc
> +infodir ?= $(prefix)/share/info
>  pdfdir ?= $(prefix)/share/doc/git-doc
>  mandir ?= $(prefix)/share/man
>  man1dir = $(mandir)/man1
> @@ -98,7 +99,6 @@ RM ?= rm -f
>  MAN_REPO = ../../git-manpages
>  HTML_REPO = ../../git-htmldocs
>  
> -infodir ?= $(prefix)/share/info
>  MAKEINFO = makeinfo

Is this another stylefix or is there a functional reason for this
change?

Re: [PATCH 2/3] Documentation/Makefile: move infodir to be with other '*dir's

[John Keeping]

On Tue, Feb 12, 2013 at 01:01:38PM -0800, Jonathan Nieder wrote:
> John Keeping wrote:
> 
> > Signed-off-by: John Keeping <john@keeping.me.uk>
> [...]
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -81,6 +81,7 @@ DOC_MAN7 = $(patsubst %.txt,%.7,$(MAN7_TXT))
> >  prefix ?= $(HOME)
> >  bindir ?= $(prefix)/bin
> >  htmldir ?= $(prefix)/share/doc/git-doc
> > +infodir ?= $(prefix)/share/info
> >  pdfdir ?= $(prefix)/share/doc/git-doc
> >  mandir ?= $(prefix)/share/man
> >  man1dir = $(mandir)/man1
> > @@ -98,7 +99,6 @@ RM ?= rm -f
> >  MAN_REPO = ../../git-manpages
> >  HTML_REPO = ../../git-htmldocs
> >  
> > -infodir ?= $(prefix)/share/info
> >  MAKEINFO = makeinfo
> 
> Is this another stylefix or is there a functional reason for this
> change?

Another stylefix - this arrangement seems more logical to me and makes
the comment in the next patch simpler.

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[Jonathan Nieder]

John Keeping wrote:

>   Documentation/Makefile: fix inherited {html,info,man}dir

This doesn't seem to have hit the list.

Thanks,
Jonathan

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[John Keeping]

On Tue, Feb 12, 2013 at 02:25:08PM -0800, Jonathan Nieder wrote:
> John Keeping wrote:
> 
> >   Documentation/Makefile: fix inherited {html,info,man}dir
> 
> This doesn't seem to have hit the list.

Hmm... it made it to gmane:

http://article.gmane.org/gmane.comp.version-control.git/216188

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[Jonathan Nieder]

John Keeping wrote:
> On Tue, Feb 12, 2013 at 02:25:08PM -0800, Jonathan Nieder wrote:
>> John Keeping wrote:

>>>   Documentation/Makefile: fix inherited {html,info,man}dir
>>
>> This doesn't seem to have hit the list.
>
> Hmm... it made it to gmane:
>
> http://article.gmane.org/gmane.comp.version-control.git/216188

So it did.  Sorry for the noise --- I'll grab it from there.

Sincerely,
Jonathan

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> John Keeping wrote:
>
>>   Documentation/Makefile: fix inherited {html,info,man}dir
>
> This doesn't seem to have hit the list.

More importantly, 

>> 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.

I am not sure what problem it is trying to address.  During every
cycle "make doc && make install-man install-html" is run for all
integration branches and it didn't cause any problems.

A wild guess.  John, are you using config.mak.autogen?

I _think_ exporting mandir/html/infodir from the top-level Makefile
is wrong to begin with.  We should drop the "export mandir" from
there.

Giving them unusual meaning (e.g. "mandir = share/man") is already
bad and that needs to be fixed by limiting this "oh, on some
platforms we compile-in GIT_MAN_PATH as a relative path to an
unspecified place" insanity only to where -DGIT_MAN_PATH=<path> is
defined.  The path used there does not help the building and
installation of the documentation at all, so the variable used for
the purpose of giving that <path> should not be named the same way
as the variable used on Documentation/Makefile to name the real path
in the first place.

Perhaps rename these to runtime_{man,html,info}dir or something and
make sure {man,html,info}dir are defined as the real paths whose
default values begin with $(prefix)?

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> I _think_ exporting mandir/html/infodir from the top-level Makefile
> is wrong to begin with.  We should drop the "export mandir" from
> there.

Ah, it is this thing, isn't it?

commit d8cf908cb6012cd4dc3d1089a849daf646150c2e
Author: Junio C Hamano <gitster@pobox.com>
Date:   Sat Feb 2 17:58:49 2013 -0800

    config.mak.in: remove unused definitions
    
    When 5566771 (autoconf: Use autoconf to write installation
    directories to config.mak.autogen, 2006-07-03) introduced support
    for autoconf generated config.mak file, it added an "export" for a
    few common makefile variables, in addition to definitions of srcdir
    and VPATH.
    
    The "export" logically does not belong there.  The make variables
    like mandir, prefix, etc, should be exported to submakes for people
    who use config.mak and people who use config.mak.autogen the same
    way; if we want to get these exported, that should be in the main
    Makefile.
    
    We do use mandir and htmldir in Documentation/Makefile, so let's
    add export for them in the main Makefile instead.
    
    We may eventually want to support VPATH, and srcdir may turn out to
    be useful for that purpose, but right now nobody uses it, so it is
    useless to define them in this file.
    
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

config.mak.in shouldn't have exported mandir in the first place, and
the commit made it worse by moving that broken export to the main
Makefile, and also added an export to htmldir as well, which was
totally wrong.

Let me revert that bit first.

I still think making "mandir" to have the real path in both the
top-level Makefile and Documentation/Makefile and renaming the
variable that is used to form the -DGIT_MAN_PATH=<path> to
optionally compile in a path relative to an unspecified location
that is discovered at runtime to something else is the sane thing to
do, but that is a separate issue, I think.

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[John Keeping]

On Tue, Feb 12, 2013 at 02:45:34PM -0800, Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
> 
> > John Keeping wrote:
> >
> >>   Documentation/Makefile: fix inherited {html,info,man}dir
> >
> > This doesn't seem to have hit the list.
> 
> More importantly, 
> 
> >> 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.
> 
> I am not sure what problem it is trying to address.  During every
> cycle "make doc && make install-man install-html" is run for all
> integration branches and it didn't cause any problems.
> 
> A wild guess.  John, are you using config.mak.autogen?

Close - plain config.mak.

I set $prefix there and ran "make install-doc".  That installed the man
pages in Documentation/share/man/ in my Git source directory.

> I _think_ exporting mandir/html/infodir from the top-level Makefile
> is wrong to begin with.  We should drop the "export mandir" from
> there.
> 
> Giving them unusual meaning (e.g. "mandir = share/man") is already
> bad and that needs to be fixed by limiting this "oh, on some
> platforms we compile-in GIT_MAN_PATH as a relative path to an
> unspecified place" insanity only to where -DGIT_MAN_PATH=<path> is
> defined.  The path used there does not help the building and
> installation of the documentation at all, so the variable used for
> the purpose of giving that <path> should not be named the same way
> as the variable used on Documentation/Makefile to name the real path
> in the first place.
> 
> Perhaps rename these to runtime_{man,html,info}dir or something and
> make sure {man,html,info}dir are defined as the real paths whose
> default values begin with $(prefix)?

Would it be sensible to define the values for these variables (with
absolute paths) in a separate top-level file like config.mak.uname
(defaults.mak maybe?) and include that in both Documentation/Makefile
and Makefile, then calculate the relative path from $(prefix) to
$({man,html,info}dir) for the compiled-in values.


John

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[John Keeping]

On Tue, Feb 12, 2013 at 02:57:25PM -0800, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I _think_ exporting mandir/html/infodir from the top-level Makefile
> > is wrong to begin with.  We should drop the "export mandir" from
> > there.
> 
> Ah, it is this thing, isn't it?
> 
> commit d8cf908cb6012cd4dc3d1089a849daf646150c2e
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Sat Feb 2 17:58:49 2013 -0800
> 
>     config.mak.in: remove unused definitions
>     
>     When 5566771 (autoconf: Use autoconf to write installation
>     directories to config.mak.autogen, 2006-07-03) introduced support
>     for autoconf generated config.mak file, it added an "export" for a
>     few common makefile variables, in addition to definitions of srcdir
>     and VPATH.
>     
>     The "export" logically does not belong there.  The make variables
>     like mandir, prefix, etc, should be exported to submakes for people
>     who use config.mak and people who use config.mak.autogen the same
>     way; if we want to get these exported, that should be in the main
>     Makefile.
>     
>     We do use mandir and htmldir in Documentation/Makefile, so let's
>     add export for them in the main Makefile instead.
>     
>     We may eventually want to support VPATH, and srcdir may turn out to
>     be useful for that purpose, but right now nobody uses it, so it is
>     useless to define them in this file.
>     
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>

Looks like it - I tried this for the first time today (with pu) so I
didn't realise it was a recent change, and I didn't think to blame the
export line.


John

[PATCH] Makefile: do not export mandir/htmldir/infodir

[Junio C Hamano]

These are defined in the main Makefile to be funny values that are
optionally relative to an unspecified location that is determined at
runtime.  They are only suitable for hardcoding in the binary via
the -DGIT_{MAN,HTML,INFO}_PATH=<value> C preprocessor options, and
are not real paths, contrary to what any sane person, and more
importantly, the Makefile in the documentation directory, would
expect.

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.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1dae2c5..26b697d 100644
--- a/Makefile
+++ b/Makefile
@@ -359,7 +359,7 @@ lib = lib
 # DESTDIR=
 pathsep = :
 
-export prefix bindir sharedir mandir htmldir sysconfdir gitwebdir localedir
+export prefix bindir sharedir sysconfdir gitwebdir localedir
 
 CC = cc
 AR = ar
-- 
1.8.1.3.675.g524d3b9

Re: [PATCH 0/3] Fix installation paths with "make install-doc"

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> Looks like it - I tried this for the first time today (with pu) so I
> didn't realise it was a recent change, and I didn't think to blame the
> export line.

Unfortunately that bogus change is already in 'next', but luckily we
caught it before it graduated to 'master' ;-)

I'll queue the regression fix patch (already sent separately); the
patch should bring everything in Makefile back to the same state as
in 'master', so in that sense it fixes the regression, but it does
not address the real cause of the confusion, solution to which is
hinted in the log message of the patch.

Thanks for catching this.

Re: [PATCH] Makefile: do not export mandir/htmldir/infodir

[Jonathan Nieder]

Junio C Hamano wrote:

> These are defined in the main Makefile to be funny values that are
> optionally relative to an unspecified location that is determined at
> runtime.
[...]
> 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.

Makes sense.

Reported-by: John Keeping <john@keeping.me.uk>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Re: [PATCH] Makefile: do not export mandir/htmldir/infodir

[John Keeping]

On Tue, Feb 12, 2013 at 03:16:51PM -0800, Jonathan Nieder wrote:
> Junio C Hamano wrote:
> 
> > These are defined in the main Makefile to be funny values that are
> > optionally relative to an unspecified location that is determined at
> > runtime.
> [...]
> > 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.
> 
> Makes sense.
> 
> Reported-by: John Keeping <john@keeping.me.uk>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Fixes my original problem, so FWIW:

Tested-by: John Keeping <john@keeping.me.uk>

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

[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 <gitster@pobox.com>
Signed-off-by: John Keeping <john@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

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

[Junio C Hamano]

John Keeping <john@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 <gitster@pobox.com>
> Signed-off-by: John Keeping <john@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 = \