From: Simon Glass <simon.glass@canonical.com> We plan to change how config adjustments are handled in buildman, so update this test to use a use single board. This will avoid a race condition with parallel builds using merge_config.sh race conditions when multiple builds try to compile Kconfig tools simultaneously. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/func_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 4a207bfb00c..ec2b2efc3b6 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -1100,7 +1100,8 @@ Idx Name Size VMA LMA File off Algn def test_reproducible(self): """Test that the -r flag works""" - lines, cfg_data = self.check_command('-r') + # Use single board to avoid parallel merge_config.sh race conditions + lines, cfg_data = self.check_command('board0', '-r') self.assertIn(b'SOURCE_DATE_EPOCH=0', lines[0]) # We should see CONFIG_LOCALVERSION_AUTO unset -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> The --adjust-cfg option currently modifies .config directly via cfgutil.adjust_cfg_file(), which does not resolve Kconfig dependencies. This means 'imply' and 'select' relationships are not honoured. For example, enabling CONFIG_UNIT_TEST via '-a UNIT_TEST' does not enable CONFIG_CONSOLE_RECORD even though UNIT_TEST implies it. Fix this by using scripts/kconfig/merge_config.sh to apply config changes. This script merges config fragments into .config and runs 'make alldefconfig' to resolve all Kconfig dependencies. To properly resolve 'imply' relationships, use 'make savedefconfig' to create a minimal defconfig as the base for merge_config.sh. The full .config contains '# CONFIG_xxx is not set' lines which Kconfig treats as "specified", preventing imply from taking effect. Using the minimal defconfig ensures only explicitly set options are 'locked', allowing imply to work correctly. Add adjust_cfg_to_fragment() and run_merge_config() functions to cfgutil to convert the adjust_cfg dictionary into a config-fragment format and handle the merge process. Add BUILDMAN_TEST_A and BUILDMAN_TEST_B options to test/Kconfig with an imply relationship for testing. Update func_test.py to test that the imply mechanism works correctly with --adjust-cfg. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/Kconfig | 14 ++++ tools/buildman/builderthread.py | 12 ++- tools/buildman/cfgutil.py | 88 +++++++++++++++++++++ tools/buildman/func_test.py | 136 ++++++++++++++++++++++++++++++-- tools/buildman/test_cfgutil.py | 31 ++++++++ 5 files changed, 266 insertions(+), 15 deletions(-) diff --git a/test/Kconfig b/test/Kconfig index 96723940bac..cd1eec169e8 100644 --- a/test/Kconfig +++ b/test/Kconfig @@ -122,6 +122,20 @@ source "test/lib/Kconfig" source "test/optee/Kconfig" source "test/fdt_overlay/Kconfig" +config BUILDMAN_TEST_A + bool "Buildman test option A" + imply BUILDMAN_TEST_B + help + Test option used by buildman to verify that --adjust-cfg correctly + resolves Kconfig 'imply' dependencies via merge_config.sh. + +config BUILDMAN_TEST_B + bool "Buildman test option B" + help + Test option implied by BUILDMAN_TEST_A. Used by buildman tests to + verify that 'imply' dependencies are resolved when using + merge_config.sh. + endif # UNIT_TEST config POST diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index dee31972e4b..dcf2d8f9ac5 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -516,7 +516,11 @@ class BuilderThread(threading.Thread): setup.config_args, config_out, cmd_list, mrproper) do_config = False # No need to configure next time if req.adjust_cfg: - cfgutil.adjust_cfg_file(cfg_file, req.adjust_cfg) + merge_result = cfgutil.run_merge_config( + setup.cwd, out_dir, cfg_file, req.adjust_cfg, setup.env) + config_out.write(merge_result.combined) + if merge_result.return_code: + result = merge_result return result, do_config, cfg_file def _build_and_get_result(self, req, setup, commit, cmd_list, config_out, @@ -537,12 +541,6 @@ class BuilderThread(threading.Thread): CommandResult: Result of the build """ if result.return_code == 0: - if req.adjust_cfg: - oldc_args = list(setup.args) + ['oldconfig'] - oldc_result = self.make(commit, req.brd, 'oldconfig', setup.cwd, - *oldc_args, env=setup.env) - if oldc_result.return_code: - return oldc_result result = self._build(commit, req.brd, setup.cwd, setup.args, setup.env, cmd_list, config_only) if req.adjust_cfg: diff --git a/tools/buildman/cfgutil.py b/tools/buildman/cfgutil.py index 2d4c6799b5c..cec33e1e62b 100644 --- a/tools/buildman/cfgutil.py +++ b/tools/buildman/cfgutil.py @@ -7,7 +7,9 @@ import os import re +import tempfile +from u_boot_pylib import command from u_boot_pylib import tools @@ -306,3 +308,89 @@ def process_config(fname, squash_config_y): value = '1' config[key] = value return config + + +def adjust_cfg_to_fragment(adjust_cfg): + """Convert adjust_cfg dict to config fragment content + + Args: + adjust_cfg (dict): Changes to make to .config file. Keys are config + names (without CONFIG_ prefix), values are the setting. Format + matches make_cfg_line(): + ~... - disable the option + ...=val - set the option to val (val contains full assignment) + other - enable the option with =y + + Returns: + str: Config fragment content suitable for merge_config.sh + """ + lines = [] + for opt, val in adjust_cfg.items(): + if val.startswith('~'): + lines.append(f'# CONFIG_{opt} is not set') + elif '=' in val: + lines.append(f'CONFIG_{val}') + else: + lines.append(f'CONFIG_{opt}=y') + return '\n'.join(lines) + '\n' if lines else '' + + +def run_merge_config(src_dir, out_dir, cfg_file, adjust_cfg, env): + """Run merge_config.sh to apply config changes with Kconfig resolution + + This uses scripts/kconfig/merge_config.sh to merge config fragments + into the .config file, then runs 'make alldefconfig' to resolve all + Kconfig dependencies including 'imply' and 'select'. + + To properly resolve 'imply' relationships, we must use a minimal + defconfig as the base (not the full .config). The full .config contains + '# CONFIG_xxx is not set' lines which count as "specified" and prevent + imply from taking effect. Using savedefconfig output ensures only + explicitly set options are in the base, allowing imply to work. + + Args: + src_dir (str): Source directory (containing scripts/kconfig) + out_dir (str): Output directory containing .config + cfg_file (str): Path to the .config file + adjust_cfg (dict): Config changes to apply + env (dict): Environment variables + + Returns: + CommandResult: Result of the merge_config.sh operation + """ + # Create a temporary fragment file with the config changes + fragment_content = adjust_cfg_to_fragment(adjust_cfg) + with tempfile.NamedTemporaryFile(mode='w', suffix='.config', + delete=False) as frag: + frag.write(fragment_content) + frag_path = frag.name + + # Create a minimal defconfig from the current .config + # This is necessary for 'imply' to work - the full .config has + # '# CONFIG_xxx is not set' lines that prevent imply from taking effect + defconfig_path = os.path.join(out_dir or '.', 'defconfig') + make_cmd = ['make', f'O={out_dir}' if out_dir else None, + f'KCONFIG_CONFIG={cfg_file}', 'savedefconfig'] + make_cmd = [x for x in make_cmd if x] # Remove None elements + result = command.run_one(*make_cmd, cwd=src_dir, env=env, capture=True, + capture_stderr=True) + if result.return_code: + if os.path.exists(frag_path): + os.unlink(frag_path) + return result + + try: + # Run merge_config.sh with the minimal defconfig as base + # -O sets output dir; defconfig is the base, fragment is merged + merge_script = os.path.join(src_dir or '.', 'scripts', 'kconfig', + 'merge_config.sh') + out = out_dir or '.' + cmd = [merge_script, '-O', out, defconfig_path, frag_path] + result = command.run_one(*cmd, cwd=src_dir, env=env, capture=True, + capture_stderr=True) + finally: + # Clean up temporary files + if os.path.exists(frag_path): + os.unlink(frag_path) + + return result diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 7f4c88701bb..7db4c086207 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -25,6 +25,7 @@ from buildman import boards from buildman.boards import Extended from buildman import bsettings from buildman import builderthread +from buildman import cfgutil from buildman import cmdline from buildman import control from buildman import toolchain @@ -446,6 +447,73 @@ Idx Name Size VMA LMA File off Algn print(line, file=buf) return command.CommandResult(stdout=buf.getvalue(), return_code=0) + def _run_merge_config(self, cmd_list, _kwargs): + """Run the real merge_config.sh script + + Runs merge_config.sh from the real U-Boot source tree with real + Kconfig files for actual dependency resolution. + + Args: + cmd_list (list): Original command and arguments + + Returns: + CommandResult: Result of running the script + """ + # Run from the real U-Boot source tree (not the test's fake git dir) + src_root = os.path.dirname(os.path.dirname(self._buildman_dir)) + merge_script = os.path.join(src_root, 'scripts', 'kconfig', + 'merge_config.sh') + + # Build command with real script path, keeping original arguments + new_cmd = [merge_script] + list(cmd_list[1:]) + + # Use a clean host environment with CROSS_COMPILE cleared so Kconfig + # uses the host gcc + env = dict(os.environ) + env['CROSS_COMPILE'] = '' + + # Temporarily disable TEST_RESULT to run the real command + old_test_result = command.TEST_RESULT + command.TEST_RESULT = None + try: + result = command.run_one(*new_cmd, cwd=src_root, env=env, + capture=True, capture_stderr=True) + finally: + command.TEST_RESULT = old_test_result + return result + + def _handle_make_savedefconfig(self, args, _kwargs): + """Handle make savedefconfig command + + This runs the real make savedefconfig to create a minimal defconfig + from the current .config file. + + Args: + args (list): Arguments to make (after 'make') + + Returns: + CommandResult: Result of running the command + """ + # Run from the U-Boot source tree + src_root = os.path.dirname(os.path.dirname(self._buildman_dir)) + + # Build the full command + new_cmd = ['make'] + list(args) + + # Use a clean host environment with CROSS_COMPILE cleared + env = dict(os.environ) + env['CROSS_COMPILE'] = '' + + # Temporarily disable TEST_RESULT to run the real command + old_test_result = command.TEST_RESULT + command.TEST_RESULT = None + try: + result = command.run_one(*new_cmd, cwd=src_root, env=env, + capture=True, capture_stderr=True) + finally: + command.TEST_RESULT = old_test_result + return result + def _handle_command(self, **kwargs): # pylint: disable=too-many-branches """Handle a command execution. @@ -482,6 +550,12 @@ Idx Name Size VMA LMA File off Algn result = self._handle_command_cpp(args) elif cmd == 'gcc' and args[0] == '-E': result = self._handle_command_cpp(args[1:]) + elif cmd.endswith('merge_config.sh'): + # Run the real merge_config.sh using command.run_one() + result = self._run_merge_config(pipe_list[0], kwargs) + elif cmd == 'make' and 'savedefconfig' in args: + # Handle make savedefconfig - create minimal defconfig from .config + result = self._handle_make_savedefconfig(args, kwargs) else: # Not handled, so abort print('unknown command', kwargs) @@ -1107,22 +1181,20 @@ Idx Name Size VMA LMA File off Algn lines, cfg_data = self.check_command('board0', '-r') self.assertIn(b'SOURCE_DATE_EPOCH=0', lines[0]) - # We should see CONFIG_LOCALVERSION_AUTO unset - self.assertEqual(b'''CONFIG_SOMETHING=1 -# CONFIG_LOCALVERSION_AUTO is not set -''', cfg_data) + # We should see CONFIG_LOCALVERSION_AUTO unset (uses real Kconfig) + self.assertIn(b'# CONFIG_LOCALVERSION_AUTO is not set', cfg_data) with terminal.capture() as (stdout, _stderr): lines, cfg_data = self.check_command('board0', '-r', '-a', 'LOCALVERSION') self.assertIn(b'SOURCE_DATE_EPOCH=0', lines[0]) - # We should see CONFIG_LOCALVERSION_AUTO unset - self.assertEqual(b'''CONFIG_SOMETHING=1 -CONFIG_LOCALVERSION=y -''', cfg_data) + # When user explicitly sets LOCALVERSION, the warning appears self.assertIn('Not dropping LOCALVERSION_AUTO', stdout.getvalue()) + # LOCALVERSION should be present in .config (it's a string config) + self.assertIn(b'CONFIG_LOCALVERSION=', cfg_data) + def test_scan_defconfigs(self): """Test scanning the defconfigs to obtain all the boards""" src = self._git_dir @@ -1619,3 +1691,51 @@ something: me # No reconfigs should be triggered self.assertEqual(0, self._builder.kconfig_reconfig) + + def test_adjust_cfg_no_imply(self): + """Test that direct .config modification does not resolve imply + + Modifying .config directly with cfgutil.adjust_cfg_file() does not + run Kconfig, so 'imply' dependencies are not resolved. This test + demonstrates the limitation that merge_config.sh fixes. + """ + # Create a temporary .config file + cfg_file = os.path.join(self._output_dir, '.config') + tools.write_file(cfg_file, b'# Empty config\n') + + # Use cfgutil to directly modify .config (old approach) + adjust_cfg = {'BUILDMAN_TEST_A': 'BUILDMAN_TEST_A'} + cfgutil.adjust_cfg_file(cfg_file, adjust_cfg) + + # Read the result + cfg_data = tools.read_file(cfg_file) + + # BUILDMAN_TEST_A should be enabled + self.assertIn(b'CONFIG_BUILDMAN_TEST_A=y', cfg_data) + + # But BUILDMAN_TEST_B should NOT be enabled - imply is not resolved + # because we didn't run Kconfig + self.assertNotIn(b'CONFIG_BUILDMAN_TEST_B=y', cfg_data, + 'Direct .config modification should not resolve imply') + + def test_adjust_cfg_imply(self): + """Test that merge_config.sh resolves Kconfig 'imply' dependencies + + The --adjust-cfg option uses merge_config.sh to apply config changes, + which runs 'make alldefconfig' to resolve all Kconfig dependencies + including 'imply'. CONFIG_BUILDMAN_TEST_A implies + CONFIG_BUILDMAN_TEST_B, so enabling BUILDMAN_TEST_A should also enable + BUILDMAN_TEST_B. + """ + # Use single board to avoid parallel merge_config.sh race conditions + # Enable UNIT_TEST since BUILDMAN_TEST_A depends on it + _lines, cfg_data = self.check_command( + 'board0', '-a', 'UNIT_TEST,BUILDMAN_TEST_A') + + # Verify BUILDMAN_TEST_A was enabled + self.assertIn(b'CONFIG_BUILDMAN_TEST_A=y', cfg_data) + + # merge_config.sh resolves imply dependencies, so enabling + # BUILDMAN_TEST_A should also enable BUILDMAN_TEST_B + self.assertIn(b'CONFIG_BUILDMAN_TEST_B=y', cfg_data, + '--adjust-cfg should resolve imply dependencies') diff --git a/tools/buildman/test_cfgutil.py b/tools/buildman/test_cfgutil.py index ba3f0468570..47e522d3d6c 100644 --- a/tools/buildman/test_cfgutil.py +++ b/tools/buildman/test_cfgutil.py @@ -115,6 +115,37 @@ class TestAdjustCfg(unittest.TestCase): '~CONFIG_ABE,CONFIG_MARK=0x456', 'CONFIG_ANNA="anna"']) self.assertEqual(expect, actual) + def test_adjust_cfg_to_fragment(self): + """Test adjust_cfg_to_fragment creates correct fragment content""" + # Empty dict returns empty string + self.assertEqual('', cfgutil.adjust_cfg_to_fragment({})) + + # Enable option + self.assertEqual('CONFIG_FRED=y\n', + cfgutil.adjust_cfg_to_fragment({'FRED': 'FRED'})) + + # Disable option + self.assertEqual('# CONFIG_FRED is not set\n', + cfgutil.adjust_cfg_to_fragment({'FRED': '~FRED'})) + + # Set value + self.assertEqual('CONFIG_FRED=0x123\n', + cfgutil.adjust_cfg_to_fragment({'FRED': 'FRED=0x123'})) + + # Set string value + self.assertEqual('CONFIG_FRED="fred"\n', + cfgutil.adjust_cfg_to_fragment({'FRED': 'FRED="fred"'})) + + # Multiple options (note: dict order is preserved in Python 3.7+) + result = cfgutil.adjust_cfg_to_fragment({ + 'FRED': 'FRED', + 'MARY': '~MARY', + 'JOHN': 'JOHN=42' + }) + self.assertIn('CONFIG_FRED=y', result) + self.assertIn('# CONFIG_MARY is not set', result) + self.assertIn('CONFIG_JOHN=42', result) + def test_check_cfg_file(self): """Test check_cfg_file detects conflicts as expected""" # Check failure to disable CONFIG -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Some toolchain tarballs from kernel.org contain symlinks with a doubled cross-compile prefix, e.g. 'x86_64-linux-x86_64-linux-gcc' alongside the correct 'x86_64-linux-gcc'. This causes buildman to print a warning about ambiguous toolchains when downloading. Add a regex-based check to detect and filter out these malformed binaries during toolchain scanning. When verbose output is enabled, these files are shown as "ignoring ... (doubled prefix)" rather than "found ..." Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/test.py | 24 ++++++++++++++++++++++++ tools/buildman/toolchain.py | 25 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index da6df1f173c..0f4a5b9e543 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -744,6 +744,30 @@ class TestBuild(TestBuildBase): # Verify downloaded priority beats system priority self.assertLess(toolchain.PRIORITY_DOWNLOADED, sys_tc.priority) + def test_is_doubled_prefix(self): + """Test detection of doubled toolchain prefixes""" + # Valid toolchain names (not doubled) + self.assertFalse( + toolchain.Toolchains.is_doubled_prefix('aarch64-linux-gcc')) + self.assertFalse( + toolchain.Toolchains.is_doubled_prefix('x86_64-linux-gcc')) + self.assertFalse( + toolchain.Toolchains.is_doubled_prefix('arm-linux-gnueabi-gcc')) + self.assertFalse( + toolchain.Toolchains.is_doubled_prefix('gcc')) + + # Doubled prefixes (should be filtered out) + self.assertTrue( + toolchain.Toolchains.is_doubled_prefix( + 'aarch64-linux-aarch64-linux-gcc')) + self.assertTrue( + toolchain.Toolchains.is_doubled_prefix( + 'x86_64-linux-x86_64-linux-gcc')) + + # Not a gcc file + self.assertFalse( + toolchain.Toolchains.is_doubled_prefix('aarch64-linux-ld')) + def test_get_env_args(self): """Test the GetEnvArgs() function""" tc = self.toolchains.select('arm') diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py index 8f3d3ab3b0c..27302f20d42 100644 --- a/tools/buildman/toolchain.py +++ b/tools/buildman/toolchain.py @@ -30,6 +30,10 @@ from u_boot_pylib import tools # Environment variable / argument types for get_env_args() (VAR_CROSS_COMPILE, VAR_PATH, VAR_ARCH, VAR_MAKE_ARGS) = range(4) +# Matches a repeated prefix, e.g. 'aarch64-linux-aarch64-linux-gcc' +RE_DOUBLED_PREFIX = re.compile(r'^(.+)\1gcc$') + + class MyHTMLParser(HTMLParser): """Simple class to collect links from a page @@ -378,6 +382,22 @@ class Toolchains: f"toolchain for arch '{toolchain.arch}' has priority " f"{self.toolchains[toolchain.arch].priority}") + @staticmethod + def is_doubled_prefix(fname): + """Check if a gcc filename has a doubled prefix + + Some toolchain tarballs contain symlinks with the cross-compile prefix + repeated, e.g. 'x86_64-linux-x86_64-linux-gcc'. These are not valid + toolchains and should be ignored. + + Args: + fname (str): Filename to check (basename, not full path) + + Returns: + bool: True if the prefix is doubled, False otherwise + """ + return bool(RE_DOUBLED_PREFIX.match(fname)) + def scan_path(self, path, verbose): """Scan a path for a valid toolchain @@ -394,6 +414,11 @@ class Toolchains: if verbose: print(f" - looking in '{dirname}'") for fname in glob.glob(dirname + '/*gcc'): + basename = os.path.basename(fname) + if self.is_doubled_prefix(basename): + if verbose: + print(f" - ignoring '{fname}' (doubled prefix)") + continue if verbose: print(f" - found '{fname}'") fnames.append(fname) -- 2.43.0