Skip to content

Author: Masala <[email protected]> #2

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Author: Masala <[email protected]> #2

wants to merge 1 commit into from

Conversation

mixmasala
Copy link
Contributor

This PR is to extend the gogio tool to support adding the android permission for FOREGROUND_SERVICE to the generated android manifest.xml metadata. Originally part of gioui/gio#67

@mixmasala mixmasala force-pushed the add_foreground_service branch from 8acf798 to 81e14a3 Compare August 13, 2022 12:01
@eliasnaur eliasnaur force-pushed the main branch 4 times, most recently from 6f881e1 to 02068d6 Compare August 18, 2022 08:12
@mixmasala mixmasala force-pushed the add_foreground_service branch from 81e14a3 to 8545def Compare August 20, 2022 15:13
@eliasnaur
Copy link
Contributor

This PR seems to mix two features: adding a new permission and adding .java support to gogio. The former seems ok, but the latter not because it adds a dependency on a Java compiler. In future, I'd like gogio to be implemented completely in Go.

@mixmasala
Copy link
Contributor Author

I believe the java compiler dependency already exists for buildandroid.go and the changes above are necessary to implement this feature (generating AndroidManifest.xml with the right class names). When do you think you will manage to eliminate the java compiler dependecy?

@eliasnaur
Copy link
Contributor

I believe the java compiler dependency already exists for buildandroid.go.

It's true that buildandroid.go depends on a Java compiler today. My point is that your change will make it much harder to avoid that dependency in future. Without your change, we can check in .class files (or a .jar) for the Java files from package gioui.org/app and add //go:generate commands to update them (which will require a Java compiler).

When do you think you will manage to eliminate the java compiler dependecy?

I don't have the work planned.

the changes above are necessary to implement this feature (generating AndroidManifest.xml with the right class names)

Can you point to the changes that require a java compiler to implement this change? I couldn't find any.

@mixmasala mixmasala force-pushed the add_foreground_service branch from 8545def to a9fefc1 Compare September 15, 2022 14:58
@mixmasala
Copy link
Contributor Author

Thanks. Looking closer, I see those changes were no longer necessary because the notification icon is now using the package icon.

eliasnaur
eliasnaur reviewed Sep 16, 2022
View reviewed changes
gogio/androidbuild.go Outdated
@@ -446,6 +448,7 @@ func exeAndroid(tmpDir string, tools *androidTools, bi *buildInfo, extraJars, pe
Features: features,
IconSnip: iconSnip,
AppName: appName,
HasService: stringsContains(permissions, foregroundPermission),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's more canonical to search for "foreground" in perms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed.

gogio/androidbuild.go
@@ -468,6 +471,19 @@ func exeAndroid(tmpDir string, tools *androidTools, bi *buildInfo, extraJars, pe
<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>
{{if .HasService}}
<service android:name="org.gioui.GioForegroundService"
Copy link
Contributor

Choose a reason for hiding this comment

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

We can't submit this change before your change that adds GioForegroundService, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct

gogio/androidbuild.go Outdated
Comment on lines 481 to 482
<meta-data android:name="org.gioui.ForegroundChannelDesc"
android:value="" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this empty value be omitted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if the corresponding getString() method in gio returns a default empty string.

@MatejMagat305
Copy link

I do not want toplay to clever, but if you want got permision you can use https://github.com/MatejMagat305/golang-prototype-permision

@mixmasala mixmasala force-pushed the add_foreground_service branch from a9fefc1 to 697be40 Compare September 17, 2023 19:01
@mixmasala mixmasala force-pushed the add_foreground_service branch 3 times, most recently from e92b820 to c2c687e Compare September 27, 2023 11:01
@mixmasala
cmd/gogio: add android foreground permission
f7c2488 
This adds the permission android.permission.FOREGROUND_SERVICE to the
android application Metadata

Signed-off-by: Masala <[email protected]>
@mixmasala mixmasala force-pushed the add_foreground_service branch from c2c687e to f7c2488 Compare August 30, 2024 06:42
@eliasnaur eliasnaur force-pushed the main branch 4 times, most recently from 207e6f8 to 01ffdf7 Compare March 21, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eliasnaur eliasnaur eliasnaur left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mixmasala @eliasnaur @MatejMagat305