[PATCH v3 1/3] patchtest: use HEAD commit as base for the selftest and not master

[PATCH v3 1/3] patchtest: use HEAD commit as base for the selftest and not master

[Alexander Kanavin]

From: Alexander Kanavin <alex@linutronix.de>

Patchtest applies patches on top of poky master branch by default;
this means selftest does the same, and any commits from the branch-under-test
are then discarded.

This can cause issues for example, if bitbake-server process started by selftest
from the master branch tries to parse bitbake.conf from the branch under test:
https://valkyrie.yoctoproject.org/#/builders/71/builds/460

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/lib/patchtest/selftest/selftest | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/patchtest/selftest/selftest b/meta/lib/patchtest/selftest/selftest
index 6fad50ce616..3cf1c361f7f 100755
--- a/meta/lib/patchtest/selftest/selftest
+++ b/meta/lib/patchtest/selftest/selftest
@@ -38,7 +38,7 @@ def test(root, patch):
     res = True
     patchpath = os.path.abspath(os.path.join(root, patch))
     
-    cmd     = 'patchtest --repodir %s --testdir %s/tests --patch %s' % (repodir, topdir, patchpath)
+    cmd     = 'patchtest --base-commit HEAD --repodir %s --testdir %s/tests --patch %s' % (repodir, topdir, patchpath)
     results = subprocess.check_output(cmd, stderr=subprocess.STDOUT, universal_newlines=True, shell=True)
 
     return results
-- 
2.39.5

[PATCH v3 2/3] bitbake.conf: add an addfragments directive for oe-core and dependent layers

[Alexander Kanavin]

From: Alexander Kanavin <alex@linutronix.de>

Please see the patch to bitbake for syntax and implementation details.

The path prefix to fragments is in its own variable so it doesn't have
to be hardcoded into tools.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/conf/bitbake.conf | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 02bbf0e7a52..b6013f66e26 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -836,6 +836,8 @@ include conf/documentation.conf
 include conf/licenses.conf
 require conf/sanity.conf
 include conf/bblock.conf
+OE_FRAGMENTS_PREFIX ?= "conf/fragments"
+addfragments ${OE_FRAGMENTS_PREFIX} OE_FRAGMENTS
 
 ##################################################################
 # Weak variables (usually to retain backwards compatibility)
-- 
2.39.5

[PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Alexander Kanavin]

From: Alexander Kanavin <alex@linutronix.de>

This allows fine-tuning local configurations with pre-frabricated
configuration snippets in a structured, controlled way. It's also
an important building block for bitbake-setup.

There are three (and a half) operations (list/enable/disable/disable all), and here's the 'list' output:

alex@Zen2:/srv/storage/alex/yocto/build-64$ bitbake-config-build list-fragments
NOTE: Starting bitbake server...
Available fragments in selftest layer located in /srv/work/alex/poky/meta-selftest:

selftest/test-fragment	(disabled)	This is a configuration fragment intended for testing in oe-selftest context
selftest/more-fragments-here/test-another-fragment	(disabled)	This is a second configuration fragment intended for testing in oe-selftest context

The tool requires that each fragment contains a one-line summary, and one or more
lines of description, as BB_CONF_FRAGMENT_SUMMARY[layerid/fragmentname] style metadata.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 .../test-another-fragment.conf                |   3 +
 .../conf/fragments/test-fragment.conf         |   3 +
 meta/lib/bbconfigbuild/configfragments.py     | 147 ++++++++++++++++++
 meta/lib/oeqa/selftest/cases/bblayers.py      |  31 ++++
 4 files changed, 184 insertions(+)
 create mode 100644 meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf
 create mode 100644 meta-selftest/conf/fragments/test-fragment.conf
 create mode 100644 meta/lib/bbconfigbuild/configfragments.py

