Patchwork [4,of,7] packages: remove support for documentation on target

login
register
mail settings
Submitter Thomas De Schampheleire
Date Feb. 5, 2014, 10:50 a.m.
Message ID <ab1e96c7872d5567b2a4.1391597410@argentina>
Download mbox | patch
Permalink /patch/316934/
State Superseded
Headers show

Comments

Thomas De Schampheleire - Feb. 5, 2014, 10:50 a.m.
This patch removes deprecated symbol BR2_HAVE_DOCUMENTATION and all its
usage. Additionally, it removes the now unused BR2_DEPRECATED_SINCE_2012_11.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
 Config.in                                              |  14 --------------
 Config.in.legacy                                       |   7 +++++++
 Makefile                                               |   2 --
 package/Makefile.in                                    |   2 --
 package/avahi/avahi.mk                                 |   2 +-
 package/berkeleydb/berkeleydb.mk                       |   4 ----
 package/enlightenment/enlightenment.mk                 |   2 --
 package/ffmpeg/ffmpeg.mk                               |   2 +-
 package/hdparm/hdparm.mk                               |   7 -------
 package/kmod/kmod.mk                                   |   5 +----
 package/libbsd/libbsd.mk                               |   6 ------
 package/logsurfer/logsurfer.mk                         |  13 -------------
 package/ncurses/ncurses.mk                             |   2 +-
 package/netsnmp/netsnmp.mk                             |   4 +---
 package/opencv/opencv.mk                               |   4 +---
 package/perl/perl.mk                                   |   4 ----
 package/pppd/pppd.mk                                   |  10 ----------
 package/pulseaudio/pulseaudio.mk                       |   2 +-
 package/python-pygame/python-pygame.mk                 |   2 --
 package/samba/samba.mk                                 |   2 --
 package/udisks/udisks.mk                               |   3 +--
 package/vala/vala.mk                                   |  10 ++--------
 package/vim/vim.mk                                     |   2 --
 package/x11r7/xproto_xcmiscproto/xproto_xcmiscproto.mk |   2 +-
 package/x11r7/xproto_xextproto/xproto_xextproto.mk     |   2 +-
 25 files changed, 19 insertions(+), 96 deletions(-)
Thomas Petazzoni - Feb. 5, 2014, 12:49 p.m.
Dear Thomas De Schampheleire,

On Wed, 05 Feb 2014 11:50:10 +0100, Thomas De Schampheleire wrote:

> diff --git a/package/Makefile.in b/package/Makefile.in
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -332,7 +332,6 @@ ifneq ($(BR2_LARGEFILE),y)
>  DISABLE_LARGEFILE= --disable-largefile
>  endif
>  
> -ifneq ($(BR2_HAVE_DOCUMENTATION),y)
>  # The configure option varies, but since unknown options are ignored
>  # we can pass all of them.
>  DISABLE_DOCUMENTATION = \
> @@ -342,7 +341,6 @@ DISABLE_DOCUMENTATION = \
>  	--disable-documentation \
>  	--with-xmlto=no \
>  	--with-fop=no
> -endif

So the DISABLE_DOCUMENTATION variable now always has the same value,
and is only used in pkg-autotools.mk in one place. So I believe it
makes sense to remove this variable altogether, and simply pass all the
appropriate --<something> options directly in the <pkg>_CONFIGURE_CMDS
of pkg-autotools.mk.


> diff --git a/package/kmod/kmod.mk b/package/kmod/kmod.mk
> --- a/package/kmod/kmod.mk
> +++ b/package/kmod/kmod.mk
> @@ -19,12 +19,9 @@ KMOD_LICENSE_FILES = libkmod/COPYING
>  # https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
>  KMOD_CONF_OPT = --disable-static --enable-shared
>  
> -# manpages not installed to host and needs xsltproc
> +# manpages not installed and needs xsltproc
>  HOST_KMOD_CONF_OPT = --disable-manpages
> -
> -ifneq ($(BR2_HAVE_DOCUMENTATION),y)
>  KMOD_CONF_OPT += --disable-manpages
> -endif

Then merge this KMOD_CONF_OPT += line with the existing KMOD_CONF_OPT
line above.

> diff --git a/package/netsnmp/netsnmp.mk b/package/netsnmp/netsnmp.mk
> --- a/package/netsnmp/netsnmp.mk
> +++ b/package/netsnmp/netsnmp.mk
> @@ -44,9 +44,7 @@ else
>  endif
>  
>  # Docs
> -ifneq ($(BR2_HAVE_DOCUMENTATION),y)
> -	NETSNMP_CONF_OPT += --disable-manuals
> -endif
> +NETSNMP_CONF_OPT += --disable-manuals

Ditto here. Now that it is no longer conditional, you can add
--disable-manuals to the already existing NETSNMP_CONF_OPT
unconditional definition.

Thomas
Thomas De Schampheleire - Feb. 5, 2014, 1:10 p.m.
Hi Thomas,

On Wed, Feb 5, 2014 at 1:49 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Wed, 05 Feb 2014 11:50:10 +0100, Thomas De Schampheleire wrote:
>
>> diff --git a/package/Makefile.in b/package/Makefile.in
>> --- a/package/Makefile.in
>> +++ b/package/Makefile.in
>> @@ -332,7 +332,6 @@ ifneq ($(BR2_LARGEFILE),y)
>>  DISABLE_LARGEFILE= --disable-largefile
>>  endif
>>
>> -ifneq ($(BR2_HAVE_DOCUMENTATION),y)
>>  # The configure option varies, but since unknown options are ignored
>>  # we can pass all of them.
>>  DISABLE_DOCUMENTATION = \
>> @@ -342,7 +341,6 @@ DISABLE_DOCUMENTATION = \
>>       --disable-documentation \
>>       --with-xmlto=no \
>>       --with-fop=no
>> -endif
>
> So the DISABLE_DOCUMENTATION variable now always has the same value,
> and is only used in pkg-autotools.mk in one place. So I believe it
> makes sense to remove this variable altogether, and simply pass all the
> appropriate --<something> options directly in the <pkg>_CONFIGURE_CMDS
> of pkg-autotools.mk.

