vitorpamplona / amethyst

Nostr client for Android
MIT License
1.13k stars 155 forks source link

proprietary dependencies added with v0.11.4 #54

Closed IzzySoft closed 1 year ago

IzzySoft commented 1 year ago

Looks like today's release added a bunch of proprietary dependencies:

Offending libs:
---------------
* Firebase Data Transport (/com/google/android/datatransport): NonFreeNet
* Google Mobile Services (/com/google/android/gms): NonFreeDep
* Firebase (/com/google/firebase): NonFreeNet,NonFreeDep
* ML Kit (/com/google/mlkit): NonFreeDep,Tracking

4 offenders.

My guess is you've added MLKit, and the other 3 were dragged in by that. Now I don't know what to do:

So what do you think, @vitorpamplona?

vitorpamplona commented 1 year ago

Those are for the QR code scanning. I tried other ones but it wasn't really working well.

We need to search for a good way to scan since Google's default camera doesn't send nostr:// uris to Amethyst. :(

IzzySoft commented 1 year ago

Maybe look what other apps use? Conversations has QR Code scanning as well and is fully FOSS (they use zxing it seems), for example. I could check what other QR Code libraries my library definitions know about.

PS: I see the APK size also more than doubled, having reached 25 MB now. With a limit of 30 MB per app, my repo will only be able to keep 1 APK then.

IzzySoft commented 1 year ago

Apart from ZXing I could only find zBarLibary (Java) in my list.

IzzySoft commented 1 year ago

since Google's default camera doesn't send nostr:// uris to Amethyst.

That camera app isn't even installed on all devices. Maybe try SecScanQR and see what they are using. I'm using that scanner for many years now and am quite happy with it. It's of course available at F-Droid, so you could even recommend it to your users. They btw use Zxing (com.google.zxing:core:3.4.0).

IzzySoft commented 1 year ago

@vitorpamplona now the APK size reached 3 times the per-app size limit my repo allows (over 80 MB – the limit is 30 MB), so we have a real issue. Could you at least provide per-ABI builds so we get the size down? And ideally a build flavor without those non-free dependencies? I can close one eye, but now I'd need more eyes than I have. Per-ABI builds might get the size down to about 30..40 MB each I guess, so I'm already compromising on that. So if this cannot be solved, I'll unfortunately will have to remove Amethyst from my repo again – which I hope we can avoid.

IzzySoft commented 1 year ago

@vitorpamplona Not having heard back from you concerning APK size and per-ABI builds, but today's release again is 3 times the limit set for my repo – so it seems I have to remove Amethyst now. Please let me know once you've established per-ABI builds so I can see whether I can re-establish your app – ideally with the mentioned proprietary components gone as well. Thanks in advance!

vitorpamplona commented 1 year ago

Hi Izzy,

Sorry for the delay and thanks for hosting the app for a while. I am have not found equivalent libraries to replace proprietary dependencies yet. It's ok. We can re add to F-Droid when that happens.

IzzySoft commented 1 year ago

Hi Vitor, and thanks for the reply! Yes, the app is also easily re-established in my repo should you finally have per-ABI builds with APK sizes down to around 30 MB or maybe even less (hope dies last) – ideally without the proprietary components or at least a reduced set of them so Tracking can be removed – looks a bit strange on an app like this.

If you cannot find a proper replacement for MLKit, an option might be to go a similar path as StreetComplete did with their StreetMeasure component (which requires Google AR), putting it into a separate app? Amethyst started without it, so it doesn't seem a component it cannot do without (but can be "loaded optionally" by those who think they need it)? That would then also contribute to the size issue: per-ABI APKs without MLKit might be well below 30 MB each again. Separating the two would probably also make Amethyst meeting F-Droid's inclusion criteria; I could then help you getting it there, too (disclosure: I'm one of the maintainers there).

I just try encouraging you here, please do not consider it a complaint :smile:

maxmoney21m commented 1 year ago

@IzzySoft I have opened #249 to use zxing and #258 which removes pretty big chunk of images that aren't even being used right now. With those 2, split abis per platform, and minification, we should be under 30 MB for one APK.

Is that enough to get back on F Droid or is the translation stuff going to be a problem still? It's Google but it's all on device AFAIU.

IzzySoft commented 1 year ago

Thanks @maxmoney21m! I cannot tell for sure before I've scanned the resulting APK, but I'm confident it will be fine for my repo. For F-Droid.org we'll need to have a much closer look as inclusion criteria there are more strict. Would it be OK to leave that question hanging until we can tell (i.e. the two referenced issues are solved)? I gladly assist with getting Amethyst to "F-Droid proper" then (disclosure: I'm also one of the maintainers there).

