Bug 12373 - Feature Backfilling doesn't work for multilib
Summary: Feature Backfilling doesn't work for multilib
Status: RESOLVED FIXED
Alias: None
Product: OE-Core
Classification: Build System, Metadata & Runtime
Component: configuration (show other bugs)
Version: 2.4
Hardware: x86 Multiple
: High normal
Target Milestone: 2.5 M3
Assignee: Richard Purdie
QA Contact:
URL:
Whiteboard:
Depends on:
Blocks:
 
Reported: 2017-11-22 02:06 UTC by Jackie Huang
Modified: 2018-03-04 21:13 UTC (History)
6 users (show)

See Also:
OS type for building Yocto: ---
Type of Regression: ---
Verified:
Documentation change: No (bug/feature does not impact docs)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jackie Huang 2017-11-22 02:06:09 UTC
Enabled multilib as the following for qemumips64:

MULTILIB_GLOBAL_VARIANTS_append = " libn32"
MULTILIBS ?= "multilib:lib32 multilib:libn32"
DEFAULTTUNE_virtclass-multilib-lib32 ?= "mips32r2"
DEFAULTTUNE_virtclass-multilib-libn32 ?= "mips64-n32"
require conf/multilib.conf


$ bitbake -e libn32-gobject-introspection | grep '^MACHINE_FEATURES'
MACHINE_FEATURES="alsa bluetooth usbgadget screen rtc qemu-usermode"
MACHINE_FEATURES_BACKFILL="rtc qemu-usermode"
MACHINE_FEATURES_BACKFILL_CONSIDERED=" qemu-usermode"

According to the doc:
http://www.yoctoproject.org/docs/2.4/ref-manual/ref-manual.html#ref-features-backfill

The expected result should be:
MACHINE_FEATURES="alsa bluetooth usbgadget screen rtc"
Comment 1 Jackie Huang 2017-11-22 02:14:32 UTC
The MACHINE_FEATURES_BACKFILL_CONSIDERED is set in meta/conf/machine/include/mips/arch-mips.inc:

# user mode qemu doesn't support mips64 n32: "Invalid ELF image for this architecture"
MACHINE_FEATURES_BACKFILL_CONSIDERED_append = " ${@bb.utils.contains('TUNE_FEATURES', 'n32', 'qemu-usermode', '', d)}"

But the backfilling is handled in bb.event.ConfigParsed(in base.bbclass):

    if isinstance(e, bb.event.ConfigParsed):
        if not d.getVar("NATIVELSBSTRING", False):
            d.setVar("NATIVELSBSTRING", lsb_distro_identifier(d))
        d.setVar('BB_VERSION', bb.__version__)
        pkgarch_mapping(d)
        oe.utils.features_backfill("DISTRO_FEATURES", d)
        oe.utils.features_backfill("MACHINE_FEATURES", d)


Which seems to be too early since multilib handler happens in bb.event.RecipePreFinalise.
Comment 2 Richard Purdie 2017-11-30 15:58:12 UTC
I believe the issue here is event ordering. I suspect that there are ConfigParsed event handlers in both base.bbclass and multilib and the ordering of them in some cases is incorrect.

Firstly could you confirm if this is an event ordering issue? To test you could add a second event and see whether that resolves the issue.

If that is the problem, we then need to figure out a way to control ordering of event handlers. There is another bug open for that but this would raise its prority. We should discuss what way there may be of doing that on the mailing list.
Comment 3 Jackie Huang 2017-12-01 02:10:07 UTC
(In reply to comment #2)
> I believe the issue here is event ordering. I suspect that there are
> ConfigParsed event handlers in both base.bbclass and multilib and the
> ordering of them in some cases is incorrect.

In multilib, the ConfigParsed event handler is:
multilib_virtclass_handler_vendor[eventmask] = "bb.event.ConfigParsed"

But the DEFAULTTUNE changes is in multilib_virtclass_handler which is RecipePreFinalise event handler.

> 
> Firstly could you confirm if this is an event ordering issue? To test you
> could add a second event and see whether that resolves the issue.

Yes, I can confirm that it's an event ordering issue. I'm not sure what you mean by
adding a second event, but I tried the following to change the ordering:

1) I changed ConfigParsed to RecipeTaskPreProcess for oe.utils.features_backfill in meta/classes/base.bbclass

-    if isinstance(e, bb.event.ConfigParsed):
+    if isinstance(e, bb.event.RecipePreFinalise):

But it didn't resolve the issue.

2) Then I changed ConfigParsed to RecipeTaskPreProcess:

-base_eventhandler[eventmask] = "bb.event.ConfigParsed bb.event.MultiConfigParsed bb.event.BuildStarted bb.event.RecipePreFinalise bb.runqueue.sceneQueueComplete bb.event.RecipeParsed"
+base_eventhandler[eventmask] = "bb.event.ConfigParsed bb.event.MultiConfigParsed bb.event.BuildStarted bb.event.RecipePreFinalise bb.runqueue.sceneQueueComplete bb.event.RecipeParsed bb.event.RecipeTaskPreProcess"
 python base_eventhandler() {
     import bb.runqueue

-    if isinstance(e, bb.event.ConfigParsed):
+    if isinstance(e, bb.event.RecipeTaskPreProcess):

And it resolved the issue, but I'm not sure if it's a proper fix or if it impacts other orderings, I'm going to do some world builds to check.


> 
> If that is the problem, we then need to figure out a way to control ordering
> of event handlers. There is another bug open for that but this would raise

what is the bug id?

> its prority. We should discuss what way there may be of doing that on the
> mailing list.
Comment 4 Jackie Huang 2017-12-01 05:39:07 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > I believe the issue here is event ordering. I suspect that there are
> > ConfigParsed event handlers in both base.bbclass and multilib and the
> > ordering of them in some cases is incorrect.
> 
> In multilib, the ConfigParsed event handler is:
> multilib_virtclass_handler_vendor[eventmask] = "bb.event.ConfigParsed"
> 
> But the DEFAULTTUNE changes is in multilib_virtclass_handler which is
> RecipePreFinalise event handler.
> 
> > 
> > Firstly could you confirm if this is an event ordering issue? To test you
> > could add a second event and see whether that resolves the issue.
> 
> Yes, I can confirm that it's an event ordering issue. I'm not sure what you
> mean by
> adding a second event, but I tried the following to change the ordering:
> 
> 1) I changed ConfigParsed to RecipeTaskPreProcess for
> oe.utils.features_backfill in meta/classes/base.bbclass
> 
> -    if isinstance(e, bb.event.ConfigParsed):
> +    if isinstance(e, bb.event.RecipePreFinalise):
> 
> But it didn't resolve the issue.
> 
> 2) Then I changed ConfigParsed to RecipeTaskPreProcess:
> 
> -base_eventhandler[eventmask] = "bb.event.ConfigParsed
> bb.event.MultiConfigParsed bb.event.BuildStarted bb.event.RecipePreFinalise
> bb.runqueue.sceneQueueComplete bb.event.RecipeParsed"
> +base_eventhandler[eventmask] = "bb.event.ConfigParsed
> bb.event.MultiConfigParsed bb.event.BuildStarted bb.event.RecipePreFinalise
> bb.runqueue.sceneQueueComplete bb.event.RecipeParsed
> bb.event.RecipeTaskPreProcess"
>  python base_eventhandler() {
>      import bb.runqueue
> 
> -    if isinstance(e, bb.event.ConfigParsed):
> +    if isinstance(e, bb.event.RecipeTaskPreProcess):
> 
> And it resolved the issue, but I'm not sure if it's a proper fix or if it
> impacts other orderings, I'm going to do some world builds to check.

This causes error:
bluez5 was skipped: missing required distro feature 'bluez5' (not in DISTRO_FEATURES)

So it's not a proper fix and impact other orderings.

> 
> 
> > 
> > If that is the problem, we then need to figure out a way to control ordering
> > of event handlers. There is another bug open for that but this would raise
> 
> what is the bug id?
> 
> > its prority. We should discuss what way there may be of doing that on the
> > mailing list.
Comment 5 Randy MacLeod 2017-12-04 19:56:56 UTC
Richard asked us to work on how to resolve this multilib problem with him simply providing guidance. I've assigned it to Robert and we'll talk about how to proceed then update the defect with our proposed fix.
Comment 6 Richard Purdie 2017-12-04 21:25:06 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Firstly could you confirm if this is an event ordering issue? To test you
> > could add a second event and see whether that resolves the issue.
> 
> Yes, I can confirm that it's an event ordering issue. I'm not sure what you
> mean by
> adding a second event, but I tried the following to change the ordering:

In cookerdata.py there is the line:

bb.event.fire(bb.event.ConfigParsed(), self.data)

so you could add:

bb.event.fire(bb.event.ConfigParsed2(), self.data)

(adding a copy of bb.event.ConfigParsed as ConfigParsed2)

immediately after it which would add a second event (or add en event before). If that fixed the problem, we'd confirm its event ordering.

> > If that is the problem, we then need to figure out a way to control ordering
> > of event handlers. There is another bug open for that but this would raise
> 
> what is the bug id?

5709, I added it as a dependency of this bug.
Comment 7 Jackie Huang 2017-12-06 07:53:58 UTC
(In reply to comment #6)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Firstly could you confirm if this is an event ordering issue? To test you
> > > could add a second event and see whether that resolves the issue.
> > 
> > Yes, I can confirm that it's an event ordering issue. I'm not sure what you
> > mean by
> > adding a second event, but I tried the following to change the ordering:
> 
> In cookerdata.py there is the line:
> 
> bb.event.fire(bb.event.ConfigParsed(), self.data)
> 
> so you could add:
> 
> bb.event.fire(bb.event.ConfigParsed2(), self.data)
> 
> (adding a copy of bb.event.ConfigParsed as ConfigParsed2)
> 
> immediately after it which would add a second event (or add en event
> before). If that fixed the problem, we'd confirm its event ordering.

Unfortunately it can not fix the problem, I think it's not only an ordering issue.

> 
> > > If that is the problem, we then need to figure out a way to control ordering
> > > of event handlers. There is another bug open for that but this would raise
> > 
> > what is the bug id?
> 
> 5709, I added it as a dependency of this bug.

The bug is about controlling handlers order in the same event, right?

But the issue here is:
features_backfill happens in bb.event.ConfigParsed, multilib_virtclass_handler happens in bb.event.RecipePreFinalise which seems to be after bb.event.ConfigParsed.
Comment 8 Jackie Huang 2017-12-07 02:41:26 UTC
I sent a patch for this:
https://patchwork.openembedded.org/series/10104/
Comment 9 Richard Purdie 2018-03-03 17:13:20 UTC
The approach in this patch has always felt like a hack/workaround. After experimentation I've concluded we should fix bitbake -e to handle anonymous python, then we can move the backfilling to anonymous python and the problematic multilib issue should be resolved. I've sent patches to bitbake and OE-Core.