OK.
By the way, I added on my own todo list that we should try to expand
the automatically passed options, for example:
--disable-manpages --disable-man-pages --disable-manuals etc. as some
of them are used in several packages...

>
>
>> diff --git a/package/kmod/kmod.mk b/package/kmod/kmod.mk
>> --- a/package/kmod/kmod.mk
>> +++ b/package/kmod/kmod.mk
>> @@ -19,12 +19,9 @@ KMOD_LICENSE_FILES = libkmod/COPYING
>>  # https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
>>  KMOD_CONF_OPT = --disable-static --enable-shared
>>
>> -# manpages not installed to host and needs xsltproc
>> +# manpages not installed and needs xsltproc
>>  HOST_KMOD_CONF_OPT = --disable-manpages
>> -
>> -ifneq ($(BR2_HAVE_DOCUMENTATION),y)
>>  KMOD_CONF_OPT += --disable-manpages
>> -endif
>
> Then merge this KMOD_CONF_OPT += line with the existing KMOD_CONF_OPT
> line above.

I did not do that because the --disable-static --enable-shared setting
is accompanied with a comment:
# static linking not supported, see
# https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
KMOD_CONF_OPT = --disable-static --enable-shared

and I felt that adding --disable-manpages here would be confusing.
With this extra info, what is your position?

>
>> diff --git a/package/netsnmp/netsnmp.mk b/package/netsnmp/netsnmp.mk
>> --- a/package/netsnmp/netsnmp.mk
>> +++ b/package/netsnmp/netsnmp.mk
>> @@ -44,9 +44,7 @@ else
>>  endif
>>
>>  # Docs
>> -ifneq ($(BR2_HAVE_DOCUMENTATION),y)
>> -     NETSNMP_CONF_OPT += --disable-manuals
>> -endif
>> +NETSNMP_CONF_OPT += --disable-manuals
>
> Ditto here. Now that it is no longer conditional, you can add
> --disable-manuals to the already existing NETSNMP_CONF_OPT
> unconditional definition.

Here I agree, I will change it.

Best regards,
Thomas
Thomas Petazzoni - Feb. 5, 2014, 1:16 p.m.
Dear Thomas De Schampheleire,

On Wed, 5 Feb 2014 14:10:09 +0100, Thomas De Schampheleire wrote:

> > So the DISABLE_DOCUMENTATION variable now always has the same value,
> > and is only used in pkg-autotools.mk in one place. So I believe it
> > makes sense to remove this variable altogether, and simply pass all the
> > appropriate --<something> options directly in the <pkg>_CONFIGURE_CMDS
> > of pkg-autotools.mk.
> 
> OK.
> By the way, I added on my own todo list that we should try to expand
> the automatically passed options, for example:
> --disable-manpages --disable-man-pages --disable-manuals etc. as some
> of them are used in several packages...

I have a mixed feeling about this. The problem is that those options
are not standard, so by far not all configure scripts understand them.
Which means that when a given configure script is called, you often
have a warning:

WARNING: options --disable-foo --enable-bar unsupported

If you have gazillions of options that are unsupported, then you may
very well miss an option passed by your .mk file on which you made a
typo, for example.

So having a long list of options to disable documentation is not
something that I personally like that much, but I believe Arnout does
not share the same opinion :)

> > Then merge this KMOD_CONF_OPT += line with the existing KMOD_CONF_OPT
> > line above.
> 
> I did not do that because the --disable-static --enable-shared setting
> is accompanied with a comment:
> # static linking not supported, see
> # https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
> KMOD_CONF_OPT = --disable-static --enable-shared
> 
> and I felt that adding --disable-manpages here would be confusing.
> With this extra info, what is your position?

Ah, ok. In that case then it would be more natural maybe to have first
the mandatory normal options, and then this special shared/static
thing. Like:

KMOD_CONF_OPT = --disable-manpages

# blabla static blabla shared
KMOD_CONF_OPT += --disable-static --enable-shared

All these comments are cosmetic comment, so I'm in favor of merging
your patch set as is, and do followup improvements as needed.

Best regards,

Thomas
Thomas De Schampheleire - Feb. 5, 2014, 1:28 p.m.
Hi Thomas,

On Wed, Feb 5, 2014 at 2:16 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
[..]
>
>> > Then merge this KMOD_CONF_OPT += line with the existing KMOD_CONF_OPT
>> > line above.
>>
>> I did not do that because the --disable-static --enable-shared setting
>> is accompanied with a comment:
>> # static linking not supported, see
>> # https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
>> KMOD_CONF_OPT = --disable-static --enable-shared
>>
>> and I felt that adding --disable-manpages here would be confusing.
>> With this extra info, what is your position?
>
> Ah, ok. In that case then it would be more natural maybe to have first
> the mandatory normal options, and then this special shared/static
> thing. Like:
>
> KMOD_CONF_OPT = --disable-manpages
>
> # blabla static blabla shared
> KMOD_CONF_OPT += --disable-static --enable-shared
>

Looking at this file, I don't know if your suggestion makes sense.
Here is part of the original file:

-----------------------------------
KMOD_VERSION = 16
KMOD_SOURCE = kmod-$(KMOD_VERSION).tar.xz
KMOD_SITE = $(BR2_KERNEL_MIRROR)/linux/utils/kernel/kmod/
KMOD_INSTALL_STAGING = YES
KMOD_DEPENDENCIES = host-pkgconf
HOST_KMOD_DEPENDENCIES = host-pkgconf

# license info for libkmod only, conditionally add more below
KMOD_LICENSE = LGPLv2.1+
KMOD_LICENSE_FILES = libkmod/COPYING

# static linking not supported, see
# https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
KMOD_CONF_OPT = --disable-static --enable-shared

# manpages not installed and needs xsltproc
HOST_KMOD_CONF_OPT = --disable-manpages
KMOD_CONF_OPT += --disable-manpages

ifeq ($(BR2_PACKAGE_ZLIB),y)
KMOD_DEPENDENCIES += zlib
KMOD_CONF_OPT += --with-zlib
endif

