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

Add support for vendor-defined bootanimation #12

Open
wants to merge 1 commit into
base: maru-0.6
Choose a base branch
from

Conversation

makinbacon21
Copy link

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 [email protected]

Copy link
Member

@utzcoz utzcoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the device makefile can override this field after including device-maru.mk, and it doesn't need a commit. cc @pdsouza .

@makinbacon21
Copy link
Author

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
Copy link
Member

pdsouza commented Apr 8, 2021

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
Copy link
Member

pdsouza commented Apr 8, 2021

@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
Copy link
Author

@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
Copy link
Member

pdsouza commented Apr 10, 2021

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
Copy link
Member

pdsouza commented Apr 10, 2021

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

@pdsouza
Copy link
Member

pdsouza commented Apr 24, 2021

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

See #14. Can someone confirm it builds correctly on maru-0.6?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants