[PATCH] Bugfix: undefined htmldir in config.mak.autogen

[PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Jiang Xin]

Html documents will be installed to root dir (/) no matter what prefix
is set, if run these commands before `make` and `make install-html`:

    $ make configure
    $ ./configure --prefix=<PREFIX>

After the installation, all the html documents will copy to rootdir (/),
and:

    $ git --html-path
    <PREFIX>

    $ git help -w something
    fatal: '<PREFIX>': not a documentation directory.

This is because the variable "htmldir" points to a undefined variable
"$(docdir)" in file "config.mak.autogen", which is generated by running
`./configure`. This bug comes from commit fc1c541 (Honor configure's
htmldir switch), since v1.8.1.3-537-g1d321.

Add the required two variables "PACKAGE_TARNAME" and "docdir" to file
"config.mak.in" will resolve this problem.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 config.mak.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.mak.in b/config.mak.in
index d7c49..fa02bd 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -8,6 +8,7 @@ LDFLAGS = @LDFLAGS@
 AR = @AR@
 TAR = @TAR@
 DIFF = @DIFF@
+PACKAGE_TARNAME = @PACKAGE_TARNAME@
 #INSTALL = @INSTALL@		# needs install-sh or install.sh in sources
 
 prefix = @prefix@
@@ -17,6 +18,7 @@ gitexecdir = @libexecdir@/git-core
 datarootdir = @datarootdir@
 template_dir = @datadir@/git-core/templates
 sysconfdir = @sysconfdir@
+docdir = @docdir@
 
 mandir = @mandir@
 htmldir = @htmldir@
-- 
1.8.2.rc0.18.g63af42f.dirty

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> Html documents will be installed to root dir (/) no matter what prefix
> is set, if run these commands before `make` and `make install-html`:
>
>     $ make configure
>     $ ./configure --prefix=<PREFIX>
>
> After the installation, all the html documents will copy to rootdir (/),
> and:
>
>     $ git --html-path
>     <PREFIX>
>
>     $ git help -w something
>     fatal: '<PREFIX>': not a documentation directory.
>
> This is because the variable "htmldir" points to a undefined variable
> "$(docdir)" in file "config.mak.autogen", which is generated by running
> `./configure`. This bug comes from commit fc1c541 (Honor configure's
> htmldir switch), since v1.8.1.3-537-g1d321.
>
> Add the required two variables "PACKAGE_TARNAME" and "docdir" to file
> "config.mak.in" will resolve this problem.
>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---

Who references PACKAGE_TARNAME and how is the symbol used?

>  config.mak.in | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/config.mak.in b/config.mak.in
> index d7c49..fa02bd 100644
> --- a/config.mak.in
> +++ b/config.mak.in
> @@ -8,6 +8,7 @@ LDFLAGS = @LDFLAGS@
>  AR = @AR@
>  TAR = @TAR@
>  DIFF = @DIFF@
> +PACKAGE_TARNAME = @PACKAGE_TARNAME@
>  #INSTALL = @INSTALL@		# needs install-sh or install.sh in sources
>  
>  prefix = @prefix@
> @@ -17,6 +18,7 @@ gitexecdir = @libexecdir@/git-core
>  datarootdir = @datarootdir@
>  template_dir = @datadir@/git-core/templates
>  sysconfdir = @sysconfdir@
> +docdir = @docdir@
>  
>  mandir = @mandir@
>  htmldir = @htmldir@

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Junio C Hamano]

Jiang Xin <worldhello.net@gmail.com> writes:

> Html documents will be installed to root dir (/) no matter what prefix
> is set, if run these commands before `make` and `make install-html`:
>
>     $ make configure
>     $ ./configure --prefix=<PREFIX>
>
> After the installation, all the html documents will copy to rootdir (/),
> and:
>
>     $ git --html-path
>     <PREFIX>
>
>     $ git help -w something
>     fatal: '<PREFIX>': not a documentation directory.

I am not sure if this description is correct.  The generated configure
seems to set

	datarootdir='${prefix}/share'
	htmldir='${docdir}'
        docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'

so it is likely you would get not <PREFIX> but <PREFIX>/share, no?

In the main Makefile, we set htmldir to "share/doc/git-doc" and that
is supposed to be relative to PREFIX, so the above will be wrong in
multiple ways (it is an absolute path with <PREFIX>/ in front, and
it ends not with share/doc/git-doc but with share/doc/git).

And the worst part is that having to know that the file needs to
export docdir and PACKAGE_TARNAME feels to me that we are tying
ourselves to too much detail in the internal implementation detail
of versions of autoconf we happen to have for testing this change.

