Patchwork [5/6] conf/distro/include/default-distrovars.inc: Create set of default 'distro' variable values

From: Richard Purdie <richard.purdie@linuxfoundation.org>

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/conf/distro/defaultsetup.conf              |    1 +
 meta/conf/distro/include/default-distrovars.inc |   43 ++++++++++++++++++++
 meta/conf/distro/poky.conf                      |   48 +----------------------
 3 files changed, 46 insertions(+), 46 deletions(-)
 create mode 100644 meta/conf/distro/include/default-distrovars.inc

Some minor remarks on the default-distrovars.inc contents:

2011/5/10 Richard Purdie <richard.purdie@linuxfoundation.org>
[...]

> diff --git a/meta/conf/distro/include/default-distrovars.inc
> b/meta/conf/distro/include/default-distrovars.inc
> new file mode 100644
> index 0000000..ab26a30
> --- /dev/null
> +++ b/meta/conf/distro/include/default-distrovars.inc
> @@ -0,0 +1,43 @@
> +QA_LOGFILE = "${TMPDIR}/qa.log"
>
Should this be here? I would expect the class file that uses this to have
this as default and/or have this in local.conf.sample.
This does not seem too distro specific

> +
> +IMAGE_ROOTFS_SIZE_ext2 ?= "131072"
>
Hm, again something that is probalby not distro related. And why only for
_ext2 and not eg for _ext3 or for _jffs2
Personally I always stuff this in my image recipe

> +
> +OEINCLUDELOGS ?= "yes"
>
in local.conf.sample?

> +KERNEL_CONSOLE ?= "ttyS0"
>
I'm inclined to put this into the machine conf. Is this really a distro
thing?

> +
> +require conf/distro/include/preferred-xorg-versions.inc
> +
> +PCMCIA_MANAGER ?= "pcmciautils"
> +
> +IMAGE_LINGUAS ?= "en-us en-gb"
> +LIMIT_BUILT_LOCALES ?= "POSIX en_US en_GB"
> +ENABLE_BINARY_LOCALE_GENERATION ?= "1"
> +LOCALE_UTF8_ONLY ?= "0"
>
Again something I tend to do in image recipes; then again this might be
because I do not use feeds, only make images.

> +
> +DISTRO_FEATURES ?= "alsa bluetooth ext2 irda pcmcia usbgadget usbhost wifi
> nfs zeroconf pci"
>
Is this the complete range? I would expect all features to be enabled by
default (and let MACHINE_FEATURES reduce that set)

> +
> +DISTRO_EXTRA_RDEPENDS += "task-core-boot"
> +DISTRO_EXTRA_RRECOMMENDS += "kernel-module-af-packet"
> +
> +IMAGE_FEATURES ?= ""
>
Has a var with ?= assignment of an empty string any meaning. Thought an
empty string would be the default.

> +
> +# This is a list of packages that are used by the build system to build
> the distribution, they are not
> +# directly part of the distribution.
> +HOSTTOOLS_WHITELIST_GPLv3 ?= ""
> +WHITELIST_GPLv3 ?= "less"
>
I'm not sure why less is listed in the line above:
the less license is not gpl (at least not less 443)


> +LGPLv2_WHITELIST_GPLv3 ?= "libassuan gnutls libtasn1 libidn libgcc
> gcc-runtime"
> +
> +# This is a list of packages that require a commercial license to ship
> +# product. If shipped as part of an image these packages may have
> +# implications so they are disabled by default
> +COMMERCIAL_LICENSE ?= "lame gst-fluendo-mp3 libmad mpeg2dec ffmpeg qmmp"
> +COMMERCIAL_AUDIO_PLUGINS ?= ""
> +# COMMERCIAL_AUDIO_PLUGINS ?= "gst-plugins-ugly-mad
> gst-plugins-ugly-mpegaudioparse"
> +COMMERCIAL_VIDEO_PLUGINS ?= ""
> +# COMMERCIAL_VIDEO_PLUGINS ?= "gst-plugins-ugly-mpeg2dec
> gst-plugins-ugly-mpegstream gst-plugins-bad-mpegvideoparse"
> +COMMERCIAL_QT ?= ""
> +# COMMERCIAL_QT ?= "qmmp"
> +# Set of common licenses used for license.bbclass
> +COMMON_LICENSE_DIR ??= "${COREBASE}/meta/files/common-licenses"
>
maybe the above could be in a separate license.inc file or so.

