meta-raspberrypi icon indicating copy to clipboard operation
meta-raspberrypi copied to clipboard

rpi-base: filter None values from DTB boot files

Open lhoward opened this issue 11 months ago • 25 comments

Filter None values from DTB boot files, to avoid errors expanding IMAGE_BOOT_FILES

lhoward avatar Jan 12 '25 23:01 lhoward

I don't really know Python, so not sure what it's about the version I've installed that caused this. It would be great if (after review) it could also be back-ported to the scarthgap branch.

lhoward avatar Jan 12 '25 23:01 lhoward

I don't really know Python, so not sure what it's about the version I've installed that caused this. It would be great if (after review) it could also be back-ported to the scarthgap branch.

Thanks for this. I would like to understand why this happened in the first place. Could you dump the environment and check the value there? Also, a fix need to firstly land in master (and you can follow up with a cherry pick PR in other branches).

agherzan avatar Jan 16 '25 17:01 agherzan

Note to self: a colleague suggested

⁠ return ' '.join(x for x in (transform(dtb) for dtb in alldtbs.split(' ') if dtb) if x is not None) ⁠

would be more idiomatic

lhoward avatar Jan 16 '25 18:01 lhoward

I'll rerun the build without the patch when I'm at my desk, but my hunch is that the whitespaces in IMAGE_BOOT_FILES are being ~~expanded~~ split to None (at least on my system).

lhoward avatar Jan 16 '25 18:01 lhoward

Error I saw was:

ERROR: ExpansionError during parsing /home/lukeh/CVSRoot/padl/inferno/build/xebros/meta/recipes-core/images/core-image-minimal.bb
bb.data_smart.ExpansionError: Failure expanding variable IMAGE_BOOT_FILES, expression was bootfiles/*                  ${@make_dtb_boot_files(d)}                   	Image u-boot.bin;kernel8.img boot.scr 	                   which triggered exception TypeError: sequence item 59: expected str instance, NoneType found
The variable dependency chain for the failure is: IMAGE_BOOT_FILES

I don't know enough about BitBake to know how to dump more...

lhoward avatar Jan 17 '25 01:01 lhoward

Just checking if there's an update on this, I'd like to get off my fork if possible.

lhoward avatar Mar 12 '25 03:03 lhoward

Doesn't this affect the master branch?

agherzan avatar Mar 12 '25 21:03 agherzan

Doesn't this affect the master branch?

I imagine it does. What was the question in relation to?

lhoward avatar Mar 15 '25 03:03 lhoward

Unless there is a fix specific to a maintained branch, we require fixes to first be pushed against master and only afterward backported (via an additional PR) to a stable branch. Do you think you could do that?

agherzan avatar Mar 21 '25 09:03 agherzan

Unless there is a fix specific to a maintained branch, we require fixes to first be pushed against master and only afterward backported (via an additional PR) to a stable branch. Do you think you could do that?

The patch should still apply against master, but I can resubmit.

lhoward avatar Mar 21 '25 09:03 lhoward

Rebased.

lhoward avatar Mar 21 '25 09:03 lhoward

Is there anything I can do to help get this merged?

lhoward avatar May 19 '25 12:05 lhoward

Is there anything I can do to help get this merged?

Please make a change and rebase on top of latest master branch so the CI state can reset

kraj avatar May 19 '25 22:05 kraj

Rebased once more.

lhoward avatar May 19 '25 23:05 lhoward

Rebased once more against master. How can I help get this merged?

lhoward avatar Jun 22 '25 22:06 lhoward

Apologies, previous commit was incorrectly rebased and had other patches in it; this one should be correct.

lhoward avatar Jun 27 '25 04:06 lhoward

Just pinging to see if there is anything I could help to get this merged. I'd love to avoid having to maintain my own branch. Alternative I suppose is to find the right Python version that doesn't exhibit this behaviour on my host system, and close the PR.

lhoward avatar Jul 02 '25 04:07 lhoward

Is there anything I can do to help progress this? I'd prefer to not have to maintain my own fork if at all possible.

lhoward avatar Aug 08 '25 09:08 lhoward

Is there anything I can do to help progress this? I'd prefer to not have to maintain my own fork if at all possible.

I will schedule it for CI over weekend.

kraj avatar Aug 08 '25 18:08 kraj

Is there anything I can do to help progress this? I'd prefer to not have to maintain my own fork if at all possible.

I will schedule it for CI over weekend.

Did CI pass?

lhoward avatar Aug 13 '25 04:08 lhoward

Is there anything I can do to help progress this? I'd prefer to not have to maintain my own fork if at all possible.

I will schedule it for CI over weekend.

Did CI pass?

there is a merge commit, please rebase so the merge commit is not there. There is a CI test which is failing due to that

kraj avatar Aug 13 '25 20:08 kraj

Damn GitHub making merge commits by default from UI. Fixed.

lhoward avatar Aug 13 '25 21:08 lhoward

GitHub is saying all checks have passed – can you let me know how we might go forward getting this merged into master and then into scarthgap? Do you require separate pull requests for version branches?

lhoward avatar Aug 14 '25 09:08 lhoward

GitHub is saying all checks have passed – can you let me know how we might go forward getting this merged into master and then into scarthgap? Do you require separate pull requests for version branches?

It would help me assess this a bit more if you can dump KERNEL_DEVICETREE variable when it fails. I would like to document exact situation where this can happen and ofcouse the patch makes the function robust so perhaps we are fine there.

kraj avatar Aug 14 '25 14:08 kraj

Well, this is mysterious.

I added some debugging (bb.warn ) calls to make_dtb_boot_files() to better introspect what's going and without the patch it now seems to be concatenating correctly:

KERNEL_DEVICETREE:          overlays/overlay_map.dtb          overlays/disable-bt.dtbo     overlays/disable-wifi.dtbo                                                                                                                                                                                                        overlays/uart0.dtbo                              overlays/uart3.dtbo                    overlays/uart5.dtbo               overlays/vc4-kms-v3d.dtbo     overlays/vc4-kms-v3d-pi4.dtbo          overlays/vc4-kms-dsi-7inch.dtbo     overlays/vc4-kms-dsi-ili9881-7inch.dtbo                    overlays/bcm2712d0.dtbo          overlays/spi0-1cs.dtbo     overlays/uart0.dtbo     overlays/uart3.dtbo     overlays/uart5.dtbo     overlays/vc4-kms-v3d.dtbo     overlays/vc4-kms-dsi-7inch.dtbo     overlays/vc4-kms-dsi-maxen-b70hm30.dtbo     overlays/mv88e6352-rmu.dtbo     overlays/mv88e6352-mdio.dtbo      broadcom/bcm2711-rpi-cm4.dtb 

make_dtb_boot_files(): overlay_map.dtb;overlays/overlay_map.dtb disable-bt.dtbo;overlays/disable-bt.dtbo disable-wifi.dtbo;overlays/disable-wifi.dtbo uart0.dtbo;overlays/uart0.dtbo uart3.dtbo;overlays/uart3.dtbo uart5.dtbo;overlays/uart5.dtbo vc4-kms-v3d.dtbo;overlays/vc4-kms-v3d.dtbo vc4-kms-v3d-pi4.dtbo;overlays/vc4-kms-v3d-pi4.dtbo vc4-kms-dsi-7inch.dtbo;overlays/vc4-kms-dsi-7inch.dtbo vc4-kms-dsi-ili9881-7inch.dtbo;overlays/vc4-kms-dsi-ili9881-7inch.dtbo bcm2712d0.dtbo;overlays/bcm2712d0.dtbo spi0-1cs.dtbo;overlays/spi0-1cs.dtbo uart0.dtbo;overlays/uart0.dtbo uart3.dtbo;overlays/uart3.dtbo uart5.dtbo;overlays/uart5.dtbo vc4-kms-v3d.dtbo;overlays/vc4-kms-v3d.dtbo vc4-kms-dsi-7inch.dtbo;overlays/vc4-kms-dsi-7inch.dtbo vc4-kms-dsi-maxen-b70hm30.dtbo;overlays/vc4-kms-dsi-maxen-b70hm30.dtbo mv88e6352-rmu.dtbo;overlays/mv88e6352-rmu.dtbo mv88e6352-mdio.dtbo;overlays/mv88e6352-mdio.dtbo bcm2711-rpi-cm4.dtb

So at this point I'm not quite sure what to do. Perhaps there was something in an earlier set of overlays that tripped it up (I recently removed all but the required entries). So I suppose we can close this in case I can duplicate it again?

lhoward avatar Aug 15 '25 02:08 lhoward