I am inclined to suggest that we probably should

 * revert fc1c5415d69d (Honor configure's htmldir switch,
   2013-02-02); and

 * fix generated "./configure --help" not to suggest that --htmldir
   can be overriden from its command line;

instead of piling on a broken "fix" like this one top of it.

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Junio C Hamano]

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

> Jiang Xin <worldhello.net@gmail.com> writes:
>
>> Html documents will be installed to root dir (/) no matter what prefix
>> is set, if run these commands before `make` and `make install-html`:
>>
>>     $ make configure
>>     $ ./configure --prefix=<PREFIX>
>>
>> After the installation, all the html documents will copy to rootdir (/),
>> and:
>>
>>     $ git --html-path
>>     <PREFIX>
>>
>>     $ git help -w something
>>     fatal: '<PREFIX>': not a documentation directory.
>
> I am not sure if this description is correct.  The generated configure
> seems to set
>
> 	datarootdir='${prefix}/share'
> 	htmldir='${docdir}'
>       docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
>
> so it is likely you would get not <PREFIX> but <PREFIX>/share, no?

This was a mis-diag; without docdir mentioned in config.mak.in, we
do not even get that far, and htmldir will end up being empty, and
the runtime code adds <PREFIX> to it in system_path().  What I was
describing was what happens when you only mention @docdir@ but not
PACKAGE_TARNAME in the file.

> And the worst part is that having to know that the file needs to
> export docdir and PACKAGE_TARNAME feels to me that we are tying
> ourselves to too much detail in the internal implementation detail
> of versions of autoconf we happen to have for testing this change.

This still stands. It really feels wrong that this file has to be
aware of such an implementation detail of autoconf.  But as an
interim workaround, setting these two otherwise unused variables may
be the best we could do.

I am not sure if such a layout can be actually used for installing,
though.  Didn't we see some issues around the relativeness of
htmldir and mandir vs passing them down to Documentation/Makefile,
or is it not an issue when ./configure and config.mak.autogen is
used?

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Jiang Xin]

2013/2/20 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>>> After the installation, all the html documents will copy to rootdir (/),
>>> and:
>>>
>>>     $ git --html-path
>>>     <PREFIX>
>>>
>>>     $ git help -w something
>>>     fatal: '<PREFIX>': not a documentation directory.
>>
>> I am not sure if this description is correct.  The generated configure
>> seems to set
>>
>>       datarootdir='${prefix}/share'
>>       htmldir='${docdir}'
>>       docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
>>
>> so it is likely you would get not <PREFIX> but <PREFIX>/share, no?
>
> This was a mis-diag; without docdir mentioned in config.mak.in, we
> do not even get that far, and htmldir will end up being empty, and
> the runtime code adds <PREFIX> to it in system_path().  What I was
> describing was what happens when you only mention @docdir@ but not
> PACKAGE_TARNAME in the file.

I also doubt about it after sleep, so I check it again:

    ## gettext is installed in non-standard location on Mac
    $ export CFLAGS=-I/usr/local/include; export LDFLAGS=-L/usr/local/lib
    $ make config
    $ ./configure --prefix=/opt/git/v1.8.2
    $ make && sudo make install
    ## already symlink /opt/git/v1.8.2/bin/* to /usr/local/bin/
    $ git --html-path
    /opt/git/v1.8.2/
    $ git help -w help
    fatal: '/opt/git/v1.8.2/': not a documentation directory.

>> And the worst part is that having to know that the file needs to
>> export docdir and PACKAGE_TARNAME feels to me that we are tying
>> ourselves to too much detail in the internal implementation detail
>> of versions of autoconf we happen to have for testing this change.

I am not familiar with autoconf. After clone autoconf and check,
I cannot find a neat way to change "htmldir" default location to
use ${datarootdir} (just like mandir).
In file "lib/autoconf/general.m4", there are:

    AC_SUBST([docdir],         [m4_ifset([AC_PACKAGE_TARNAME],
                 ['${datarootdir}/doc/${PACKAGE_TARNAME}'],
                 ['${datarootdir}/doc/${PACKAGE}'])])dnl
    ...
    AC_SUBST([htmldir],        ['${docdir}'])dnl
    ...
    AC_SUBST([pdfdir],         ['${docdir}'])dnl
    ...
    AC_SUBST([mandir],         ['${datarootdir}/man'])dnl

> This still stands. It really feels wrong that this file has to be
> aware of such an implementation detail of autoconf.  But as an
> interim workaround, setting these two otherwise unused variables may
> be the best we could do.
>
> I am not sure if such a layout can be actually used for installing,
> though.  Didn't we see some issues around the relativeness of
> htmldir and mandir vs passing them down to Documentation/Makefile,
> or is it not an issue when ./configure and config.mak.autogen is
> used?


