Merge patch series "buildman: Add initial support for config fragments"

Simon Glass <sjg@chromium.org> says:

This series updates buildman to process #include lines in defconfig
files. With this, it is no-longer necessary to duplicate lines certain
lines from the include-file in the defconfig, e.g. CONFIG_ARM and
CONFIG_SOC_...

Link: https://lore.kernel.org/r/20241108152350.3686274-1-sjg@chromium.org

tools/buildman/boards.py

@@ -19,7 +19,10 @@ import time
 from buildman import board
 from buildman import kconfiglib
 
+from u_boot_pylib import command
 from u_boot_pylib.terminal import print_clear, tprint
+from u_boot_pylib import tools
+from u_boot_pylib import tout
 
 ### constant variables ###
 OUTPUT_FILE = 'boards.cfg'
@@ -202,6 +205,7 @@ class KconfigScanner:
         os.environ['KCONFIG_OBJDIR'] = ''
         self._tmpfile = None
         self._conf = kconfiglib.Kconfig(warn=False)
+        self._srctree = srctree
 
     def __del__(self):
         """Delete a leftover temporary file before exit.
@@ -239,7 +243,26 @@ class KconfigScanner:
         expect_target, match, rear = leaf.partition('_defconfig')
         assert match and not rear, f'{leaf} : invalid defconfig'
 
-        self._conf.load_config(defconfig)
+        temp = None
+        if b'#include' in tools.read_file(defconfig):
+            cmd = [
+                os.getenv('CPP', 'cpp'),
+                '-nostdinc', '-P',
+                '-I', self._srctree,
+                '-undef',
+                '-x', 'assembler-with-cpp',
+                defconfig]
+            result = command.run_pipe([cmd], capture=True, capture_stderr=True)
+            temp = tempfile.NamedTemporaryFile(prefix='buildman-')
+            tools.write_file(temp.name, result.stdout, False)
+            fname = temp.name
+            tout.info(f'Processing #include to produce {defconfig}')
+        else:
+            fname = defconfig
+
+        self._conf.load_config(fname)
+        if temp:
+            del temp
         self._tmpfile = None
 
         params = {}

tools/buildman/buildman.rst

@@ -203,7 +203,6 @@ Setting up
       sh = sh4
       x86: i386
 
-
    This selects the available toolchain paths. Add the base directory for
    each of your toolchains here. Buildman will search inside these directories
    and also in any '/usr' and '/usr/bin' subdirectories.
@@ -934,6 +933,18 @@ a set of (tag, value) pairs.
     For example powerpc-linux-gcc will be noted as a toolchain for 'powerpc'
     and CROSS_COMPILE will be set to powerpc-linux- when using it.
 
+    The tilde character ``~`` is supported in paths, to represent the home
+    directory.
+
+'[toolchain-prefix]' section
+    This can be used to provide the full toolchain-prefix for one or more
+    architectures. The full CROSS_COMPILE prefix must be provided. These
+    typically have a higher priority than matches in the '[toolchain]', due to
+    this prefix.
+
+    The tilde character ``~`` is supported in paths, to represent the home
+    directory.
+
 '[toolchain-alias]' section
     This converts toolchain architecture names to U-Boot names. For example,
     if an x86 toolchains is called i386-linux-gcc it will not normally be
@@ -1112,6 +1123,30 @@ The -U option uses the u-boot.env files which are produced by a build.
 Internally, buildman writes out an out-env file into the build directory for
 later comparison.
 
+defconfig fragments
+-------------------
+
+Buildman provides some initial support for configuration fragments. It can scan
+these when present in defconfig files and handle the resuiting Kconfig
+correctly. Thus it is possible to build a board which has a ``#include`` in the
+defconfig file.
+
+For now, Buildman simply includes the files to produce a single output file,
+using the C preprocessor. It does not call the ``merge_config.sh`` script. The
+redefined/redundant logic in that script could fairly easily be repeated in
+Buildman, to detect potential problems. For now it is not clear that this is
+useful.
+
+To specify the C preprocessor to use, set the ``CPP`` environment variable. The
+default is ``cpp``.
+
+Note that Buildman does not support adding fragments to existing boards, e.g.
+like::
+
+    make qemu_riscv64_defconfig acpi.config
+
+This is partly because there is no way for Buildman to know which fragments are
+valid on which boards.
 
 Building with clang
 -------------------

tools/buildman/func_test.py

@@ -2,8 +2,10 @@
 # Copyright (c) 2014 Google, Inc
 #
 
+import io
 import os
 from pathlib import Path
+import re
 import shutil
 import sys
 import tempfile
@@ -373,6 +375,22 @@ class TestFunctional(unittest.TestCase):
     def _HandleCommandSize(self, args):
         return command.CommandResult(return_code=0)
 
+    def _HandleCommandCpp(self, args):
+        # args ['-nostdinc', '-P', '-I', '/tmp/tmp7f17xk_o/src', '-undef',
+        # '-x', 'assembler-with-cpp', fname]
+        fname = args[7]
+        buf = io.StringIO()
+        for line in tools.read_file(fname, False).splitlines():
+            if line.startswith('#include'):
+                # Example: #include <configs/renesas_rcar2.config>
+                m_incfname = re.match('#include <(.*)>', line)
+                data = tools.read_file(m_incfname.group(1), False)
+                for line in data.splitlines():
+                    print(line, file=buf)
+            else:
+                print(line, file=buf)
+        return command.CommandResult(stdout=buf.getvalue(), return_code=0)
+
     def _HandleCommand(self, **kwargs):
         """Handle a command execution.
 
