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

IzzyOnDroid #2

Open
farfromrefug opened this issue Oct 10, 2024 · 26 comments
Open

IzzyOnDroid #2

farfromrefug opened this issue Oct 10, 2024 · 26 comments

Comments

@farfromrefug
Copy link
Contributor

@IzzySoft here is a new app of mine. Would love for it to be on IzzyOnDroid ;)
Thanks!

@IzzySoft
Copy link

! repo/com.akylas.conty_13.apk declares sensitive permission(s):
  android.permission.READ_EXTERNAL_STORAGE android.permission.READ_PHONE_STATE
  android.permission.MANAGE_EXTERNAL_STORAGE

READ_EXTERNAL_STORAGE I can understand. But MANAGE? Why would you need that? And why READ_PHONE_STATE (for incoming-call-detection, just register to the onAudioFocusChanged broadcast).

Another question: where are the stories downloaded from?

Apart from that: it will go online with the next sync around 6 pm UTC 😃

@farfromrefug
Copy link
Contributor Author

farfromrefug commented Oct 11, 2024

@IzzySoft i removed the READ_PHONE_STATE which was coming from a dependency.
Now about MANAGE_EXTERNAL_STORAGE. I use it because the app heavily uses file read, unzip, copy... (to import packaged stories in zip). SAF is a nightmare and is amazingly slow. Using MANAGE_EXTERNAL_STORAGE allows me to work directly with java.io.File and makes the app package processing 10 times faster.
I will add a first dialog prompt to explain why i ask for that permission.
BTW the app will still work if you refuse it.

EDIT: and i forgot to say thank you!

@IzzySoft
Copy link

Thank you! Yes, SAF is a pain especially with Android 5-11 (and the permission mess there). And for heavy file operations pretty slow, agreed. But why MANAGE and not just READ/WRITE? Ah, legacy and "ignored" with Android 12+ or something, right?

@farfromrefug
Copy link
Contributor Author

@IzzySoft because READ/WRITE does not work on android >= 11. It is either SAF or MANAGE
And btw it is even worth on android 10 where even with READ/WRITE there are some operations which wont work without SAF (like creating dirs on sdcard)

@IzzySoft
Copy link

because READ/WRITE does not work on android >= 11. It is either SAF or MANAGE

Thanks for confirming! I wasn't 100% sure. And now wonder how Goog then deals with it in their toy shop if they limit MANAGE to "well proven cases, mostly only file managers and such"… 🤷‍♂️

on android 10

On 10 (and only there) you can opt into legacy, though. But that would make it complicated if you want to cover > 10 as well. Crazy stuff that…

@farfromrefug
Copy link
Contributor Author

@IzzySoft i do opt in on android 10 for legacy storage but still does not work. Dont rrmeber were but it is written that some File operations might fail.... And indeed create dirs, delete them, fails even with the legacy storage flag...

@IzzySoft
Copy link

Wow. That's weird. But well, I believe you. Makes it even more crazy how they deal with it concerning PlayStore admission…

That leaves the two questions:

  • what about android.permission.READ_EXTERNAL_STORAGE?
  • where are books downloaded from?

@farfromrefug
Copy link
Contributor Author

@IzzySoft books can be either downloaded from the web or imported from a zip. I am 100% but then I think I need read storage for pre SAF devices ?

@IzzySoft
Copy link

books can be either downloaded from the web or imported from a zip.

Ah, that implicitly answers the reason of my question. So nothing reaching out to some non-free/fixed server by default – and the app would even work without any server as one can import from ZIP. Very good, so we don't need some NonFreeNet or such 😉

I am 100% but then I think I need read storage for pre SAF devices ?

If there were any affected, yes. But which should that be? Conty requires at least Android-5, which was when SAF was introduced. Would you support devices on Android < 5, then yes. But you don't. So if you consequently use SAF for all file operations outside your app's dedicated storage (i.e. outside /data/data/com.akylas.conty/), you should need neither READ nor WRITE permissions. On first file access you'd need to pop up the file picker so one can choose a location – and as long as you operate inside that, all should be fine (to my understanding – I'm not an Android dev, so some things are "theory" to me and not first-hand experience).

@IzzySoft
Copy link

Oh, PS: I miss the IzzyOnDroid badge in the Readme (I just wanted to use that to quickly check your min Android version on the repo browser, but only found the PlayStore and Github badges there) ☺️

@farfromrefug
Copy link
Contributor Author

I am 100% but then I think I need read storage for pre SAF devices ?

If there were any affected, yes. But which should that be? Conty requires at least Android-5, which was when SAF was introduced. Would you support devices on Android < 5, then yes. But you don't. So if you consequently use SAF for all file operations outside your app's dedicated storage (i.e. outside /data/data/com.akylas.conty/), you should need neither READ nor WRITE permissions. On first file access you'd need to pop up the file picker so one can choose a location – and as long as you operate inside that, all should be fine (to my understanding – I'm not an Android dev, so some things are "theory" to me and not first-hand experience).