> +
> +BB_GENERATE_MIRROR_TARBALLS ??= "0"
>
local.conf.sample ?

Enjoy! Frans

On Tue, 2011-05-10 at 16:26 +0200, Frans Meulenbroeks wrote:
> Some minor remarks on the default-distrovars.inc contents:

To quote the email prefacing this patch series:

"""
I did dump a load of "default" variables into default-distrovars.inc,
I'm not calling that file finished, I just had to draw the line
somewhere and start a discussion about this :)
"""

but we can have this discussion.

> 2011/5/10 Richard Purdie <richard.purdie@linuxfoundation.org>
> [...]
> 
> > diff --git a/meta/conf/distro/include/default-distrovars.inc
> > b/meta/conf/distro/include/default-distrovars.inc
> > new file mode 100644
> > index 0000000..ab26a30
> > --- /dev/null
> > +++ b/meta/conf/distro/include/default-distrovars.inc
> > @@ -0,0 +1,43 @@
> > +QA_LOGFILE = "${TMPDIR}/qa.log"
> >
> Should this be here? I would expect the class file that uses this to have
> this as default and/or have this in local.conf.sample.
> This does not seem too distro specific

If this is unset, the logfile isn't generated at all with the current
defaults. I don't claim the current defaults are correct but its a
separate patch if we do want to change the defaults.

> > +
> > +IMAGE_ROOTFS_SIZE_ext2 ?= "131072"
> >
> Hm, again something that is probalby not distro related. And why only for
> _ext2 and not eg for _ext3 or for _jffs2
> Personally I always stuff this in my image recipe

Left there pending a review of where various image sizes are set.

> > +
> > +OEINCLUDELOGS ?= "yes"
> >
> in local.conf.sample?

If its there, this can be dropped, yes. I think forcing it on by default
may be a good thing though. I'm not 100% sure that variable even still
exists tbh.

> > +KERNEL_CONSOLE ?= "ttyS0"
> >
> I'm inclined to put this into the machine conf. Is this really a distro
> thing?

No, left for compatibility pending cleanup.

> > +
> > +require conf/distro/include/preferred-xorg-versions.inc
> > +
> > +PCMCIA_MANAGER ?= "pcmciautils"
> > +
> > +IMAGE_LINGUAS ?= "en-us en-gb"
> > +LIMIT_BUILT_LOCALES ?= "POSIX en_US en_GB"
> > +ENABLE_BINARY_LOCALE_GENERATION ?= "1"
> > +LOCALE_UTF8_ONLY ?= "0"
> >
> Again something I tend to do in image recipes; then again this might be
> because I do not use feeds, only make images.

These ones are more distro policy and whilst an image can change them a
default is good in those cases. If there are defaults set elsewhere we
should review and pick the correct ones.

> > +DISTRO_FEATURES ?= "alsa bluetooth ext2 irda pcmcia usbgadget usbhost wifi
> > nfs zeroconf pci"
> >
> Is this the complete range? I would expect all features to be enabled by
> default (and let MACHINE_FEATURES reduce that set)

Needs further investigation.

> > +
> > +DISTRO_EXTRA_RDEPENDS += "task-core-boot"
> > +DISTRO_EXTRA_RRECOMMENDS += "kernel-module-af-packet"
> > +
> > +IMAGE_FEATURES ?= ""
> >
> Has a var with ?= assignment of an empty string any meaning. Thought an
> empty string would be the default.

No, its not.

