[RFC][PATCH 0/6] development vs. production builds

[RFC][PATCH 0/6] development vs. production builds

[Patrick Ohly]

At OEDAM [1] I took the AR to flesh out some of my ideas for
introducing global and per-image settings for switching between
development and production builds. The goal is partly to establish
common configure options that then can be used by different layers,
partly to have some actual useful functionality attached to them
already in OE-core.

"development" builds are what a developer does when trying out a
distro or working on his own personal device. "production" is what
device manufacturer put onto the actual end-user hardware.

At OEDAM we already concluded that per-image settings are more
useful. However, sometimes a component also has compile-time choices
that cannot be changed later on in an image, and indeed I found one
example for that (kmod) in OE-core.

Therefore I have included "debug-build" DISTRO_FEATURES support. It's a
bit similar to manpages.bbclass, but in contrast to that (currently)
is meant to be inherited globally - that's partly due to
misunderstanding how manpages.bbclass was meant to be used. This can
be changed, for now I just want to demonstrate that such a distro
feature is not entirely useless, and what effect it could have already
in OE-core.

In refkit, we also switch globally between development and production
builds via .inc files. However, I am in the process of replacing that
with the more flexible per-image IMAGE_MODE check. So from my
perspective, the IMAGE_MODE is more important and useful than the
"debug-build" DISTRO_FEATURE.

From a design perspective, the approach taken here is to let a
developer or image define what mode it wants, and default features can
be configured accordingly. That alternative would be to continue defining
features as before and use the mode to configure QA warnings or errors.

But I find that less flexible, and I suspect it would be harder to
keep track of what is meant to be usable in which mode. Developers
also would have a harder time overriding the defaults.

[1] https://www.openembedded.org/wiki/OEDAM_2017

Patrick Ohly (6):
  build-mode.bbclass: distro-wide debug-build mode
  basefiles: warn about non-production DISTRO_FEATURES in motd
  defaultsetup.conf: enable special "debug-build" DISTRO_FEATURES support
  image-mode.bbclass: per-image production/development/debug mode
  image.bbclass: include IMAGE_MODE support
  local.conf.sample: make debug-tweaks depend on IMAGE_MODE

 meta/classes/build-mode.bbclass                   | 16 ++++-
 meta/classes/image-mode.bbclass                   | 62 ++++++++++++++++-
 meta/classes/image.bbclass                        |  3 +-
 meta/conf/distro/defaultsetup.conf                |  2 +-
 meta/conf/local.conf.sample                       |  5 +-
 meta/recipes-core/base-files/base-files_3.0.14.bb | 11 +++-
 6 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100644 meta/classes/build-mode.bbclass
 create mode 100644 meta/classes/image-mode.bbclass

base-commit: 9f9ebf2e1ba6eda48fb5e3f20d4ca5bbabe3dad4
-- 
git-series 0.9.1

[RFC][PATCH 1/6] build-mode.bbclass: distro-wide debug-build mode

[Patrick Ohly]

Some packages (for example, kmod) have built-time configure options
for additional debugging support. Similar to manpages.bbclass, this
new class turns the new "debug-build" DISTRO_FEATURE into "debug" and
"logging" PACKAGECONFIGs if the current recipe has those.

Another usage will be to mark images when undesired distro features
like "debug-build" were used. This protects against accidentally using
such an image as production image on a device.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/build-mode.bbclass | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 meta/classes/build-mode.bbclass

diff --git a/meta/classes/build-mode.bbclass b/meta/classes/build-mode.bbclass
new file mode 100644
index 0000000..e5564dd
--- /dev/null
+++ b/meta/classes/build-mode.bbclass
@@ -0,0 +1,16 @@
+# Enable additional logging and debugging features that are unsuitable
+# for production images when building in "debug" mode. "kmod" is
+# an example of a recipe which has both "debug" and "logging"
+# PACKAGECONFIGs.
+#
+# This distro feature is not very flexible. More useful in practice
+# is the IMAGE_MODE in image.bbclass, which allows building production
+# and development images in the same build configuration.
+PACKAGECONFIG_append_class-target = "${@ \
+    (' ' + ' '.join(set(d.getVarFlags('PACKAGECONFIG').keys()).intersection(('debug', 'logging')))) if \
+    bb.utils.contains('DISTRO_FEATURES', 'debug-build', True, False, d) \
+    else ''}"
+
+# The following DISTRO_FEATURES have to be considered too dangerous
+# and/or unsuitable for use in production.
+DISTRO_FEATURES_NON_PRODUCTION ??= "debug-build"
-- 
git-series 0.9.1

[RFC][PATCH 2/6] basefiles: warn about non-production DISTRO_FEATURES in motd

[Patrick Ohly]

The default motd is created at build-time by the basefiles recipes.
When the entire distro is built in a way that is unsuitable for
production, we can warn about that here.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/recipes-core/base-files/base-files_3.0.14.bb | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/meta/recipes-core/base-files/base-files_3.0.14.bb b/meta/recipes-core/base-files/base-files_3.0.14.bb
index ca7bf06..eff70a0 100644
--- a/meta/recipes-core/base-files/base-files_3.0.14.bb
+++ b/meta/recipes-core/base-files/base-files_3.0.14.bb
@@ -127,6 +127,17 @@ do_install () {
 	install -m 0644 ${WORKDIR}/nsswitch.conf ${D}${sysconfdir}/nsswitch.conf
 	install -m 0644 ${WORKDIR}/host.conf ${D}${sysconfdir}/host.conf
 	install -m 0644 ${WORKDIR}/motd ${D}${sysconfdir}/motd
+	non_production="${@ ' '.join(set(d.getVar('DISTRO_FEATURES').split()).intersection((d.getVar('DISTRO_FEATURES_NON_PRODUCTION') or '').split()))}"
+	if [ "$non_production" ]; then
+		cat >>${D}${sysconfdir}/motd <<EOF
+******************************************************************
+*** This distro was built using these non-production features: ***
+*** `printf "%-58s" "$non_production"` ***
+*** Do not use this distro and its images in production.       ***
+******************************************************************
+
+EOF
+	fi
 
 	ln -sf /proc/mounts ${D}${sysconfdir}/mtab
 }
-- 
git-series 0.9.1

[RFC][PATCH 3/6] defaultsetup.conf: enable special "debug-build" DISTRO_FEATURES support

[Patrick Ohly]

The changes introduced by build-mode.bbclass should be simple enough
that they can be enabled by default. Doing so helps establish
"debug-build" consistently in different distros.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/conf/distro/defaultsetup.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/conf/distro/defaultsetup.conf b/meta/conf/distro/defaultsetup.conf
index ca2f917..fc7cee6 100644
--- a/meta/conf/distro/defaultsetup.conf
+++ b/meta/conf/distro/defaultsetup.conf
@@ -20,5 +20,5 @@ CACHE = "${TMPDIR}/cache/${TCMODE}-${TCLIBC}${@['', '/' + str(d.getVar('MACHINE'
 USER_CLASSES ?= ""
 PACKAGE_CLASSES ?= "package_ipk"
 INHERIT_BLACKLIST = "blacklist"
-INHERIT_DISTRO ?= "debian devshell sstate license remove-libtool"
+INHERIT_DISTRO ?= "debian devshell sstate license remove-libtool build-mode"
 INHERIT += "${PACKAGE_CLASSES} ${USER_CLASSES} ${INHERIT_DISTRO} ${INHERIT_BLACKLIST}"
-- 
git-series 0.9.1

[RFC][PATCH 4/6] image-mode.bbclass: per-image production/development/debug mode