ifeq ($(BR2_PACKAGE_XZ),y)
KMOD_DEPENDENCIES += xz
KMOD_CONF_OPT += --with-xz
endif
------------------------------------

I can certainly put the two manpage-related lines above the static
linking, but this is just a move in the file, right?

Thanks,
Thomas
Thomas Petazzoni - Feb. 5, 2014, 1:34 p.m.
Dear Thomas De Schampheleire,

On Wed, 5 Feb 2014 14:28:48 +0100, Thomas De Schampheleire wrote:

> Looking at this file, I don't know if your suggestion makes sense.
> Here is part of the original file:
> 
> -----------------------------------
> KMOD_VERSION = 16
> KMOD_SOURCE = kmod-$(KMOD_VERSION).tar.xz
> KMOD_SITE = $(BR2_KERNEL_MIRROR)/linux/utils/kernel/kmod/
> KMOD_INSTALL_STAGING = YES
> KMOD_DEPENDENCIES = host-pkgconf
> HOST_KMOD_DEPENDENCIES = host-pkgconf
> 
> # license info for libkmod only, conditionally add more below
> KMOD_LICENSE = LGPLv2.1+
> KMOD_LICENSE_FILES = libkmod/COPYING
> 
> # static linking not supported, see
> # https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
> KMOD_CONF_OPT = --disable-static --enable-shared
> 
> # manpages not installed and needs xsltproc
> HOST_KMOD_CONF_OPT = --disable-manpages
> KMOD_CONF_OPT += --disable-manpages
> 
> ifeq ($(BR2_PACKAGE_ZLIB),y)
> KMOD_DEPENDENCIES += zlib
> KMOD_CONF_OPT += --with-zlib
> endif
> 
> ifeq ($(BR2_PACKAGE_XZ),y)
> KMOD_DEPENDENCIES += xz
> KMOD_CONF_OPT += --with-xz
> endif
> ------------------------------------
> 
> I can certainly put the two manpage-related lines above the static
> linking, but this is just a move in the file, right?

You're right, I agree my suggestion doesn't make much sense. Just keep
your patch as it was, it is good.

Sorry for the noise :/

Thomas
Arnout Vandecappelle - Feb. 5, 2014, 9:21 p.m.
On 05/02/14 14:16, Thomas Petazzoni wrote:
> Dear Thomas De Schampheleire,
> 
> On Wed, 5 Feb 2014 14:10:09 +0100, Thomas De Schampheleire wrote:
> 
>>> > > So the DISABLE_DOCUMENTATION variable now always has the same value,
>>> > > and is only used in pkg-autotools.mk in one place. So I believe it
>>> > > makes sense to remove this variable altogether, and simply pass all the
>>> > > appropriate --<something> options directly in the <pkg>_CONFIGURE_CMDS
>>> > > of pkg-autotools.mk.
>> > 
>> > OK.
>> > By the way, I added on my own todo list that we should try to expand
>> > the automatically passed options, for example:
>> > --disable-manpages --disable-man-pages --disable-manuals etc. as some
>> > of them are used in several packages...
> I have a mixed feeling about this. The problem is that those options
> are not standard, so by far not all configure scripts understand them.
> Which means that when a given configure script is called, you often
> have a warning:
> 
> WARNING: options --disable-foo --enable-bar unsupported
> 
> If you have gazillions of options that are unsupported, then you may
> very well miss an option passed by your .mk file on which you made a
> typo, for example.
> 
> So having a long list of options to disable documentation is not
> something that I personally like that much, but I believe Arnout does
> not share the same opinion :)

 I have a mixed opinion about this...

 I do think that the configure options should ideally be as exact as
possible. However, documentation is something that is wont to cause hard
to track build failures, e.g. trying to run system asciidoc with host
python, or system makeinfo with host-perl. So if we remove it from the
defaults, then we should make sure that _all_ autotools packages disable
their documentation.

 Hm, now I think about that, it would actually be a good idea to do
that... We currently still miss some of these cases because we assume
--disable-documentation works, but if we make a habit of paying attention
to it, then there is less risk of this happening.


 Regards,
 Arnout


[snip]
Thomas Petazzoni - Feb. 6, 2014, 8:37 a.m.
Dear Arnout Vandecappelle,

On Wed, 05 Feb 2014 22:21:30 +0100, Arnout Vandecappelle wrote:

> > WARNING: options --disable-foo --enable-bar unsupported
> > 
> > If you have gazillions of options that are unsupported, then you may
> > very well miss an option passed by your .mk file on which you made a
> > typo, for example.
> > 
> > So having a long list of options to disable documentation is not
> > something that I personally like that much, but I believe Arnout does
> > not share the same opinion :)
> 
>  I have a mixed opinion about this...
> 
>  I do think that the configure options should ideally be as exact as
> possible. However, documentation is something that is wont to cause hard
> to track build failures

Not sure what you meant here. Will those build failures be hard to
track, or *not* hard to track ?

Remember that the Free Electrons autobuilder builds run in a chroot
that has only the minimal hard dependencies required by Buildroot. This
typically allows us to catch invisible dependencies on asciidoc or
other documentation utilities.

>, e.g. trying to run system asciidoc with host
> python, or system makeinfo with host-perl. So if we remove it from the
> defaults, then we should make sure that _all_ autotools packages disable
> their documentation.

Indeed.

>  Hm, now I think about that, it would actually be a good idea to do
> that... We currently still miss some of these cases because we assume
> --disable-documentation works, but if we make a habit of paying attention
> to it, then there is less risk of this happening.

So your conclusion is that instead of passing those options globally,
we should pass them in a per-package basis, so that we remember to
always verify that the documentation disabling has been done?

Best regards,