> > +
> > +# This is a list of packages that are used by the build system to build
> > the distribution, they are not
> > +# directly part of the distribution.
> > +HOSTTOOLS_WHITELIST_GPLv3 ?= ""
> > +WHITELIST_GPLv3 ?= "less"
> >
> I'm not sure why less is listed in the line above:
> the less license is not gpl (at least not less 443)
> 
> 
> > +LGPLv2_WHITELIST_GPLv3 ?= "libassuan gnutls libtasn1 libidn libgcc
> > gcc-runtime"
> > +
> > +# This is a list of packages that require a commercial license to ship
> > +# product. If shipped as part of an image these packages may have
> > +# implications so they are disabled by default
> > +COMMERCIAL_LICENSE ?= "lame gst-fluendo-mp3 libmad mpeg2dec ffmpeg qmmp"
> > +COMMERCIAL_AUDIO_PLUGINS ?= ""
> > +# COMMERCIAL_AUDIO_PLUGINS ?= "gst-plugins-ugly-mad
> > gst-plugins-ugly-mpegaudioparse"
> > +COMMERCIAL_VIDEO_PLUGINS ?= ""
> > +# COMMERCIAL_VIDEO_PLUGINS ?= "gst-plugins-ugly-mpeg2dec
> > gst-plugins-ugly-mpegstream gst-plugins-bad-mpegvideoparse"
> > +COMMERCIAL_QT ?= ""
> > +# COMMERCIAL_QT ?= "qmmp"
> > +# Set of common licenses used for license.bbclass
> > +COMMON_LICENSE_DIR ??= "${COREBASE}/meta/files/common-licenses"
> >
> maybe the above could be in a separate license.inc file or so.

Possibly, setting these to defaults is good though.

> > +
> > +BB_GENERATE_MIRROR_TARBALLS ??= "0"
> >
> local.conf.sample ?

I regard local.conf.sample as things a new user cares about. I don't
think this fits that category. Its here pending the creation of a more
"advanced settings" version of local.conf.sample we've talked about
being created.

Cheers,

Richard

2011/5/10 Richard Purdie <richard.purdie@linuxfoundation.org>

> On Tue, 2011-05-10 at 16:26 +0200, Frans Meulenbroeks wrote:
> > Some minor remarks on the default-distrovars.inc contents:
>
> To quote the email prefacing this patch series:
>
> """
> I did dump a load of "default" variables into default-distrovars.inc,
> I'm not calling that file finished, I just had to draw the line
> somewhere and start a discussion about this :)
> """
>
> but we can have this discussion.
>

I saw that note. Idea was indeed to initiate the discussion and make
suggestions for improvements.


>
> > 2011/5/10 Richard Purdie <richard.purdie@linuxfoundation.org>
> > [...]
> >
> > > diff --git a/meta/conf/distro/include/default-distrovars.inc
> > > b/meta/conf/distro/include/default-distrovars.inc
> > > new file mode 100644
> > > index 0000000..ab26a30
> > > --- /dev/null
> > > +++ b/meta/conf/distro/include/default-distrovars.inc
> > > @@ -0,0 +1,43 @@
> > > +QA_LOGFILE = "${TMPDIR}/qa.log"
> > >
> > Should this be here? I would expect the class file that uses this to have
> > this as default and/or have this in local.conf.sample.
> > This does not seem too distro specific
>
> If this is unset, the logfile isn't generated at all with the current
> defaults. I don't claim the current defaults are correct but its a
> separate patch if we do want to change the defaults.
>

It is good to to have the qa file made by default, but I am not sure whether
it is distro specific.
Actually I probably would expect things like QA_LOGFILE to live in the same
spot as where TMPDIR and DOWNLOADDIR etc are defined

>
> > > +
> > > +IMAGE_ROOTFS_SIZE_ext2 ?= "131072"
> > >
> > Hm, again something that is probalby not distro related. And why only for
> > _ext2 and not eg for _ext3 or for _jffs2
> > Personally I always stuff this in my image recipe
>
> Left there pending a review of where various image sizes are set.
>

Ok understood

>
> > > +
> > > +OEINCLUDELOGS ?= "yes"
> > >
> > in local.conf.sample?
>
> If its there, this can be dropped, yes. I think forcing it on by default
> may be a good thing though. I'm not 100% sure that variable even still
> exists tbh.
>