[Patrick Ohly]

A global switch between different modes allows configuring image
recipes so that they adapt to the intended usage automatically. The
implementation here introduces three modes which progressively enable
more debugging features. That different modes cannot be combined is
meant to make the feature easier to use.

When a mode is explicitly set to something other than "production",
a warning about that is added to /etc/motd.

The default is to not set a mode explicitly. Image recipes then can
decide what they want to enable, depending on the normal usage, just
as they do now.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/image-mode.bbclass | 62 ++++++++++++++++++++++++++++++++++-
 1 file changed, 62 insertions(+)
 create mode 100644 meta/classes/image-mode.bbclass

diff --git a/meta/classes/image-mode.bbclass b/meta/classes/image-mode.bbclass
new file mode 100644
index 0000000..b036045
--- /dev/null
+++ b/meta/classes/image-mode.bbclass
@@ -0,0 +1,62 @@
+# Images can be built for "production", "debugging" or "development".
+# Additional modes might get defined by a distro or developer. This is
+# intentionally a separate variable. That way, IMAGE_FEATURES can be
+# set dynamically depending on IMAGE_MODE.
+#
+# They can be set independently from the "debug-build" DISTRO_FEATURE.
+# However, images that are explicitly marked as "production" get
+# disabled when "debug-build" is active.
+#
+# The intended usage is:
+# - production - enable and allow only features suitable for production
+# - development - enable also features useful while developing the distro and/or image content,
+#                 like easier root access
+# - debugging - same as development plus additional debug output
+#
+# The default is not to have a fixed mode. Which features an image
+# recipe enables in that case depends on whether the image and/or project is more
+# geared towards development or production. For example, the OE-core
+# local.conf.sample enables "debug-tweaks" by default unless "production"
+# is set.
+IMAGE_MODE ??= ""
+IMAGE_MODE_VALID ??= "production development debugging"
+
+python () {
+    # Sanity checks for IMAGE_MODE.
+    image_mode = d.getVar('IMAGE_MODE', d)
+    mode = set(image_mode.split())
+    if len(mode) == 0:
+        return
+    if len(mode) > 1:
+        bb.fatal('An image can only be built in exactly one mode: IMAGE_MODE=%s' % image_mode)
+    mode = mode.pop()
+    valid = d.getVar('IMAGE_MODE_VALID')
+    if mode not in valid.split():
+        bb.fatal('Invalid image mode: IMAGE_MODE=%s (not in %s)' % (image_mode, valid))
+
+    # Disable development images when we can't be sure that they'll only contain
+    # production content. This errs on the side of caution.
+    if mode == 'production' and 'debug-build' in d.getVar('DISTRO_FEATURES').split():
+        raise bb.parse.SkipRecipe("Production images are disabled when the 'debug-build' DISTRO_FEATURE is active.")
+}
+
+# Optional additional text appended to an image's /etc/motd
+# when some mode other than "production" is chosen explicitly.
+#
+# Set to empty to disable this.
+IMAGE_DEV_MOTD_DEFAULT () {
+*********************************************
+*** This is a ${IMAGE_MODE} image! ${@ ' ' * (19 - len(d.getVar('IMAGE_MODE')))} ***
+*** Do not use in production.             ***
+*********************************************
+}
+IMAGE_DEV_MOTD ??= "${IMAGE_DEV_MOTD_DEFAULT}"
+
+python dev_motd () {
+    motd = d.getVar('IMAGE_DEV_MOTD')
+    if motd:
+        with open(d.expand('${IMAGE_ROOTFS}${sysconfdir}/motd'), 'a') as f:
+            f.write(motd)
+}
+
+ROOTFS_POSTPROCESS_COMMAND += "${@'' if (d.getVar('IMAGE_MODE') or 'production') == 'production' else ' dev_motd;'}"
-- 
git-series 0.9.1

[RFC][PATCH 5/6] image.bbclass: include IMAGE_MODE support

[Patrick Ohly]

Inheriting image-mode.bbclass does not change anything by default, but
ensures that the variable is getting checked consistently in all image
recipes and distros when it gets set.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/image.bbclass | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 405fd73..976db0d 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -20,6 +20,9 @@ INHIBIT_DEFAULT_DEPS = "1"
 TESTIMAGECLASS = "${@base_conditional('TEST_IMAGE', '1', 'testimage-auto', '', d)}"
 inherit ${TESTIMAGECLASS}
 
+# Add support for IMAGE_MODE.
+inherit image-mode
+
 # IMAGE_FEATURES may contain any available package group
 IMAGE_FEATURES ?= ""
 IMAGE_FEATURES[type] = "list"
-- 
git-series 0.9.1

[RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Patrick Ohly]

Enabling "debug-tweaks" unconditionally, even if it is only in the
local.conf.sample file, runs the risk of that getting used in
production images.

By checking the per-image IMAGE_MODE, the debug tweaks only get
enabled for images not meant for production.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/conf/local.conf.sample | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
index 85c5e21..edadbb7 100644
--- a/meta/conf/local.conf.sample
+++ b/meta/conf/local.conf.sample
@@ -114,8 +114,9 @@ PACKAGE_CLASSES ?= "package_ipk"
 #                     e.g. ssh root access has a blank password
 # There are other application targets that can be used here too, see
 # meta/classes/image.bbclass and meta/classes/core-image.bbclass for more details.
-# We default to enabling the debugging tweaks.
-EXTRA_IMAGE_FEATURES ?= "debug-tweaks"
+# We default to enabling the debugging tweaks unless an image is explicitly
+# requested to be built for production.
+EXTRA_IMAGE_FEATURES ?= "${@ '' if 'production' == d.getVar('IMAGE_MODE') else 'debug-tweaks'}"
 
 #
 # Additional image features
-- 
git-series 0.9.1

Re: [RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Khem Raj]

On Mon, May 15, 2017 at 6:27 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> Enabling "debug-tweaks" unconditionally, even if it is only in the
> local.conf.sample file, runs the risk of that getting used in
> production images.
>
> By checking the per-image IMAGE_MODE, the debug tweaks only get
> enabled for images not meant for production.
>
> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
>  meta/conf/local.conf.sample | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
> index 85c5e21..edadbb7 100644
> --- a/meta/conf/local.conf.sample
> +++ b/meta/conf/local.conf.sample
> @@ -114,8 +114,9 @@ PACKAGE_CLASSES ?= "package_ipk"
>  #                     e.g. ssh root access has a blank password
>  # There are other application targets that can be used here too, see
>  # meta/classes/image.bbclass and meta/classes/core-image.bbclass for more details.
> -# We default to enabling the debugging tweaks.
> -EXTRA_IMAGE_FEATURES ?= "debug-tweaks"
> +# We default to enabling the debugging tweaks unless an image is explicitly
> +# requested to be built for production.
> +EXTRA_IMAGE_FEATURES ?= "${@ '' if 'production' == d.getVar('IMAGE_MODE') else 'debug-tweaks'}"

is IMAGE_MODE defined per image recipe ?

Re: [RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Patrick Ohly]

