maruos / vendor_maruos

Common files for Maru OS.
Apache License 2.0
5 stars 15 forks source link

Add support for vendor-defined bootanimation #12

Open makinbacon21 opened 3 years ago

makinbacon21 commented 3 years ago

Allow external vendors to set bootanimation without being overwritten by maru--checks if maru version is set before setting TARGET_BOOTANIMATION.

Signed-off-by: Thomas Makin halorocker89@gmail.com

makinbacon21 commented 3 years ago

Yea but my concern was with setting other stuff with parsing order—if I wanted to override the rootfs AND bootanimation from the same makefile in another vendor repo, the order the makefile are executed in matters. This would eliminate that necessity, though I see your point there and it might just be better for me to work off a fork instead of adding an unnecessary conditional.

pdsouza commented 3 years ago

I agree with @utzcoz. I suggest making sure you include device-maru.mk first, and then setting whichever overrides you need after that. Here's how we override any LOS variables for hammerhead for example: https://github.com/maruos/android_device_lge_hammerhead/blob/maru-0.7/maru_hammerhead.mk. This has worked well for us.

pdsouza commented 3 years ago

@makinbacon21 Actually, I see your point about the ordering with the rootfs the way you're using the lazily set variable: you need to first set PREBUILT_REPO, then include device-maru.mk, then override TARGET_BOOTANIMATION. This is brittle.

What we need here is the ability for you to override anything you need after including device-maru.mk.

It would be ideal if we could export a variable like TARGET_DESKTOP_ROOTFS or something that pointed to the prebuilt rootfs, and instead of using PREBUILT_REPO, we just copied over TARGET_DESKTOP_ROOTFS. This is similar to how TARGET_BOOTANIMATION overriding works on LOS. I think the trick would be to make the desktop rootfs a package, just like with bootanimation.zip, so that all the Makefiles are parsed and combined before running the actual package makefile commands.

This shouldn't be a big change and I'll see if this works tonight when I have some time.

Open to other ideas as well of course.

makinbacon21 commented 3 years ago

@pdsouza I really like that idea--I was experimenting with something similar the other day when I was pondering ways to build rootfs as part of android build, but abandoned that project upon realizing it'd need sudo anyway, and giving build system root access is bad form.

I think a TARGET_DESKTOP_ROOTFS var would work perfectly, and could just be ifneq'd in device-maru.mk to copy prebuilt. Thanks for your work!

pdsouza commented 3 years ago

OK, I think I have it working. See #13. I'll merge this into maru-0.7 and then cherry-pick it into maru-0.6 if it's good.

pdsouza commented 3 years ago

13 has been merged into maru-0.7. I will cherry-pick it into maru-0.6 sometime this weekend.

pdsouza commented 3 years ago

13 has been merged into maru-0.7. I will cherry-pick it into maru-0.6 sometime this weekend.

See https://github.com/maruos/vendor_maruos/pull/14. Can someone confirm it builds correctly on maru-0.6?