Indeed i could use SAF bu i am thinking to not use SAF for devices < 10 since it is much faster (10 is sadly the one version where everything is messed up without real solution). So i guess i prefer READ/WRITE over SAF. Is that ok?

And I brought back the badge!
Thank you

@IzzySoft
Copy link

Sure. SAF would be more granular and mostly guaranteeing which directories the app can access – but my question is mostly about transparency. So I've added the reasoning now (putting all storage permissions to your app's "green list" with an explanation). Thanks!

@IzzySoft
Copy link

I see you added Sentry. Is it strictly opt-in? Or is something of it enabled by default?

@farfromrefug
Copy link
Contributor Author

@IzzySoft Damn that s a mistake should only be in playstore builds :s Will be removed in next ones...
Really sorry

@farfromrefug
Copy link
Contributor Author

@IzzySoft
Copy link

Thanks for the swift action! Just manually triggered an update now to verify… Oops, now android.permission.MANAGE_EXTERNAL_STORAGE is back. And looks like you didn't increase versionCode (it's still 17 despite the tag indicating 18, which will break RB if set up – ah, wasn't (checking) ah, Typescript – well, no idea how to do RB with that). But Sentry is gone now, thanks!

@farfromrefug
Copy link
Contributor Author

Managé permission is normal ! As we explained before in this issue. But indeed I forgot to increase the build number :( will.make a new one tomorrow morning
Sorry

@IzzySoft
Copy link

No probs, and no need to hurry it. In this specific case it's even somehow useful it overwrites the existing APK – but only for new installs, and for those who did not yet update. So a timely update would be great indeed, but tomorrow is totally sufficient, thanks!

@farfromrefug
Copy link
Contributor Author

@IzzySoft the new build is up https://github.com/Akylas/conty/releases/tag/android%2Fgithub%2F1.5.6%2F19. Sorry for all the trouble and thank your for your great support as usual!

@IzzySoft
Copy link

Thanks – and it just went live with today's sync 🤩

@farfromrefug
Copy link
Contributor Author

@IzzySoft i have a little question for you. iOS users are asking me to release ipa file through github (thanks to the new european law). So to do that i would need to create new releases on github (all my apps) specially for iOS.
My question is would it be a problem for izzydroid if there was releases with only ipa and not apk?:

  • would your process ignore releases without apk?
  • do you need a special rule to ignore those releases tags?
    Thanks

@IzzySoft
Copy link

would your process ignore releases without apk?

It would log an error on each try – but yeah, it wouldn't "break" anything.

do you need a special rule to ignore those releases tags?

Well, that would be nice to avoid the error, yes. My guess is it won't even come to that as you most likely already plan what I suggest now:

AutoUpdateMode: Version ^android/github/[^/]+/%c$

is what's set up for Conty here currently. Your IPAs will most likely come with tags named matching the RegEx ^ios/github/.*, right? Those tags would be entirely ignored by the IoD updater (which only considers those matching the above RegEx). And before you ask: the %c stands for versionCode. So if the check finds a tag pointing to a versionCode not higher than what's already here, that would be ignored as well 😜

@farfromrefug
Copy link
Contributor Author

Well, that would be nice to avoid the error, yes. My guess is it won't even come to that as you most likely already plan what I suggest now:

AutoUpdateMode: Version ^android/github/[^/]+/%c$

is what's set up for Conty here currently. Your IPAs will most likely come with tags named matching the RegEx ^ios/github/.*, right? Those tags would be entirely ignored by the IoD updater (which only considers those matching the above RegEx). And before you ask: the %c stands for versionCode. So if the check finds a tag pointing to a versionCode not higher than what's already here, that would be ignored as well 😜

Yes that s the case i use ios in the tags and not android! So awesome! BTW will be the same for :
https://github.com/Akylas/OSS-DocumentScanner
https://github.com/Akylas/alpimaps/
https://github.com/Akylas/oss-weather/

@IzzySoft
Copy link

Ah, while we're here… fastlane. Could you maybe adjust the full_description.txt a little? At least using bullet-points (asterisks, i.e. *, no graphical dots) in "how it works")? Then it could be rendered as Markdown – which would look much better than as-is. Be welcome to the IzzyOnDroid Fastlane Documentation for further hints 😉

@farfromrefug
Copy link
Contributor Author

@IzzySoft should be better now.

@IzzySoft
Copy link

A final touch is still needed: current version would render fine as Github flavored Markdown, but not otherwise. Can you make sure the bullet-point lists are "separated" (i.e. have an empty line before and after)? In en-US that would currently mean inserting a newline before lines 10 and 4.

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

No branches or pull requests

2 participants