On Mon, 2017-05-15 at 08:50 -0700, Khem Raj wrote:
> On Mon, May 15, 2017 at 6:27 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > Enabling "debug-tweaks" unconditionally, even if it is only in the
> > local.conf.sample file, runs the risk of that getting used in
> > production images.
> >
> > By checking the per-image IMAGE_MODE, the debug tweaks only get
> > enabled for images not meant for production.
> >
> > Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> > ---
> >  meta/conf/local.conf.sample | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
> > index 85c5e21..edadbb7 100644
> > --- a/meta/conf/local.conf.sample
> > +++ b/meta/conf/local.conf.sample
> > @@ -114,8 +114,9 @@ PACKAGE_CLASSES ?= "package_ipk"
> >  #                     e.g. ssh root access has a blank password
> >  # There are other application targets that can be used here too, see
> >  # meta/classes/image.bbclass and meta/classes/core-image.bbclass for more details.
> > -# We default to enabling the debugging tweaks.
> > -EXTRA_IMAGE_FEATURES ?= "debug-tweaks"
> > +# We default to enabling the debugging tweaks unless an image is explicitly
> > +# requested to be built for production.
> > +EXTRA_IMAGE_FEATURES ?= "${@ '' if 'production' == d.getVar('IMAGE_MODE') else 'debug-tweaks'}"
> 
> is IMAGE_MODE defined per image recipe ?

Conceptually it is, although I guess it might get set globally in
practice.

The class just defines the empty string (= no specific mode) as ??=
default. Then a distro's local.conf sample can define a weak ?= default,
probably "development" (similar to the current practice of enabling
debug-tweaks in local.conf.sample). Finally, specific image recipes
(like a core-image-minimal-development.bb which includes
core-image-minimal.bb) can force a fixed mode with
IMAGE_MODE_forcevariable = "development".

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Khem Raj]

On Mon, May 15, 2017 at 12:18 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Mon, 2017-05-15 at 08:50 -0700, Khem Raj wrote:
>> On Mon, May 15, 2017 at 6:27 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> > Enabling "debug-tweaks" unconditionally, even if it is only in the
>> > local.conf.sample file, runs the risk of that getting used in
>> > production images.
>> >
>> > By checking the per-image IMAGE_MODE, the debug tweaks only get
>> > enabled for images not meant for production.
>> >
>> > Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
>> > ---
>> >  meta/conf/local.conf.sample | 5 +++--
>> >  1 file changed, 3 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
>> > index 85c5e21..edadbb7 100644
>> > --- a/meta/conf/local.conf.sample
>> > +++ b/meta/conf/local.conf.sample
>> > @@ -114,8 +114,9 @@ PACKAGE_CLASSES ?= "package_ipk"
>> >  #                     e.g. ssh root access has a blank password
>> >  # There are other application targets that can be used here too, see
>> >  # meta/classes/image.bbclass and meta/classes/core-image.bbclass for more details.
>> > -# We default to enabling the debugging tweaks.
>> > -EXTRA_IMAGE_FEATURES ?= "debug-tweaks"
>> > +# We default to enabling the debugging tweaks unless an image is explicitly
>> > +# requested to be built for production.
>> > +EXTRA_IMAGE_FEATURES ?= "${@ '' if 'production' == d.getVar('IMAGE_MODE') else 'debug-tweaks'}"
>>
>> is IMAGE_MODE defined per image recipe ?
>
> Conceptually it is, although I guess it might get set globally in
> practice.
>
> The class just defines the empty string (= no specific mode) as ??=
> default. Then a distro's local.conf sample can define a weak ?= default,
> probably "development" (similar to the current practice of enabling
> debug-tweaks in local.conf.sample). Finally, specific image recipes
> (like a core-image-minimal-development.bb which includes
> core-image-minimal.bb) can force a fixed mode with
> IMAGE_MODE_forcevariable = "development".

People use same distro with two different images for production and
devcelopment and conrols are inserted via IMAGE_FEATURES
with this change once effectively needs two different distros for prod and dev

Re: [RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Patrick Ohly]

On Mon, 2017-05-15 at 12:34 -0700, Khem Raj wrote:
> On Mon, May 15, 2017 at 12:18 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > On Mon, 2017-05-15 at 08:50 -0700, Khem Raj wrote:
> >> On Mon, May 15, 2017 at 6:27 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> >> > Enabling "debug-tweaks" unconditionally, even if it is only in the
> >> > local.conf.sample file, runs the risk of that getting used in
> >> > production images.
> >> >
> >> > By checking the per-image IMAGE_MODE, the debug tweaks only get
> >> > enabled for images not meant for production.
> >> >
> >> > Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> >> > ---
> >> >  meta/conf/local.conf.sample | 5 +++--
> >> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
> >> > index 85c5e21..edadbb7 100644
> >> > --- a/meta/conf/local.conf.sample
> >> > +++ b/meta/conf/local.conf.sample
> >> > @@ -114,8 +114,9 @@ PACKAGE_CLASSES ?= "package_ipk"
> >> >  #                     e.g. ssh root access has a blank password
> >> >  # There are other application targets that can be used here too, see
> >> >  # meta/classes/image.bbclass and meta/classes/core-image.bbclass for more details.
> >> > -# We default to enabling the debugging tweaks.
> >> > -EXTRA_IMAGE_FEATURES ?= "debug-tweaks"
> >> > +# We default to enabling the debugging tweaks unless an image is explicitly
> >> > +# requested to be built for production.
> >> > +EXTRA_IMAGE_FEATURES ?= "${@ '' if 'production' == d.getVar('IMAGE_MODE') else 'debug-tweaks'}"
> >>
> >> is IMAGE_MODE defined per image recipe ?
> >
> > Conceptually it is, although I guess it might get set globally in
> > practice.
> >
> > The class just defines the empty string (= no specific mode) as ??=
> > default. Then a distro's local.conf sample can define a weak ?= default,
> > probably "development" (similar to the current practice of enabling
> > debug-tweaks in local.conf.sample). Finally, specific image recipes
> > (like a core-image-minimal-development.bb which includes
> > core-image-minimal.bb) can force a fixed mode with
> > IMAGE_MODE_forcevariable = "development".
> 
> People use same distro with two different images for production and
> devcelopment and conrols are inserted via IMAGE_FEATURES
> with this change once effectively needs two different distros for prod and dev

Why that?

The production image recipe "foobar-image-production.bb" can use
IMAGE_MODE_forcevariable = "production"
and the development image recipe "foobar-image-development.bb" can use
IMAGE_MODE_forcevariable = "development".

Then whatever the user might configure in local.conf is ignored in favor
of the fixed recipe values. If there's a concern about using
_forcevariable: that could be addressed by configuring a global
IMAGE_MODE_DEFAULT ??= "" and an IMAGE_MODE ??= "${IMAGE_MODE_DEFAULT}"
in image-mode.bbclass and changing IMAGE_MODE_DEFAULT in distro or local
conf. Then individual recipes can set IMAGE_MODE =
"development/production" without having to fall back to _forcevariable.

Or do you mean that there's just one image .bb and traditionally
IMAGE_FEATURES were changed to switch back and forth? The same works
with IMAGE_MODE. The advantage over enabling or disabling dangerous
IMAGE_FEATURES is that users of a distro don't need to know about them.
They get the guarantee that (for a responsible distro) the dangerous
once will not get enabled by default for IMAGE_MODE=development.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Khem Raj]