In openembedded/classes and openembedded/site this var does not exist. I do
not have an oe-core/yocto tree handy to check if it is there.
BTW I do have BBINCLUDELOGS in my local.conf.sample. As far as I know this
is a bitbake var and actually being used.

>
> > > +KERNEL_CONSOLE ?= "ttyS0"
> > >
> > I'm inclined to put this into the machine conf. Is this really a distro
> > thing?
>
> No, left for compatibility pending cleanup.
>
> > > +
> > > +require conf/distro/include/preferred-xorg-versions.inc
> > > +
> > > +PCMCIA_MANAGER ?= "pcmciautils"
> > > +
> > > +IMAGE_LINGUAS ?= "en-us en-gb"
> > > +LIMIT_BUILT_LOCALES ?= "POSIX en_US en_GB"
> > > +ENABLE_BINARY_LOCALE_GENERATION ?= "1"
> > > +LOCALE_UTF8_ONLY ?= "0"
> > >
> > Again something I tend to do in image recipes; then again this might be
> > because I do not use feeds, only make images.
>
> These ones are more distro policy and whilst an image can change them a
> default is good in those cases. If there are defaults set elsewhere we
> should review and pick the correct ones.
>
> > > +DISTRO_FEATURES ?= "alsa bluetooth ext2 irda pcmcia usbgadget usbhost
> wifi
> > > nfs zeroconf pci"
> > >
> > Is this the complete range? I would expect all features to be enabled by
> > default (and let MACHINE_FEATURES reduce that set)
>
> Needs further investigation.
>
> > > +
> > > +DISTRO_EXTRA_RDEPENDS += "task-core-boot"
> > > +DISTRO_EXTRA_RRECOMMENDS += "kernel-module-af-packet"
> > > +
> > > +IMAGE_FEATURES ?= ""
> > >
> > Has a var with ?= assignment of an empty string any meaning. Thought an
> > empty string would be the default.
>
> No, its not.
>
> > > +
> > > +# This is a list of packages that are used by the build system to
> build
> > > the distribution, they are not
> > > +# directly part of the distribution.
> > > +HOSTTOOLS_WHITELIST_GPLv3 ?= ""
> > > +WHITELIST_GPLv3 ?= "less"
> > >
> > I'm not sure why less is listed in the line above:
> > the less license is not gpl (at least not less 443)
> >
> >
> > > +LGPLv2_WHITELIST_GPLv3 ?= "libassuan gnutls libtasn1 libidn libgcc
> > > gcc-runtime"
> > > +
> > > +# This is a list of packages that require a commercial license to ship
> > > +# product. If shipped as part of an image these packages may have
> > > +# implications so they are disabled by default
> > > +COMMERCIAL_LICENSE ?= "lame gst-fluendo-mp3 libmad mpeg2dec ffmpeg
> qmmp"
> > > +COMMERCIAL_AUDIO_PLUGINS ?= ""
> > > +# COMMERCIAL_AUDIO_PLUGINS ?= "gst-plugins-ugly-mad
> > > gst-plugins-ugly-mpegaudioparse"
> > > +COMMERCIAL_VIDEO_PLUGINS ?= ""
> > > +# COMMERCIAL_VIDEO_PLUGINS ?= "gst-plugins-ugly-mpeg2dec
> > > gst-plugins-ugly-mpegstream gst-plugins-bad-mpegvideoparse"
> > > +COMMERCIAL_QT ?= ""
> > > +# COMMERCIAL_QT ?= "qmmp"
> > > +# Set of common licenses used for license.bbclass
> > > +COMMON_LICENSE_DIR ??= "${COREBASE}/meta/files/common-licenses"
> > >
> > maybe the above could be in a separate license.inc file or so.embedded
>
> Possibly, setting these to defaults is good though.
>
> > > +
> > > +BB_GENERATE_MIRROR_TARBALLS ??= "0"
> > >
> > local.conf.sample ?
>
> I regard local.conf.sample as things a new user cares about. I don't
> think this fits that category. Its here embeddedpending the creation of a
> more
> "advanced settings" version of local.conf.sample we've talked about
> being created.
>
> I'm unaware of any discussion on an advanced settings version of
local.conf.sample.