diff --git a/meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf b/meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf
new file mode 100644
index 00000000000..cf9ba6a6132
--- /dev/null
+++ b/meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf
@@ -0,0 +1,3 @@
+BB_CONF_FRAGMENT_SUMMARY[selftest/more-fragments-here/test-another-fragment] = "This is a second configuration fragment intended for testing in oe-selftest context"
+BB_CONF_FRAGMENT_DESCRIPTION[selftest/more-fragments-here/test-another-fragment] = "It defines another variable that can be checked inside the test."
+SELFTEST_FRAGMENT_ANOTHER_VARIABLE = "someothervalue"
diff --git a/meta-selftest/conf/fragments/test-fragment.conf b/meta-selftest/conf/fragments/test-fragment.conf
new file mode 100644
index 00000000000..63ebc1fca68
--- /dev/null
+++ b/meta-selftest/conf/fragments/test-fragment.conf
@@ -0,0 +1,3 @@
+BB_CONF_FRAGMENT_SUMMARY[selftest/test-fragment] = "This is a configuration fragment intended for testing in oe-selftest context"
+BB_CONF_FRAGMENT_DESCRIPTION[selftest/test-fragment] = "It defines a variable that can be checked inside the test."
+SELFTEST_FRAGMENT_VARIABLE = "somevalue"
diff --git a/meta/lib/bbconfigbuild/configfragments.py b/meta/lib/bbconfigbuild/configfragments.py
new file mode 100644
index 00000000000..39031b43435
--- /dev/null
+++ b/meta/lib/bbconfigbuild/configfragments.py
@@ -0,0 +1,147 @@
+#
+# Copyright OpenEmbedded Contributors
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+
+import logging
+import os
+import sys
+import os.path
+
+import bb.utils
+
+from bblayers.common import LayerPlugin
+
+logger = logging.getLogger('bitbake-config-layers')
+
+sys.path.insert(0, os.path.dirname(os.path.dirname(__file__)))
+
+def plugin_init(plugins):
+    return ConfigFragmentsPlugin()
+
+class ConfigFragmentsPlugin(LayerPlugin):
+    def get_fragment_info(self, path, name):
+        d = bb.data.init()
+        bb.parse.handle(path, d, True)
+        summary = d.getVarFlag('BB_CONF_FRAGMENT_SUMMARY', name)
+        description = d.getVarFlag('BB_CONF_FRAGMENT_DESCRIPTION', name)
+        if not summary:
+            raise Exception('Please add a one-line summary as BB_CONF_FRAGMENT_SUMMARY[{}] = \"...\" variable at the beginning of {}'.format(name, path))
+
+        if not description:
+            raise Exception('Please add a description as BB_CONF_FRAGMENT_DESCRIPTION[{}] = \"...\" variable at the beginning of {}'.format(name, path))
+
+        return summary, description
+
+    def discover_fragments(self):
+        fragments_path_prefix = self.tinfoil.config_data.getVar('OE_FRAGMENTS_PREFIX')
+        allfragments = {}
+        for layername in self.bbfile_collections:
+             layerdir = self.bbfile_collections[layername]
+             fragments = []
+             for topdir, dirs, files in os.walk(os.path.join(layerdir, fragments_path_prefix)):
+                 fragmentdir = os.path.relpath(topdir, os.path.join(layerdir, fragments_path_prefix))
+                 for fragmentfile in sorted(files):
+                     fragmentname = os.path.normpath("/".join((layername, fragmentdir, fragmentfile.split('.')[0])))
+                     fragmentpath = os.path.join(topdir, fragmentfile)
+                     fragmentsummary, fragmentdesc = self.get_fragment_info(fragmentpath, fragmentname)
+                     fragments.append({'path':fragmentpath, 'name':fragmentname, 'summary':fragmentsummary, 'description':fragmentdesc})
+             if fragments:
+                 allfragments[layername] = {'layerdir':layerdir,'fragments':fragments}
+        return allfragments
+
+    def do_list_fragments(self, args):
+        """ List available configuration fragments """
+        enabled_fragments = (self.tinfoil.config_data.getVar('OE_FRAGMENTS') or "").split()
+
+        for layername, layerdata in self.discover_fragments().items():
+            layerdir = layerdata['layerdir']
+            fragments = layerdata['fragments']
+
+            print('Available fragments in {} layer located in {}:\n'.format(layername, layerdir))
+            for f in fragments:
+                if not args.verbose:
+                    print('{}\t{}\t{}'.format(f['name'], '(enabled)' if f['name'] in enabled_fragments else '(disabled)', f['summary']))
+                else:
+                    print('Name: {}\nPath: {}\nEnabled: {}\nSummary: {}\nDescription:\n{}\n'.format(f['name'], f['path'], 'yes' if f['name'] in enabled_fragments else 'no', f['summary'],''.join(f['description'])))
+            print('')
+
+    def fragment_exists(self, fragmentname):
+        for layername, layerdata in self.discover_fragments().items():
+            for f in layerdata['fragments']:
+              if f['name'] == fragmentname:
+                  return True
+        return False
+
+    def create_conf(self, confpath):
+        if not os.path.exists(confpath):
+            with open(confpath, 'w') as f:
+                f.write('')
+        with open(confpath, 'r') as f:
+            lines = f.read()
+        if "OE_FRAGMENTS += " not in lines:
+            lines += "\nOE_FRAGMENTS += \"\"\n"
+        with open(confpath, 'w') as f:
+            f.write(lines)
+
+    def do_enable_fragment(self, args):
+        """ Enable a fragment in the local build configuration """
+        def enable_helper(varname, origvalue, op, newlines):
+            enabled_fragments = origvalue.split()
+            if args.fragmentname in enabled_fragments:
+                print("Fragment {} already included in {}".format(args.fragmentname, args.confpath))
+            else:
+                enabled_fragments.append(args.fragmentname)
+            return " ".join(enabled_fragments), None, 0, True
+
+        if not self.fragment_exists(args.fragmentname):
+            raise Exception("Fragment {} does not exist; use 'list-fragments' to see the full list.".format(args.fragmentname))
+
+        self.create_conf(args.confpath)
+        modified = bb.utils.edit_metadata_file(args.confpath, ["OE_FRAGMENTS"], enable_helper)
+        if modified:
+            print("Fragment {} added to {}.".format(args.fragmentname, args.confpath))
+
+    def do_disable_fragment(self, args):
+        """ Disable a fragment in the local build configuration """
+        def disable_helper(varname, origvalue, op, newlines):
+            enabled_fragments = origvalue.split()
+            if args.fragmentname in enabled_fragments:
+                enabled_fragments.remove(args.fragmentname)
+            else:
+                print("Fragment {} not currently enabled in {}".format(args.fragmentname, args.confpath))
+            return " ".join(enabled_fragments), None, 0, True
+
+        self.create_conf(args.confpath)
+        modified = bb.utils.edit_metadata_file(args.confpath, ["OE_FRAGMENTS"], disable_helper)
+        if modified:
+            print("Fragment {} removed from {}.".format(args.fragmentname, args.confpath))
+
+    def do_disable_all_fragments(self, args):
+        """ Disable all fragments in the local build configuration """
+        def disable_all_helper(varname, origvalue, op, newlines):
+            return "", None, 0, True
+
+        self.create_conf(args.confpath)
+        modified = bb.utils.edit_metadata_file(args.confpath, ["OE_FRAGMENTS"], disable_all_helper)
+        if modified:
+            print("All fragments removed from {}.".format(args.confpath))
+
+    def register_commands(self, sp):
+        default_confpath = os.path.join(os.environ["BBPATH"], "conf/auto.conf")
+
+        parser_list_fragments = self.add_command(sp, 'list-fragments', self.do_list_fragments, parserecipes=False)
+        parser_list_fragments.add_argument("--confpath", default=default_confpath, help='Configuration file which contains a list of enabled fragments (default is {}).'.format(default_confpath))
+        parser_list_fragments.add_argument('--verbose', '-v', action='store_true', help='Print extended descriptions of the fragments')
+
+        parser_enable_fragment = self.add_command(sp, 'enable-fragment', self.do_enable_fragment, parserecipes=False)
+        parser_enable_fragment.add_argument("--confpath", default=default_confpath, help='Configuration file which contains a list of enabled fragments (default is {}).'.format(default_confpath))
+        parser_enable_fragment.add_argument('fragmentname', help='The name of the fragment (use list-fragments to see them)')
+
+        parser_disable_fragment = self.add_command(sp, 'disable-fragment', self.do_disable_fragment, parserecipes=False)
+        parser_disable_fragment.add_argument("--confpath", default=default_confpath, help='Configuration file which contains a list of enabled fragments (default is {}).'.format(default_confpath))
+        parser_disable_fragment.add_argument('fragmentname', help='The name of the fragment')
+
+        parser_disable_all = self.add_command(sp, 'disable-all-fragments', self.do_disable_all_fragments, parserecipes=False)
+        parser_disable_all.add_argument("--confpath", default=default_confpath, help='Configuration file which contains a list of enabled fragments (default is {}).'.format(default_confpath))
diff --git a/meta/lib/oeqa/selftest/cases/bblayers.py b/meta/lib/oeqa/selftest/cases/bblayers.py
index 695d17377d4..68b03777201 100644
--- a/meta/lib/oeqa/selftest/cases/bblayers.py
+++ b/meta/lib/oeqa/selftest/cases/bblayers.py
@@ -240,3 +240,34 @@ class BitbakeLayers(OESelftestTestCase):
         self.assertEqual(first_desc_2, '', "Describe not cleared: '{}'".format(first_desc_2))
         self.assertEqual(second_rev_2, second_rev_1, "Revision should not be updated: '{}'".format(second_rev_2))
         self.assertEqual(second_desc_2, second_desc_1, "Describe should not be updated: '{}'".format(second_desc_2))