@@ -406,6 +424,8 @@ class TestFunctional(unittest.TestCase):
             return self._HandleCommandObjcopy(args)
         elif cmd.endswith( 'size'):
             return self._HandleCommandSize(args)
+        elif cmd.endswith( 'cpp'):
+            return self._HandleCommandCpp(args)
 
         if not result:
             # Not handled, so abort
@@ -1067,3 +1087,68 @@ endif
             result = self._RunControl('--print-arch', 'board0')
         self.assertEqual('arm\n', stdout.getvalue())
         self.assertEqual('', stderr.getvalue())
+
+    def test_kconfig_scanner(self):
+        """Test using the kconfig scanner to determine important values
+
+        Note that there is already a test_scan_defconfigs() which checks the
+        higher-level scan_defconfigs() function. This test checks just the
+        scanner itself
+        """
+        src = self._git_dir
+        scanner = boards.KconfigScanner(src)
+
+        # First do a simple sanity check
+        norm = os.path.join(src, 'board0_defconfig')
+        tools.write_file(norm, 'CONFIG_TARGET_BOARD0=y', False)
+        res = scanner.scan(norm, True)
+        self.assertEqual(({
+            'arch': 'arm',
+            'cpu': 'armv7',
+            'soc': '-',
+            'vendor': 'Tester',
+            'board': 'ARM Board 0',
+            'config': 'config0',
+            'target': 'board0'}, []), res)
+
+        # Check that the SoC cannot be changed and the filename does not affect
+        # the resulting board
+        tools.write_file(norm, '''CONFIG_TARGET_BOARD2=y
+CONFIG_SOC="fred"
+''', False)
+        res = scanner.scan(norm, True)
+        self.assertEqual(({
+            'arch': 'powerpc',
+            'cpu': 'ppc',
+            'soc': 'mpc85xx',
+            'vendor': 'Tester',
+            'board': 'PowerPC board 1',
+            'config': 'config2',
+            'target': 'board0'}, []), res)
+
+        # Check handling of missing information
+        tools.write_file(norm, '', False)
+        res = scanner.scan(norm, True)
+        self.assertEqual(({
+            'arch': '-',
+            'cpu': '-',
+            'soc': '-',
+            'vendor': '-',
+            'board': '-',
+            'config': '-',
+            'target': 'board0'},
+            ['WARNING: board0_defconfig: No TARGET_BOARD0 enabled']), res)
+
+        # check handling of #include files; see _HandleCommandCpp()
+        inc = os.path.join(src, 'common')
+        tools.write_file(inc, b'CONFIG_TARGET_BOARD0=y\n')
+        tools.write_file(norm, f'#include <{inc}>', False)
+        res = scanner.scan(norm, True)
+        self.assertEqual(({
+            'arch': 'arm',
+            'cpu': 'armv7',
+            'soc': '-',
+            'vendor': 'Tester',
+            'board': 'ARM Board 0',
+            'config': 'config0',
+            'target': 'board0'}, []), res)

tools/buildman/main.py

@@ -25,6 +25,7 @@ from buildman import cmdline
 from buildman import control
 from u_boot_pylib import test_util
 from u_boot_pylib import tools