Anyway there are two different groups of users for oe/yocto:
Those who want to do a distro with package feeds etc and those (like me) who
are more into creating (lean &lightweight) images for embedded,
non-networked (or non network upgradeable)  devices (like a microwave etc).
For the latter it could be very useful to have a "distro-less" system or a
very simple default distro. Main interest for those people is probably the
cross environment and the image creation capabilities.
If  one wants full control on what is being in the image it helps to have
that in a logical location.

But I am digressing...

Frans.

On Tue, May 10, 2011 at 7:00 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/conf/distro/defaultsetup.conf              |    1 +
>  meta/conf/distro/include/default-distrovars.inc |   43 ++++++++++++++++++++
>  meta/conf/distro/poky.conf                      |   48 +----------------------
>  3 files changed, 46 insertions(+), 46 deletions(-)
>  create mode 100644 meta/conf/distro/include/default-distrovars.inc
>
> diff --git a/meta/conf/distro/defaultsetup.conf b/meta/conf/distro/defaultsetup.conf
> index af5ef7b..8da6c0a 100644
> --- a/meta/conf/distro/defaultsetup.conf
> +++ b/meta/conf/distro/defaultsetup.conf
> @@ -1,5 +1,6 @@
>  include conf/distro/include/default-providers.inc
>  include conf/distro/include/default-versions.inc
> +include conf/distro/include/default-distrovars.inc
>  include conf/distro/include/world-broken.inc
>
>  TARGET_VENDOR ?= "-oecore"
> diff --git a/meta/conf/distro/include/default-distrovars.inc b/meta/conf/distro/include/default-distrovars.inc
> new file mode 100644
> index 0000000..ab26a30
> --- /dev/null
> +++ b/meta/conf/distro/include/default-distrovars.inc
> @@ -0,0 +1,43 @@
> +QA_LOGFILE = "${TMPDIR}/qa.log"
> +
> +IMAGE_ROOTFS_SIZE_ext2 ?= "131072"
> +
> +OEINCLUDELOGS ?= "yes"
> +KERNEL_CONSOLE ?= "ttyS0"
> +
> +require conf/distro/include/preferred-xorg-versions.inc
> +
> +PCMCIA_MANAGER ?= "pcmciautils"
> +
> +IMAGE_LINGUAS ?= "en-us en-gb"
> +LIMIT_BUILT_LOCALES ?= "POSIX en_US en_GB"
> +ENABLE_BINARY_LOCALE_GENERATION ?= "1"
> +LOCALE_UTF8_ONLY ?= "0"
> +
> +DISTRO_FEATURES ?= "alsa bluetooth ext2 irda pcmcia usbgadget usbhost wifi nfs zeroconf pci"
> +
> +DISTRO_EXTRA_RDEPENDS += "task-core-boot"
> +DISTRO_EXTRA_RRECOMMENDS += "kernel-module-af-packet"
> +


others can be overridden but the above two are a problem. they are forcing
distros to have these in RDEPENDS and RRECOMMENDS e.g. a distro
which does not want kernel-module-af-packet will have to have some way
to remove it. My opinion is do not add them to policy

-Khem

On Sun, 2011-05-15 at 13:48 -0700, Khem Raj wrote:
> > +++ b/meta/conf/distro/include/default-distrovars.inc
> > @@ -0,0 +1,43 @@

> > +DISTRO_EXTRA_RDEPENDS += "task-core-boot"
> > +DISTRO_EXTRA_RRECOMMENDS += "kernel-module-af-packet"
> > +
> 
> 
> others can be overridden but the above two are a problem. they are forcing
> distros to have these in RDEPENDS and RRECOMMENDS e.g. a distro
> which does not want kernel-module-af-packet will have to have some way
> to remove it. My opinion is do not add them to policy

Agreed, I merged your patch to remove these two things.

Cheers,

Richard