+
+class BitbakeConfigBuild(OESelftestTestCase):
+    def test_enable_disable_fragments(self):
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_VARIABLE'), None)
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_ANOTHER_VARIABLE'), None)
+
+        runCmd('bitbake-config-build enable-fragment selftest/test-fragment')
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_VARIABLE'), 'somevalue')
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_ANOTHER_VARIABLE'), None)
+
+        runCmd('bitbake-config-build enable-fragment selftest/more-fragments-here/test-another-fragment')
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_VARIABLE'), 'somevalue')
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_ANOTHER_VARIABLE'), 'someothervalue')
+
+        fragment_metadata_command = "bitbake-getvar -f {} --value {}"
+        result = runCmd(fragment_metadata_command.format("selftest/test-fragment", "BB_CONF_FRAGMENT_SUMMARY"))
+        self.assertIn("This is a configuration fragment intended for testing in oe-selftest context", result.output)
+        result = runCmd(fragment_metadata_command.format("selftest/test-fragment", "BB_CONF_FRAGMENT_DESCRIPTION"))
+        self.assertIn("It defines a variable that can be checked inside the test.", result.output)
+        result = runCmd(fragment_metadata_command.format("selftest/more-fragments-here/test-another-fragment", "BB_CONF_FRAGMENT_SUMMARY"))
+        self.assertIn("This is a second configuration fragment intended for testing in oe-selftest context", result.output)
+        result = runCmd(fragment_metadata_command.format("selftest/more-fragments-here/test-another-fragment", "BB_CONF_FRAGMENT_DESCRIPTION"))
+        self.assertIn("It defines another variable that can be checked inside the test.", result.output)
+
+        runCmd('bitbake-config-build disable-fragment selftest/test-fragment')
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_VARIABLE'), None)
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_ANOTHER_VARIABLE'), 'someothervalue')
+
+        runCmd('bitbake-config-build disable-fragment selftest/more-fragments-here/test-another-fragment')
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_VARIABLE'), None)
+        self.assertEqual(get_bb_var('SELFTEST_FRAGMENT_ANOTHER_VARIABLE'), None)
-- 
2.39.5

Patchtest results for [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[patchtest]

[-- Attachment #1: Type: text/plain, Size: 3098 bytes --]

Thank you for your submission. Patchtest identified one
or more issues with the patch. Please see the log below for
more information:

---
Testing patch /home/patchtest/share/mboxes/v3-3-3-bitbake-config-build-add-a-plugin-for-config-fragments.patch

FAIL: test max line length: Patch line too long (current length 213, maximum is 200) (test_metadata.TestMetadata.test_max_line_length)

PASS: pretest pylint (test_python_pylint.PyLint.pretest_pylint)
PASS: test Signed-off-by presence (test_mbox.TestMbox.test_signed_off_by_presence)
PASS: test author valid (test_mbox.TestMbox.test_author_valid)
PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence)
PASS: test commit message user tags (test_mbox.TestMbox.test_commit_message_user_tags)
PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
PASS: test pylint (test_python_pylint.PyLint.test_pylint)
PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)
PASS: test target mailing list (test_mbox.TestMbox.test_target_mailing_list)