-- 
蒋鑫

北京群英汇信息技术有限公司
邮件: worldhello.net@gmail.com
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 010-51262007, 18601196889

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Stefano Lattarini]

On 02/20/2013 02:39 AM, Jiang Xin wrote:
>
> [SNIP]
> 
> I am not familiar with autoconf. After clone autoconf and check,
> I cannot find a neat way to change "htmldir" default location to
> use ${datarootdir} (just like mandir).
>
This one-line change should be enough to do what you want:

  diff --git a/configure.ac b/configure.ac
  index 1991258..2bfbec9 100644
  --- a/configure.ac
  +++ b/configure.ac
  @@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
   config_file=config.mak.autogen
   config_in=config.mak.in

  +AC_SUBST([htmldir], ['${datarootdir}'])
  +
   GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])

   # Directories holding "saner" versions of common or POSIX binaries.

Not sure whether this a good idea though (I haven't really followed the
discussion); but it is easily doable.

HTH,
  Stefano

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[John Keeping]

On Tue, Feb 19, 2013 at 03:40:16PM -0800, Junio C Hamano wrote:
> I am not sure if such a layout can be actually used for installing,
> though.  Didn't we see some issues around the relativeness of
> htmldir and mandir vs passing them down to Documentation/Makefile,
> or is it not an issue when ./configure and config.mak.autogen is
> used?

If these variables are set explicitly in config.mak.autogen (or indeed
config.mak) then these values should override the ones calculated in the
Makefiles so that we avoid that problem - the problem occurs if the
relative paths from the top-level Makefile are exported to
Documentation/Makefile.


John

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Jiang Xin]

 2013/2/20 Stefano Lattarini <stefano.lattarini@gmail.com>:
> On 02/20/2013 02:39 AM, Jiang Xin wrote:
>>
>> [SNIP]
>>
>> I am not familiar with autoconf. After clone autoconf and check,
>> I cannot find a neat way to change "htmldir" default location to
>> use ${datarootdir} (just like mandir).
>>
> This one-line change should be enough to do what you want:
>
>   diff --git a/configure.ac b/configure.ac
>   index 1991258..2bfbec9 100644
>   --- a/configure.ac
>   +++ b/configure.ac
>   @@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
>    config_file=config.mak.autogen
>    config_in=config.mak.in
>
>   +AC_SUBST([htmldir], ['${datarootdir}'])
>   +
>    GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])
>

If changed like that, set:

     AC_SUBST([htmldir], ['${datarootdir}/doc/git-doc'])

In the generated "configure" file, this instruction will be inserted
after the option_parse block (not before), and will override what
the user provided by running "./configure --htmldir=DOCDIR".

BTW, add "docdir = @docdir@" to "config.mak.in", also let
"./configure --docdir=DIR" works properly.

-- 
Jiang Xin

Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen

[Stefano Lattarini]

On 02/20/2013 11:42 AM, Jiang Xin wrote:
>  2013/2/20 Stefano Lattarini <stefano.lattarini@gmail.com>:
>> On 02/20/2013 02:39 AM, Jiang Xin wrote:
>>>
>>> [SNIP]
>>>
>>> I am not familiar with autoconf. After clone autoconf and check,
>>> I cannot find a neat way to change "htmldir" default location to
>>> use ${datarootdir} (just like mandir).
>>>
>> This one-line change should be enough to do what you want:
>>
>>   diff --git a/configure.ac b/configure.ac
>>   index 1991258..2bfbec9 100644
>>   --- a/configure.ac
>>   +++ b/configure.ac
>>   @@ -149,6 +149,8 @@ AC_CONFIG_SRCDIR([git.c])
>>    config_file=config.mak.autogen
>>    config_in=config.mak.in
>>
>>   +AC_SUBST([htmldir], ['${datarootdir}'])
>>   +
>>    GIT_CONF_SUBST([AUTOCONFIGURED], [YesPlease])
>>
> 
> If changed like that, set:
> 
>      AC_SUBST([htmldir], ['${datarootdir}/doc/git-doc'])
> 
> In the generated "configure" file, this instruction will be inserted
> after the option_parse block (not before), and will override what
> the user provided by running "./configure --htmldir=DOCDIR".
>
Yikes, you're right.  Scratch my suggestion then; the issue should
probably be brought up on the autoconf mailing list.  Albeit I think
it is by design that autoconf doesn't let a package to override the
defaults for installation directory: this way, the end users can
expect consistent, well-documented defaults for all autoconf-based
packages.


> BTW, add "docdir = @docdir@" to "config.mak.in", also let
> "./configure --docdir=DIR" works properly.
> 

Thanks, and sorry for the noise,
  Stefano