+from u_boot_pylib import tout
 
 def run_tests(skip_net_tests, debug, verbose, args):
     """Run the buildman tests
@@ -93,8 +94,12 @@ def run_buildman():
 
     # Build selected commits for selected boards
     else:
+        try:
+            tout.init(tout.INFO if args.verbose else tout.WARNING)
             bsettings.setup(args.config_file)
             ret_code = control.do_buildman(args)
+        finally:
+            tout.uninit()
         return ret_code
 
 

tools/buildman/test.py

@@ -46,6 +46,16 @@ main: /usr/sbin
 wrapper = ccache
 '''
 
+settings_data_homedir = '''
+# Buildman settings file
+
+[toolchain]
+main = ~/mypath
+
+[toolchain-prefix]
+x86 = ~/mypath-x86-
+'''
+
 migration = '''===================== WARNING ======================
 This board does not use CONFIG_DM. CONFIG_DM will be
 compulsory starting with the v2020.01 release.
@@ -1030,6 +1040,46 @@ class TestBuild(unittest.TestCase):
         finally:
             os.environ['PATH'] = old_path
 
+    def testHomedir(self):
+        """Test using ~ in a toolchain or toolchain-prefix section"""
+        # Add some test settings
+        bsettings.setup(None)
+        bsettings.add_file(settings_data_homedir)
+
+        # Set up the toolchains
+        home = os.path.expanduser('~')
+        toolchains = toolchain.Toolchains()
+        toolchains.GetSettings()
+        self.assertEqual([f'{home}/mypath'], toolchains.paths)
+
+        # Check scanning
+        with test_util.capture_sys_output() as (stdout, _):
+            toolchains.Scan(verbose=True, raise_on_error=False)
+        lines = iter(stdout.getvalue().splitlines() + ['##done'])
+        self.assertEqual('Scanning for tool chains', next(lines))
+        self.assertEqual(f"   - scanning prefix '{home}/mypath-x86-'",
+                         next(lines))
+        self.assertEqual(
+            f"Error: No tool chain found for prefix '{home}/mypath-x86-gcc'",
+            next(lines))
+        self.assertEqual(f"   - scanning path '{home}/mypath'", next(lines))
+        self.assertEqual(f"      - looking in '{home}/mypath/.'", next(lines))
+        self.assertEqual(f"      - looking in '{home}/mypath/bin'", next(lines))
+        self.assertEqual(f"      - looking in '{home}/mypath/usr/bin'",
+                         next(lines))
+        self.assertEqual('##done', next(lines))
+
+        # Check adding a toolchain
+        with test_util.capture_sys_output() as (stdout, _):
+            toolchains.Add('~/aarch64-linux-gcc', test=True, verbose=True)
+        lines = iter(stdout.getvalue().splitlines() + ['##done'])
+        self.assertEqual('Tool chain test:  BAD', next(lines))
+        self.assertEqual(f'Command: {home}/aarch64-linux-gcc --version',
+                         next(lines))
+        self.assertEqual('', next(lines))
+        self.assertEqual('', next(lines))
+        self.assertEqual('##done', next(lines))
+
 
 if __name__ == "__main__":
     unittest.main()

tools/buildman/toolchain.py

@@ -65,12 +65,13 @@ class Toolchain:
         """Create a new toolchain object.
 
         Args:
-            fname: Filename of the gcc component
+            fname: Filename of the gcc component, possibly with ~ or $HOME in it
             test: True to run the toolchain to test it
             verbose: True to print out the information
             priority: Priority to use for this toolchain, or PRIORITY_CALC to
                 calculate it
         """
+        fname = os.path.expanduser(fname)
         self.gcc = fname
         self.path = os.path.dirname(fname)
         self.override_toolchain = override_toolchain
@@ -109,7 +110,7 @@ class Toolchain:
                                                           self.priority))
                 else:
                     print('BAD')
-                    print('Command: ', cmd)
+                    print(f"Command: {' '.join(cmd)}")
                     print(result.stdout)
                     print(result.stderr)
         else:
@@ -296,10 +297,11 @@ class Toolchains:
 
         paths = []
         for name, value in toolchains:
+            fname = os.path.expanduser(value)
             if '*' in value:
-                paths += glob.glob(value)
+                paths += glob.glob(fname)
             else:
-                paths.append(value)
+                paths.append(fname)
         return paths
 
     def GetSettings(self, show_warning=True):
@@ -327,6 +329,7 @@ class Toolchains:
         toolchain = Toolchain(fname, test, verbose, priority, arch,
                               self.override_toolchain)
         add_it = toolchain.ok
+        if add_it:
             if toolchain.arch in self.toolchains:
                 add_it = (toolchain.priority <
                           self.toolchains[toolchain.arch].priority)
@@ -372,7 +375,7 @@ class Toolchains:
                 pathname_list.append(pathname)
         return pathname_list
 
-    def Scan(self, verbose):
+    def Scan(self, verbose, raise_on_error=True):
         """Scan for available toolchains and select the best for each arch.
 
         We look for all the toolchains we can file, figure out the
@@ -384,11 +387,12 @@ class Toolchains:
         """
         if verbose: print('Scanning for tool chains')
         for name, value in self.prefixes:
-            if verbose: print("   - scanning prefix '%s'" % value)
+            fname = os.path.expanduser(value)
-            if os.path.exists(value):
+            if verbose: print("   - scanning prefix '%s'" % fname)
-                self.Add(value, True, verbose, PRIORITY_FULL_PREFIX, name)
+            if os.path.exists(fname):
+                self.Add(fname, True, verbose, PRIORITY_FULL_PREFIX, name)
                 continue
-            fname = value + 'gcc'
+            fname += 'gcc'
             if os.path.exists(fname):
                 self.Add(fname, True, verbose, PRIORITY_PREFIX_GCC, name)
                 continue
@@ -396,8 +400,11 @@ class Toolchains:
             for f in fname_list:
                 self.Add(f, True, verbose, PRIORITY_PREFIX_GCC_PATH, name)
             if not fname_list:
-                raise ValueError("No tool chain found for prefix '%s'" %
+                msg = f"No tool chain found for prefix '{fname}'"
-                                   value)
+                if raise_on_error:
+                    raise ValueError(msg)
+                else:
+                    print(f'Error: {msg}')
         for path in self.paths:
             if verbose: print("   - scanning path '%s'" % path)
             fnames = self.ScanPath(path, verbose)