SKIP: pretest src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.pretest_src_uri_left_files)
SKIP: test CVE check ignore: No modified recipes or older target branch, skipping test (test_metadata.TestMetadata.test_cve_check_ignore)
SKIP: test CVE tag format: No new CVE patches introduced (test_patch.TestPatch.test_cve_tag_format)
SKIP: test Signed-off-by presence: No new CVE patches introduced (test_patch.TestPatch.test_signed_off_by_presence)
SKIP: test Upstream-Status presence: No new CVE patches introduced (test_patch.TestPatch.test_upstream_status_presence_format)
SKIP: test bugzilla entry format: No bug ID found (test_mbox.TestMbox.test_bugzilla_entry_format)
SKIP: test lic files chksum modified not mentioned: No modified recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence)
SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence)
SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head)
SKIP: test src uri left files: No modified recipes, skipping pretest (test_metadata.TestMetadata.test_src_uri_left_files)
SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence)

---

Please address the issues identified and
submit a new revision of the patch, or alternatively, reply to this
email with an explanation of why the patch should be accepted. If you
believe these results are due to an error in patchtest, please submit a
bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
under 'Yocto Project Subprojects'). For more information on specific
failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
you!

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Richard Purdie]

On Mon, 2024-11-18 at 17:26 +0100, Alexander Kanavin via lists.openembedded.org wrote:
> From: Alexander Kanavin <alex@linutronix.de>
> 
> This allows fine-tuning local configurations with pre-frabricated
> configuration snippets in a structured, controlled way. It's also
> an important building block for bitbake-setup.
> 
> There are three (and a half) operations (list/enable/disable/disable all), and here's the 'list' output:
> 
> alex@Zen2:/srv/storage/alex/yocto/build-64$ bitbake-config-build list-fragments
> NOTE: Starting bitbake server...
> Available fragments in selftest layer located in /srv/work/alex/poky/meta-selftest:
> 
> selftest/test-fragment	(disabled)	This is a configuration fragment intended for testing in oe-selftest context
> selftest/more-fragments-here/test-another-fragment	(disabled)	This is a second configuration fragment intended for testing in oe-selftest context
> 
> The tool requires that each fragment contains a one-line summary, and one or more
> lines of description, as BB_CONF_FRAGMENT_SUMMARY[layerid/fragmentname] style metadata.
> 
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>  .../test-another-fragment.conf                |   3 +
>  .../conf/fragments/test-fragment.conf         |   3 +
>  meta/lib/bbconfigbuild/configfragments.py     | 147 ++++++++++++++++++
>  meta/lib/oeqa/selftest/cases/bblayers.py      |  31 ++++
>  4 files changed, 184 insertions(+)
>  create mode 100644 meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf
>  create mode 100644 meta-selftest/conf/fragments/test-fragment.conf
>  create mode 100644 meta/lib/bbconfigbuild/configfragments.py
> 
> diff --git a/meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf b/meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf
> new file mode 100644
> index 00000000000..cf9ba6a6132
> --- /dev/null
> +++ b/meta-selftest/conf/fragments/more-fragments-here/test-another-fragment.conf
> @@ -0,0 +1,3 @@
> +BB_CONF_FRAGMENT_SUMMARY[selftest/more-fragments-here/test-another-fragment] = "This is a second configuration fragment intended for testing in oe-selftest context"
> +BB_CONF_FRAGMENT_DESCRIPTION[selftest/more-fragments-here/test-another-fragment] = "It defines another variable that can be checked inside the test."
> +SELFTEST_FRAGMENT_ANOTHER_VARIABLE = "someothervalue"

One thing which is still bugging me about this and causing some of my
hesitation to merge things is the repeat of the name in the file path
and in the flag name itself. This is going to be a pain to keep in sync
over time.

I've been wondering if:

a) We should have some of variable that gets expanded. Similar examples
are THIDDIR, LAYERDIR, FILE_DIRNAME and FILE but all those have
problems.

-or-

b) Whether the fragment inclusion code should do a rename of any 
BB_CONF_FRAGMENT_SUMMARY -> BB_CONF_FRAGMENT_SUMMARY[<name>] during
parsing. We could list the variables that need processing as parameters
to the addfragments directive or in a variable.

Of the two, I can see b) possibly working. We've never had a) work in a
way that I've liked.

Does anyone have any thoughts on this?

Cheers,

Richard

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Alexander Kanavin]

On Mon, 9 Dec 2024 at 17:33, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> One thing which is still bugging me about this and causing some of my
> hesitation to merge things is the repeat of the name in the file path
> and in the flag name itself. This is going to be a pain to keep in sync
> over time.

I agree. It also simply visually clutters the fragment content without
need, fragments should be easy on the eye.

> b) Whether the fragment inclusion code should do a rename of any
> BB_CONF_FRAGMENT_SUMMARY -> BB_CONF_FRAGMENT_SUMMARY[<name>] during
> parsing. We could list the variables that need processing as parameters
> to the addfragments directive or in a variable.
>
> Of the two, I can see b) possibly working. We've never had a) work in a
> way that I've liked.

I was going to try this out, but I need to choose from two options.
The fragment path is given to parser like this:

                bb.parse.ConfHandler.include(self.filename,
fragment_path, self.lineno, data, "include fragment")

