diff mbox series

Bug with SkipRecipe and multiconfig

Message ID DM8PR12MB5430DF7192F8D0BF7E6F2BE58B302@DM8PR12MB5430.namprd12.prod.outlook.com
State New
Headers show
Series Bug with SkipRecipe and multiconfig | expand

Commit Message

chris.laplante@agilent.com Dec. 5, 2024, 8:24 p.m. UTC 
Hi all,

I believe I have tracked down a bug with SkipRecipe and multiconfig. Before I submit a patch, I wanted to check my understanding of the code and my suggested fix.

The issue I observed is that when you have a recipe that is incompatible (e.g. due to COMPATIBLE_MACHINE) with more than one active multiconfig, including 'default', then BitBake will inconsistently report why the recipe is unavailable.

For an example, please see the below patch. It adds a test case to oeqa selftest that adds a 'musl' multiconfig (with MACHINE = qemumips), and writes a 'm4' bbappend setting COMPATIBLE_MACHINE="qemuarm". The 'default' multiconfig has MACHINE = qemux86-64. I also modified the 'incompatible' message to report BB_CURRENT_MC. If you run the test a few times, you should see that the reason is not consistent: 

Sometimes it will correctly report:

    ERROR: Nothing PROVIDES 'm4'
    m4 was skipped: incompatible with machine qemux86-64 (not in COMPATIBLE_MACHINE), mc: default

while other times you get a message seemingly from the 'musl' multiconfig context:

    ERROR: Nothing PROVIDES 'm4'
    m4 was skipped: incompatible with machine qemumips (not in COMPATIBLE_MACHINE), mc: musl	

I believe the bug is caused by the fact that BBCooker has a single 'skiplist' dict, which is manipulated across all mcs. This dict is populated by CookerParser::parse_next:

        for virtualfn, info_array in result:
            if info_array[0].skipped:	
                self.skipped += 1
                self.cooker.skiplist[virtualfn] = SkippedPackage(info_array[0])
            self.bb_caches[mc].add_info(virtualfn, info_array, self.cooker.recipecaches[mc],
                                        parsed=parsed, watcher = self.cooker.add_filewatch)

CookerParser has a separate MulticonfigCache for each mc (self.bb_caches), but it is using the same 'skiplist' regardless of mc. 

I think BBCooker either needs a separate skiplist per mc, or switch to using tuples of (mc, virtualfn) as the keys. Then we'd adjust code elsewhere to respect the mc. For example, in BBCooker::buildTaskData, this code:

        for mc in self.multiconfigs:
            taskdata[mc] = bb.taskdata.TaskData(halt, skiplist=self.skiplist, allowincomplete=allowincomplete)
            localdata[mc] = data.createCopy(self.databuilder.mcdata[mc])
            bb.data.expandKeys(localdata[mc])

becomes something like:

        for mc in self.multiconfigs:
            # Assuming we use the per-mc skiplist approach
            taskdata[mc] = bb.taskdata.TaskData(halt, skiplist=self.skiplist_by_mc[mc], allowincomplete=allowincomplete)
            localdata[mc] = data.createCopy(self.databuilder.mcdata[mc])
            bb.data.expandKeys(localdata[mc])


Does this sound reasonable?

Thanks,
Chris

Comments

Richard Purdie Dec. 5, 2024, 10:13 p.m. UTC | #1 
On Thu, 2024-12-05 at 20:24 +0000, chris.laplante@agilent.com wrote:
> I believe I have tracked down a bug with SkipRecipe and multiconfig.
> Before I submit a patch, I wanted to check my understanding of the
> code and my suggested fix.
> 
> The issue I observed is that when you have a recipe that is
> incompatible (e.g. due to COMPATIBLE_MACHINE) with more than one
> active multiconfig, including 'default', then BitBake will
> inconsistently report why the recipe is unavailable.
> 
[..]

> 
> 
> Does this sound reasonable?

Short summary is yes, it does. We retrofitted multiconfig to bitbake so
this looks like something we just missed at the time.

Cheers,

Richard
chris.laplante@agilent.com Dec. 5, 2024, 10:49 p.m. UTC | #2 
Hi Richard,

> > The issue I observed is that when you have a recipe that is
> > incompatible (e.g. due to COMPATIBLE_MACHINE) with more than one
> > active multiconfig, including 'default', then BitBake will
> > inconsistently report why the recipe is unavailable.
> >
> [..]
> 
> >
> >
> > Does this sound reasonable?
> 
> Short summary is yes, it does. We retrofitted multiconfig to bitbake so this
> looks like something we just missed at the time.

OK, great. I'll work on a patch + selftest of some sort :).

Thanks,
Chris
diff mbox series

Patch

diff --git a/meta/classes-global/base.bbclass b/meta/classes-global/base.bbclass
index b81e61fdb7..a639792504 100644
--- a/meta/classes-global/base.bbclass
+++ b/meta/classes-global/base.bbclass
@@ -560,7 +560,7 @@  python () {
             if re.match(need_machine, m):
                 break
         else:
-            raise bb.parse.SkipRecipe("incompatible with machine %s (not in COMPATIBLE_MACHINE)" % d.getVar('MACHINE'))
+            raise bb.parse.SkipRecipe(f"incompatible with machine {d.getVar('MACHINE')} (not in COMPATIBLE_MACHINE), mc: {d.getVar('BB_CURRENT_MC')}")

     source_mirror_fetch = d.getVar('SOURCE_MIRROR_FETCH', False) or d.getVar('PARSE_ALL_RECIPES', False)
     if not source_mirror_fetch:
diff --git a/meta/lib/oeqa/selftest/cases/multiconfig.py b/meta/lib/oeqa/selftest/cases/multiconfig.py
index f509cbf607..c74bbd2fcf 100644
--- a/meta/lib/oeqa/selftest/cases/multiconfig.py
+++ b/meta/lib/oeqa/selftest/cases/multiconfig.py
@@ -85,3 +85,24 @@  BBMULTICONFIG = "muslmc"

         # Build a core-image-minimal, only dry run needed to check config is present
         bitbake('mc:muslmc:bash -n')
+
+    def test_multiconfig_skip_recipe(self):
+        config = """
+BBMULTICONFIG = "musl"
+"""
+        self.write_config(config)
+
+        muslconfig = """
+MACHINE = "qemumips"
+DISTRO = "poky"
+TCLIBC = "musl"
+TMPDIR = "${TOPDIR}/tmp-mc-musl"
+"""
+        self.write_config(muslconfig, 'musl')
+
+        # Write a recipe that is not compatible with qemux86-64
+        self.write_recipeinc('m4', 'COMPATIBLE_MACHINE="qemuarm"')
+
+        m = bitbake("m4 -n")
+        m = [line for line in m.output.splitlines() if "MACHINE=" in line]
+