On Mon, May 15, 2017 at 12:47 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> On Mon, 2017-05-15 at 12:34 -0700, Khem Raj wrote:
>> On Mon, May 15, 2017 at 12:18 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> > On Mon, 2017-05-15 at 08:50 -0700, Khem Raj wrote:
>> >> On Mon, May 15, 2017 at 6:27 AM, Patrick Ohly <patrick.ohly@intel.com> wrote:
>> >> > Enabling "debug-tweaks" unconditionally, even if it is only in the
>> >> > local.conf.sample file, runs the risk of that getting used in
>> >> > production images.
>> >> >
>> >> > By checking the per-image IMAGE_MODE, the debug tweaks only get
>> >> > enabled for images not meant for production.
>> >> >
>> >> > Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
>> >> > ---
>> >> >  meta/conf/local.conf.sample | 5 +++--
>> >> >  1 file changed, 3 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
>> >> > index 85c5e21..edadbb7 100644
>> >> > --- a/meta/conf/local.conf.sample
>> >> > +++ b/meta/conf/local.conf.sample
>> >> > @@ -114,8 +114,9 @@ PACKAGE_CLASSES ?= "package_ipk"
>> >> >  #                     e.g. ssh root access has a blank password
>> >> >  # There are other application targets that can be used here too, see
>> >> >  # meta/classes/image.bbclass and meta/classes/core-image.bbclass for more details.
>> >> > -# We default to enabling the debugging tweaks.
>> >> > -EXTRA_IMAGE_FEATURES ?= "debug-tweaks"
>> >> > +# We default to enabling the debugging tweaks unless an image is explicitly
>> >> > +# requested to be built for production.
>> >> > +EXTRA_IMAGE_FEATURES ?= "${@ '' if 'production' == d.getVar('IMAGE_MODE') else 'debug-tweaks'}"
>> >>
>> >> is IMAGE_MODE defined per image recipe ?
>> >
>> > Conceptually it is, although I guess it might get set globally in
>> > practice.
>> >
>> > The class just defines the empty string (= no specific mode) as ??=
>> > default. Then a distro's local.conf sample can define a weak ?= default,
>> > probably "development" (similar to the current practice of enabling
>> > debug-tweaks in local.conf.sample). Finally, specific image recipes
>> > (like a core-image-minimal-development.bb which includes
>> > core-image-minimal.bb) can force a fixed mode with
>> > IMAGE_MODE_forcevariable = "development".
>>
>> People use same distro with two different images for production and
>> devcelopment and conrols are inserted via IMAGE_FEATURES
>> with this change once effectively needs two different distros for prod and dev
>
> Why that?
>
> The production image recipe "foobar-image-production.bb" can use
> IMAGE_MODE_forcevariable = "production"
> and the development image recipe "foobar-image-development.bb" can use
> IMAGE_MODE_forcevariable = "development".
>
> Then whatever the user might configure in local.conf is ignored in favor
> of the fixed recipe values. If there's a concern about using
> _forcevariable: that could be addressed by configuring a global
> IMAGE_MODE_DEFAULT ??= "" and an IMAGE_MODE ??= "${IMAGE_MODE_DEFAULT}"
> in image-mode.bbclass and changing IMAGE_MODE_DEFAULT in distro or local
> conf. Then individual recipes can set IMAGE_MODE =
> "development/production" without having to fall back to _forcevariable.
>
> Or do you mean that there's just one image .bb and traditionally
> IMAGE_FEATURES were changed to switch back and forth? The same works
> with IMAGE_MODE. The advantage over enabling or disabling dangerous
> IMAGE_FEATURES is that users of a distro don't need to know about them.
> They get the guarantee that (for a responsible distro) the dangerous
> once will not get enabled by default for IMAGE_MODE=development.
>

IMAGE_MODE is a distro settings not image setting is that correct ?

Re: [RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Patrick Ohly]

On Mon, 2017-05-15 at 13:25 -0700, Khem Raj wrote:
> On Mon, May 15, 2017 at 12:47 PM, Patrick Ohly <patrick.ohly@intel.com> wrote:
> > The production image recipe "foobar-image-production.bb" can use
> > IMAGE_MODE_forcevariable = "production"
> > and the development image recipe "foobar-image-development.bb" can use
> > IMAGE_MODE_forcevariable = "development".

I'll probably automate the creation of these extra image variants via
BBCLASSEXTEND in an utility class. This seems to be of general
usefulness: base image with default IMAGE_MODE, one variant per valid
IMAGE_MODE where that mode is selected explicitly.

> > Then whatever the user might configure in local.conf is ignored in favor
> > of the fixed recipe values. If there's a concern about using
> > _forcevariable: that could be addressed by configuring a global
> > IMAGE_MODE_DEFAULT ??= "" and an IMAGE_MODE ??= "${IMAGE_MODE_DEFAULT}"
> > in image-mode.bbclass and changing IMAGE_MODE_DEFAULT in distro or local
> > conf. Then individual recipes can set IMAGE_MODE =
> > "development/production" without having to fall back to _forcevariable.
> >
> > Or do you mean that there's just one image .bb and traditionally
> > IMAGE_FEATURES were changed to switch back and forth? The same works
> > with IMAGE_MODE. The advantage over enabling or disabling dangerous
> > IMAGE_FEATURES is that users of a distro don't need to know about them.
> > They get the guarantee that (for a responsible distro) the dangerous
> > once will not get enabled by default for IMAGE_MODE=development.
> >
> 
> IMAGE_MODE is a distro settings not image setting is that correct ?

No, it is per-image. The code that checks the variable always runs in
individual image recipes.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 6/6] local.conf.sample: make debug-tweaks depend on IMAGE_MODE

[Patrick Ohly]

On Mon, 2017-05-15 at 15:27 +0200, Patrick Ohly wrote:
> Enabling "debug-tweaks" unconditionally, even if it is only in the
> local.conf.sample file, runs the risk of that getting used in
> production images.
> 
> By checking the per-image IMAGE_MODE, the debug tweaks only get
> enabled for images not meant for production.
> 
> Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
> ---
>  meta/conf/local.conf.sample | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
> index 85c5e21..edadbb7 100644
> --- a/meta/conf/local.conf.sample
> +++ b/meta/conf/local.conf.sample
> @@ -114,8 +114,9 @@ PACKAGE_CLASSES ?= "package_ipk"
>  #                     e.g. ssh root access has a blank password
>  # There are other application targets that can be used here too, see
>  # meta/classes/image.bbclass and meta/classes/core-image.bbclass for more details.
> -# We default to enabling the debugging tweaks.
> -EXTRA_IMAGE_FEATURES ?= "debug-tweaks"
> +# We default to enabling the debugging tweaks unless an image is explicitly
> +# requested to be built for production.

Would it help to add an additional comment here, like this?

# Note that this expression gets evaluated separately for each image
# and that IMAGE_MODE can get set differently for different images,
# even in the same build.

> +EXTRA_IMAGE_FEATURES ?= "${@ '' if 'production' == d.getVar('IMAGE_MODE') else 'debug-tweaks'}"

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 0/6] development vs. production builds

[Richard Purdie]

On Mon, 2017-05-15 at 15:26 +0200, Patrick Ohly wrote:
> At OEDAM [1] I took the AR to flesh out some of my ideas for
> introducing global and per-image settings for switching between
> development and production builds. The goal is partly to establish
> common configure options that then can be used by different layers,
> partly to have some actual useful functionality attached to them
> already in OE-core.
> 
> "development" builds are what a developer does when trying out a
> distro or working on his own personal device. "production" is what
> device manufacturer put onto the actual end-user hardware.
> 
> At OEDAM we already concluded that per-image settings are more
> useful. However, sometimes a component also has compile-time choices
> that cannot be changed later on in an image, and indeed I found one
> example for that (kmod) in OE-core.
> 
> Therefore I have included "debug-build" DISTRO_FEATURES support. It's
> a
> bit similar to manpages.bbclass, but in contrast to that (currently)
> is meant to be inherited globally - that's partly due to
> misunderstanding how manpages.bbclass was meant to be used. This can
> be changed, for now I just want to demonstrate that such a distro
> feature is not entirely useless, and what effect it could have
> already
> in OE-core.
> 
> In refkit, we also switch globally between development and production
> builds via .inc files. However, I am in the process of replacing that
> with the more flexible per-image IMAGE_MODE check. So from my
> perspective, the IMAGE_MODE is more important and useful than the
> "debug-build" DISTRO_FEATURE.
> 
> From a design perspective, the approach taken here is to let a
> developer or image define what mode it wants, and default features
> can
> be configured accordingly. That alternative would be to continue
> defining
> features as before and use the mode to configure QA warnings or
> errors.
> 
> But I find that less flexible, and I suspect it would be harder to
> keep track of what is meant to be usable in which mode. Developers
> also would have a harder time overriding the defaults.
> 
> [1] https://www.openembedded.org/wiki/OEDAM_2017