maxmoney21m commented 1 year ago

Cool, let's see what @vitorpamplona wants to do.

maxmoney21m commented 1 year ago

@IzzySoft can you check if 0.25.0 is OK? @vitorpamplona he will need the abi builds though, they should be around 30 MB now.

Please let us know what else we need to do for F Droid proper. I'd like to use it from F Droid myself, so I'll work on the changes required.

IzzySoft commented 1 year ago

can you check if 0.25.0 is OK?

On it now. 0.25.1 is NOT:

Offending libs:
---------------
* Firebase Data Transport (/com/google/android/datatransport): NonFreeNet
* Google Mobile Services (/com/google/android/gms): NonFreeDep
* Firebase (/com/google/firebase): NonFreeNet,NonFreeDep
* ML Kit (/com/google/mlkit): NonFreeDep,Tracking

4 offenders.

so still the same as above. As you've explicitly asked for 0.25.0 I've checked that, too – same results. MLKit is still in there, dragging in the others along.

Please let us know what else we need to do for F Droid proper.

We can check that once this issue has been solved (makes no sense before). When we reached that poiunt I'd suggest opening an RFP to have the entire issuebot module set run over it (my library scanner is just one of those).

maxmoney21m commented 1 year ago

Ok, as I suspected this is about the translation models, not just the QR camera. I will start working on a flavor to remove them.

IzzySoft commented 1 year ago

As long as MLKit is in, the problem will persist: MLKit itself is non-free.

maxmoney21m commented 1 year ago

@IzzySoft @vitorpamplona how can we get you an APK built using #276 to test before vitor pulls it in? There's no point in pulling or releasing it if it does not meet the requirements to solve this issue.

IzzySoft commented 1 year ago

Can you attach it here (optionally removing it later) – or link to the corresponding artifact? Or maybe you wish to test it yourself? My library scanner is used by multiple projects (e.g. F-Droid uses it in its IssueBot, and I know of a few projects running it in their release pipelines to make sure nothing "slipped in"). If you're interested in that, find description and instructions here: Identify modules in apps. It's FOSS, so "no strings attached" :wink:

vitorpamplona commented 1 year ago

Scripts seem to be working! https://github.com/vitorpamplona/amethyst/releases/tag/v0.25.3

IzzySoft commented 1 year ago

And even the universal APK is below 30 MB! Not much reason to go "split" with that little difference (especially as the ARM variants are missing :see_no_evil:), congrats! How did you get it that small again? Looks like a "reverse" of when you added MLKit, so was it just removing that? I see the Google variant still has 80+ MB. So double-win with that!

Oh, and congrats², my scanner says:

No offending libs found.

So I'm no re-establishing Amethyst in my repo, with all anti-features removed. As metadata was just archived, that took me an eye-blink – automation took care of the rest (refreshing from fastlane etc). Next sync at around 7 pm UTC will see it back :partying_face:

Next station: F-Droid.org I guess? If I can assist, let me know. Amethyst then will be the first Nostr client at F-Droid :smiley:

IzzySoft commented 1 year ago

PS: I see the Playstore listing has that 512x250px purple image with birds, which would well fit as fastlane/metadata/android/en-US/images/featureGraphic.jpg if you wish. Would show as "title banner" (background) on the app's detail page in the F-Droid client then. I've added it here already, so you can take a look for yourself tomorrow then :wink:

vitorpamplona commented 1 year ago

Yep, this flavor doesn't have any translation at all. So, fully de-googled. :)

How do we get into F-Droid.org?

IzzySoft commented 1 year ago

Usually you open an RFP (Request For Packaging) to get started (initial scan and all that). Should go pretty straight as we covered most of it already – though at that stage, IssueBot will complain about MLKit etc. as it cannot distinguish the flavors, so don't be surprised about that. Next step then would be a merge request at fdroiddata. I can see to set that up tomorrow; as your app is written in Kotlin even I as a non-dev can do that, flavor and all (I hope at least :see_no_evil:). I'd then link here for "author does not oppose inclusion".

IzzySoft commented 1 year ago

Now cross your fingers, press your thumbs, or what other actions you prefer: here you go, engines running…

Update 1: Ignore the failed "schema validation", fixed already. Just holding back pushing the changes until the build is through so I can get the full picture. Same for lint.