So fixing up the flags would either require passing in the list of
variables (that need fixing) into this call, which could turn out to
be an invasive change. Or we could parse everything as usual, and then
fix up the flags after the fact, e.g. just after this call. I'm
leaning towards the latter.

Alex

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Richard Purdie]

On Mon, 2024-11-18 at 17:26 +0100, Alexander Kanavin via
lists.openembedded.org wrote:
> From: Alexander Kanavin <alex@linutronix.de>
> 
> This allows fine-tuning local configurations with pre-frabricated
> configuration snippets in a structured, controlled way. It's also
> an important building block for bitbake-setup.
> 
> There are three (and a half) operations (list/enable/disable/disable
> all), and here's the 'list' output:
> 
> alex@Zen2:/srv/storage/alex/yocto/build-64$ bitbake-config-build list-fragments
> NOTE: Starting bitbake server...
> Available fragments in selftest layer located in /srv/work/alex/poky/meta-selftest:
> 
> selftest/test-fragment	(disabled)	This is a configuration fragment intended for testing in oe-selftest context
> selftest/more-fragments-here/test-another-fragment	(disabled)	This is a second configuration fragment intended for testing in oe-selftest context
> 
> The tool requires that each fragment contains a one-line summary, and one or more
> lines of description, as BB_CONF_FRAGMENT_SUMMARY[layerid/fragmentname] style metadata.

The other area I've been wondering about is this output. I think we
need to take a quick step back and look at it from a usability
perspective and make sure we're showing the information in the optimal
way.

I'm far from a UI expert, in fact I've been told I shouldn't comment on
such things however I do have some thoughts.

Firstly, I think the "(disabled)" is the wrong way to show that
information. It would be more compact and perhaps more usable to show
the elements sorted into two sections along the lines of:

"""
Enabled fragments:

selftest/test-fragment - This is a configuration fragment intended for testing in oe-selftest context

Unused fragments:

selftest/more-fragments-here/test-another-fragment - This is a second configuration fragment intended for testing in oe-selftest context
"""

Secondly when listing the fragments, the current approach does lend
itself to cut and past but also doesn't group neatly. For example you
could do something like:

selftest/
  test-fragment - This is a configuration fragment intended for testing in oe-selftest context
  more-fragments-here/test-another-fragment - This is a second configuration fragment intended for testing in oe-selftest context

I'm torn on that but it may be easier for people to read. We should
also think about how terminal line wrapping will effect the output and
try to ensure it remains readable.

As with the previous email, I'm open to other thoughts on this.

Cheers,

Richard

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Alexander Kanavin]

On Mon, 9 Dec 2024 at 18:00, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> Firstly, I think the "(disabled)" is the wrong way to show that
> information. It would be more compact and perhaps more usable to show
> the elements sorted into two sections along the lines of:
>
> """
> Enabled fragments:
>
> selftest/test-fragment - This is a configuration fragment intended for testing in oe-selftest context
>
> Unused fragments:
>
> selftest/more-fragments-here/test-another-fragment - This is a second configuration fragment intended for testing in oe-selftest context
> """

This is fine, I can fix it.

> Secondly when listing the fragments, the current approach does lend
> itself to cut and past but also doesn't group neatly. For example you
> could do something like:
>
> selftest/
>   test-fragment - This is a configuration fragment intended for testing in oe-selftest context
>   more-fragments-here/test-another-fragment - This is a second configuration fragment intended for testing in oe-selftest context

If we start going down this path, we might as well write an
interactive curses UI, and keep the command line output 'flat' like it
is now. I'm also not sure what is the best approach, we'd need to take
fragments into practical use and accumulate a sizable amount of them
perhaps. Such things can be easily tweaked from experience.

Alex

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Richard Purdie]

On Tue, 2024-12-10 at 14:05 +0100, Alexander Kanavin via
lists.openembedded.org wrote:
> On Mon, 9 Dec 2024 at 18:00, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > Firstly, I think the "(disabled)" is the wrong way to show that
> > information. It would be more compact and perhaps more usable to
> > show
> > the elements sorted into two sections along the lines of:
> > 
> > """
> > Enabled fragments:
> > 
> > selftest/test-fragment - This is a configuration fragment intended
> > for testing in oe-selftest context
> > 
> > Unused fragments:
> > 
> > selftest/more-fragments-here/test-another-fragment - This is a
> > second configuration fragment intended for testing in oe-selftest
> > context
> > """
> 
> This is fine, I can fix it.

Right, this on is not too hard to implement from a code perspective.

> 
> > Secondly when listing the fragments, the current approach does lend
> > itself to cut and past but also doesn't group neatly. For example
> > you
> > could do something like:
> > 
> > selftest/
> >   test-fragment - This is a configuration fragment intended for
> > testing in oe-selftest context
> >   more-fragments-here/test-another-fragment - This is a second
> > configuration fragment intended for testing in oe-selftest context
> 
> If we start going down this path, we might as well write an
> interactive curses UI, and keep the command line output 'flat' like it
> is now. I'm also not sure what is the best approach, we'd need to take
> fragments into practical use and accumulate a sizable amount of them
> perhaps. Such things can be easily tweaked from experience.

Having experimented with curses UIs, they're a pain and don't really do
what you hope/think they would.

Some of the other tools like bitbake-layers do "group" output for ease
of usability so this is something our commandline tools do to some
extent already. I'm not saying we have to do this, or do it now. I did
want to at least put the idea out there to see what people's thoughts
were.

I just know from experience that showing users large blocks of text
without something to guide the eye can make them harder to use.

We can leave this part for now...

Cheers,