Thomas
Arnout Vandecappelle - Feb. 6, 2014, 10:26 a.m.
On 06/02/14 09:37, Thomas Petazzoni wrote:
> Dear Arnout Vandecappelle,
> 
> On Wed, 05 Feb 2014 22:21:30 +0100, Arnout Vandecappelle wrote:
> 
>>> WARNING: options --disable-foo --enable-bar unsupported
>>>
>>> If you have gazillions of options that are unsupported, then you may
>>> very well miss an option passed by your .mk file on which you made a
>>> typo, for example.
>>>
>>> So having a long list of options to disable documentation is not
>>> something that I personally like that much, but I believe Arnout does
>>> not share the same opinion :)
>>
>>  I have a mixed opinion about this...
>>
>>  I do think that the configure options should ideally be as exact as
>> possible. However, documentation is something that is wont to cause hard
>> to track build failures
> 
> Not sure what you meant here. Will those build failures be hard to
> track, or *not* hard to track ?
> 
> Remember that the Free Electrons autobuilder builds run in a chroot
> that has only the minimal hard dependencies required by Buildroot. This
> typically allows us to catch invisible dependencies on asciidoc or
> other documentation utilities.

 Yes, and this is exactly the problem. On the autobuilders, autotools
will detect that asciidoc is missing and will not build documentation. On
a user's machine, however, autotools will detect that asciidoc is present
and use it, but then /usr/bin/asciidoc will try to use host-python
instead of /usr/bin/python and fails because of some missing modules.

> 
>> , e.g. trying to run system asciidoc with host
>> python, or system makeinfo with host-perl. So if we remove it from the
>> defaults, then we should make sure that _all_ autotools packages disable
>> their documentation.
> 
> Indeed.
> 
>>  Hm, now I think about that, it would actually be a good idea to do
>> that... We currently still miss some of these cases because we assume
>> --disable-documentation works, but if we make a habit of paying attention
>> to it, then there is less risk of this happening.
> 
> So your conclusion is that instead of passing those options globally,
> we should pass them in a per-package basis, so that we remember to
> always verify that the documentation disabling has been done?

 Exactly.

 But of course that's a large change, since all 846 autotools packages
would have to be checked...

 So let's start with not adding anything to the global CONF_OPT :-)

 Regards,
 Arnout

> 
> Best regards,
> 
> Thomas
>
Thomas De Schampheleire - Feb. 6, 2014, 11:34 a.m.
On Thu, Feb 6, 2014 at 11:26 AM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 06/02/14 09:37, Thomas Petazzoni wrote:
>> Dear Arnout Vandecappelle,
>>
>> On Wed, 05 Feb 2014 22:21:30 +0100, Arnout Vandecappelle wrote:
>>
>>>> WARNING: options --disable-foo --enable-bar unsupported
>>>>
>>>> If you have gazillions of options that are unsupported, then you may
>>>> very well miss an option passed by your .mk file on which you made a
>>>> typo, for example.
>>>>
>>>> So having a long list of options to disable documentation is not
>>>> something that I personally like that much, but I believe Arnout does
>>>> not share the same opinion :)
>>>
>>>  I have a mixed opinion about this...
>>>
>>>  I do think that the configure options should ideally be as exact as
>>> possible. However, documentation is something that is wont to cause hard
>>> to track build failures
>>
>> Not sure what you meant here. Will those build failures be hard to
>> track, or *not* hard to track ?
>>
>> Remember that the Free Electrons autobuilder builds run in a chroot
>> that has only the minimal hard dependencies required by Buildroot. This
>> typically allows us to catch invisible dependencies on asciidoc or
>> other documentation utilities.
>
>  Yes, and this is exactly the problem. On the autobuilders, autotools
> will detect that asciidoc is missing and will not build documentation. On
> a user's machine, however, autotools will detect that asciidoc is present
> and use it, but then /usr/bin/asciidoc will try to use host-python
> instead of /usr/bin/python and fails because of some missing modules.
>
>>
>>> , e.g. trying to run system asciidoc with host
>>> python, or system makeinfo with host-perl. So if we remove it from the
>>> defaults, then we should make sure that _all_ autotools packages disable
>>> their documentation.
>>
>> Indeed.
>>
>>>  Hm, now I think about that, it would actually be a good idea to do
>>> that... We currently still miss some of these cases because we assume
>>> --disable-documentation works, but if we make a habit of paying attention
>>> to it, then there is less risk of this happening.
>>
>> So your conclusion is that instead of passing those options globally,
>> we should pass them in a per-package basis, so that we remember to
>> always verify that the documentation disabling has been done?
>
>  Exactly.
>
>  But of course that's a large change, since all 846 autotools packages
> would have to be checked...
>
>  So let's start with not adding anything to the global CONF_OPT :-)
>

With this conclusion, I'd say we mark the last patch as Rejected (the
one removing the separate DISABLE_DOCUMENTATION variable).

What about the other patches? Good to proceed?

Thanks,
Thomas
Thomas Petazzoni - Feb. 6, 2014, 11:40 a.m.
Dear Thomas De Schampheleire,

On Thu, 6 Feb 2014 12:34:00 +0100, Thomas De Schampheleire wrote:

> >  But of course that's a large change, since all 846 autotools packages
> > would have to be checked...
> >
> >  So let's start with not adding anything to the global CONF_OPT :-)
> 
> With this conclusion, I'd say we mark the last patch as Rejected (the
> one removing the separate DISABLE_DOCUMENTATION variable).

I don't really come to the same conclusion. Arnout said to not add more
things to the global CONF_OPT.

The existing options in DISABLE_DOCUMENTATION are already part of the
CONF_OPT, so having a small refactoring that removes
DISABLE_DOCUMENTATION and makes it part of the global CONF_OPT doesn't
change anything in terms of removing those global options and moving
them to a per-package solution.

Thomas
Thomas De Schampheleire - Feb. 6, 2014, 12:15 p.m.
On Thu, Feb 6, 2014 at 12:40 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Thu, 6 Feb 2014 12:34:00 +0100, Thomas De Schampheleire wrote:
>
>> >  But of course that's a large change, since all 846 autotools packages
>> > would have to be checked...
>> >
>> >  So let's start with not adding anything to the global CONF_OPT :-)
>>
>> With this conclusion, I'd say we mark the last patch as Rejected (the
>> one removing the separate DISABLE_DOCUMENTATION variable).
>
> I don't really come to the same conclusion. Arnout said to not add more
> things to the global CONF_OPT.
>
> The existing options in DISABLE_DOCUMENTATION are already part of the
> CONF_OPT, so having a small refactoring that removes
> DISABLE_DOCUMENTATION and makes it part of the global CONF_OPT doesn't
> change anything in terms of removing those global options and moving
> them to a per-package solution.

