[PATCH v3 00/10] optparse => argparse

[PATCH v3 00/10] optparse => argparse

[Chris Laplante]

v1: initial
v2: fix intermixed argument parsing
v3: fix TinfoilConfigParameters defaulting of 'ui' with string, reorder
    patches

Chris Laplante (10):
  cookerdata: tweak to avoid mutable default argument
  tests/arg_parser: add stub for testing arg parsing
  tests/arg_parser: add test for 'bitbake -S none world'
  main: migrate from optparse to argparse
  main: group --help options to make them easier to read
  main: rename list_extension_modules => load_extension_modules to
    clarify its function
  tests/arg_parser: add test for lazy loading of bb.ui modules
  main: fix parsing of intermixed arguments
  tinfoil: use knotty module itself as default 'ui' arg to keep up with
    changes in setup_bitbake
  tests/arg_parser: add test for default TinfoilConfigParameters 'ui'

 bin/bitbake-selftest       |   3 +-
 lib/bb/cookerdata.py       |   6 +-
 lib/bb/main.py             | 327 +++++++++++++++++++------------------
 lib/bb/tests/arg_parser.py |  62 +++++++
 lib/bb/tinfoil.py          |   9 +-
 5 files changed, 240 insertions(+), 167 deletions(-)
 create mode 100644 lib/bb/tests/arg_parser.py

--
2.17.1

[PATCH v3 01/10] cookerdata: tweak to avoid mutable default argument

[Chris Laplante]

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/cookerdata.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/bb/cookerdata.py b/lib/bb/cookerdata.py
index 91cc4347..c39b5681 100644
--- a/lib/bb/cookerdata.py
+++ b/lib/bb/cookerdata.py
@@ -23,8 +23,8 @@ logger      = logging.getLogger("BitBake")
 parselog    = logging.getLogger("BitBake.Parsing")
 
 class ConfigParameters(object):
-    def __init__(self, argv=sys.argv):
-        self.options, targets = self.parseCommandLine(argv)
+    def __init__(self, argv=None):
+        self.options, targets = self.parseCommandLine(argv or sys.argv)
         self.environment = self.parseEnvironment()
 
         self.options.pkgs_to_build = targets or []
@@ -209,7 +209,7 @@ def findConfigFile(configfile, data):
     return None
 
 #
-# We search for a conf/bblayers.conf under an entry in BBPATH or in cwd working 
+# We search for a conf/bblayers.conf under an entry in BBPATH or in cwd working
 # up to /. If that fails, we search for a conf/bitbake.conf in BBPATH.
 #
 
-- 
2.17.1

[PATCH v3 02/10] tests/arg_parser: add stub for testing arg parsing

[Chris Laplante]

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 bin/bitbake-selftest       |  3 ++-
 lib/bb/tests/arg_parser.py | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 lib/bb/tests/arg_parser.py

diff --git a/bin/bitbake-selftest b/bin/bitbake-selftest
index 6c073741..cc794bd6 100755
--- a/bin/bitbake-selftest
+++ b/bin/bitbake-selftest
@@ -17,7 +17,8 @@ try:
 except RuntimeError as exc:
     sys.exit(str(exc))
 