Richard

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Alexander Kanavin]

On Tue, 10 Dec 2024 at 16:27, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:

> > If we start going down this path, we might as well write an
> > interactive curses UI, and keep the command line output 'flat' like it
> > is now. I'm also not sure what is the best approach, we'd need to take
> > fragments into practical use and accumulate a sizable amount of them
> > perhaps. Such things can be easily tweaked from experience.
>
> Having experimented with curses UIs, they're a pain and don't really do
> what you hope/think they would.

We might be thinking of different things: my suggestion is that
interactive 'kernel menuconfig' type UI is worth exploring for
fragments (or bitbake-setup, for that matter).

Alex

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Richard Purdie]

On Wed, 2024-12-11 at 12:24 +0100, Alexander Kanavin wrote:
> On Tue, 10 Dec 2024 at 16:27, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> 
> > > If we start going down this path, we might as well write an
> > > interactive curses UI, and keep the command line output 'flat'
> > > like it
> > > is now. I'm also not sure what is the best approach, we'd need to
> > > take
> > > fragments into practical use and accumulate a sizable amount of
> > > them
> > > perhaps. Such things can be easily tweaked from experience.
> > 
> > Having experimented with curses UIs, they're a pain and don't
> > really do
> > what you hope/think they would.
> 
> We might be thinking of different things: my suggestion is that
> interactive 'kernel menuconfig' type UI is worth exploring for
> fragments (or bitbake-setup, for that matter).

We were talking about different things. I agree, I think something
kconfig like for some of this would be much appreciated by some users.

Cheers,

Richard

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Joshua Watt]

OK, I gave this a try and here is some feedback:

1) bitbake-config-build is way too aggressive in the files it looks
for as fragments. My editor (vim) creates hidden temporary files that
I kept having to manually delete or the tool would attempt to parse
them and fail. I would recommend that it only consider non-hidden
files that end in .conf as valid fragments.
2) It would be convenient if `bitbake-config-build enable-fragment`
and `bitbake-config-build disable-fragment` could take more than one
fragment, as this would make scripting in CI more straight forward.
3) The biggest problem I had with this is that I can't set variables
like MACHINE/DISTRO etc. in fragments, I think this is due to how
deferred the evaluation of this is. Without being able to do this, I'm
not sure how useful this system will be for me (or at least, it won't
be able to replace any of the current mechanisms I use). I think what
users are generally after here is that they can really easily share a
configuration in source control (which this does well), but if it
can't control everything it's of limited use.

As a practical example of what I would like to see for this to be
useful, consider https://github.com/JPEWdev/oe-doom-demo. In this
repo, we are building a demo program for multiple different hardware
platforms; In an ideal world, the instructions I could give someone
would look like (NOTE: this is fictitious because the repo doesn't
work this way today):
```
git clone https://github.com/JPEWdev/oe-doom-demo
cd oe-doom-demo
./scripts/setup-layers
. core/oe-init-build-env
bitbake-config-build enable-fragment doom-demo/le-potato
bitbake core-image-doom
```

This specifies all the things required to build the demo for le-potato
hardware (including MACHINE/DISTRO/etc.), so they should get the exact
same demo image I made when building.

On Mon, Dec 16, 2024 at 9:58 AM Richard Purdie via
lists.openembedded.org
<richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote:
>
> On Wed, 2024-12-11 at 12:24 +0100, Alexander Kanavin wrote:
> > On Tue, 10 Dec 2024 at 16:27, Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> >
> > > > If we start going down this path, we might as well write an
> > > > interactive curses UI, and keep the command line output 'flat'
> > > > like it
> > > > is now. I'm also not sure what is the best approach, we'd need to
> > > > take
> > > > fragments into practical use and accumulate a sizable amount of
> > > > them
> > > > perhaps. Such things can be easily tweaked from experience.
> > >
> > > Having experimented with curses UIs, they're a pain and don't
> > > really do
> > > what you hope/think they would.
> >
> > We might be thinking of different things: my suggestion is that
> > interactive 'kernel menuconfig' type UI is worth exploring for
> > fragments (or bitbake-setup, for that matter).
>
> We were talking about different things. I agree, I think something
> kconfig like for some of this would be much appreciated by some users.
>
> Cheers,
>
> Richard
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#208780): https://lists.openembedded.org/g/openembedded-core/message/208780
> Mute This Topic: https://lists.openembedded.org/mt/109647318/3616693
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [JPEWhacker@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Alexander Kanavin]

On Fri, 20 Dec 2024 at 18:39, Joshua Watt <jpewhacker@gmail.com> wrote:
> 1) bitbake-config-build is way too aggressive in the files it looks
> for as fragments. My editor (vim) creates hidden temporary files that
> I kept having to manually delete or the tool would attempt to parse
> them and fail. I would recommend that it only consider non-hidden
> files that end in .conf as valid fragments.
> 2) It would be convenient if `bitbake-config-build enable-fragment`
> and `bitbake-config-build disable-fragment` could take more than one
> fragment, as this would make scripting in CI more straight forward.

I see the patches for these have been sent, thanks.

> 3) The biggest problem I had with this is that I can't set variables
> like MACHINE/DISTRO etc. in fragments, I think this is due to how
> deferred the evaluation of this is. Without being able to do this, I'm
> not sure how useful this system will be for me (or at least, it won't
> be able to replace any of the current mechanisms I use). I think what
> users are generally after here is that they can really easily share a
> configuration in source control (which this does well), but if it
> can't control everything it's of limited use.

