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

Factor out wait for /dev/.coldboot to separate command #168

Merged
merged 3 commits into from
Dec 2, 2023

Conversation

beroset
Copy link
Member

@beroset beroset commented Jul 4, 2023

Every target except for the qemux86 target included android-init indirectly as part of the conf/machine/include/hybris-watch.inc file that each image includes. Without this, asteroid-launcher for qemux86 never starts because it is waiting for something to create the file /dev/.coldboot_done. Since every target except qemux86 already includes android-init, this does not affect any other target but fixes AsteroidOS/asteroid#259 for qemux86.

@MagneFire
Copy link
Member

I'm fine with merging this. @dodoradio do you agree?

@dodoradio
Copy link
Contributor

I don't think it's a good idea to add a second init system to all builds just because hybris is our current default porting strategy.
Accepting this kind of android integration as default also breaks packaging for postmarketos, who won't ship any android dependency frameworks.
I think that asteroid-launcher should have a proper behaviour added instead of this plaster solution.

@beroset beroset marked this pull request as draft July 5, 2023 15:08
@beroset
Copy link
Member Author

beroset commented Jul 5, 2023

@dodoradio has suggested that a better approach might be to conditionally add the check for /dev/.coldboot_done for watches that include hybris-watch.inc which I agree sounds like a better approach. So I'm converting this to draft for now and will see about trying such an alternative. Thanks all!

@beroset
Copy link
Member Author

beroset commented Jul 5, 2023

I could create two different versions of asteroid-launcher.service, one of which omits this line:

ExecStartPre=/bin/sh -ec 'while [ ! -f /dev/.coldboot_done ]; do sleep 1; done'

Then we could selectively choose which one to install, based on whether the target it qemux86 or not. Is that a better way to do this? Seems hackish, but I don't know how else to do it.

@FlorentRevest
Copy link
Member

I agree with @dodoradio here :)

I could create two different versions of asteroid-launcher.service, one of which omits this line:

I'd approve that but I wonder if it'll be more maintainable to have a ExecStartPre=/usr/bin/asteroid-launcher-precondition and have a script there that waits for /dev/.coldboot_done only if there is an android init installed for example. Or something along that line.

This adds a new asteroid-launcher-precondition shell script which waits
for the existence of a /dev/.coldboot_done file only on machines that
have the android-boot-completed.service installed.  This allows both
real watches and the qemux86 simulator to boot properly to a GUI.

Signed-off-by: Ed Beroset <beroset@ieee.org>
This adds the asteroid-launcher-precondition shell script to the
installation.

Signed-off-by: Ed Beroset <beroset@ieee.org>
This uses the new asteroid-launcher-precondition command instead of
inlining a one-line shell command.  This fixes #259 and allows both
watches and the emulator to properly boot to a GUI screen.

Signed-off-by: Ed Beroset <beroset@ieee.org>
@beroset beroset changed the title Add android-init to installed packages Factor out wait for /dev/.coldboot to separate command Nov 11, 2023
@beroset beroset marked this pull request as ready for review November 11, 2023 22:03
@beroset
Copy link
Member Author

beroset commented Nov 11, 2023

This version was tested on both catfish and qemux86 and works as expected.

Copy link
Member

@MagneFire MagneFire left a comment

Choose a reason for hiding this comment

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

Minor comment, otherwise looks fine. Thank you 😄

@@ -7,7 +7,7 @@ ConditionUser=!root
Type=notify
WorkingDirectory=/
EnvironmentFile=-/var/lib/environment/compositor/*.conf
ExecStartPre=/bin/sh -ec 'while [ ! -f /dev/.coldboot_done ]; do sleep 1; done'
ExecStartPre=/usr/bin/asteroid-launcher-precondition
ExecStartPre=-/bin/sh -ec '/bin/echo QUIT > /run/psplash_fifo'
Copy link
Member

Choose a reason for hiding this comment

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

What's your opinion on moving this to the new script as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure what you're asking. Do you mean adding the -bin/sh -ec '/bin/echo QUIT > /run/psplash_fifo' line to the new script? If so, then I'm fine with that and will do that. If not, please explain further.

@FlorentRevest
Copy link
Member

Thank you!

@FlorentRevest FlorentRevest merged commit 06f4f32 into AsteroidOS:master Dec 2, 2023
@beroset beroset deleted the add-android-init branch December 3, 2023 12:45
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.

[qemux86] Emulator fails to show GUI
4 participants