Update 2: Build succeeded, then failed due to multiple APKs :see_no_evil:

2023-03-14 23:15:57,497 INFO: Successfully built version 0.26.0 of com.vitorpamplona.amethyst from f8bb4a3168b2adfdec7286aaf3d35daf63a05f71
2023-03-14 23:15:58,092 ERROR: Could not build app com.vitorpamplona.amethyst: More than one resulting apks found in build/com.vitorpamplona.amethyst/app/build/outputs/apk/fdroid/release

Fixed that as well (pinning it to the "universal"). Pipelines running again, in 20 minutes we know more.

Update 3: You're in a release marathon, huh? Ignore the CheckUpdates error. While I still try getting 0.26.0 to succeed, you've released 0.26.1. Will take care for that later. Lint & Schema-Validator succeeded this time, now waiting for the build to see if we can go for the "supreme discipline" and enable reproducible builds, so F-Droid can ship the APK you've built and signed.

IzzySoft commented 1 year ago

OK, builds fine – and we are just some millimeters away from reproducible builds. Could you please check with sort baseline.profm in build.gradle using com.android.tools.profgen and implement that here? Additional hints in my comment on the MR. Thanks!

maxmoney21m commented 1 year ago

@IzzySoft I think 0.26.2 has the changes?

IzzySoft commented 1 year ago

My eager team mate already updated my MR accordingly :see_no_evil: CI is currently running. :crossed_fingers:

IzzySoft commented 1 year ago

Yay, reproducible build succeeded! :partying_face: Now I need to ask something for testing the APK. Can you give me a quick start – like how to set up the keys and what I need to configure, plus maybe even how to contact one of you two via Amethyst? Then I'd see to do the review soon™, so we can merge and get Amethyst in.

maxmoney21m commented 1 year ago

@IzzySoft

  1. Open the app
  2. Accept terms and generate a key
  3. Go to the side menu (drawer) and hit "backup keys" to back up your private key
  4. Go to your profile and click the edit button to change any info you'd like
  5. You can message someone by searching for their name or npub (Search is currently on the Global icon page). i.e. search for "vitor pamplona" or npub1gcxzte5zlkncx26j68ez60fzkvtkm9e0vrwdcvsjakxf9mu9qewqlfnj5z. I'm maxmoney21m or npub1max2lm5977tkj4zc28djq25g2muzmjgh2jqf83mq7vy539hfs7eqgec4et.
IzzySoft commented 1 year ago

OK, it's back online again! And after a nice cliff-hanger this morning, I just announced it now. Next: The review. On it. Was distracted by the day job, apologies :see_no_evil:

IzzySoft commented 1 year ago

First hint: Please take a look at the SAST report right below the initial comment of the MR. It has some "high risk" entries:

image

For cross-checking, I'm sure Pithus will reflect that once it has completed its scan.

vitorpamplona commented 1 year ago

I have no idea where that's coming from... hum...

IzzySoft commented 1 year ago

Nor do I. It's not a stopper unless you say so – after all its "potential" issues. Guess it's OK when I publish my review for additional details – except for those SAST findings (which are already public anyway) it's rather positive.

Btw: did you get my messages on Nostr itself (via Amethyst)? I've pinged you and Max but got no "pong". One open question from my end is what's that with the "flash icon" (asking me for 500. 1k or 5k flashes, Volt … coin? "Zeps"?)

IzzySoft commented 1 year ago

PS: The fdroid build still asks the CAMERA permission. Maybe I'm blind, but I could find no button for that (QR I guess to confirm a contact). As MLKit was removed, was QR replaced by something? Similarly: Biometrics?

vitorpamplona commented 1 year ago

There is still QR Scanning with Zxing. Bottom button in the drawer

IzzySoft commented 1 year ago

Bottom button in the drawer

Ah, there – that shows my QR. But how'd you scan it with Amethyst if I showed you mine?

vitorpamplona commented 1 year ago

Can you see the Scan QR Button in the bottom of the QR Code itself?

IzzySoft commented 1 year ago

No, but knowing where it should be I found it. Must be a display issue: I see a small bar at the bottom there, tapping that opens the camera:

2023-03-15_22 45 59

vitorpamplona commented 1 year ago

Wow.. which phone is this? Super squared aspect ratio :)

It's the purple button that is cropped :)

IzzySoft commented 1 year ago