I tried this locally with poky and adding DISTRO/MACHINE to one of the
selftest fragments, and it works properly, so if you can provide a way
to reproduce this problem I'd appreciate. Or there's something else on
your side that gets in the way.

Here's the output of bitbake -e (DISTRO is similarly adjusted as expected):

#
# $MACHINE [3 operations]
#   set /srv/storage/alex/yocto/build-64/conf/local.conf:38
#     [_defaultval] "qemux86-64"
#   set /srv/work/alex/poky/meta/conf/documentation.conf:279
#     [doc] "Specifies the target device for which the image is built.
You define MACHINE in the conf/local.conf file in the Build
Directory."
#   set /srv/work/alex/poky/meta-selftest/conf/fragments/test-fragment.conf:5
#     "qemuarm"
# pre-expansion value:
#   "qemuarm"
MACHINE="qemuarm"

Alex

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Joshua Watt]

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

Hmm, ok, I'll try again.

On Wed, Dec 25, 2024, 8:29 AM Alexander Kanavin <alex.kanavin@gmail.com>
wrote:

> On Fri, 20 Dec 2024 at 18:39, Joshua Watt <jpewhacker@gmail.com> wrote:
> > 1) bitbake-config-build is way too aggressive in the files it looks
> > for as fragments. My editor (vim) creates hidden temporary files that
> > I kept having to manually delete or the tool would attempt to parse
> > them and fail. I would recommend that it only consider non-hidden
> > files that end in .conf as valid fragments.
> > 2) It would be convenient if `bitbake-config-build enable-fragment`
> > and `bitbake-config-build disable-fragment` could take more than one
> > fragment, as this would make scripting in CI more straight forward.
>
> I see the patches for these have been sent, thanks.
>
> > 3) The biggest problem I had with this is that I can't set variables
> > like MACHINE/DISTRO etc. in fragments, I think this is due to how
> > deferred the evaluation of this is. Without being able to do this, I'm
> > not sure how useful this system will be for me (or at least, it won't
> > be able to replace any of the current mechanisms I use). I think what
> > users are generally after here is that they can really easily share a
> > configuration in source control (which this does well), but if it
> > can't control everything it's of limited use.
>
> I tried this locally with poky and adding DISTRO/MACHINE to one of the
> selftest fragments, and it works properly, so if you can provide a way
> to reproduce this problem I'd appreciate. Or there's something else on
> your side that gets in the way.
>
> Here's the output of bitbake -e (DISTRO is similarly adjusted as expected):
>
> #
> # $MACHINE [3 operations]
> #   set /srv/storage/alex/yocto/build-64/conf/local.conf:38
> #     [_defaultval] "qemux86-64"
> #   set /srv/work/alex/poky/meta/conf/documentation.conf:279
> #     [doc] "Specifies the target device for which the image is built.
> You define MACHINE in the conf/local.conf file in the Build
> Directory."
> #   set
> /srv/work/alex/poky/meta-selftest/conf/fragments/test-fragment.conf:5
> #     "qemuarm"
> # pre-expansion value:
> #   "qemuarm"
> MACHINE="qemuarm"
>
> Alex
>

[-- Attachment #2: Type: text/html, Size: 2853 bytes --]

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Joshua Watt]

Ok, I tried it again, and it didn't work for me (or at least, didn't
do what I would have liked). You can set MACHINE and DISTRO in config
fragments, *but* it doesn't affect the machine or distro include in
bitbake.conf. This means you can't really specify MACHINE or DISTRO in
a fragment, or you're likely to get really strange results. You can
see this pretty easily in the environment output; the MACHINE variable
proper may be set to whatever is in the fragment, but the included
machine file is not the one that is specified and thus the actual
machine settings will be really weird (same for DISTRO).

I won't claim to speak for everyone who wants config fragments, but in
my experience, this means that these config fragments aren unlikely to
be able to replace many of the existing tooling that does
configuration (e.g. whisk, kas, et. al). As the number of developers
working on project grows, and you try to build more and more
builds/projects/products (hereafter just called "products") out of the
same repo, it gets increasing difficult to expect users to manage
switching between different products by changing environment variables
and/or local.conf settings. What the existing configuration management
tools have going for them is that they make this very easy; it's just
a single pretty easy to understand system that allows an end user to
exactly replicate the product "as intended" (e.g. you can precisely
replicate the configuration that all other users are using). In the
case of whisk specifically, it's designed such that everything the
user needs to replicate the build is specified by whisk, and
local.conf only needs to have anything in it if they want to change
something for their local build. This has worked extremely well for us
in our use cases, as it's well understood that if you are having
problems, the first step is to see what changes you've made in
local.conf and/or remove them to see if the problem can still be
replicated.

https://github.com/JPEWdev/oe-doom-demo works in a very similar vein,
except it does so without using whisk. The idea is that the user can
set a single environment variable (BUILD_DEVICE) and precisely
replicate the intended build. I would love to be able to do this with
config fragements instead, as like pretty much all the other features
of them, but if I can't set MACHINE and DISTRO, it doesn't really feel
worth it (IMHO).