My reasoning was that if we are going to migrate the global options to
per-package options, then it is not needed to change anything about
this now. Maybe this is too swift a conclusion...

I have no very strong opinion about the last patch, so feel free to
apply or reject it.

Best regards,
Thomas
Arnout Vandecappelle - Feb. 6, 2014, 1:33 p.m.
On 06/02/14 12:40, Thomas Petazzoni wrote:
> Dear Thomas De Schampheleire,
> 
> On Thu, 6 Feb 2014 12:34:00 +0100, Thomas De Schampheleire wrote:
> 
>>>  But of course that's a large change, since all 846 autotools packages
>>> would have to be checked...
>>>
>>>  So let's start with not adding anything to the global CONF_OPT :-)
>>
>> With this conclusion, I'd say we mark the last patch as Rejected (the
>> one removing the separate DISABLE_DOCUMENTATION variable).
> 
> I don't really come to the same conclusion. Arnout said to not add more
> things to the global CONF_OPT.
> 
> The existing options in DISABLE_DOCUMENTATION are already part of the
> CONF_OPT, so having a small refactoring that removes
> DISABLE_DOCUMENTATION and makes it part of the global CONF_OPT doesn't
> change anything in terms of removing those global options and moving
> them to a per-package solution.

 Exactly.

 Regards,
 Arnout
Peter Korsgaard - Feb. 6, 2014, 10:19 p.m.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > I don't really come to the same conclusion. Arnout said to not add more
 > things to the global CONF_OPT.

 > The existing options in DISABLE_DOCUMENTATION are already part of the
 > CONF_OPT, so having a small refactoring that removes
 > DISABLE_DOCUMENTATION and makes it part of the global CONF_OPT doesn't
 > change anything in terms of removing those global options and moving
 > them to a per-package solution.

Agreed.

Patch

diff --git a/Config.in b/Config.in
--- a/Config.in
+++ b/Config.in
@@ -264,10 +264,6 @@  config BR2_DEPRECATED
 
 if BR2_DEPRECATED
 
-config BR2_DEPRECATED_SINCE_2012_11
-	bool
-	default y
-
 config BR2_DEPRECATED_SINCE_2013_02
 	bool
 	default y
@@ -476,16 +472,6 @@  config BR2_PREFER_STATIC_LIB
 
 	  WARNING: This is highly experimental at the moment.
 
-config BR2_HAVE_DOCUMENTATION
-	bool "documentation on the target"
-	# We no longer want to support a toolchain on the target
-	depends on BR2_DEPRECATED_SINCE_2012_11
-	help
-	  Install the documentation, including manual pages and info
-	  pages, on the target.
-	  If you say n here, your target will not contain any
-	  documentation.
-
 config BR2_PACKAGE_OVERRIDE_FILE
 	string "location of a package override file"
 	default "$(TOPDIR)/local.mk"
diff --git a/Config.in.legacy b/Config.in.legacy
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -101,6 +101,13 @@  endif
 ###############################################################################
 comment "Legacy options removed in 2014.02"
 
+config BR2_HAVE_DOCUMENTATION
+	bool "support for documentation on target has been removed"
+	select BR2_LEGACY
+	help
+	  Support for documentation on target has been removed since it has
+	  been deprecated for more than four buildroot releases.
+
 config BR2_PACKAGE_AUTOMAKE
 	bool "automake target package has been removed"
 	select BR2_LEGACY
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -500,13 +500,11 @@  target-finalize:
 ifneq ($(BR2_PACKAGE_GDB),y)
 	rm -rf $(TARGET_DIR)/usr/share/gdb
 endif
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
 	rm -rf $(TARGET_DIR)/usr/man $(TARGET_DIR)/usr/share/man
 	rm -rf $(TARGET_DIR)/usr/info $(TARGET_DIR)/usr/share/info
 	rm -rf $(TARGET_DIR)/usr/doc $(TARGET_DIR)/usr/share/doc
 	rm -rf $(TARGET_DIR)/usr/share/gtk-doc
 	-rmdir $(TARGET_DIR)/usr/share 2>/dev/null
-endif
 ifeq ($(BR2_PACKAGE_PYTHON_PY_ONLY),y)
 	find $(TARGET_DIR)/usr/lib/ -name '*.pyc' -print0 | xargs -0 rm -f
 endif
diff --git a/package/Makefile.in b/package/Makefile.in
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -332,7 +332,6 @@  ifneq ($(BR2_LARGEFILE),y)
 DISABLE_LARGEFILE= --disable-largefile
 endif
 
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
 # The configure option varies, but since unknown options are ignored
 # we can pass all of them.
 DISABLE_DOCUMENTATION = \
@@ -342,7 +341,6 @@  DISABLE_DOCUMENTATION = \
 	--disable-documentation \
 	--with-xmlto=no \
 	--with-fop=no
-endif
 
 ifeq ($(BR2_INET_IPV6),y)
 DISABLE_IPV6= --enable-ipv6
diff --git a/package/avahi/avahi.mk b/package/avahi/avahi.mk
--- a/package/avahi/avahi.mk
+++ b/package/avahi/avahi.mk
@@ -78,7 +78,7 @@  AVAHI_CONF_OPT = --localstatedir=/var \
 		--disable-monodoc \
 		--disable-stack-protector \
 		--with-distro=none \
-		$(if $(BR2_HAVE_DOCUMENTATION),--enable,--disable)-manpages \
+		--disable-manpages \
 		$(if $(BR2_PACKAGE_AVAHI_AUTOIPD),--enable,--disable)-autoipd \
 		--with-avahi-user=default \
 		--with-avahi-group=default \