I'm not really sure I like this change, it seems fairly invasive and
complex for gains which don't seem to add up to me.

For example after your patchset, we end up with two different places
motd may be generated based on various flags. I think it would be
confusing for the user to find out where a message as coming from.

One of the reasons I have grown to hate "debug-tweaks" is that when you
enable it, you have no idea what you're really changing as the name
doesn't tell you what it does. I worry "debug-build" will become the
same problem, it doesn't do one thing well but many things badly. If it
really just changes debug and logging PACKAGECONFIG (if present), why
doesn't the distro just do that?

I also worry about the number of classes this introduces. Whilst
classes are cheap and easy, I have heard concerns we have too many of
them. In this case image-mode gets hardcoded into image.bbclass.

Also, have you seen bb.utils.filter()? It might be useful instead of
some of your .intersection() code?

I do think you're right that we need to make some changes in this area
but overall this patchset seems too complex to me somehow and I think
we probably can achieve something similar in a simpler way...

Cheers,

Richard

Re: [RFC][PATCH 0/6] development vs. production builds

[Mike Looijmans]

This seems to assume that every project is similar and that develop vs 
production is something well defined. Well, it's not.

I for one would welcome the demise of debug-tweaks. It's an option that 
enforces one of two policies that may both be wrong.

For example, if the target is a settop box where users tend to port-forward 
the SSH port to the internet, and which is open so that they're expected to 
log in and make changes, the policy we want is that there is initially no root 
password, and that SSH access requires a valid password. This makes the system 
secure against outside attacks. Setting debug-tweaks will allow blank SSH 
passwords, while not setting it would invalidate the root login. Neither is 
acceptable for this project, so one has to resort to overriding the image 
class methods zap_root_password and ssh_allow_empty_password to do nothing.

Instead of some obscure switch with implications hidden somewhere in various 
recipes, classes, distros and local configs, can't we just define options that 
actually do what they say, like for example a boolean option 
"ssh_allow_empty_password"?

While the "development/production" switch may be great for some projects, 
it'll make things only more complicated for others while gaining nothing above 
what we have now.



On 15-05-17 15:26, Patrick Ohly wrote:
> At OEDAM [1] I took the AR to flesh out some of my ideas for
> introducing global and per-image settings for switching between
> development and production builds. The goal is partly to establish
> common configure options that then can be used by different layers,
> partly to have some actual useful functionality attached to them
> already in OE-core.
>
> "development" builds are what a developer does when trying out a
> distro or working on his own personal device. "production" is what
> device manufacturer put onto the actual end-user hardware.
>
> At OEDAM we already concluded that per-image settings are more
> useful. However, sometimes a component also has compile-time choices
> that cannot be changed later on in an image, and indeed I found one
> example for that (kmod) in OE-core.
>
> Therefore I have included "debug-build" DISTRO_FEATURES support. It's a
> bit similar to manpages.bbclass, but in contrast to that (currently)
> is meant to be inherited globally - that's partly due to
> misunderstanding how manpages.bbclass was meant to be used. This can
> be changed, for now I just want to demonstrate that such a distro
> feature is not entirely useless, and what effect it could have already
> in OE-core.
>
> In refkit, we also switch globally between development and production
> builds via .inc files. However, I am in the process of replacing that
> with the more flexible per-image IMAGE_MODE check. So from my
> perspective, the IMAGE_MODE is more important and useful than the
> "debug-build" DISTRO_FEATURE.
>
> From a design perspective, the approach taken here is to let a
> developer or image define what mode it wants, and default features can
> be configured accordingly. That alternative would be to continue defining
> features as before and use the mode to configure QA warnings or errors.
>
> But I find that less flexible, and I suspect it would be harder to
> keep track of what is meant to be usable in which mode. Developers
> also would have a harder time overriding the defaults.
>
> [1] https://www.openembedded.org/wiki/OEDAM_2017
>
> Patrick Ohly (6):
>   build-mode.bbclass: distro-wide debug-build mode
>   basefiles: warn about non-production DISTRO_FEATURES in motd
>   defaultsetup.conf: enable special "debug-build" DISTRO_FEATURES support
>   image-mode.bbclass: per-image production/development/debug mode
>   image.bbclass: include IMAGE_MODE support
>   local.conf.sample: make debug-tweaks depend on IMAGE_MODE
>
>  meta/classes/build-mode.bbclass                   | 16 ++++-
>  meta/classes/image-mode.bbclass                   | 62 ++++++++++++++++-
>  meta/classes/image.bbclass                        |  3 +-
>  meta/conf/distro/defaultsetup.conf                |  2 +-
>  meta/conf/local.conf.sample                       |  5 +-
>  meta/recipes-core/base-files/base-files_3.0.14.bb | 11 +++-
>  6 files changed, 96 insertions(+), 3 deletions(-)
>  create mode 100644 meta/classes/build-mode.bbclass
>  create mode 100644 meta/classes/image-mode.bbclass
>
> base-commit: 9f9ebf2e1ba6eda48fb5e3f20d4ca5bbabe3dad4
>



Kind regards,

Mike Looijmans
System Expert

TOPIC Products
Materiaalweg 4, NL-5681 RJ Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail



Join our presentation at Electronics & Applications 2017:
FPGA for real-time data processing, subject “Hardware platform for industrial ultrasound steel plate Inspection” Topic Embedded Systems - Herman Kuster, 1st June 10 AM

Visit http://eabeurs.nl/author/612884/ for more information

Re: [RFC][PATCH 0/6] development vs. production builds

[Patrick Ohly]

On Tue, 2017-05-16 at 08:29 +0100, Richard Purdie wrote:
> I'm not really sure I like this change, it seems fairly invasive and
> complex for gains which don't seem to add up to me.
> 
> For example after your patchset, we end up with two different places
> motd may be generated based on various flags. I think it would be
> confusing for the user to find out where a message as coming from.

Fair enough. I'm not a big fan of the "debug-build" DISTRO_FEATURES
either, and just included it as reference for what could be done at the
distro level.

What about just patch 4, the one which introduces image-mode.bbclass? Is
that of enough interest to carry it in OE-core? With or without enabling
it by default for images (patch 5)?

That classes currently defines three modes in IMAGE_MODE_VALID, but
intentionally with the weakest default possible. Distros can override
that to match their intended usage modes, or (better, now that I think
about it) we make the default empty and when users try to set IMAGE_MODE
(because they might have seen it in some other distro) they get an error
(assuming that we inherit the class in image.bbclass).

My concern with not having this class in OE-core is that each distro
will have to come up with their own way of doing something like this,
with no consistency between distros. That is going to confuse some
users. If a distro does not want this feature, they can just ignore the
class, once we make IMAGE_MODE_VALID empty by default.