-tests = ["bb.tests.codeparser",
+tests = ["bb.tests.arg_parser",
+         "bb.tests.codeparser",
          "bb.tests.color",
          "bb.tests.cooker",
          "bb.tests.cow",
diff --git a/lib/bb/tests/arg_parser.py b/lib/bb/tests/arg_parser.py
new file mode 100644
index 00000000..e6b55f69
--- /dev/null
+++ b/lib/bb/tests/arg_parser.py
@@ -0,0 +1,17 @@
+#
+# BitBake Test for the main 'bitbake' argument parser
+#
+# Copyright (C) 2020  Agilent Technologies, Inc.
+# Author: Chris Laplante <chris.laplante@agilent.com>
+#
+# SPDX-License-Identifier: MIT
+#
+
+import unittest
+
+from bb.main import create_bitbake_parser
+
+
+class ArgParserTests(unittest.TestCase):
+    def setUp(self):
+        self._parser = create_bitbake_parser()
-- 
2.17.1

[PATCH v3 03/10] tests/arg_parser: add test for 'bitbake -S none world'

[Chris Laplante]

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/tests/arg_parser.py | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/lib/bb/tests/arg_parser.py b/lib/bb/tests/arg_parser.py
index e6b55f69..fd46fe8c 100644
--- a/lib/bb/tests/arg_parser.py
+++ b/lib/bb/tests/arg_parser.py
@@ -7,11 +7,37 @@
 # SPDX-License-Identifier: MIT
 #
 
+import shlex
 import unittest
+import collections
 
-from bb.main import create_bitbake_parser
+
+from bb.main import create_bitbake_parser, BitBakeConfigParameters
+
+ParseResult = collections.namedtuple("ParseResult", ("options", "targets"))
 
 
 class ArgParserTests(unittest.TestCase):
     def setUp(self):
         self._parser = create_bitbake_parser()
+
+    def _parse_helper(self, arg_str):
+        self.assertIs(type(arg_str), str)
+        params = BitBakeConfigParameters(argv=shlex.split(arg_str))
+        return ParseResult(params.options, params.options.pkgs_to_build)
+
+    def test_parse_s_none_world(self):
+        # All of these should give the same result
+        arg_strs = [
+            "bitbake -S none world",
+            "bitbake world -S none",
+            "bitbake -Snone world",
+            # optparse doesn't handle =
+            # "bitbake -S=none world",
+            # "bitbake world -S=none"
+        ]
+
+        for arg_str in arg_strs:
+            res = self._parse_helper(arg_str)
+            self.assertListEqual(res.targets, ["world"])
+            self.assertListEqual(res.options.dump_signatures, ["none"])
-- 
2.17.1

[PATCH v3 04/10] main: migrate from optparse to argparse

[Chris Laplante]

Modernizes the argument parsing code and opens the door to grouping
the options (e.g. YOCTO #12018).

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/main.py | 263 ++++++++++++++++++++++++-------------------------
 1 file changed, 127 insertions(+), 136 deletions(-)

diff --git a/lib/bb/main.py b/lib/bb/main.py
index 06bad495..5bd41b1c 100755
--- a/lib/bb/main.py
+++ b/lib/bb/main.py
@@ -12,10 +12,10 @@
 import os
 import sys
 import logging
-import optparse
 import warnings
 import fcntl
 import time
+import argparse
 import traceback
 
 import bb
@@ -31,30 +31,14 @@ import bb.server.xmlrpcclient
 
 logger = logging.getLogger("BitBake")
 
+
 class BBMainException(Exception):
     pass
 
+
 class BBMainFatal(bb.BBHandledException):
     pass
 
-def present_options(optionlist):
-    if len(optionlist) > 1:
-        return ' or '.join([', '.join(optionlist[:-1]), optionlist[-1]])
-    else:
-        return optionlist[0]
-
-class BitbakeHelpFormatter(optparse.IndentedHelpFormatter):
-    def format_option(self, option):
-        # We need to do this here rather than in the text we supply to
-        # add_option() because we don't want to call list_extension_modules()
-        # on every execution (since it imports all of the modules)
-        # Note also that we modify option.help rather than the returned text
-        # - this is so that we don't have to re-format the text ourselves
-        if option.dest == 'ui':
-            valid_uis = list_extension_modules(bb.ui, 'main')
-            option.help = option.help.replace('@CHOICES@', present_options(valid_uis))
-
-        return optparse.IndentedHelpFormatter.format_option(self, option)
 
 def list_extension_modules(pkg, checkattr):
     """
@@ -69,36 +53,22 @@ def list_extension_modules(pkg, checkattr):
             as the type of extension you are looking for
     """
     import pkgutil
-    pkgdir = os.path.dirname(pkg.__file__)
+    import importlib
 
     modules = []
-    for _, modulename, _ in pkgutil.iter_modules([pkgdir]):
-        if os.path.isdir(os.path.join(pkgdir, modulename)):
-            # ignore directories
-            continue
+    # bb.ui uses namespace packaging: https://packaging.python.org/guides/packaging-namespace-packages/
+    for _, modulename, _ in pkgutil.iter_modules(pkg.__path__, pkg.__name__ + "."):
         try:
-            module = __import__(pkg.__name__, fromlist=[modulename])
+            module = importlib.import_module(modulename)
         except:
             # If we can't import it, it's not valid
             continue
-        module_if = getattr(module, modulename)
-        if getattr(module_if, 'hidden_extension', False):
-            continue
-        if not checkattr or hasattr(module_if, checkattr):
-            modules.append(modulename)
+        # if getattr(module_if, 'hidden_extension', False):
+        #     continue
+        if not checkattr or hasattr(module, checkattr):
+            modules.append(module)
     return modules
 
-def import_extension_module(pkg, modulename, checkattr):
-    try:
-        # Dynamically load the UI based on the ui name. Although we
-        # suggest a fixed set this allows you to have flexibility in which
-        # ones are available.
-        module = __import__(pkg.__name__, fromlist=[modulename])
-        return getattr(module, modulename)
-    except AttributeError:
-        modules = present_options(list_extension_modules(pkg, checkattr))
-        raise BBMainException('FATAL: Unable to import extension module "%s" from %s. '
-                              'Valid extension modules: %s' % (modulename, pkg.__name__, modules))
 
 # Display bitbake/OE warnings via the BitBake.Warnings logger, ignoring others"""
 warnlog = logging.getLogger("BitBake.Warnings")
@@ -120,213 +90,234 @@ warnings.filterwarnings("ignore", category=DeprecationWarning, module="<string>$
 warnings.filterwarnings("ignore", message="With-statements now directly support multiple context managers")
 
 
+class LazyUiChoices:
+    """
+    A proxy object that will act as both the 'choices' (via __iter__ and __contains__) and the 'type' (via __call__) for
+    the 'ui' argparse argument.
+    """
+    def __init__(self):
+        # map module name => imported module
+        self._modules = {}
+
+    def _lazy_load_modules(self):
+        if not self._modules:
+            self._modules = {module.__name__.split(".")[-1]: module for module in list_extension_modules(bb.ui, "main")}
+
+    def __iter__(self):
+        self._lazy_load_modules()
+        yield from self._modules.keys()
+
+    def __contains__(self, item):
+        self._lazy_load_modules()
+        return item in self._modules.values()
+
+    def __call__(self, *args, **kwargs):
+        """ Called by argparse to convert string name into the module itself """
+        self._lazy_load_modules()
+        try:
+            return self._modules[args[0]]
+        except KeyError:
+            # invalid choice, just return the string - will be rejected when argparse uses  __contains__ to check it
+            return args[0]
+
+
 def create_bitbake_parser():
-    parser = optparse.OptionParser(
-        formatter=BitbakeHelpFormatter(),
-        version="BitBake Build Tool Core version %s" % bb.__version__,
-        usage="""%prog [options] [recipename/target recipe:do_task ...]
+    parser = argparse.ArgumentParser(usage="""%(prog)s [options] [recipename/target recipe:do_task ...]
 
     Executes the specified task (default is 'build') for a given set of target recipes (.bb files).
     It is assumed there is a conf/bblayers.conf available in cwd or in BBPATH which
-    will provide the layer, BBFILES and other configuration information.""")
+    will provide the layer, BBFILES and other configuration information.
+    """)
+    parser.add_argument("targets", nargs="*")
+    parser.add_argument("--version",
+                        action="version",
+                        version="BitBake Build Tool Core version {0}".format(bb.__version__))
+
+    ui_choices = LazyUiChoices()
+    # Setting metavar="" is necessary otherwise argparse will immediately try to expand the choices
+    parser.add_argument("-u", "--ui",
+                        default=os.environ.get('BITBAKE_UI', 'knotty'),
+                        help="The user interface to use (%(choices)s) - default: %(default)s",
+                        choices=ui_choices, metavar="", type=ui_choices)
+
+    parser.add_argument("-v", "--verbose", action="store_true", dest="verbose",
+                      help="Enable tracing of shell tasks (with 'set -x'). "
+                           "Also print bb.note(...) messages to stdout (in "
+                           "addition to writing them to ${T}/log.do_<task>).")
 
-    parser.add_option("-b", "--buildfile", action="store", dest="buildfile", default=None,
+    parser.add_argument("-q", "--quiet", action="count", dest="quiet", default=0,
+                      help="Output less log message data to the terminal. You can specify this more than once.")
+
+    parser.add_argument("-D", "--debug", action="count", dest="debug", default=0,
+                      help="Increase the debug level. You can specify this "
+                           "more than once. -D sets the debug level to 1, "
+                           "where only bb.debug(1, ...) messages are printed "
+                           "to stdout; -DD sets the debug level to 2, where "
+                           "both bb.debug(1, ...) and bb.debug(2, ...) "
+                           "messages are printed; etc. Without -D, no debug "
+                           "messages are printed. Note that -D only affects "
+                           "output to stdout. All debug messages are written "
+                           "to ${T}/log.do_taskname, regardless of the debug "
+                           "level.")
+
+    parser.add_argument("-b", "--buildfile", dest="buildfile",
                       help="Execute tasks from a specific .bb recipe directly. WARNING: Does "
                            "not handle any dependencies from other recipes.")
 
-    parser.add_option("-k", "--continue", action="store_false", dest="abort", default=True,
+    parser.add_argument("-k", "--continue", action="store_false", dest="abort",
                       help="Continue as much as possible after an error. While the target that "
                            "failed and anything depending on it cannot be built, as much as "
                            "possible will be built before stopping.")
 
-    parser.add_option("-f", "--force", action="store_true", dest="force", default=False,
+    parser.add_argument("-f", "--force", action="store_true", dest="force",
                       help="Force the specified targets/task to run (invalidating any "
                            "existing stamp file).")
 
-    parser.add_option("-c", "--cmd", action="store", dest="cmd",
+    parser.add_argument("-c", "--cmd", dest="cmd",
                       help="Specify the task to execute. The exact options available "
                            "depend on the metadata. Some examples might be 'compile'"
                            " or 'populate_sysroot' or 'listtasks' may give a list of "
                            "the tasks available.")
 
-    parser.add_option("-C", "--clear-stamp", action="store", dest="invalidate_stamp",
+    parser.add_argument("-C", "--clear-stamp", dest="invalidate_stamp",
                       help="Invalidate the stamp for the specified task such as 'compile' "
                            "and then run the default task for the specified target(s).")
 
-    parser.add_option("-r", "--read", action="append", dest="prefile", default=[],
+    parser.add_argument("-r", "--read", action="append", dest="prefile", default=[],
                       help="Read the specified file before bitbake.conf.")
 
-    parser.add_option("-R", "--postread", action="append", dest="postfile", default=[],
+    parser.add_argument("-R", "--postread", action="append", dest="postfile", default=[],
                       help="Read the specified file after bitbake.conf.")
 
-    parser.add_option("-v", "--verbose", action="store_true", dest="verbose", default=False,
-                      help="Enable tracing of shell tasks (with 'set -x'). "
-                           "Also print bb.note(...) messages to stdout (in "
-                           "addition to writing them to ${T}/log.do_<task>).")
-
-    parser.add_option("-D", "--debug", action="count", dest="debug", default=0,
-                      help="Increase the debug level. You can specify this "
-                           "more than once. -D sets the debug level to 1, "
-                           "where only bb.debug(1, ...) messages are printed "
-                           "to stdout; -DD sets the debug level to 2, where "
-                           "both bb.debug(1, ...) and bb.debug(2, ...) "
-                           "messages are printed; etc. Without -D, no debug "
-                           "messages are printed. Note that -D only affects "
-                           "output to stdout. All debug messages are written "
-                           "to ${T}/log.do_taskname, regardless of the debug "
-                           "level.")
-
-    parser.add_option("-q", "--quiet", action="count", dest="quiet", default=0,
-                      help="Output less log message data to the terminal. You can specify this more than once.")
-
-    parser.add_option("-n", "--dry-run", action="store_true", dest="dry_run", default=False,
+    parser.add_argument("-n", "--dry-run", action="store_true", dest="dry_run", default=[],
                       help="Don't execute, just go through the motions.")
 
-    parser.add_option("-S", "--dump-signatures", action="append", dest="dump_signatures",
-                      default=[], metavar="SIGNATURE_HANDLER",
-                      help="Dump out the signature construction information, with no task "
+    parser.add_argument("-S", "--dump-signatures", action="append", dest="dump_signatures", metavar="SIGNATURE_HANDLER",
+                        default=[], help="Dump out the signature construction information, with no task "
                            "execution. The SIGNATURE_HANDLER parameter is passed to the "
                            "handler. Two common values are none and printdiff but the handler "
                            "may define more/less. none means only dump the signature, printdiff"
                            " means compare the dumped signature with the cached one.")
 
-    parser.add_option("-p", "--parse-only", action="store_true",
-                      dest="parse_only", default=False,
+    parser.add_argument("-p", "--parse-only", action="store_true", dest="parse_only",
                       help="Quit after parsing the BB recipes.")
 
-    parser.add_option("-s", "--show-versions", action="store_true",
-                      dest="show_versions", default=False,
+    parser.add_argument("-s", "--show-versions", action="store_true", dest="show_versions",
                       help="Show current and preferred versions of all recipes.")
 
-    parser.add_option("-e", "--environment", action="store_true",
-                      dest="show_environment", default=False,
+    parser.add_argument("-e", "--environment", action="store_true", dest="show_environment",
                       help="Show the global or per-recipe environment complete with information"
                            " about where variables were set/changed.")
 
-    parser.add_option("-g", "--graphviz", action="store_true", dest="dot_graph", default=False,
+    parser.add_argument("-g", "--graphviz", action="store_true", dest="dot_graph",
                       help="Save dependency tree information for the specified "
                            "targets in the dot syntax.")
 
-    parser.add_option("-I", "--ignore-deps", action="append",
-                      dest="extra_assume_provided", default=[],
+    parser.add_argument("-I", "--ignore-deps", action="append", default=[], dest="extra_assume_provided",
                       help="Assume these dependencies don't exist and are already provided "
                            "(equivalent to ASSUME_PROVIDED). Useful to make dependency "
                            "graphs more appealing")
 
-    parser.add_option("-l", "--log-domains", action="append", dest="debug_domains", default=[],
+    parser.add_argument("-l", "--log-domains", action="append", dest="debug_domains", default=[],
                       help="Show debug logging for the specified logging domains")
 
-    parser.add_option("-P", "--profile", action="store_true", dest="profile", default=False,
+    parser.add_argument("-P", "--profile", action="store_true", dest="profile", default=[],
                       help="Profile the command and save reports.")
 
-    # @CHOICES@ is substituted out by BitbakeHelpFormatter above
-    parser.add_option("-u", "--ui", action="store", dest="ui",
-                      default=os.environ.get('BITBAKE_UI', 'knotty'),
-                      help="The user interface to use (@CHOICES@ - default %default).")
-
-    parser.add_option("", "--token", action="store", dest="xmlrpctoken",
-                      default=os.environ.get("BBTOKEN"),
+    parser.add_argument("--token", dest="xmlrpctoken", default=os.environ.get("BBTOKEN"),
                       help="Specify the connection token to be used when connecting "
                            "to a remote server.")
 
-    parser.add_option("", "--revisions-changed", action="store_true",
-                      dest="revisions_changed", default=False,
+    parser.add_argument("--revisions-changed", action="store_true", dest="revisions_changed",
                       help="Set the exit code depending on whether upstream floating "
                            "revisions have changed or not.")
 
-    parser.add_option("", "--server-only", action="store_true",
-                      dest="server_only", default=False,
+    parser.add_argument("--server-only", action="store_true", dest="server_only",
                       help="Run bitbake without a UI, only starting a server "
                            "(cooker) process.")
 
-    parser.add_option("-B", "--bind", action="store", dest="bind", default=False,
+    parser.add_argument("-B", "--bind", dest="bind",
                       help="The name/address for the bitbake xmlrpc server to bind to.")
 
-    parser.add_option("-T", "--idle-timeout", type=float, dest="server_timeout",
+    parser.add_argument("-T", "--idle-timeout", type=float, dest="server_timeout",
                       default=os.getenv("BB_SERVER_TIMEOUT"),
                       help="Set timeout to unload bitbake server due to inactivity, "
                            "set to -1 means no unload, "
                            "default: Environment variable BB_SERVER_TIMEOUT.")
 
-    parser.add_option("", "--no-setscene", action="store_true",
-                      dest="nosetscene", default=False,
+    parser.add_argument("--no-setscene", action="store_true", dest="nosetscene",
                       help="Do not run any setscene tasks. sstate will be ignored and "
                            "everything needed, built.")
 
-    parser.add_option("", "--skip-setscene", action="store_true",
-                      dest="skipsetscene", default=False,
+    parser.add_argument("--skip-setscene", action="store_true", dest="skipsetscene",
                       help="Skip setscene tasks if they would be executed. Tasks previously "
                            "restored from sstate will be kept, unlike --no-setscene")
 
-    parser.add_option("", "--setscene-only", action="store_true",
-                      dest="setsceneonly", default=False,
+    parser.add_argument("--setscene-only", action="store_true", dest="setsceneonly",
                       help="Only run setscene tasks, don't run any real tasks.")
 
-    parser.add_option("", "--remote-server", action="store", dest="remote_server",
-                      default=os.environ.get("BBSERVER"),
+    parser.add_argument("--remote-server", dest="remote_server", default=os.environ.get("BBSERVER"),
                       help="Connect to the specified server.")
 
-    parser.add_option("-m", "--kill-server", action="store_true",
-                      dest="kill_server", default=False,
+    parser.add_argument("-m", "--kill-server", action="store_true", dest="kill_server",
                       help="Terminate any running bitbake server.")
 
-    parser.add_option("", "--observe-only", action="store_true",
-                      dest="observe_only", default=False,
+    parser.add_argument("--observe-only", action="store_true", dest="observe_only",
                       help="Connect to a server as an observing-only client.")
 
-    parser.add_option("", "--status-only", action="store_true",
-                      dest="status_only", default=False,
+    parser.add_argument("--status-only", action="store_true", dest="status_only",
                       help="Check the status of the remote bitbake server.")
 
-    parser.add_option("-w", "--write-log", action="store", dest="writeeventlog",
-                      default=os.environ.get("BBEVENTLOG"),
+    parser.add_argument("-w", "--write-log", dest="writeeventlog", default=os.environ.get("BBEVENTLOG"),
                       help="Writes the event log of the build to a bitbake event json file. "
                            "Use '' (empty string) to assign the name automatically.")
 
-    parser.add_option("", "--runall", action="append", dest="runall",
-                      help="Run the specified task for any recipe in the taskgraph of the specified target (even if it wouldn't otherwise have run).")
+    parser.add_argument("--runall", action="append", dest="runall", default=[],
+                      help="Run the specified task for any recipe in the taskgraph of the specified target (even if "
+                           "it wouldn't otherwise have run).")
 
-    parser.add_option("", "--runonly", action="append", dest="runonly",
-                      help="Run only the specified task within the taskgraph of the specified targets (and any task dependencies those tasks may have).")
+    parser.add_argument("--runonly", action="append", dest="runonly", default=[],
+                      help="Run only the specified task within the taskgraph of the specified targets (and any task "
+                           "dependencies those tasks may have).")
     return parser
 
 
 class BitBakeConfigParameters(cookerdata.ConfigParameters):
-    def parseCommandLine(self, argv=sys.argv):
+    def parseCommandLine(self, argv=None):
         parser = create_bitbake_parser()
-        options, targets = parser.parse_args(argv)
+        args = parser.parse_args(argv or sys.argv)
 
-        if options.quiet and options.verbose:
+        if args.quiet and args.verbose:
             parser.error("options --quiet and --verbose are mutually exclusive")
 
-        if options.quiet and options.debug:
+        if args.quiet and args.debug:
             parser.error("options --quiet and --debug are mutually exclusive")
 
         # use configuration files from environment variables
         if "BBPRECONF" in os.environ:
-            options.prefile.append(os.environ["BBPRECONF"])
+            args.prefile.append(os.environ["BBPRECONF"])
 
         if "BBPOSTCONF" in os.environ:
-            options.postfile.append(os.environ["BBPOSTCONF"])
+            args.postfile.append(os.environ["BBPOSTCONF"])
 
         # fill in proper log name if not supplied
-        if options.writeeventlog is not None and len(options.writeeventlog) == 0:
+        if args.writeeventlog is not None and len(args.writeeventlog) == 0:
             from datetime import datetime
             eventlog = "bitbake_eventlog_%s.json" % datetime.now().strftime("%Y%m%d%H%M%S")
-            options.writeeventlog = eventlog
+            args.writeeventlog = eventlog
 
-        if options.bind:
+        if args.bind:
             try:
                 #Checking that the port is a number and is a ':' delimited value
-                (host, port) = options.bind.split(':')
+                (host, port) = args.bind.split(':')
                 port = int(port)
             except (ValueError,IndexError):
                 raise BBMainException("FATAL: Malformed host:port bind parameter")
-            options.xmlrpcinterface = (host, port)
+            args.xmlrpcinterface = (host, port)
         else:
-            options.xmlrpcinterface = (None, 0)
+            args.xmlrpcinterface = (None, 0)
 
-        return options, targets[1:]
+        return args, args.targets[1:]
 
 
 def bitbake_main(configParams, configuration):
@@ -402,9 +393,9 @@ def setup_bitbake(configParams, extrafeatures=None):
         featureset = []
         ui_module = None
     else:
-        ui_module = import_extension_module(bb.ui, configParams.ui, 'main')
         # Collect the feature set for the UI
-        featureset = getattr(ui_module, "featureSet", [])
+        featureset = getattr(configParams.ui, "featureSet", [])
+        ui_module = configParams.ui
 
     if extrafeatures:
         for feature in extrafeatures:
-- 
2.17.1

[PATCH v3 05/10] main: group --help options to make them easier to read

[Chris Laplante]

(YOCTO #12018)

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/main.py | 175 +++++++++++++++++++++++++++----------------------
 1 file changed, 95 insertions(+), 80 deletions(-)

diff --git a/lib/bb/main.py b/lib/bb/main.py
index 5bd41b1c..e211768f 100755
--- a/lib/bb/main.py
+++ b/lib/bb/main.py
@@ -127,28 +127,56 @@ def create_bitbake_parser():
     Executes the specified task (default is 'build') for a given set of target recipes (.bb files).
     It is assumed there is a conf/bblayers.conf available in cwd or in BBPATH which
     will provide the layer, BBFILES and other configuration information.
-    """)
+    """, add_help=False)
     parser.add_argument("targets", nargs="*")
-    parser.add_argument("--version",
-                        action="version",
-                        version="BitBake Build Tool Core version {0}".format(bb.__version__))
 
+    # List options that the user is most likely to use first
+    frequent_group = parser.add_argument_group("frequently used")
+    frequent_group.add_argument("-c", "--cmd", dest="cmd",
+                                help="Specify the task to execute. The exact options available "
+                                     "depend on the metadata. Some examples might be 'compile'"
+                                     " or 'populate_sysroot' or 'listtasks' may give a list of "
+                                     "the tasks available.")
+
+    frequent_group.add_argument("-k", "--continue", action="store_false", dest="abort",
+                                help="Continue as much as possible after an error. While the target that "
+                                     "failed and anything depending on it cannot be built, as much as "
+                                     "possible will be built before stopping.")
+
+    frequent_group.add_argument("-C", "--clear-stamp", dest="invalidate_stamp",
+                                help="Invalidate the stamp for the specified task such as 'compile' "
+                                     "and then run the default task for the specified target(s).")
+
+    frequent_group.add_argument("--runall", action="append", dest="runall", default=[],
+                                help="Run the specified task for any recipe in the taskgraph of the specified target (even if "
+                                     "it wouldn't otherwise have run).")
+
+    frequent_group.add_argument("--runonly", action="append", dest="runonly", default=[],
+                                help="Run only the specified task within the taskgraph of the specified targets (and any task "
+                                     "dependencies those tasks may have).")
+
+    frequent_group.add_argument("-f", "--force", action="store_true", dest="force",
+                            help="Force the specified targets/task to run (invalidating any "
+                                 "existing stamp file).")
+
+    # Options having to do with UI or verbosity/debug reporting
+    output_control_group = parser.add_argument_group("output control")
     ui_choices = LazyUiChoices()
     # Setting metavar="" is necessary otherwise argparse will immediately try to expand the choices
-    parser.add_argument("-u", "--ui",
+    output_control_group.add_argument("-u", "--ui",
                         default=os.environ.get('BITBAKE_UI', 'knotty'),
                         help="The user interface to use (%(choices)s) - default: %(default)s",
                         choices=ui_choices, metavar="", type=ui_choices)
 
-    parser.add_argument("-v", "--verbose", action="store_true", dest="verbose",
+    output_control_group.add_argument("-v", "--verbose", action="store_true", dest="verbose",
                       help="Enable tracing of shell tasks (with 'set -x'). "
                            "Also print bb.note(...) messages to stdout (in "
                            "addition to writing them to ${T}/log.do_<task>).")
 
-    parser.add_argument("-q", "--quiet", action="count", dest="quiet", default=0,
+    output_control_group.add_argument("-q", "--quiet", action="count", dest="quiet", default=0,
                       help="Output less log message data to the terminal. You can specify this more than once.")
 
-    parser.add_argument("-D", "--debug", action="count", dest="debug", default=0,
+    output_control_group.add_argument("-D", "--debug", action="count", dest="debug", default=0,
                       help="Increase the debug level. You can specify this "
                            "more than once. -D sets the debug level to 1, "
                            "where only bb.debug(1, ...) messages are printed "
@@ -160,125 +188,112 @@ def create_bitbake_parser():
                            "to ${T}/log.do_taskname, regardless of the debug "
                            "level.")
 
-    parser.add_argument("-b", "--buildfile", dest="buildfile",
-                      help="Execute tasks from a specific .bb recipe directly. WARNING: Does "
-                           "not handle any dependencies from other recipes.")
+    output_control_group.add_argument("-w", "--write-log", dest="writeeventlog", default=os.environ.get("BBEVENTLOG"),
+                        help="Writes the event log of the build to a bitbake event json file. "
+                             "Use '' (empty string) to assign the name automatically.")
 
-    parser.add_argument("-k", "--continue", action="store_false", dest="abort",
-                      help="Continue as much as possible after an error. While the target that "
-                           "failed and anything depending on it cannot be built, as much as "
-                           "possible will be built before stopping.")
+    output_control_group.add_argument("-l", "--log-domains", action="append", dest="debug_domains", default=[],
+                        help="Show debug logging for the specified logging domains")
 
-    parser.add_argument("-f", "--force", action="store_true", dest="force",
-                      help="Force the specified targets/task to run (invalidating any "
-                           "existing stamp file).")
+    # Debugging options
+    debugging_group = parser.add_argument_group("debugging and introspection")
+    debugging_group.add_argument("-e", "--environment", action="store_true", dest="show_environment",
+                                 help="Show the global or per-recipe environment complete with information"
+                                      " about where variables were set/changed.")
 
-    parser.add_argument("-c", "--cmd", dest="cmd",
-                      help="Specify the task to execute. The exact options available "
-                           "depend on the metadata. Some examples might be 'compile'"
-                           " or 'populate_sysroot' or 'listtasks' may give a list of "
-                           "the tasks available.")
-
-    parser.add_argument("-C", "--clear-stamp", dest="invalidate_stamp",
-                      help="Invalidate the stamp for the specified task such as 'compile' "
-                           "and then run the default task for the specified target(s).")
-
-    parser.add_argument("-r", "--read", action="append", dest="prefile", default=[],
-                      help="Read the specified file before bitbake.conf.")
-
-    parser.add_argument("-R", "--postread", action="append", dest="postfile", default=[],
-                      help="Read the specified file after bitbake.conf.")
-
-    parser.add_argument("-n", "--dry-run", action="store_true", dest="dry_run", default=[],
+    debugging_group.add_argument("-n", "--dry-run", action="store_true", dest="dry_run", default=[],
                       help="Don't execute, just go through the motions.")
 
-    parser.add_argument("-S", "--dump-signatures", action="append", dest="dump_signatures", metavar="SIGNATURE_HANDLER",
+    debugging_group.add_argument("-S", "--dump-signatures", action="append", dest="dump_signatures", metavar="SIGNATURE_HANDLER",
                         default=[], help="Dump out the signature construction information, with no task "
                            "execution. The SIGNATURE_HANDLER parameter is passed to the "
                            "handler. Two common values are none and printdiff but the handler "
                            "may define more/less. none means only dump the signature, printdiff"
                            " means compare the dumped signature with the cached one.")
 
-    parser.add_argument("-p", "--parse-only", action="store_true", dest="parse_only",
+    debugging_group.add_argument("-p", "--parse-only", action="store_true", dest="parse_only",
                       help="Quit after parsing the BB recipes.")
 
-    parser.add_argument("-s", "--show-versions", action="store_true", dest="show_versions",
+    debugging_group.add_argument("-s", "--show-versions", action="store_true", dest="show_versions",
                       help="Show current and preferred versions of all recipes.")
 
-    parser.add_argument("-e", "--environment", action="store_true", dest="show_environment",
-                      help="Show the global or per-recipe environment complete with information"
-                           " about where variables were set/changed.")
-
-    parser.add_argument("-g", "--graphviz", action="store_true", dest="dot_graph",
+    debugging_group.add_argument("-g", "--graphviz", action="store_true", dest="dot_graph",
                       help="Save dependency tree information for the specified "
                            "targets in the dot syntax.")
 
-    parser.add_argument("-I", "--ignore-deps", action="append", default=[], dest="extra_assume_provided",
+    debugging_group.add_argument("--revisions-changed", action="store_true", dest="revisions_changed",
+                        help="Set the exit code depending on whether upstream floating "
+                             "revisions have changed or not.")
+
+    debugging_group.add_argument("-P", "--profile", action="store_true", dest="profile", default=[],
+                                              help="Profile the command and save reports.")
+
+    # Developer options that aren't commonly used by normal users
+    developer_group = parser.add_argument_group("developer")
+    developer_group.add_argument("-r", "--read", action="append", dest="prefile", default=[],
+                                   help="Read the specified file before bitbake.conf.")
+
+    developer_group.add_argument("-R", "--postread", action="append", dest="postfile", default=[],
+                                   help="Read the specified file after bitbake.conf.")
+
+    developer_group.add_argument("-I", "--ignore-deps", action="append", default=[], dest="extra_assume_provided",
                       help="Assume these dependencies don't exist and are already provided "
                            "(equivalent to ASSUME_PROVIDED). Useful to make dependency "
                            "graphs more appealing")
 
-    parser.add_argument("-l", "--log-domains", action="append", dest="debug_domains", default=[],
-                      help="Show debug logging for the specified logging domains")
+    developer_group.add_argument("--no-setscene", action="store_true", dest="nosetscene",
+                        help="Do not run any setscene tasks. sstate will be ignored and "
+                             "everything needed, built.")
 
-    parser.add_argument("-P", "--profile", action="store_true", dest="profile", default=[],
-                      help="Profile the command and save reports.")
+    developer_group.add_argument("--skip-setscene", action="store_true", dest="skipsetscene",
+                        help="Skip setscene tasks if they would be executed. Tasks previously "
+                             "restored from sstate will be kept, unlike --no-setscene")
 
-    parser.add_argument("--token", dest="xmlrpctoken", default=os.environ.get("BBTOKEN"),
+    developer_group.add_argument("--setscene-only", action="store_true", dest="setsceneonly",
+                        help="Only run setscene tasks, don't run any real tasks.")
+
+    developer_group.add_argument("-b", "--buildfile", dest="buildfile",
+                               help="Execute tasks from a specific .bb recipe directly. WARNING: Does "
+                                    "not handle any dependencies from other recipes.")
+    # Server control options
+    server_control_group = parser.add_argument_group("server control")
+    server_control_group.add_argument("--token", dest="xmlrpctoken", default=os.environ.get("BBTOKEN"),
                       help="Specify the connection token to be used when connecting "
                            "to a remote server.")
 
-    parser.add_argument("--revisions-changed", action="store_true", dest="revisions_changed",
-                      help="Set the exit code depending on whether upstream floating "
-                           "revisions have changed or not.")
-
-    parser.add_argument("--server-only", action="store_true", dest="server_only",
+    server_control_group.add_argument("--server-only", action="store_true", dest="server_only",
                       help="Run bitbake without a UI, only starting a server "
                            "(cooker) process.")
 
-    parser.add_argument("-B", "--bind", dest="bind",
+    server_control_group.add_argument("-B", "--bind", dest="bind",
                       help="The name/address for the bitbake xmlrpc server to bind to.")
 
-    parser.add_argument("-T", "--idle-timeout", type=float, dest="server_timeout",
+    server_control_group.add_argument("-T", "--idle-timeout", type=float, dest="server_timeout",
                       default=os.getenv("BB_SERVER_TIMEOUT"),
                       help="Set timeout to unload bitbake server due to inactivity, "
                            "set to -1 means no unload, "
                            "default: Environment variable BB_SERVER_TIMEOUT.")
 
-    parser.add_argument("--no-setscene", action="store_true", dest="nosetscene",
-                      help="Do not run any setscene tasks. sstate will be ignored and "
-                           "everything needed, built.")
-
-    parser.add_argument("--skip-setscene", action="store_true", dest="skipsetscene",
-                      help="Skip setscene tasks if they would be executed. Tasks previously "
-                           "restored from sstate will be kept, unlike --no-setscene")
-
-    parser.add_argument("--setscene-only", action="store_true", dest="setsceneonly",
-                      help="Only run setscene tasks, don't run any real tasks.")
-
-    parser.add_argument("--remote-server", dest="remote_server", default=os.environ.get("BBSERVER"),
+    server_control_group.add_argument("--remote-server", dest="remote_server", default=os.environ.get("BBSERVER"),
                       help="Connect to the specified server.")
 
-    parser.add_argument("-m", "--kill-server", action="store_true", dest="kill_server",
+    server_control_group.add_argument("-m", "--kill-server", action="store_true", dest="kill_server",
                       help="Terminate any running bitbake server.")
 
-    parser.add_argument("--observe-only", action="store_true", dest="observe_only",
+    server_control_group.add_argument("--observe-only", action="store_true", dest="observe_only",
                       help="Connect to a server as an observing-only client.")
 
-    parser.add_argument("--status-only", action="store_true", dest="status_only",
+    server_control_group.add_argument("--status-only", action="store_true", dest="status_only",
                       help="Check the status of the remote bitbake server.")
 
-    parser.add_argument("-w", "--write-log", dest="writeeventlog", default=os.environ.get("BBEVENTLOG"),
-                      help="Writes the event log of the build to a bitbake event json file. "
-                           "Use '' (empty string) to assign the name automatically.")
+    # Options that don't really fit elsewhere
+    other_options = parser.add_argument_group("other")
+    other_options.add_argument("--version",
+                    action="version",
+                    version="BitBake Build Tool Core version {0}".format(bb.__version__))
 
-    parser.add_argument("--runall", action="append", dest="runall", default=[],
-                      help="Run the specified task for any recipe in the taskgraph of the specified target (even if "
-                           "it wouldn't otherwise have run).")
+    other_options.add_argument("-h", "--help", action="help", help="show this help message and exit")
 
-    parser.add_argument("--runonly", action="append", dest="runonly", default=[],
-                      help="Run only the specified task within the taskgraph of the specified targets (and any task "
-                           "dependencies those tasks may have).")
     return parser
 
 
-- 
2.17.1

[PATCH v3 06/10] main: rename list_extension_modules => load_extension_modules to clarify its function

[Chris Laplante]

Also clearly document in docstring that it is the modules themselves
that are returned.

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/main.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/bb/main.py b/lib/bb/main.py
index e211768f..ccb844bb 100755
--- a/lib/bb/main.py
+++ b/lib/bb/main.py
@@ -40,9 +40,9 @@ class BBMainFatal(bb.BBHandledException):
     pass
 
 
-def list_extension_modules(pkg, checkattr):
+def load_extension_modules(pkg, checkattr):
     """
-    Lists extension modules in a specific Python package
+    Loads extension modules in a specific Python namespace package.
     (e.g. UIs, servers). NOTE: Calling this function will import all of the
     submodules of the specified module in order to check for the specified
     attribute; this can have unusual side-effects. As a result, this should
@@ -51,6 +51,7 @@ def list_extension_modules(pkg, checkattr):
         pkg: previously imported Python package to list
         checkattr: attribute to look for in module to determine if it's valid
             as the type of extension you are looking for
+    Returns: list of modules (imported via importlib)
     """
     import pkgutil
     import importlib
@@ -101,7 +102,7 @@ class LazyUiChoices:
 
     def _lazy_load_modules(self):
         if not self._modules:
-            self._modules = {module.__name__.split(".")[-1]: module for module in list_extension_modules(bb.ui, "main")}
+            self._modules = {module.__name__.split(".")[-1]: module for module in load_extension_modules(bb.ui, "main")}
 
     def __iter__(self):
         self._lazy_load_modules()
-- 
2.17.1

[PATCH v3 07/10] tests/arg_parser: add test for lazy loading of bb.ui modules

[Chris Laplante]

Ensures bb.ui modules are loaded lazily during construction
of the 'bitbake' argument parser.

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/main.py             |  5 ++++-
 lib/bb/tests/arg_parser.py | 18 +++++++++++++++---
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/lib/bb/main.py b/lib/bb/main.py
index ccb844bb..f8a19bc3 100755
--- a/lib/bb/main.py
+++ b/lib/bb/main.py
@@ -98,12 +98,15 @@ class LazyUiChoices:
     """
     def __init__(self):
         # map module name => imported module
-        self._modules = {}
+        self._modules = None
 
     def _lazy_load_modules(self):
         if not self._modules:
             self._modules = {module.__name__.split(".")[-1]: module for module in load_extension_modules(bb.ui, "main")}
 
+    @property
+    def has_loaded_modules(self): return self._modules is not None
+
     def __iter__(self):
         self._lazy_load_modules()
         yield from self._modules.keys()
diff --git a/lib/bb/tests/arg_parser.py b/lib/bb/tests/arg_parser.py
index fd46fe8c..be2d86fa 100644
--- a/lib/bb/tests/arg_parser.py
+++ b/lib/bb/tests/arg_parser.py
@@ -7,12 +7,11 @@
 # SPDX-License-Identifier: MIT
 #
 
+import collections
 import shlex
 import unittest
-import collections
-
 
-from bb.main import create_bitbake_parser, BitBakeConfigParameters
+from bb.main import create_bitbake_parser, BitBakeConfigParameters, LazyUiChoices
 
 ParseResult = collections.namedtuple("ParseResult", ("options", "targets"))
 
@@ -41,3 +40,16 @@ class ArgParserTests(unittest.TestCase):
             res = self._parse_helper(arg_str)
             self.assertListEqual(res.targets, ["world"])
             self.assertListEqual(res.options.dump_signatures, ["none"])
+
+    def test_lazy_module_loading(self):
+        # This test ensures that the bb.ui modules weren't loaded immediately upon creation of the arg parser
+
+        # Find the UI action
+        ui_actions = [action for action in self._parser._actions if action.dest == "ui"]
+        if len(ui_actions) > 1:
+            self.fail("Found more than one 'ui' action in the argument parser?")
+        self.assertEqual(len(ui_actions), 1, "Didn't find the 'ui' action in the argument parser?")
+
+        ui_action = ui_actions[0]
+        self.assertIs(type(ui_action.type), LazyUiChoices, "'ui' action has wrong type")
+        self.assertFalse(ui_action.type.has_loaded_modules, "Creation of the arg parser loaded the bb.ui modules")
-- 
2.17.1

[PATCH v3 08/10] main: fix parsing of intermixed arguments

[Chris Laplante]

use parse_known_args instead of parse_args. Otherwise, argparse
incorreclty interprets (for example) 'bitbake -S none world'.

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/main.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/bb/main.py b/lib/bb/main.py
index f8a19bc3..9ce931d6 100755
--- a/lib/bb/main.py
+++ b/lib/bb/main.py
@@ -132,7 +132,6 @@ def create_bitbake_parser():
     It is assumed there is a conf/bblayers.conf available in cwd or in BBPATH which
     will provide the layer, BBFILES and other configuration information.
     """, add_help=False)
-    parser.add_argument("targets", nargs="*")
 
     # List options that the user is most likely to use first
     frequent_group = parser.add_argument_group("frequently used")
@@ -304,7 +303,7 @@ def create_bitbake_parser():
 class BitBakeConfigParameters(cookerdata.ConfigParameters):
     def parseCommandLine(self, argv=None):
         parser = create_bitbake_parser()
-        args = parser.parse_args(argv or sys.argv)
+        args, targets = parser.parse_known_args(argv or sys.argv)
 
         if args.quiet and args.verbose:
             parser.error("options --quiet and --verbose are mutually exclusive")
@@ -336,7 +335,7 @@ class BitBakeConfigParameters(cookerdata.ConfigParameters):
         else:
             args.xmlrpcinterface = (None, 0)
 
-        return args, args.targets[1:]
+        return args, targets[1:]
 
 
 def bitbake_main(configParams, configuration):
-- 
2.17.1

[PATCH v3 09/10] tinfoil: use knotty module itself as default 'ui' arg to keep up with changes in setup_bitbake

[Chris Laplante]

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/tinfoil.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/bb/tinfoil.py b/lib/bb/tinfoil.py
index 763c3298..766a45db 100644
--- a/lib/bb/tinfoil.py
+++ b/lib/bb/tinfoil.py
@@ -867,13 +867,14 @@ class TinfoilConfigParameters(BitBakeConfigParameters):
     def __init__(self, config_only, **options):
         self.initial_options = options
         # Apply some sane defaults
-        if not 'parse_only' in options:
+        if 'parse_only' not in options:
             self.initial_options['parse_only'] = not config_only
         #if not 'status_only' in options:
         #    self.initial_options['status_only'] = config_only
-        if not 'ui' in options:
-            self.initial_options['ui'] = 'knotty'
-        if not 'argv' in options:
+        if 'ui' not in options:
+            import importlib
+            self.initial_options['ui'] = importlib.import_module("bb.ui.knotty")
+        if 'argv' not in options:
             self.initial_options['argv'] = []
 
         super(TinfoilConfigParameters, self).__init__()
-- 
2.17.1

[PATCH v3 10/10] tests/arg_parser: add test for default TinfoilConfigParameters 'ui'

[Chris Laplante]

Signed-off-by: Chris Laplante <chris.laplante@agilent.com>
---
 lib/bb/tests/arg_parser.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/bb/tests/arg_parser.py b/lib/bb/tests/arg_parser.py
index be2d86fa..73264066 100644
--- a/lib/bb/tests/arg_parser.py
+++ b/lib/bb/tests/arg_parser.py
@@ -9,9 +9,11 @@
 
 import collections
 import shlex
+import types
 import unittest
 
 from bb.main import create_bitbake_parser, BitBakeConfigParameters, LazyUiChoices
+from bb.tinfoil import TinfoilConfigParameters
 
 ParseResult = collections.namedtuple("ParseResult", ("options", "targets"))
 
@@ -53,3 +55,8 @@ class ArgParserTests(unittest.TestCase):
         ui_action = ui_actions[0]
         self.assertIs(type(ui_action.type), LazyUiChoices, "'ui' action has wrong type")
         self.assertFalse(ui_action.type.has_loaded_modules, "Creation of the arg parser loaded the bb.ui modules")
+
+    def test_tinfoil_default_ui_arg(self):
+        # Ensure the default 'ui' parameter is a module, not a str
+        params = TinfoilConfigParameters(config_only=True)
+        self.assertIs(type(params.ui), types.ModuleType)
-- 
2.17.1

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Richard Purdie]

On Sun, 2020-11-08 at 17:10 -0500, Chris Laplante via
lists.openembedded.org wrote:
> v1: initial
> v2: fix intermixed argument parsing
> v3: fix TinfoilConfigParameters defaulting of 'ui' with string,
> reorder
>     patches

Unfortunately oe-selftest is still throwing a few issues. In
particular, "oe-selftest -r tinfoil" is relatively fast but other
tinfoil users such as recipe tool and oe-pkgdata-util are failing.

https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/1533/steps/8/logs/step2d

Cheers,

Richard

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Chris Laplante]

> Unfortunately oe-selftest is still throwing a few issues. In particular, "oe-
> selftest -r tinfoil" is relatively fast but other tinfoil users such as recipe tool
> and oe-pkgdata-util are failing.
> 
> https://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fauto
> builder.yoctoproject.org%2Ftyphoon%2F%23%2Fbuilders%2F80%2Fbuilds%2
> F1533%2Fsteps%2F8%2Flogs%2Fstep2d&data=04%7C01%7Cchris.lapla
> nte%40agilent.com%7Cf26e5cc4b75b4a27cfc108d885c043f7%7Ca9c0bc098
> b46420693512ba12fb4a5c0%7C0%7C0%7C637406408102611086%7CUnkno
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1
> haWwiLCJXVCI6Mn0%3D%7C1000&sdata=pqpItkkPu%2BntgO1lr%2BiCL
> UKCSphuniUFFrcTtSjvtjU%3D&reserved=0


To be honest I forgot about oe-selftest... I was only running bitbake-selftest. I will re-spin the patches after all tests pass.

Thanks,
Chris

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Richard Purdie]

On Sun, 2020-11-08 at 17:10 -0500, Chris Laplante via lists.openembedded.org wrote:
> v1: initial
> v2: fix intermixed argument parsing
> v3: fix TinfoilConfigParameters defaulting of 'ui' with string,
> reorder
>     patches

One other thing which would really help me is if you could either fix
your SMTP server setup so the list isn't as unhappy and therefore
doesn't present you as "Chris Laplante via lists.openembedded.org <
chris.laplante=agilent.com@lists.openembedded.org>", or add From: lines
to the start of the patches so the author is set correctly, or CC: me
directly on the patches so I get a copy which hasn't hit the mailing
list and been mangled.

I've been fixing these up manually but its getting a bit wearing given
the number you've sent. I guess the alternative is I improve my
scripting which may be the other options, its kind of inevitable.

Cheers,

Richard

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Chris Laplante]

> One other thing which would really help me is if you could either fix your
> SMTP server setup so the list isn't as unhappy and therefore doesn't present
> you as "Chris Laplante via lists.openembedded.org <
> chris.laplante=agilent.com@lists.openembedded.org>", or add From: lines to
> the start of the patches so the author is set correctly, or CC: me directly on
> the patches so I get a copy which hasn't hit the mailing list and been
> mangled.
> 
> I've been fixing these up manually but its getting a bit wearing given the
> number you've sent. I guess the alternative is I improve my scripting which
> may be the other options, its kind of inevitable.

Sorry about that - I'll make sure to add the "From:" lines. 

Chris

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Chris Laplante]

Hi Richard,

> One other thing which would really help me is if you could either fix your
> SMTP server setup so the list isn't as unhappy and therefore doesn't present
> you as "Chris Laplante via lists.openembedded.org <
> chris.laplante=agilent.com@lists.openembedded.org>", or add From: lines to
> the start of the patches so the author is set correctly, or CC: me directly on
> the patches so I get a copy which hasn't hit the mailing list and been
> mangled.
> 
> I've been fixing these up manually but its getting a bit wearing given the
> number you've sent. I guess the alternative is I improve my scripting which
> may be the other options, its kind of inevitable.

Can you please tell me specifically how the patches I'm sending are getting mangled? I'm having trouble finding information online about how I may fix this.

I also can't find an automated way of adding 'From: ' lines to the emails produced by git send-email. 

Thanks,
Chris

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Richard Purdie]

On Wed, 2020-11-18 at 16:37 +0000, chris.laplante@agilent.com wrote:
> Hi Richard,
> 
> > One other thing which would really help me is if you could either
> > fix your
> > SMTP server setup so the list isn't as unhappy and therefore
> > doesn't present
> > you as "Chris Laplante via lists.openembedded.org <
> > chris.laplante=agilent.com@lists.openembedded.org>", or add From:
> > lines to
> > the start of the patches so the author is set correctly, or CC: me
> > directly on
> > the patches so I get a copy which hasn't hit the mailing list and
> > been
> > mangled.
> > 
> > I've been fixing these up manually but its getting a bit wearing
> > given the
> > number you've sent. I guess the alternative is I improve my
> > scripting which
> > may be the other options, its kind of inevitable.
> 
> Can you please tell me specifically how the patches I'm sending are
> getting mangled? I'm having trouble finding information online about
> how I may fix this.
> 
> I also can't find an automated way of adding 'From: ' lines to the
> emails produced by git send-email. 

groups.io (where the list is hosted) is rewriting your email address
since your domain does not allow it to send out messages 'pretending'
to be <something>@agilent.com. This is a general problem mailing lists
have with increased spam protections that are now in place. The
rewritten version is "Chris Laplante via lists.openembedded.org 
 <chris.laplante=agilent.com@lists.openembedded.org>" which works as we
allow groups.io to sent from lists.openembedded.org.

If you can influence the DNS/SPF for agilent.com, you can probably
improve things otherwise we're into workaround territory on one end or
the other.

I can handle it worst case, I probably just need to increase the
automation of my patch handling and some brute force sed hackery...

Cheers,

Richard

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Chris Laplante]

> > Can you please tell me specifically how the patches I'm sending are
> > getting mangled? I'm having trouble finding information online about
> > how I may fix this.
> >
> > I also can't find an automated way of adding 'From: ' lines to the
> > emails produced by git send-email.
> 
> groups.io (where the list is hosted) is rewriting your email address since your
> domain does not allow it to send out messages 'pretending'
> to be <something>@agilent.com. This is a general problem mailing lists have
> with increased spam protections that are now in place. The rewritten version
> is "Chris Laplante via lists.openembedded.org
> <chris.laplante=agilent.com@lists.openembedded.org>" which works as we
> allow groups.io to sent from lists.openembedded.org.
> 
> If you can influence the DNS/SPF for agilent.com, you can probably improve
> things otherwise we're into workaround territory on one end or the other.
> 
> I can handle it worst case, I probably just need to increase the automation of
> my patch handling and some brute force sed hackery...

Unfortunately I doubt I can do anything about the DNS/SPF:(

I'd rather avoid you needing to make changes to your scripting just to deal with my patches, so I'll make sure to set the 'From: ' header. I'm going to submit a patch soon as a kind of test to see if it works (though it is an actual patch for an issue I found, separate from all the argparse stuff).

N.B.: I really wish git had a hook that runs at 'send-email' time. As I understand it, that would be the correct place to automatically prefix the message body with the 'From: ' header. I tried using 'prepare-commit-msg' to prefix the entire commit message with 'From: ' but that dirties the log and breaks 'git log --oneline'. Maybe I'm missing something but I just don't see a way to do this automatically without writing a wrapper script around send-email, which I'll probably end up doing.

Thanks,
Chris

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Richard Purdie]

On Wed, 2020-11-18 at 17:07 +0000, chris.laplante@agilent.com wrote:
> > > Can you please tell me specifically how the patches I'm sending are
> > > getting mangled? I'm having trouble finding information online about
> > > how I may fix this.
> > > 
> > > I also can't find an automated way of adding 'From: ' lines to the
> > > emails produced by git send-email.
> > 
> > groups.io (where the list is hosted) is rewriting your email address since your
> > domain does not allow it to send out messages 'pretending'
> > to be <something>@agilent.com. This is a general problem mailing lists have
> > with increased spam protections that are now in place. The rewritten version
> > is "Chris Laplante via lists.openembedded.org
> > <chris.laplante=agilent.com@lists.openembedded.org>" which works as we
> > allow groups.io to sent from lists.openembedded.org.
> > 
> > If you can influence the DNS/SPF for agilent.com, you can probably improve
> > things otherwise we're into workaround territory on one end or the other.
> > 
> > I can handle it worst case, I probably just need to increase the automation of
> > my patch handling and some brute force sed hackery...
> 
> Unfortunately I doubt I can do anything about the DNS/SPF:(
> 
> I'd rather avoid you needing to make changes to your scripting just
> to deal with my patches, so I'll make sure to set the 'From: '
> header. I'm going to submit a patch soon as a kind of test to see if
> it works (though it is an actual patch for an issue I found, separate
> from all the argparse stuff).

You aren't the only one! Most of our regular contributors don't have
the problem and it has improved for others over time as people
understand the issues and set the SPF more favourably. I don't know the
details, I've just noticed some people who had the issue don't now.

Its not been problematic enough that I've had to add scripts, I've just
hacked the odd patch but its probably inevitable I'll reach a threshold
at some point.

Ironically whilst my emails to the lists now work (I think/hope), the
copies I get back of my emails are always mangled and nothing can be
done about that.

> N.B.: I really wish git had a hook that runs at 'send-email' time. As
> I understand it, that would be the correct place to automatically
> prefix the message body with the 'From: ' header. I tried using
> 'prepare-commit-msg' to prefix the entire commit message with 'From:
> ' but that dirties the log and breaks 'git log --oneline'. Maybe I'm
> missing something but I just don't see a way to do this automatically
> without writing a wrapper script around send-email, which I'll
> probably end up doing.

I wish I had good answers but its a pain. Mailing lists are becoming
harder to run and maintain (one reason we switched to groups.io).

Thanks for looking at it!

Cheers,

Richard

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Chris Laplante]

> You aren't the only one! Most of our regular contributors don't have the
> problem and it has improved for others over time as people understand the
> issues and set the SPF more favourably. I don't know the details, I've just
> noticed some people who had the issue don't now.
> 
> Its not been problematic enough that I've had to add scripts, I've just hacked
> the odd patch but its probably inevitable I'll reach a threshold at some point.
> 
> Ironically whilst my emails to the lists now work (I think/hope), the copies I
> get back of my emails are always mangled and nothing can be done about
> that.
> 
> > N.B.: I really wish git had a hook that runs at 'send-email' time. As
> > I understand it, that would be the correct place to automatically
> > prefix the message body with the 'From: ' header. I tried using
> > 'prepare-commit-msg' to prefix the entire commit message with 'From:
> > ' but that dirties the log and breaks 'git log --oneline'. Maybe I'm
> > missing something but I just don't see a way to do this automatically
> > without writing a wrapper script around send-email, which I'll
> > probably end up doing.
> 
> I wish I had good answers but its a pain. Mailing lists are becoming harder to
> run and maintain (one reason we switched to groups.io).
> 
> Thanks for looking at it!


I hacked up a small Python script as my .git/hooks/sendemail-validate to ensure the From: line is the first line in the message body. I just tested it by sending a patch to OE-Core for something I found a couple days ago.

If it looks OK please let me know. I can share the sendemail-validate if you think it could be useful to others.

Thanks,
Chris

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Richard Purdie]

On Wed, 2020-11-18 at 21:01 +0000, chris.laplante@agilent.com wrote:
> I hacked up a small Python script as my .git/hooks/sendemail-
> validate 
> to ensure the From: line is the first line in the message body. I
> just tested it by sending a patch to OE-Core for something I found a
> couple days ago.
> 
> If it looks OK please let me know. I can share the sendemail-validate 
> if you think it could be useful to others.

Sorry, I've been a bit distracted. I did just check and this worked
fine, thanks!

I think sharing that script somewhere (maybe OE-Core's contrib
directory?) would be useful. I'd like to add some documentation about
how to handle this as I doubt you'll be the first or last person to run
into this!

Cheers,

Richard

Re: [bitbake-devel] [PATCH v3 00/10] optparse => argparse

[Chris Laplante]

> Unfortunately I doubt I can do anything about the DNS/SPF:(
> 
> I'd rather avoid you needing to make changes to your scripting just to deal
> with my patches, so I'll make sure to set the 'From: ' header. I'm going to
> submit a patch soon as a kind of test to see if it works (though it is an actual
> patch for an issue I found, separate from all the argparse stuff).
> 
> N.B.: I really wish git had a hook that runs at 'send-email' time. As I
> understand it, that would be the correct place to automatically prefix the
> message body with the 'From: ' header. I tried using 'prepare-commit-msg' to
> prefix the entire commit message with 'From: ' but that dirties the log and
> breaks 'git log --oneline'. Maybe I'm missing something but I just don't see a
> way to do this automatically without writing a wrapper script around send-
> email, which I'll probably end up doing.


Ah, I sort of take it back. It seems there is a sendemail-validate hook. If I can't modify the message, then I can at least error out if the 'From: ' line is not present.

Chris