diff --git a/package/berkeleydb/berkeleydb.mk b/package/berkeleydb/berkeleydb.mk
--- a/package/berkeleydb/berkeleydb.mk
+++ b/package/berkeleydb/berkeleydb.mk
@@ -54,14 +54,10 @@  BERKELEYDB_POST_INSTALL_TARGET_HOOKS += 
 
 endif
 
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
-
 define BERKELEYDB_REMOVE_DOCS
 	rm -rf $(TARGET_DIR)/usr/docs
 endef
 
 BERKELEYDB_POST_INSTALL_TARGET_HOOKS += BERKELEYDB_REMOVE_DOCS
 
-endif
-
 $(eval $(autotools-package))
diff --git a/package/enlightenment/enlightenment.mk b/package/enlightenment/enlightenment.mk
--- a/package/enlightenment/enlightenment.mk
+++ b/package/enlightenment/enlightenment.mk
@@ -35,13 +35,11 @@  else
 ENLIGHTENMENT_CONF_ENV += enable_alsa=no
 endif
 
-ifeq ($(BR2_HAVE_DOCUMENTATION),)
 define ENLIGHTENMENT_REMOVE_DOCUMENTATION
 	rm -rf $(TARGET_DIR)/usr/share/enlightenment/doc/
 	rm -f $(TARGET_DIR)/usr/share/enlightenment/COPYING
 	rm -f $(TARGET_DIR)/usr/share/enlightenment/AUTHORS
 endef
 ENLIGHTENMENT_POST_INSTALL_TARGET_HOOKS += ENLIGHTENMENT_REMOVE_DOCUMENTATION
-endif
 
 $(eval $(autotools-package))
diff --git a/package/ffmpeg/ffmpeg.mk b/package/ffmpeg/ffmpeg.mk
--- a/package/ffmpeg/ffmpeg.mk
+++ b/package/ffmpeg/ffmpeg.mk
@@ -69,7 +69,7 @@  FFMPEG_CONF_OPT = \
 	--disable-vis \
 	--disable-sram \
 	--disable-symver \
-	$(if $(BR2_HAVE_DOCUMENTATION),,--disable-doc)
+	--disable-doc
 
 FFMPEG_DEPENDENCIES += $(if $(BR2_PACKAGE_LIBICONV),libiconv)
 
diff --git a/package/hdparm/hdparm.mk b/package/hdparm/hdparm.mk
--- a/package/hdparm/hdparm.mk
+++ b/package/hdparm/hdparm.mk
@@ -15,15 +15,8 @@  define HDPARM_BUILD_CMDS
 		LDFLAGS="$(TARGET_LDFLAGS)"
 endef
 
-ifeq ($(BR2_HAVE_DOCUMENTATION),y)
-define HDPARM_INSTALL_DOCUMENTATION
-	$(INSTALL) -D $(@D)/hdparm.8 $(TARGET_DIR)/usr/share/man/man8/hdparm.8
-endef
-endif
-
 define HDPARM_INSTALL_TARGET_CMDS
 	$(INSTALL) -D -m 0755 $(@D)/hdparm $(TARGET_DIR)/sbin/hdparm
-	$(HDPARM_INSTALL_DOCUMENTATION)
 endef
 
 $(eval $(generic-package))
diff --git a/package/kmod/kmod.mk b/package/kmod/kmod.mk
--- a/package/kmod/kmod.mk
+++ b/package/kmod/kmod.mk
@@ -19,12 +19,9 @@  KMOD_LICENSE_FILES = libkmod/COPYING
 # https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=b7016153ec8
 KMOD_CONF_OPT = --disable-static --enable-shared
 
-# manpages not installed to host and needs xsltproc
+# manpages not installed and needs xsltproc
 HOST_KMOD_CONF_OPT = --disable-manpages
-
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
 KMOD_CONF_OPT += --disable-manpages
-endif
 
 ifeq ($(BR2_PACKAGE_ZLIB),y)
 KMOD_DEPENDENCIES += zlib
diff --git a/package/libbsd/libbsd.mk b/package/libbsd/libbsd.mk
--- a/package/libbsd/libbsd.mk
+++ b/package/libbsd/libbsd.mk
@@ -9,12 +9,6 @@  LIBBSD_SITE            = http://libbsd.f
 LIBBSD_LICENSE         = BSD-3c MIT
 LIBBSD_LICENSE_FILES   = COPYING
 
-# man-pages are BSD-4c, so that license only matters
-# if doc is kept in the target rootfs
-ifeq ($(BR2_HAVE_DOCUMENTATION),y)
-LIBBSD_LICENSE        += (libraries), BSD-4c (documentation)
-endif
-
 LIBBSD_INSTALL_STAGING = YES
 
 $(eval $(autotools-package))
diff --git a/package/logsurfer/logsurfer.mk b/package/logsurfer/logsurfer.mk
--- a/package/logsurfer/logsurfer.mk
+++ b/package/logsurfer/logsurfer.mk
@@ -12,17 +12,4 @@  define LOGSURFER_INSTALL_TARGET_CMDS
 		$(TARGET_DIR)/usr/bin/logsurfer
 endef
 
-ifeq ($(BR2_HAVE_DOCUMENTATION),y)
-
-define LOGSURFER_INSTALL_TARGET_MAN
-	$(INSTALL) -D -m 0644 $(@D)/man/logsurfer.1 \
-		$(TARGET_DIR)/usr/man/man1/logsurfer.1
-	$(INSTALL) -D -m 0644 $(@D)/man/logsurfer.conf.4 \
-		$(TARGET_DIR)/usr/man/man4/logsurfer.conf.4
-endef
-
-LOGSURFER_POST_INSTALL_TARGET_HOOKS += LOGSURFER_INSTALL_TARGET_MAN
-
-endif
-
 $(eval $(autotools-package))
diff --git a/package/ncurses/ncurses.mk b/package/ncurses/ncurses.mk
--- a/package/ncurses/ncurses.mk
+++ b/package/ncurses/ncurses.mk
@@ -29,7 +29,7 @@  NCURSES_CONF_OPT = \
 	--enable-overwrite \
 	--enable-pc-files \
 	$(if $(BR2_PACKAGE_NCURSES_TARGET_PROGS),,--without-progs) \
