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

2013-02-12 Thread Junio C Hamano
John Keeping  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.
--
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


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

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 02:57:25PM -0800, Junio C Hamano wrote:
> Junio C Hamano  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 
> 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 

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


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

2013-02-12 Thread John Keeping
On Tue, Feb 12, 2013 at 02:45:34PM -0800, Junio C Hamano wrote:
> Jonathan Nieder  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= 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  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
--
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


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

2013-02-12 Thread Junio C Hamano
Junio C Hamano  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 
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 

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

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


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

2013-02-12 Thread Junio C Hamano
Jonathan Nieder  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= 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  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)?
--
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


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

2013-02-12 Thread 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
--
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


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

2013-02-12 Thread 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
--
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


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

2013-02-12 Thread Jonathan Nieder
John Keeping wrote:

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

This doesn't seem to have hit the list.

Thanks,
Jonathan
--
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