diff mbox series

scripts/lib/buildstats: allow aggregation to skip mismatched recipes

Message ID 20260806062342.3737135-1-fjpedrazag@gmail.com
State Under Review
Headers show
Series scripts/lib/buildstats: allow aggregation to skip mismatched recipes | expand

Commit Message

Francisco Pedraza Aug. 6, 2026, 6:23 a.m. UTC
oe-build-perf-report aggregates the buildstats of every test run sharing
an openembedded-core commit. Since buildperf began tracking
openembedded-core rather than poky, metadata changes in meta-yocto no
longer alter the commit hash, so two runs at the same revision can
legitimately contain different recipe versions. When that happens
BSRecipe.aggregate() raises and report generation aborts entirely:

  Refusing to aggregate buildstats, recipe version differs:
  linux-yocto-6.16.11+git-r0 vs. linux-yocto-6.18.1+git-r0

Two problems follow. The failure discards the entire run rather than the
recipe that differs, so data for every other recipe is lost with it. And
because BuildStats.aggregate() mutates as it iterates, recipes processed
before the mismatch have already been converted to BSTaskAggregate when
the exception is raised, leaving the object partially merged; which
recipes survive depends on the ordering returned by os.listdir() in
from_dir().

Check every recipe before modifying any of them, and add a strict
argument to BuildStats.aggregate(). With strict=True, the default, a
ValueError is still raised, but now before anything has been mutated, so
buildstats-diff keeps refusing to combine buildstats that a user has
explicitly asked it to combine. The message it raises now names all
offending recipes rather than only the first. oe-build-perf-report
passes strict=False, which skips the recipes that cannot be aggregated,
logs which ones and why, and aggregates the rest.

Skipping only the mismatched recipes preserves the most data, at the
cost of recipes having differing sample counts within a revision.
BSTaskAggregate already handles that, as it averages over its own task
list.

Tested with:

  oe-selftest -r oescripts.OEBuildStatsAggregateTests

covering the strict and non-strict paths in both recipe orderings, the
differing-task-set condition, and that identical runs still aggregate.