-	$(if $(BR2_HAVE_DOCUMENTATION),,--without-manpages)
+	--without-manpages
 
 # Install after busybox for the full-blown versions
 ifeq ($(BR2_PACKAGE_BUSYBOX),y)
diff --git a/package/netsnmp/netsnmp.mk b/package/netsnmp/netsnmp.mk
--- a/package/netsnmp/netsnmp.mk
+++ b/package/netsnmp/netsnmp.mk
@@ -44,9 +44,7 @@  else
 endif
 
 # Docs
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
-	NETSNMP_CONF_OPT += --disable-manuals
-endif
+NETSNMP_CONF_OPT += --disable-manuals
 
 ifneq ($(BR2_PACKAGE_NETSNMP_ENABLE_MIBS),y)
 	NETSNMP_CONF_OPT += --disable-mib-loading
diff --git a/package/opencv/opencv.mk b/package/opencv/opencv.mk
--- a/package/opencv/opencv.mk
+++ b/package/opencv/opencv.mk
@@ -12,7 +12,7 @@  OPENCV_INSTALL_STAGING = YES
 OPENCV_CONF_OPT += \
 	-DCMAKE_BUILD_TYPE=$(if $(BR2_ENABLE_DEBUG),Debug,Release)   \
 	-DBUILD_WITH_STATIC_CRT=OFF                                  \
-	-DBUILD_DOCS=$(if $(BR2_HAVE_DOCUMENTATION),ON,OFF)          \
+	-DBUILD_DOCS=OFF                                             \
 	-DBUILD_EXAMPLES=OFF                                         \
 	-DBUILD_PACKAGE=OFF                                          \
 	-DBUILD_TESTS=$(if $(BR2_PACKAGE_OPENCV_BUILD_TESTS),ON,OFF) \
@@ -161,12 +161,10 @@  OPENCV_CONF_OPT += -DWITH_V4L=OFF
 endif
 
 # Installation hooks:
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
 define OPENCV_CLEAN_INSTALL_DOC
 	$(RM) -fr $(TARGET_DIR)/usr/share/OpenCV/doc
 endef
 OPENCV_POST_INSTALL_TARGET_HOOKS += OPENCV_CLEAN_INSTALL_DOC
-endif
 
 define OPENCV_CLEAN_INSTALL_CMAKE
 	$(RM) -f $(TARGET_DIR)/usr/share/OpenCV/OpenCVConfig*.cmake
diff --git a/package/perl/perl.mk b/package/perl/perl.mk
--- a/package/perl/perl.mk
+++ b/package/perl/perl.mk
@@ -91,10 +91,6 @@  define PERL_INSTALL_STAGING_CMDS
 endef
 
 PERL_INSTALL_TARGET_GOALS = install.perl
-ifeq ($(BR2_HAVE_DOCUMENTATION),y)
-PERL_INSTALL_TARGET_GOALS += install.man
-endif
-
 
 define PERL_INSTALL_TARGET_CMDS
 	$(MAKE1) -C $(@D) DESTDIR="$(TARGET_DIR)" $(PERL_INSTALL_TARGET_GOALS)
diff --git a/package/pppd/pppd.mk b/package/pppd/pppd.mk
--- a/package/pppd/pppd.mk
+++ b/package/pppd/pppd.mk
@@ -12,8 +12,6 @@  PPPD_LICENSE_FILES = pppd/tdb.c pppd/plu
 	pppdump/bsd-comp.c pppd/ccp.c pppd/plugins/passprompt.c
 
 PPPD_TARGET_BINS = chat pppd pppdump pppstats
-PPPD_MANPAGES = $(if $(BR2_HAVE_DOCUMENTATION),chat pppd pppdump pppstats)
-PPPD_RADIUS_MANPAGES = $(if $(BR2_HAVE_DOCUMENTATION),pppd-radattr pppd-radius)
 PPPD_RADIUS_CONF = dictionary dictionary.ascend dictionary.compat \
 			dictionary.merit dictionary.microsoft \
 			issue port-id-map realms server radiusclient.conf
@@ -65,10 +63,6 @@  define PPPD_INSTALL_RADIUS
 		$(TARGET_DIR)/etc/ppp/radius/radiusclient.conf
 	$(SED) 's:/etc/radiusclient:/etc/ppp/radius:g' \
 		$(TARGET_DIR)/etc/ppp/radius/*
-	for m in $(PPPD_RADIUS_MANPAGES); do \
-		$(INSTALL) -m 644 -D $(PPPD_DIR)/pppd/plugins/radius/$$m.8 \
-			$(TARGET_DIR)/usr/share/man/man8/$$m.8; \
-	done
 endef
 endif
 
@@ -96,10 +90,6 @@  define PPPD_INSTALL_TARGET_CMDS
 	$(INSTALL) -D $(PPPD_DIR)/pppd/plugins/pppol2tp/pppol2tp.so \
 		$(TARGET_DIR)/usr/lib/pppd/$(PPPD_VERSION)/pppol2tp.so
 	$(PPPD_INSTALL_RADIUS)
-	for m in $(PPPD_MANPAGES); do \
-		$(INSTALL) -m 644 -D $(PPPD_DIR)/$$m/$$m.8 \
-			$(TARGET_DIR)/usr/share/man/man8/$$m.8; \
-	done
 endef
 
 $(eval $(generic-package))
diff --git a/package/pulseaudio/pulseaudio.mk b/package/pulseaudio/pulseaudio.mk
--- a/package/pulseaudio/pulseaudio.mk
+++ b/package/pulseaudio/pulseaudio.mk
@@ -15,7 +15,7 @@  PULSEAUDIO_CONF_OPT = \
 	--disable-default-build-tests \
 	--disable-legacy-runtime-dir \
 	--disable-legacy-database-entry-format \
-	$(if $(BR2_HAVE_DOCUMENTATION),,--disable-manpages)
+	--disable-manpages
 
 PULSEAUDIO_DEPENDENCIES = \
 	host-pkgconf libtool json-c libsndfile speex host-intltool \
diff --git a/package/python-pygame/python-pygame.mk b/package/python-pygame/python-pygame.mk
--- a/package/python-pygame/python-pygame.mk
+++ b/package/python-pygame/python-pygame.mk
@@ -82,13 +82,11 @@  define PYTHON_PYGAME_CONFIGURE_CMDS
 	$(PYTHON_PYGAME_UNCONFIGURE_SCRAP)
 endef
 
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
 define PYTHON_PYGAME_REMOVE_DOC
 	rm -rf $(TARGET_DIR)/usr/lib/python*/site-packages/pygame/docs
 endef
 
 PYTHON_PYGAME_POST_INSTALL_TARGET_HOOKS += PYTHON_PYGAME_REMOVE_DOC