> One of the reasons I have grown to hate "debug-tweaks" is that when you
> enable it, you have no idea what you're really changing as the name
> doesn't tell you what it does.

I'm fine with eliminating "debug-tweaks" entirely. In that case,
local.conf.sample should just list the individual features directly?

>  I worry "debug-build" will become the
> same problem, it doesn't do one thing well but many things badly. If it
> really just changes debug and logging PACKAGECONFIG (if present), why
> doesn't the distro just do that?

The idea was that the distro or user can set a global flag without
having to know all these details. For example, consider the case where a
user adds a custom recipe to a distro. If the distro has a
"development.inc" and "production.inc" with PACKAGECONFIGs that tune
recipes to match the intended usage, then those lists won't configure
the custom recipe. 

> I also worry about the number of classes this introduces. Whilst
> classes are cheap and easy, I have heard concerns we have too many of
> them. In this case image-mode gets hardcoded into image.bbclass.

We could merge the content of image-mode.bbclass also into
image.bbclass, if there is consensus that this code should be active by
default.

I've done it differently here because I wanted to have the ability to
use the standalone image-mode.bbclass in refkit.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 0/6] development vs. production builds

[Patrick Ohly]

On Tue, 2017-05-16 at 09:35 +0200, Mike Looijmans wrote:
> This seems to assume that every project is similar and that develop vs 
> production is something well defined. Well, it's not.
> 
> I for one would welcome the demise of debug-tweaks. It's an option that 
> enforces one of two policies that may both be wrong.

If that's an option, then I'm also for it, and I don't mind dropping the
"debug-build" distro feature.

> While the "development/production" switch may be great for some projects, 
> it'll make things only more complicated for others while gaining nothing above 
> what we have now.

What about the approach I outlined in my reponse to Richard, where we
just introduce the IMAGE_MODE mechanism in OE-core without defining
specific modes? Would you find that useful?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 0/6] development vs. production builds

[Alexander Kanavin]

On 05/16/2017 11:21 AM, Patrick Ohly wrote:

>> While the "development/production" switch may be great for some projects,
>> it'll make things only more complicated for others while gaining nothing above
>> what we have now.
>
> What about the approach I outlined in my reponse to Richard, where we
> just introduce the IMAGE_MODE mechanism in OE-core without defining
> specific modes? Would you find that useful?

Please no. Global variables are a headache, and the less we have of them 
the better.

Here in particularly, what is wrong with defining:

patricks-awesome-image-developers.bb
(containing IMAGE_FEATURE = "no-password-for-anything")

patricks-awesome-image-production.bb
(containing IMAGE_FEATURE = "serious-security")

and the common bits can go to patricks-awesome-image.inc.

It's simple, direct and self-explanatory. At the end of the day, you 
*always* need to drill down to the code that does the work, and learn 
the details, to truly understand what is happening behind a 
configuration setting, and adding abstraction layers and scattering 
functionality over many different files does not help - quite the opposite.

Alex

Re: [RFC][PATCH 0/6] development vs. production builds

[Patrick Ohly]

On Tue, 2017-05-16 at 14:49 +0300, Alexander Kanavin wrote:
> On 05/16/2017 11:21 AM, Patrick Ohly wrote:
> 
> >> While the "development/production" switch may be great for some projects,
> >> it'll make things only more complicated for others while gaining nothing above
> >> what we have now.
> >
> > What about the approach I outlined in my reponse to Richard, where we
> > just introduce the IMAGE_MODE mechanism in OE-core without defining
> > specific modes? Would you find that useful?
> 
> Please no. Global variables are a headache, and the less we have of them 
> the better.
>
> Here in particularly, what is wrong with defining:
> 
> patricks-awesome-image-developers.bb
> (containing IMAGE_FEATURE = "no-password-for-anything")
> 
> patricks-awesome-image-production.bb
> (containing IMAGE_FEATURE = "serious-security")
> 
> and the common bits can go to patricks-awesome-image.inc.

Then why is not already done like that in practice? Is it just because
OE-core and Poky set such a bad precedence with teaching developers to
add EXTRA_IMAGE_FEATURES ?= "debug-tweaks" to make the images usable,
and then that approach gets copied?

I think everyone agrees that removing "debug-tweaks" would be a good
idea. But completely removing the global (sic!) EXTRA_IMAGE_FEATURES in
local.conf.sample would go even further, and I am not sure how the
reactions to that would be. I suspect there are people who find it
useful to have just one image recipe that gets build in different
configurations (dangerous and not so dangerous).

Feel free to prepare and propose a patch that implements the idea above
for OE-core; I personally won't, I've had enough negative feedback for
this week already ;-}

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 0/6] development vs. production builds

[Alexander Kanavin]

On 05/16/2017 04:47 PM, Patrick Ohly wrote:

> Then why is not already done like that in practice? Is it just because
> OE-core and Poky set such a bad precedence with teaching developers to
> add EXTRA_IMAGE_FEATURES ?= "debug-tweaks" to make the images usable,
> and then that approach gets copied?

It is done like that already, it's just not very consistent from what I 
can see. For example, core-image-sato-dev.bb:
=============
require core-image-sato.bb

DESCRIPTION = "Image with Sato for development work. It includes 
everything \
within core-image-sato plus a native toolchain, application development 
and \
testing libraries, profiling and debug symbols."

IMAGE_FEATURES += "dev-pkgs"
=============

> I think everyone agrees that removing "debug-tweaks" would be a good
> idea. But completely removing the global (sic!) EXTRA_IMAGE_FEATURES in
> local.conf.sample would go even further, and I am not sure how the
> reactions to that would be. I suspect there are people who find it
> useful to have just one image recipe that gets build in different
> configurations (dangerous and not so dangerous).

I'm not sure either, but I think that's not actually a bad idea - 
dropping support for it altogether. :)

I'm not a big fan of placing INHERIT into local.conf either, by the way. 
I believe in functional programming principles, and this goes directly 
against them.

Alex

Re: [RFC][PATCH 0/6] development vs. production builds

[Patrick Ohly]

On Tue, 2017-05-16 at 17:02 +0300, Alexander Kanavin wrote:
> On 05/16/2017 04:47 PM, Patrick Ohly wrote:
> 
> > Then why is not already done like that in practice? Is it just because
> > OE-core and Poky set such a bad precedence with teaching developers to
> > add EXTRA_IMAGE_FEATURES ?= "debug-tweaks" to make the images usable,
> > and then that approach gets copied?
> 
> It is done like that already, it's just not very consistent from what I 
> can see. For example, core-image-sato-dev.bb:
> =============
> require core-image-sato.bb
> 
> DESCRIPTION = "Image with Sato for development work. It includes 
> everything \
> within core-image-sato plus a native toolchain, application development 
> and \
> testing libraries, profiling and debug symbols."
> 
> IMAGE_FEATURES += "dev-pkgs"
> =============

That's different. Here an image recipe specifies what it is meant to
*contain*, not how it is meant to *behave*.

One would need core-image-sato-dev-production.bb (no debug-tweaks,
dev-pkgs), core-image-sato-dev-debug.bb (debug-tweaks, dev-pkgs),
core-image-sato-production.bb (no debug-tweaks, no dev-pkgs),
core-image-sato-debug.bb (debug-tweaks, no dev-pkgs).

Allowing EXTRA_IMAGE_FEATURES in local.conf.sample avoids that.

> I'm not a big fan of placing INHERIT into local.conf either, by the way. 
> I believe in functional programming principles, and this goes directly 
> against them.