On Wed, Dec 25, 2024 at 8:33 AM Joshua Watt <jpewhacker@gmail.com> wrote:
>
> Hmm, ok, I'll try again.
>
> On Wed, Dec 25, 2024, 8:29 AM Alexander Kanavin <alex.kanavin@gmail.com> wrote:
>>
>> On Fri, 20 Dec 2024 at 18:39, Joshua Watt <jpewhacker@gmail.com> wrote:
>> > 1) bitbake-config-build is way too aggressive in the files it looks
>> > for as fragments. My editor (vim) creates hidden temporary files that
>> > I kept having to manually delete or the tool would attempt to parse
>> > them and fail. I would recommend that it only consider non-hidden
>> > files that end in .conf as valid fragments.
>> > 2) It would be convenient if `bitbake-config-build enable-fragment`
>> > and `bitbake-config-build disable-fragment` could take more than one
>> > fragment, as this would make scripting in CI more straight forward.
>>
>> I see the patches for these have been sent, thanks.
>>
>> > 3) The biggest problem I had with this is that I can't set variables
>> > like MACHINE/DISTRO etc. in fragments, I think this is due to how
>> > deferred the evaluation of this is. Without being able to do this, I'm
>> > not sure how useful this system will be for me (or at least, it won't
>> > be able to replace any of the current mechanisms I use). I think what
>> > users are generally after here is that they can really easily share a
>> > configuration in source control (which this does well), but if it
>> > can't control everything it's of limited use.
>>
>> I tried this locally with poky and adding DISTRO/MACHINE to one of the
>> selftest fragments, and it works properly, so if you can provide a way
>> to reproduce this problem I'd appreciate. Or there's something else on
>> your side that gets in the way.
>>
>> Here's the output of bitbake -e (DISTRO is similarly adjusted as expected):
>>
>> #
>> # $MACHINE [3 operations]
>> #   set /srv/storage/alex/yocto/build-64/conf/local.conf:38
>> #     [_defaultval] "qemux86-64"
>> #   set /srv/work/alex/poky/meta/conf/documentation.conf:279
>> #     [doc] "Specifies the target device for which the image is built.
>> You define MACHINE in the conf/local.conf file in the Build
>> Directory."
>> #   set /srv/work/alex/poky/meta-selftest/conf/fragments/test-fragment.conf:5
>> #     "qemuarm"
>> # pre-expansion value:
>> #   "qemuarm"
>> MACHINE="qemuarm"
>>
>> Alex

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Alexander Kanavin]

On Fri, 27 Dec 2024 at 19:12, Joshua Watt <jpewhacker@gmail.com> wrote:
> Ok, I tried it again, and it didn't work for me (or at least, didn't
> do what I would have liked). You can set MACHINE and DISTRO in config
> fragments, *but* it doesn't affect the machine or distro include in
> bitbake.conf. This means you can't really specify MACHINE or DISTRO in
> a fragment, or you're likely to get really strange results. You can
> see this pretty easily in the environment output; the MACHINE variable
> proper may be set to whatever is in the fragment, but the included
> machine file is not the one that is specified and thus the actual
> machine settings will be really weird (same for DISTRO).

I have sent a patch for it, can you please try it? It does solve the
issue on my side.

Alex

Re: [OE-core] [PATCH v3 3/3] bitbake-config-build: add a plugin for config fragments

[Joshua Watt]

On Fri, Dec 27, 2024 at 11:44 AM Alexander Kanavin
<alex.kanavin@gmail.com> wrote:
>
> On Fri, 27 Dec 2024 at 19:12, Joshua Watt <jpewhacker@gmail.com> wrote:
> > Ok, I tried it again, and it didn't work for me (or at least, didn't
> > do what I would have liked). You can set MACHINE and DISTRO in config
> > fragments, *but* it doesn't affect the machine or distro include in
> > bitbake.conf. This means you can't really specify MACHINE or DISTRO in
> > a fragment, or you're likely to get really strange results. You can
> > see this pretty easily in the environment output; the MACHINE variable
> > proper may be set to whatever is in the fragment, but the included
> > machine file is not the one that is specified and thus the actual
> > machine settings will be really weird (same for DISTRO).
>
> I have sent a patch for it, can you please try it? It does solve the
> issue on my side.

Yes, that fixes it. Thanks.

>
> Alex

Re: [OE-core] [PATCH v3 1/3] patchtest: use HEAD commit as base for the selftest and not master

[Trevor Gamblin]

On 2024-11-18 11:26, Alexander Kanavin via lists.openembedded.org wrote:
> From: Alexander Kanavin <alex@linutronix.de>
>
> Patchtest applies patches on top of poky master branch by default;
> this means selftest does the same, and any commits from the branch-under-test
> are then discarded.
>
> This can cause issues for example, if bitbake-server process started by selftest
> from the master branch tries to parse bitbake.conf from the branch under test:
> https://valkyrie.yoctoproject.org/#/builders/71/builds/460
>
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
LGTM.
> ---
>   meta/lib/patchtest/selftest/selftest | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meta/lib/patchtest/selftest/selftest b/meta/lib/patchtest/selftest/selftest
> index 6fad50ce616..3cf1c361f7f 100755
> --- a/meta/lib/patchtest/selftest/selftest
> +++ b/meta/lib/patchtest/selftest/selftest
> @@ -38,7 +38,7 @@ def test(root, patch):
>       res = True
>       patchpath = os.path.abspath(os.path.join(root, patch))
>       
> -    cmd     = 'patchtest --repodir %s --testdir %s/tests --patch %s' % (repodir, topdir, patchpath)
> +    cmd     = 'patchtest --base-commit HEAD --repodir %s --testdir %s/tests --patch %s' % (repodir, topdir, patchpath)
>       results = subprocess.check_output(cmd, stderr=subprocess.STDOUT, universal_newlines=True, shell=True)
>   
>       return results
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#207222): https://lists.openembedded.org/g/openembedded-core/message/207222
> Mute This Topic: https://lists.openembedded.org/mt/109647316/7611679
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [tgamblin@baylibre.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>