-endif
 
 define PYTHON_PYGAME_REMOVE_TESTS
 	rm -rf $(TARGET_DIR)/usr/lib/python*/site-packages/pygame/tests
diff --git a/package/samba/samba.mk b/package/samba/samba.mk
--- a/package/samba/samba.mk
+++ b/package/samba/samba.mk
@@ -157,10 +157,8 @@  endif
 SAMBA_CONF_OPT += CFLAGS="$(TARGET_CFLAGS) -DMAX_DEBUG_LEVEL=$(BR2_PACKAGE_SAMBA_MAX_DEBUGLEVEL)"
 
 ifeq ($(BR2_PACKAGE_SAMBA_SWAT),y)
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
 SAMBA_POST_INSTALL_TARGET_HOOKS += SAMBA_REMOVE_SWAT_DOCUMENTATION
 endif
-endif
 
 define SAMBA_INSTALL_INITSCRIPTS_CONFIG
 	# install start/stop script
diff --git a/package/udisks/udisks.mk b/package/udisks/udisks.mk
--- a/package/udisks/udisks.mk
+++ b/package/udisks/udisks.mk
@@ -20,8 +20,7 @@  UDISKS_DEPENDENCIES = 	\
 	lvm2		\
 	libatasmart
 
-UDISKS_CONF_OPT = --disable-remote-access \
-	$(if $(BR2_HAVE_DOCUMENTATION),,--disable-man-pages)
+UDISKS_CONF_OPT = --disable-remote-access --disable-man-pages
 
 ifeq ($(BR2_PACKAGE_UDISKS_LVM2),y)
 UDISKS_CONF_OPT += --enable-lvm2
diff --git a/package/vala/vala.mk b/package/vala/vala.mk
--- a/package/vala/vala.mk
+++ b/package/vala/vala.mk
@@ -15,15 +15,9 @@  VALA_LICENSE_FILES = COPYING
 VALA_DEPENDENCIES = host-flex host-bison libglib2 \
 		$(if $(BR2_NEEDS_GETTEXT_IF_LOCALE),gettext)
 
-# If we want the documentation, then xsltproc is needed. If we don't
-# want the documentation, force Vala to not use the host xsltproc even
-# if available, because it may or may not work with Vala documentation
-# (some versions of xsltproc segfault)
-ifeq ($(BR2_HAVE_DOCUMENTATION),y)
-VALA_DEPENDENCIES += host-libxslt
-else
+# Force Vala to not use the host xsltproc even if available, because it may or
+# may not work with Vala documentation (some versions of xsltproc segfault)
 VALA_CONF_ENV = ac_cv_path_XSLTPROC=:
-endif
 
 HOST_VALA_DEPENDENCIES = host-flex host-libglib2
 # Yes, the autoconf script understands ':' as "xsltproc is not
diff --git a/package/vim/vim.mk b/package/vim/vim.mk
--- a/package/vim/vim.mk
+++ b/package/vim/vim.mk
@@ -42,9 +42,7 @@  endef
 
 ifeq ($(BR2_PACKAGE_VIM_RUNTIME),y)
 VIM_POST_INSTALL_TARGET_HOOKS += VIM_INSTALL_RUNTIME_CMDS
-ifneq ($(BR2_HAVE_DOCUMENTATION),y)
 VIM_POST_INSTALL_TARGET_HOOKS += VIM_REMOVE_DOCS
 endif
-endif
 
 $(eval $(autotools-package))
diff --git a/package/x11r7/xproto_xcmiscproto/xproto_xcmiscproto.mk b/package/x11r7/xproto_xcmiscproto/xproto_xcmiscproto.mk
--- a/package/x11r7/xproto_xcmiscproto/xproto_xcmiscproto.mk
+++ b/package/x11r7/xproto_xcmiscproto/xproto_xcmiscproto.mk
@@ -10,7 +10,7 @@  XPROTO_XCMISCPROTO_SITE = http://xorg.fr
 XPROTO_XCMISCPROTO_LICENSE = MIT
 XPROTO_XCMISCPROTO_LICENSE_FILES = COPYING
 XPROTO_XCMISCPROTO_INSTALL_STAGING = YES
-XPROTO_XCMISCPROTO_CONF_OPT = $(if $(BR2_HAVE_DOCUMENTATION),,--disable-specs)
+XPROTO_XCMISCPROTO_CONF_OPT = --disable-specs
 HOST_XPROTO_XCMISCPROTO_CONF_OPT = --disable-specs
 
 $(eval $(autotools-package))
diff --git a/package/x11r7/xproto_xextproto/xproto_xextproto.mk b/package/x11r7/xproto_xextproto/xproto_xextproto.mk
--- a/package/x11r7/xproto_xextproto/xproto_xextproto.mk
+++ b/package/x11r7/xproto_xextproto/xproto_xextproto.mk
@@ -10,7 +10,7 @@  XPROTO_XEXTPROTO_SITE = http://xorg.free
 XPROTO_XEXTPROTO_LICENSE = MIT
 XPROTO_XEXTPROTO_LICENSE_FILES = COPYING
 XPROTO_XEXTPROTO_INSTALL_STAGING = YES
-XPROTO_XEXTPROTO_CONF_OPT = $(if $(BR2_HAVE_DOCUMENTATION),,--disable-specs)
+XPROTO_XEXTPROTO_CONF_OPT = --disable-specs
 HOST_XPROTO_XEXTPROTO_CONF_OPT = --disable-specs
 
 $(eval $(autotools-package))