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.pur...@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 _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core