Yupp, that's what I figured by playing around, knowing it must be "somewhere there". Phone as mentioned in the review: My good old (2016) Wileyfox Swift, running Android 10 thanks to LineageOS (display resolution: 720x1280). I always test on older devices to make sure it works there as well. And I always use my devices until they fall apart; a new one usually isn't due for 5 years approximately :see_no_evil: Sustainibility. Why change if it works :wink:

IzzySoft commented 1 year ago

PS: Biometrics? The Fox has no fingerprint sensor, maybe that's the reason I see nothing? But if there's Biometrics, I'd expect it to unlock. I see nothing to set up protection.

vitorpamplona commented 1 year ago

Frankly, I don't even know how your phone has the processing power and memory to even load the number of messages we are loading.. It must be really slow.

Biometric is used to export the nsec. But only in phones that support it.

IzzySoft commented 1 year ago

I don't even know how your phone has the processing power and memory

image image

(from a report generated by Adebar. For more details: Wileyfox Swift - Specifications

IzzySoft commented 1 year ago

OK, no vetoes, so let's merge. And yes:

It must be really slow.

It was not right after install. But running for a couple of ours, it now got really slow and I received a bunch of ANRs. Uninstalled it from the test device, so I currently have no active account anymore – but work is done. Thanks for your great support, Vitor – and maybe some of the hints in my review gave you the one or other idea, too (last thing I saw in Amethyst was your message about "not wife friendly" and "onboarding process" for brand new users, that could reflect some of my thoughts. Once I grasped the idea I found my way – but that needed your help for some things :wink:

Now, welcome aboard! If you wish I can keep Amethyst in my repo even when it showed up at F-Droid (which is when I usually remove apps, given a decent overlap for users to switch). Thanks to reproducible builds existing users can easily switch over (probably would without noticing) – but for the same reason you might wish it to stay, as your release cycle is definitely fasterr than F-Droid's build cycle. Just let me know if you'ld like it that way.

PS: "F-Droid doesn't like Google translate" – that's right, and for good reasons (using that by default definitely would have caused NonFreeNet). But take a look at translation apps at F-Droid, there are FOSS alternatives supporting many languages. Maybe you'd like some of them, e.g. LibreTranslate. Without Google and without Tracking™ by Google. And users could even choose the server they like – which would fit a Nostr app much better than a centralized service, right? :wink:

maxmoney21m commented 1 year ago

Great stuff, thanks @IzzySoft. Looks like the QR screen needs to be scrollable for devices like yours. Also the key export should work with device unlock if fingerprint is unavailable, not sure if you resolved that one already.

IzzySoft commented 1 year ago

Thanks Max! The test device has no FP scanner, so that part is already clarified. And yes, scrollable would help (I indeed intuitively tried to scroll there).

I see the merge is through but checkupdates failed. Guess you just made another release then :rofl: Please let me know in time whether you want me to keep the app in my repo (background outlined in my previous comment). Default is to remove it about 2 weeks after it showing up at F-Droid. No prob if that happens – can be added back easily when needed, as metadata will be archived.

maxmoney21m commented 1 year ago

How many people are using it from your repo already? I don't really have an opinion on that, and also not really my call, but unless Vitor wants to do something different I'd say just do whatever the default is. 2 week overlap then just at FDroid is probably fine.

IzzySoft commented 1 year ago

How many people are using it from your repo already?

I cannot answer that as there's no tracking. At best I could check the web server logs on how often it was downloaded from there.

unless Vitor wants to do something different I'd say just do whatever the default is.

Fine with me, I just give the options. Usually I "just do it" unless explicitly requested; there are few cases where I put out the offer explicitly. In your case due to the release frequency F-Droid won't be able to keep up with (2 releases per week is the best F-Droid can keep up with normally; it's not often that there are more than 2-3 build cycles to complete within a week, including signing and all).

OK, let's wait for Vitor's "final words" then.

vitorpamplona commented 1 year ago

Thanks Izzy! 2 weeks should be more than enough.

So, do we need to slow down our release cycle for fdroid? We have been doing 1-2 releases a day...

IzzySoft commented 1 year ago

So, do we need to slow down our release cycle for fdroid.

No. You can keep that up if you wish. Even my repo would have only picked up one per day, F-Droid will probably pick up 2 per week – but "version junkies" can pick up one per hour directly from here if you wish to offer that :see_no_evil:

2 weeks should be more than enough.

OK, will use the default then. Should you need it back, just let me know and I have it re-established quickly. Thanks to reproducible builds switching should be no issues for existing users. All the best then!

vitorpamplona commented 1 year ago

Shouldnt Amethyst be findable in the F-Droid page by now? What am I missing?