It makes sense to me when the functionality is orthogonal to the
content, like enabling buildhistory.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [RFC][PATCH 0/6] development vs. production builds

[Alexander Kanavin]

On 05/16/2017 05:25 PM, Patrick Ohly wrote:
>>
>> It is done like that already, it's just not very consistent from what I
>> can see. For example, core-image-sato-dev.bb:
>> =============
>> require core-image-sato.bb
>>
>> DESCRIPTION = "Image with Sato for development work. It includes
>> everything \
>> within core-image-sato plus a native toolchain, application development
>> and \
>> testing libraries, profiling and debug symbols."
>>
>> IMAGE_FEATURES += "dev-pkgs"
>> =============
>
> That's different. Here an image recipe specifies what it is meant to
> *contain*, not how it is meant to *behave*.

You can totally specify behavioral IMAGE_FEATURES in image recipes. The 
distinction is not formally specified or enforced anywhere, as far as I 
know.

> One would need core-image-sato-dev-production.bb (no debug-tweaks,
> dev-pkgs), core-image-sato-dev-debug.bb (debug-tweaks, dev-pkgs),
> core-image-sato-production.bb (no debug-tweaks, no dev-pkgs),
> core-image-sato-debug.bb (debug-tweaks, no dev-pkgs).
>
> Allowing EXTRA_IMAGE_FEATURES in local.conf.sample avoids that.

Consider that the suffix in image recipe name is essentially same as 
your IMAGE_MODE idea. It specifies who is the target audience for the 
image. Then you can avoid combinatorial explosion, if you define who the 
image is meant for:

someimage-developers.bb (debug tweaks, dev-pkgs)
someimage-endusers.bb (no debug tweaks, no dev-pkgs)

>> I'm not a big fan of placing INHERIT into local.conf either, by the way.
>> I believe in functional programming principles, and this goes directly
>> against them.
>
> It makes sense to me when the functionality is orthogonal to the
> content, like enabling buildhistory.

Unfortunately, there is no way to enforce orthogonality. The door is 
wide open for awful hacks here.

Alex

[PATCH v2 0/1] development vs. production builds

[Patrick Ohly]

The previous revision of this patch series showed what could be done
at a distro level and image level. It was not suitable for OE-core
because it encoded too specific use-cases and the usefulness of a
distro-level switch is questionable.

The image-level mode switching however seems useful. This revision
focuses on just that aspect. Instead of pre-defining certain modes and
their meaning, only the generic infrastructure is left.

It's still a separate class for reasons given in the commit
message. That could be changed.

Changes in V2:
- dropped "build-mode" DISTRO_FEATURES
- moved "development/debug/production" modes from actual code into
  comments

Patrick Ohly (1):
  image-mode.bbclass: common infrastructure for choosing image defaults

 meta/classes/image-mode.bbclass | 66 ++++++++++++++++++++++++++++++++++-
 meta/classes/image.bbclass      |  2 +-
 2 files changed, 68 insertions(+)
 create mode 100644 meta/classes/image-mode.bbclass

base-commit: c59fa3bd71b42410bf032846ee8fdb6e6eb1b95c
-- 
git-series 0.9.1

[PATCH v2 1/1] image-mode.bbclass: common infrastructure for choosing image defaults

[Patrick Ohly]

A distro might want to offer developers different, pre-defined ways of
building an image, for example a "development" mode with easy (but
insecure) login methods and a "production" mode with hardened
settings.

image-mode.bbclass introduces IMAGE_MODE as the switch that defines
how developers want to build an image. It can be set differently for
different images. The intention is that the image recipe checks
IMAGE_MODE when setting the default IMAGE_FEATURES or whether
pre-generated signing keys are to be used for features like dm-verity.

All functionality is opt-in: if a distro does not change the defaults,
then the class only warns when a developer sets IMAGE_MODE althought
it isn't supported. The purpose of having this class in OE-core
although it doesn't do much by default is to achieve consistent
behavior between different distros.

Distros can define different modes (if desired, even differently for
different images) and can define additional text that gets appended to
/etc/motd of an image when a certain mode was used for the image.

The functionality is provided as a separate helper class to ease
copying of the functionality into distros not based on OE-core master
yet and to keep it in one place. The alternative is to copy the code
into image.bbclass and rootfs-postcommands.bbclass.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/image-mode.bbclass | 66 ++++++++++++++++++++++++++++++++++-
 meta/classes/image.bbclass      |  2 +-
 2 files changed, 68 insertions(+)
 create mode 100644 meta/classes/image-mode.bbclass

diff --git a/meta/classes/image-mode.bbclass b/meta/classes/image-mode.bbclass
new file mode 100644
index 0000000..6137010
--- /dev/null
+++ b/meta/classes/image-mode.bbclass
@@ -0,0 +1,66 @@
+# Some distros may want to build images in different modes ("development",
+# "production", etc.). The same image recipe might get built differently
+# based on local configuration, or different image recipes might inherit
+# from the same base and just differ in their mode. This class introduces
+# "IMAGE_MODE" as the per-image variable which controls this mode.
+#
+# IMAGE_MODE is intentionally a separate variable. This way, IMAGE_FEATURES
+# can be set dynamically depending on IMAGE_MODE.
+#
+# There are no pre-defined modes. Distros which want to use modes
+# must set the IMAGE_MODE_VALID variable, either globally or per image.
+# Distros which don't have modes can still use the class to trigger
+# an error message when developers set the variable although it doesn't
+# have an effect.
+
+# The default is to have no fixed mode.
+IMAGE_MODE ??= ""
+
+# Set this in the distro or image to enable mode support. For example:
+# IMAGE_MODE_VALID ??= "production development debugging"
+
+# Optionally this class extends /etc/motd in each image depending on the
+# IMAGE_MODE of the image. To use this, set IMAGE_MODE_MOTD[<image mode>]
+# to a string for that mode or IMAGE_MODE_MOTD[none] for empty or
+# unset IMAGE_MODE. IMAGE_MODE_MOTD is used as default when the varflag
+# is not set.
+#
+# When the motd text is empty, /etc/motd is not touched at all.
+#
+# Example:
+# IMAGE_MODE_MOTD_NOT_PRODUCTION () {
+# *********************************************
+# *** This is a ${IMAGE_MODE} image! ${@ ' ' * (19 - len(d.getVar('IMAGE_MODE')))} ***
+# *** Do not use in production.             ***
+# *********************************************
+# }
+# IMAGE_MODE_MOTD = "${IMAGE_MODE_MOTD_NOT_PRODUCTION}"
+# IMAGE_MODE_MOTD[production] = ""
+
+# Empty when IMAGE_MODE is unset, otherwise -<IMAGE_MODE>.
+IMAGE_MODE_SUFFIX = "${@ '-' + d.getVar('IMAGE_MODE') if d.getVar('IMAGE_MODE') else '' }"
+
+python () {
+    # Sanity checks for IMAGE_MODE.
+    image_mode = d.getVar('IMAGE_MODE')
+    mode = set(image_mode.split())
+    if len(mode) == 0:
+        return
+    if len(mode) > 1:
+        bb.fatal('An image can only be built in exactly one mode: IMAGE_MODE=%s' % image_mode)
+    mode = mode.pop()
+    valid = d.getVar('IMAGE_MODE_VALID') or ''
+    if mode not in valid.split():
+        bb.fatal('Invalid image mode: IMAGE_MODE=%s (not in %s)' % (image_mode, valid))
+}
+
+python image_mode_motd () {
+    image_mode = d.getVar('IMAGE_MODE')
+    motd = d.getVarFlag('IMAGE_MODE_MOTD', image_mode or 'none')
+    if motd is None:
+        motd = d.getVar('IMAGE_MODE_MOTD')
+    if motd:
+        with open(d.expand('${IMAGE_ROOTFS}${sysconfdir}/motd'), 'a') as f:
+            f.write(motd)
+}
+ROOTFS_POSTPROCESS_COMMAND += "image_mode_motd;"
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 405fd73..b381d53 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -204,7 +204,9 @@ LINGUAS_INSTALL ?= "${@" ".join(map(lambda s: "locale-base-%s" % s, d.getVar('IM
 # aren't yet available.
 PSEUDO_PASSWD = "${IMAGE_ROOTFS}:${STAGING_DIR_NATIVE}"
 