Fixes [YOCTO #16119]

Signed-off-by: Francisco Pedraza <fjpedrazag@gmail.com>
---
 meta/lib/oeqa/selftest/cases/oescripts.py | 44 +++++++++++++++++++++++
 scripts/lib/buildstats.py                 | 34 ++++++++++++++++--
 scripts/oe-build-perf-report              |  2 +-
 3 files changed, 77 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/meta/lib/oeqa/selftest/cases/oescripts.py b/meta/lib/oeqa/selftest/cases/oescripts.py
index 3e0bd6f4ec..2082898081 100644
--- a/meta/lib/oeqa/selftest/cases/oescripts.py
+++ b/meta/lib/oeqa/selftest/cases/oescripts.py
@@ -7,6 +7,7 @@ 
 import os
 import shutil
 import importlib
+import sys
 import unittest
 from oeqa.selftest.case import OESelftestTestCase
 from oeqa.utils.commands import runCmd, bitbake, get_bb_var
@@ -172,3 +173,46 @@  class OEListPackageconfigTests(OESelftestTestCase):
 
         self.check_endlines(results, expected_endlines)
 
+
+class OEBuildStatsAggregateTests(OESelftestTestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        scripts_lib = os.path.join(get_bb_var('COREBASE'), 'scripts', 'lib')
+        if scripts_lib not in sys.path:
+            sys.path.insert(0, scripts_lib)
+
+    def _make_run(self, kernel_version, order):
+        from buildstats import BuildStats
+        task = {'start_time': 1000.0, 'elapsed_time': 42.0, 'status': 'PASSED',
+                'iostat': {}, 'rusage': {'ru_stime': 1.0, 'ru_utime': 2.0},
+                'child_rusage': {}}
+        recipes = {
+            'busybox': {'name': 'busybox', 'epoch': None, 'version': '1.37.0',
+                        'revision': 'r0', 'tasks': {'do_compile': dict(task)}},
+            'linux-yocto': {'name': 'linux-yocto', 'epoch': None,
+                            'version': kernel_version, 'revision': 'r0',
+                            'tasks': {'do_compile': dict(task)}},
+        }
+        return BuildStats.from_json([recipes[n] for n in order])
+
+    def test_aggregate_strict_does_not_mutate(self):
+        # Matching recipe first: it must not be aggregated when a later one fails
+        from buildstats import BSTask
+        order = ['busybox', 'linux-yocto']
+        bs1 = self._make_run('6.16.11+git', order)
+        bs2 = self._make_run('6.18.1+git', order)
+        with self.assertRaises(ValueError):
+            bs1.aggregate(bs2)
+        self.assertIsInstance(bs1['busybox'].tasks['do_compile'], BSTask)
+
+    def test_aggregate_non_strict_skips_mismatch(self):
+        from buildstats import BSTask, BSTaskAggregate
+        for order in (['busybox', 'linux-yocto'], ['linux-yocto', 'busybox']):
+            bs1 = self._make_run('6.16.11+git', order)
+            bs2 = self._make_run('6.18.1+git', order)
+            bs1.aggregate(bs2, strict=False)
+            self.assertIsInstance(bs1['busybox'].tasks['do_compile'], BSTaskAggregate)
+            self.assertIsInstance(bs1['linux-yocto'].tasks['do_compile'], BSTask)
+
diff --git a/scripts/lib/buildstats.py b/scripts/lib/buildstats.py
index 6db60d5bcf..20e17f07cd 100644
--- a/scripts/lib/buildstats.py
+++ b/scripts/lib/buildstats.py
@@ -274,12 +274,42 @@  class BuildStats(dict):
 
         return buildstats
 
-    def aggregate(self, buildstats):
-        """Aggregate other buildstats into this"""
+    def aggregate(self, buildstats, strict=True):
+        """Aggregate other buildstats into this
+
+        Recipes that cannot be aggregated, i.e. those whose version or set of
+        tasks differs, are collected before any data is modified. With
+        strict=True a ValueError is raised and this object is left untouched.
+        With strict=False those recipes are skipped with a warning and the
+        remaining ones are aggregated.
+        """
         if set(self.keys()) != set(buildstats.keys()):
             raise ValueError("Refusing to aggregate buildstats, set of "
                              "recipes is different: %s" % (set(self.keys()) ^ set(buildstats.keys())))
+
+        # Aggregation mutates in place, so every recipe must be checked before
+        # any of them is modified: failing partway through would leave this
+        # object partially merged.
+        skip = {}
         for pkg, data in buildstats.items():
+            if self[pkg].nevr != data.nevr:
+                skip[pkg] = "recipe version differs: {} vs. {}".format(
+                    self[pkg].nevr, data.nevr)
+            elif set(self[pkg].tasks.keys()) != set(data.tasks.keys()):
+                skip[pkg] = "set of tasks differs"
+
+        if skip:
+            details = ", ".join("{} ({})".format(pkg, reason)
+                                for pkg, reason in sorted(skip.items()))
+            if strict:
+                raise ValueError("Refusing to aggregate buildstats, {} recipe(s) "
+                                 "cannot be aggregated: {}".format(len(skip), details))
+            log.warning("Skipping %d recipe(s) that cannot be aggregated: %s",
+                        len(skip), details)
+
+        for pkg, data in buildstats.items():
+            if pkg in skip:
+                continue
             self[pkg].aggregate(data)
 
 
diff --git a/scripts/oe-build-perf-report b/scripts/oe-build-perf-report
index a36f3c1bca..c53d4c7b9c 100755
--- a/scripts/oe-build-perf-report
+++ b/scripts/oe-build-perf-report
@@ -427,7 +427,7 @@  def get_buildstats(repo, notes_ref, notes_ref2, revs, outdir=None):
                 if measurement not in buildstats[rev.commit_number]:
                     buildstats[rev.commit_number][measurement] = _bs
                 else:
-                    buildstats[rev.commit_number][measurement].aggregate(_bs)
+                    buildstats[rev.commit_number][measurement].aggregate(_bs, strict=False)
 
     if missing:
         log.info("Buildstats were missing for some test runs, please "