From: Simon Glass <simon.glass@canonical.com> Extract the check_output() inner function from make() as a separate method _check_output_for_loop() so it can be unit-tested. This function detects Kconfig restart loops caused by missing defaults. Add unit tests covering: - Normal output (no restart message) - Restart message sets the flag - Single NEW item after restart (no loop) - Different NEW items (no loop) - Duplicate items trigger loop detection - Duplicates without restart flag (no loop) - Multiple items with one duplicate Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 64 ++++++++++++++------------- tools/buildman/main.py | 1 + tools/buildman/test_builder.py | 79 ++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 30 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 307249b5e13..d24fad9a550 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -585,50 +585,54 @@ class Builder: if checkout and self.checkout: gitutil.checkout(commit.hash) - def make(self, _commit, _brd, _stage, cwd, *args, **kwargs): - """Run make + def _check_output_for_loop(self, data): + """Check output for config restart loops + + This detects when Kconfig enters a restart loop due to missing + defaults. It looks for 'Restart config' followed by multiple + occurrences of the same Kconfig item with no default. Args: - cwd (str): Directory where make should be run - args: Arguments to pass to make - kwargs: Arguments to pass to command.run_one() + data (bytes): Output data to check Returns: - CommandResult: Result of the make operation + bool: True to terminate the command, False to continue """ + if b'Restart config' in data: + self._restarting_config = True - def check_output(_stream, data): - """Check output for config restart loops + # If we see 'Restart config' followed by multiple errors + if self._restarting_config: + matches = RE_NO_DEFAULT.findall(data) - Args: - data (bytes): Output data to check - - Returns: - bool: True to terminate the command, False to continue - """ - if b'Restart config' in data: - self._restarting_config = True + # Number of occurrences of each Kconfig item + multiple = [matches.count(val) for val in set(matches)] - # If we see 'Restart config' following by multiple errors - if self._restarting_config: - m = RE_NO_DEFAULT.findall(data) + # If any of them occur more than once, we have a loop + if [val for val in multiple if val > 1]: + self._terminated = True + return True + return False - # Number of occurences of each Kconfig item - multiple = [m.count(val) for val in set(m)] + def make(self, _commit, _brd, _stage, cwd, *args, **kwargs): + """Run make - # If any of them occur more than once, we have a loop - if [val for val in multiple if val > 1]: - self._terminated = True - return True - return False + Args: + cwd (str): Directory where make should be run + args: Arguments to pass to make + kwargs: Arguments to pass to command.run_one() + Returns: + CommandResult: Result of the make operation + """ self._restarting_config = False self._terminated = False cmd = [self.gnu_make] + list(args) - result = command.run_one(*cmd, capture=True, capture_stderr=True, - cwd=cwd, raise_on_error=False, - infile='/dev/null', output_func=check_output, - **kwargs) + result = command.run_one( + *cmd, capture=True, capture_stderr=True, cwd=cwd, + raise_on_error=False, infile='/dev/null', + output_func=lambda stream, data: self._check_output_for_loop(data), + **kwargs) if self._terminated: # Try to be helpful diff --git a/tools/buildman/main.py b/tools/buildman/main.py index 18809d843c6..dadfd629506 100755 --- a/tools/buildman/main.py +++ b/tools/buildman/main.py @@ -58,6 +58,7 @@ def run_tests(skip_net_tests, debug, verbose, args): test_builder.TestPrepareWorkingSpace, test_builder.TestShowNotBuilt, test_builder.TestPrepareOutputSpace, + test_builder.TestCheckOutputForLoop, 'buildman.toolchain']) return (0 if result.wasSuccessful() else 1) diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index 78c80aa6c43..09809a07706 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -548,5 +548,84 @@ class TestPrepareOutputSpace(unittest.TestCase): self.assertFalse(lines[0].newline) +class TestCheckOutputForLoop(unittest.TestCase): + """Tests for Builder._check_output_for_loop()""" + + def setUp(self): + """Set up test fixtures""" + self.builder = builder.Builder( + toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', + num_threads=4, num_jobs=1) + # Reset state before each test + self.builder._restarting_config = False + self.builder._terminated = False + + def test_no_restart_message(self): + """Test that normal output does not trigger termination""" + result = self.builder._check_output_for_loop(b'Building target...') + + self.assertFalse(result) + self.assertFalse(self.builder._restarting_config) + self.assertFalse(self.builder._terminated) + + def test_restart_message_sets_flag(self): + """Test that 'Restart config' sets the restarting flag""" + result = self.builder._check_output_for_loop(b'Restart config...') + + self.assertFalse(result) # No loop detected yet + self.assertTrue(self.builder._restarting_config) + self.assertFalse(self.builder._terminated) + + def test_single_new_item_no_loop(self): + """Test that a single NEW item after restart is not a loop""" + self.builder._restarting_config = True + + result = self.builder._check_output_for_loop( + b'(CONFIG_ITEM) [] (NEW)') + + self.assertFalse(result) + self.assertFalse(self.builder._terminated) + + def test_different_new_items_no_loop(self): + """Test that different NEW items do not trigger a loop""" + self.builder._restarting_config = True + + result = self.builder._check_output_for_loop( + b'(CONFIG_A) [] (NEW)\n(CONFIG_B) [] (NEW)') + + self.assertFalse(result) + self.assertFalse(self.builder._terminated) + + def test_duplicate_items_triggers_loop(self): + """Test that duplicate NEW items trigger loop detection""" + self.builder._restarting_config = True + + result = self.builder._check_output_for_loop( + b'(CONFIG_ITEM) [] (NEW)\n(CONFIG_ITEM) [] (NEW)') + + self.assertTrue(result) + self.assertTrue(self.builder._terminated) + + def test_no_loop_without_restart(self): + """Test that duplicates without restart flag do not trigger loop""" + # _restarting_config is False by default + + result = self.builder._check_output_for_loop( + b'(CONFIG_ITEM) [] (NEW)\n(CONFIG_ITEM) [] (NEW)') + + self.assertFalse(result) + self.assertFalse(self.builder._terminated) + + def test_multiple_items_one_duplicate(self): + """Test loop detection with multiple items, one duplicated""" + self.builder._restarting_config = True + + result = self.builder._check_output_for_loop( + b'(CONFIG_A) [] (NEW)\n(CONFIG_B) [] (NEW)\n(CONFIG_A) [] (NEW)') + + self.assertTrue(result) + self.assertTrue(self.builder._terminated) + + if __name__ == '__main__': unittest.main() -- 2.43.0