+# Inherit helper classes.
 inherit rootfs-postcommands
+inherit image-mode
 
 PACKAGE_EXCLUDE ??= ""
 PACKAGE_EXCLUDE[type] = "list"
-- 
git-series 0.9.1

Re: [PATCH v2 1/1] image-mode.bbclass: common infrastructure for choosing image defaults

[Patrick Ohly]

On Wed, 2017-05-17 at 09:58 +0200, Patrick Ohly wrote:
[...]
> +    valid = d.getVar('IMAGE_MODE_VALID') or ''
> +    if mode not in valid.split():
> +        bb.fatal('Invalid image mode: IMAGE_MODE=%s (not in %s)' % (image_mode, valid))
> +}

A dedicated error message for the "IMAGE_MODE_VALID is empty" case is
needed. But let me wait for reactions before posting a v3.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2 1/1] image-mode.bbclass: common infrastructure for choosing image defaults

[Alexander Kanavin]

On 05/17/2017 10:58 AM, Patrick Ohly wrote:
> A distro might want to offer developers different, pre-defined ways of
> building an image, for example a "development" mode with easy (but
> insecure) login methods and a "production" mode with hardened
> settings.

Apologies Patrick, I still think this brings more complication than 
benefits. And it's not even showcased anywhere in poky: you add the 
variable, but don't take it into use anywhere. Which means it also won't 
get tested.

You can do this thing by creating separate image recipes, each with 
their hand picked IMAGE_FEATURES. And that includes showing appropriate 
motd as well - but that perhaps would not be necessary because the image 
mode would be encoded in its file name.


Alex

Re: [PATCH v2 1/1] image-mode.bbclass: common infrastructure for choosing image defaults

[Patrick Ohly]

On Wed, 2017-05-17 at 12:49 +0300, Alexander Kanavin wrote:
> On 05/17/2017 10:58 AM, Patrick Ohly wrote:
> > A distro might want to offer developers different, pre-defined ways of
> > building an image, for example a "development" mode with easy (but
> > insecure) login methods and a "production" mode with hardened
> > settings.
> 
> Apologies Patrick, I still think this brings more complication than 
> benefits. And it's not even showcased anywhere in poky: you add the 
> variable, but don't take it into use anywhere. Which means it also won't 
> get tested.
>
> You can do this thing by creating separate image recipes, each with 
> their hand picked IMAGE_FEATURES. And that includes showing appropriate 
> motd as well - but that perhaps would not be necessary because the image 
> mode would be encoded in its file name.

Can you be specific? Which image recipes should I add?

The feature by design is now meant to be used by distros. OE-core
doesn't contain a distro definition, so the feature cannot really be
used in OE-core in a realistic, reusable way. I can write a selftest
with a custom distro or image configuration, if that addresses your
concern about not getting the code tested.

I know that you prefer defining more image recipes over allowing the
reconfiguration of the same image recipe. I disagree on that, because
when you have independent aspects (like content and login
configuration), then you end up with various combinations of those
configuration options. Writing down all combinations in pre-defined
image recipes just doesn't scale. But you are welcome to proof me wrong
by showing how the existing image recipes in OE-core should be changed
so that they not only cover different content selection, but also what's
currently done via EXTRA_IMAGE_FEATURES in local.conf.sample.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2 1/1] image-mode.bbclass: common infrastructure for choosing image defaults

[Alexander Kanavin]

On 05/17/2017 01:47 PM, Patrick Ohly wrote:

> I know that you prefer defining more image recipes over allowing the
> reconfiguration of the same image recipe. I disagree on that, because
> when you have independent aspects (like content and login
> configuration), then you end up with various combinations of those
> configuration options. Writing down all combinations in pre-defined
> image recipes just doesn't scale. But you are welcome to proof me wrong
> by showing how the existing image recipes in OE-core should be changed
> so that they not only cover different content selection, but also what's
> currently done via EXTRA_IMAGE_FEATURES in local.conf.sample.

I disagree that it doesn't scale. It scales just fine with a 
well-constructed include files hierarchy. I'd even argue that's the only 
way to stay sane when there's more than a few product configurations:

all-products.inc
(things common to all products)

product1.inc
(things specific for product1)

product2.inc
(things specific for product2)

production.inc
(where serious security is configured)

development.inc
(where things are made easy for developers)

and then you have

product1-development.bb
====
include product1.inc
include development.inc
====

product2-development.bb
====
include product2.inc
include development.inc
====

product1-production.bb
====
include product1.inc
include production.inc
====

product2-production.bb
====
include product2.inc
include production.inc
====

This pattern is used throughout openembedded. Take a look at machine or 
distro configurations for example, or how both python 2 and 3 can be 
supported with a single recipe include and two very short recipes in 
meta-python. What does IMAGE_MODE do that include files cannot?

Alex

Re: [PATCH v2 1/1] image-mode.bbclass: common infrastructure for choosing image defaults

[Patrick Ohly]

On Wed, 2017-05-17 at 15:56 +0300, Alexander Kanavin wrote:
> On 05/17/2017 01:47 PM, Patrick Ohly wrote:
> 
> > I know that you prefer defining more image recipes over allowing the
> > reconfiguration of the same image recipe. I disagree on that, because
> > when you have independent aspects (like content and login
> > configuration), then you end up with various combinations of those
> > configuration options. Writing down all combinations in pre-defined
> > image recipes just doesn't scale. But you are welcome to proof me wrong
> > by showing how the existing image recipes in OE-core should be changed
> > so that they not only cover different content selection, but also what's
> > currently done via EXTRA_IMAGE_FEATURES in local.conf.sample.
> 
> I disagree that it doesn't scale. It scales just fine with a 
> well-constructed include files hierarchy. I'd even argue that's the only 
> way to stay sane when there's more than a few product configurations:

[...]

Now add one more mode and you end up with six instead of two image
recipes. That's what I consider not scalable.

> This pattern is used throughout openembedded. Take a look at machine or 
> distro configurations for example, or how both python 2 and 3 can be 
> supported with a single recipe include and two very short recipes in 
> meta-python. What does IMAGE_MODE do that include files cannot?

It does the same thing in a different way. What is preferred is
subjective.

I think we've both made our opinion clear. Let's hear from others.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2 1/1] image-mode.bbclass: common infrastructure for choosing image defaults

[Alexander Kanavin]

On 05/17/2017 04:39 PM, Patrick Ohly wrote:
> [...]
>
> Now add one more mode and you end up with six instead of two image
> recipes. That's what I consider not scalable.

Wait a moment. The contents of recipe files is short, regular and can be 
easily produced (or re-produced) by a script from the two lists of 
products and modes. You never even need to look at them, let alone edit 
them. The total amount of these recipe files is products*modes - there 
is no combinatorial explosion. What does not scale here?

Alex