Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

recipes-zarhus: images: zarhus-base-image-debug: delete #17

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

DaniilKl
Copy link
Contributor

@DaniilKl DaniilKl commented Aug 9, 2024

We can use overrides to add debug functionalities to images, this will decrease amount of images files, improve scalability and maintenance. This has been originally proposed here #15 (comment).

@DaniilKl DaniilKl self-assigned this Aug 9, 2024
@DaniilKl
Copy link
Contributor Author

DaniilKl commented Aug 9, 2024

@m-iwanicki, it does work now, I suppose I messed up with overrides to IMAGE_INSTALL, I used :dbg:append, but should have used :append:dbg.

But here is another problem to solve, pre-commit argues about :dbg, I suppose it does not notice we declare this override inside .yml file:

Advanced oelint..........................................................Failed
- hook id: oelint-adv
- exit code: 2

/home/danillklimuk/Projects/Zarhus/meta-zarhus/recipes-zarhus/images/zarhus-base-image.inc:4:error:oelint.vars.specific:'IMAGE_FEATURES' is set specific to ['dbg'], but isn't known from PACKAGES, MACHINE, DISTRO or resources
/home/danillklimuk/Projects/Zarhus/meta-zarhus/recipes-zarhus/images/zarhus-base-image.inc:14:error:oelint.vars.specific:'IMAGE_INSTALL' is set specific to ['dbg'], but isn't known from PACKAGES, MACHINE, DISTRO or resources

@DaniilKl
Copy link
Contributor Author

DaniilKl commented Aug 9, 2024

It is strange thoulg that it does not argue about your changes in linux_yocto_%.bbappend where you use the override for Kernel configs.

@m-iwanicki
Copy link
Contributor

m-iwanicki commented Aug 14, 2024

@DaniilKl I pushed fix for oelint error (we have to inform oelint about our overrides as it might not know them). We can generate correct format in kas:

sudo apt update
sudo apt install jq
bitbake-getvar --quiet --value OVERRIDES | tr ':' '\n' | jq -Rn  '{replacements:{ distros: [inputs]}}' > /repo/.oelint-custom-overrides.json

Also I think this part is needed:

 EXTRA_USERS_PARAMS = "usermod -p '${ROOT_PASSWD}' root;"
+EXTRA_USERS_PARAMS:dbg = ""

This way dbg version won't have password.

Not sure if there won't be problems later when meta-zarhus will grow but this split on debug/prod target has downsides e.g. when including meta-zarhus in other layers we can't create .bbappend for .inc file so I'm for this debug/prod target merge.
Maybe modify somehow .wic filename to contain debug to make it easier to differentiate images?

@DaniilKl DaniilKl changed the base branch from main to develop August 16, 2024 08:22
@DaniilKl
Copy link
Contributor Author

@m-iwanicki, thank you!

@DaniilKl
Copy link
Contributor Author

DaniilKl commented Aug 16, 2024

This way dbg version won't have password.

This way the entire variable EXTRA_USERS_PARAMS for dbg will be overwritten, won't it?

@DaniilKl DaniilKl marked this pull request as ready for review August 16, 2024 08:28
@m-iwanicki
Copy link
Contributor

This way the entire variable EXTRA_USERS_PARAMS for dbg will be overwritten, won't it?

Yes.

@macpijan
Copy link
Contributor

macpijan commented Aug 16, 2024

Not sure if there won't be problems later when meta-zarhus will grow but this split on debug/prod target has downsides > e.g. when including meta-zarhus in other layers we can't create .bbappend for .inc file so I'm for this debug/prod target > merge.
Maybe modify somehow .wic filename to contain debug to make it easier to differentiate images?

I am a bit worried here, as well.
Can we take a step back and consider what actual value we get by skipping the -dbg image recipe?

Having two recipes is advantageous to me, not only from the point as @m-iwanicki has mentioned, but we also have different image names. With this proposal, I cannot guess just from the image name, if it was debug or production image. Debug features can then easily sneak into production image, which is not a good thing to happen.

Maybe w can fix the original problem in another way?

@DaniilKl
Copy link
Contributor Author

Some rebasing.

@DaniilKl
Copy link
Contributor Author

DaniilKl commented Aug 21, 2024

This way the entire variable EXTRA_USERS_PARAMS for dbg will be overwritten, won't it?

Yes.

And I am a bit worried here.


e.g. when including meta-zarhus in other layers we can't create .bbappend for .inc file

But .inc files are just being included into recipes, so we can overwrite features they provide.

Maybe modify somehow .wic filename to contain debug to make it easier to differentiate images?

We could make a note in documentation about this override and features it provides, so users will be aware of it. Users are also aware of it during building when they provide debug.yml file. And during runtime we could utilize e.g. uname -a output and these promts:

(...)
[    1.425516] systemd[1]: Detected architecture arm64.

Welcome to Distro for Zarhus product 0.1.0 (scarthgap)!

[    1.444064] systemd[1]: Hostname set to <zarhus-machine-cm3>.
(...)
[    6.571714] mmc_host mmc2: Bus speed (slot 0) = 375000Hz (slot req 375000Hz, actual 375000HZ div = 0)

Distro for Zarhus product 0.1.0 zarhus-machine-cm3 ttyS2

zarhus-machine-cm3 login:

@DaniilKl
Copy link
Contributor Author

Can we take a step back and consider what actual value we get by skipping the -dbg image recipe?

Reduce amount of surplus code.

DaniilKl and others added 2 commits August 23, 2024 14:04
We can use overrides to add debug functionalities to images, this will
decrease amount of images files, improve scalability and maintanance.

Signed-off-by: Daniil Klimuk <daniil.klimuk@3mdeb.com>