streetcomplete / StreetComplete

Easy to use OpenStreetMap editor for Android
https://streetcomplete.app
GNU General Public License v3.0
3.83k stars 348 forks source link

non-free dependency: Google AR Core #4289

Closed IzzySoft closed 1 year ago

IzzySoft commented 2 years ago

I've just scanned the APK built by F-Droid, and see:

Offending libs:
---------------
* Google ARCode (/com/google/ar/core): NonFreeDep

1 offenders.

This would mean all affected builds need to be disabled, as that's a no-go for F-Droid. So the questions are:

Being a StreetComplete user myself, and just having made a big announcement at Mastodon how useful the app is, I'd hate seeing it disabled at F-Droid – so I really, really hope you can find a way to mitigate this.

licaon-kter commented 2 years ago

@westnordost How would it work exactly? Pop-up? Option?

mnalis commented 2 years ago

Is the compiled StreetComplete.apk really GPLv3-licensed?

Yes. That is the nature of copyleft / viral licenses like GPL3 or CC-BY-SA. Linked wikipedia articles explain it in layman terms as "[...] which requires that any derivative work also be licensed under compatible licenses with the GPL". The actual term to search for in GPLv3 legalese is "covered works" (and act of conveying it).

Uff, I didn't know that. I was under the impression that GPL would only be viral in that if a GPL licensed piece of code is used (e.g. a library), the (binary) distribution as a whole must also be GPL, not the other way round (e.g. it is not enough if all the code owned by the project is GPL, also all dependencies must be compatible to GPL).

If you think it about it, it would be trivial to circumvent GPL copyleft protections if it mattered which part is a library and which is a main program (e.g. someone wishing to add GPL library in their non-free program, could simply make their proprietary program a library instead, and then make trivial GPL "main program" which consists only of calling both free and non-free libraries). So strong copyleft licenses like GPL prohibit distributing a mix a free software (open source) with non-free software (closed source) - no matter the amount of free or non-free code or their location ("main program" or "library").

There are other class of free software licenses (jointly called permissive licenses) like mentioned Apache license which do not guarantee copyleft protections (i.e. while the code itself is free/open source it may be mixed with non-free code are resulting binary may be non-free/closed-source, including a non-free fork of library like famous Embrace,Extend,Extinguish tactics). And then there are licenses like LGPL which are in between.

So, to simplify:

westnordost commented 2 years ago

@licaon-kter Amongst many other things, the app may ask about what is the width of some road. When answering this, an input field to input the width is displayed. Next to this input field, there is a "measure with AR" button. Clicking this button will either open the AR measurement view if ARCore is installed or ask to install it if it is not. After AR measurement is complete, whatever was the result is pre-filled into the input field.

westnordost commented 2 years ago

@mnalis are there any licenses that enforce a "share alike" but only for the code owned by that project? (E.g. permissive licenses do not enforce share alike)

mnalis commented 2 years ago

@mnalis are there any licenses that enforce a "share alike" but only for the code owned by that project?

Yes, the GNU Lesser GPL (LGPL) that I just mentioned. But it depends on what exactly you want to use it for (and how tolerable one is to losing some of GPL protections). It is mostly used only for libraries, but there are exceptions IIRC.

mnalis commented 2 years ago

Wouldn't it be the same already even if the Android ARCore library was open source? After all, it "promotes" (i.e. doesn't work without) the "Google Play Services for AR" APK and even offers a short-cut to install it AFAIK.

Non-Free Addons antifeature says "This Anti-Feature is applied to apps that, although Free Software themselves, promote other Non-Free applications or plugins." While it is f-droid discretion to interpret it, it would look to me:

If it comes to creating a addon, I myself would prefer former, even if it means adding a new FAQ entry (and mention in release notes) and explaining to few confused users "where have AR measurements disappeared".

IzzySoft commented 2 years ago

@mnalis this exactly matches my understanding. I'd even go a little step further and say if there were a link from the app to "how to use the measurement feature" which goes to the wiki/FAQ, and the latter then (next to how to do it without) a.o. mentions one could also use that addon (or alternativ app flavor), the anti-feature would not apply as the app itself is not doing any promotion. Might be hard at the border, but IMHO at the "good side" of it.

mnalis commented 2 years ago

Also, according to https://github.com/google-ar/arcore-android-sdk/blob/master/README.md it seems that "By downloading the ARCore SDK for Android, you agree that the ARCore Additional Terms of Service governs your use thereof."

And those seems to require (among other things - as there are several sub-documents linked in which I have not yet looked at!) updating privacy policy (of the new addon or the main SC app, depending how the situation turns out) with extra cruft like:

The terms of service for your API Client (e.g., your app) will (1) notify end users that the API Client includes ARCore functionality provided by Google; and (2) state that use of ARCore functionality is subject to the then-current versions of the: (A) Google Terms of Service at policies.google.com/terms; and (B) Google Privacy Policy at google.com/policies/privacy.

I've starting to think becoming convinced that even if they do decide to release the full source, it would still be prudent to at least remove all ARcore functionality to separate SC addon (companion app) (instead of forcing all that google baggage on all unsuspecting SC users - i.e. if it remains in the main SC app we can kiss goodbye that nice SC privacy preamble "Ah, someone who cares about privacy, nice! I think I have only good news for you..." :cry: )

mnalis commented 2 years ago

FYI I plan to defer working on this to re-enable F-Droid builds because I like to first wait a bit to see if the Google folks at arcore-android-sdk have a change of heart. It doesn't seem to be a big team, maybe they can make such decisions on their own measure.

It has been a month since it has been clarified that ARcore library is indeed incompatible with GPL, with no further comments from Google side since then. It thus seems unlikely to me that "maybe they can make such decisions on their own measure" and we should probably start considering how to address that for future releases, i.e.:

  1. If eventually they respond that they do not intend / will not consider to make this open source, I will create a separate Apache-licensed app and publish it to the app store that contains all the code for the measuring (on the upside: People could then use that app to also measure things independently of StreetComplete). StreetComplete will then simply ask the user to download that app from the play store or github when it is not installed and the user taps on the "measure with AR" button

However, making a separate app will likely take some time, and f-droid is stuck on quite old 40.2, which already confuses users (e.g. https://github.com/streetcomplete/StreetComplete/issues/4379). It would make sense to do a patch that removes ARcore support from f-droid build so F-DROID can continue to get updated in the meantime. Would that be acceptable interim solution @westnordost? Or do you foresee better short term solution?

westnordost commented 2 years ago

As a short term solution, this would be acceptable. It should be done in one clean PR so it can be reverted once the full solution is done (which is either simply revert because Google made it properly open source or which is creating a new non-GPL app that does the measuring and report back to StreetComplete)

mnalis commented 1 year ago

I did a remove-arcore-nonfree branch that removes com.google.ar* and related stuff based on latest v47.0-beta1 (which happens to be master), and when testing it seems to work fine, i.e. does not show AR icon and collects manually entered values normally. Hopefully I didn't forget anything.

@IzzySoft do you know how to correctly push that patch to f-droid so it can publish new version when it arrives?

westnordost commented 1 year ago

Are then the quests always disabled by default? The quests should be disabled by default if no AR measure is available (because you need to have a measuring tool with you otherwise to solve the quest)

westnordost commented 1 year ago

It would be fine to push this to a branch on StreetComplete, e.g. "no-arcore" too. However, not sure how the releases with F-Droid should happen then - currently it is done via tags (e.g. v47.0-beta1). But the tags will continue to be set on master, not on no-arcore. So what's the best solution here?

TheLastProject commented 1 year ago

Most Android developers tend to solve these things using "gradle flavors", so it's all in one codebase. F-Droid can build the fdroid/foss/whatever flavor then.

However, if you don't want to mess with gradle flavours you could maybe just each release:

  1. Rebase/merge new code into no-arcode branch
  2. Create a v47.0-beta1-fdroid tag (with the appropriate number)

(Personally, I would always re-base so the no-arcode patch is always the patch "on top" of master).

Then F-Droid can just change the metadata rules so it only picks up the -fdroid tags.

If that's all too inconvenient, F-Droid can also apply a patch during build. See https://gitlab.com/fdroid/fdroiddata/-/blob/bc5bc6c329f6b3d72bc79086ab028648018dec2c/metadata/im.vector.app.yml#L943-944 for example. In such a case the patch file updating would have to go through the F-Droid team as it's in fdroiddata then, so this is the slowest option (if it's not updated on time the build will fail and updates need to go through the F-Droid team).

In the end it all comes down to your preference I think, if you're only planning on removing arcore from StreetComplete for like one or two releases, the fdroiddata patch method might be easiest. But for long-term it can get quite frustrating.

mnalis commented 1 year ago

Are then the quests always disabled by default? The quests should be disabled by default if no AR measure is available (because you need to have a measuring tool with you otherwise to solve the quest)

Yes, quests are disabled by default - the patch basically just makes ArSupportChecker() return false (and removes all offending imports and affected code), and so the SC then behaves as if the phone does not have AR support (so it disables quests by default and does not show AR button in them).

So what's the best solution here?

Ah, I have no experience there, but as @TheLastProject notes, and since we plan to make AR functionality a plugin anyway in relatively near future (or even Google might decide to open source it), the f-droid patch solution would seem simplest to me. Thanks to @TheLastProject example, I could make and submit such MR to fdroiddata, if all agree that is the best approach?

westnordost commented 1 year ago

For the F-Droid patch solution: What happens if one of the files the patch is applied to is changed? The files that this patch touches are not likely being touched except the app/build.gradle.kts which is changed for every release (but at a different position)

TheLastProject commented 1 year ago

They're regular git patches. Git can do some small resolving and generally doesn't mind if unrelated sections get changed (as you can see, I can re-use the Element patches for several versions), but it's really dependent on git itself.

In the patch no longer applies cleanly, git will throw an error and the build will fail until the patch is fixed by someone.

westnordost commented 1 year ago

I think then the solution to post the patch at F-Droid is the easier solution because of the relative isolation of the code and no necessity to rebase+add a fdroid specific tag to a branch on each release

mnalis commented 1 year ago

OK, I've done a fdroiddata merge request here: https://gitlab.com/fdroid/fdroiddata/-/merge_requests/11785

@TheLastProject you seem to have experience with the process, if you have a minute, could you take a look and say if that looks good to you? I've tested that the patch applies against v46.0, v46.1, v47.0-alpha1 and v47.0-beta1; also CI/CD for that MR on GitLab seem to have passed OK.

I've updated added a new block for v46.0 with a patch, do you know will f-droid automatically realize it should also use that patch for v46.1, v47.0 and newer when it tries to add them? Or what would be the procedure for the future versions?

TheLastProject commented 1 year ago

I see linsui responded to the MR already, they are a lot more active in fdroiddata than I am so I'd recommend listening to them over me :)

I've updated added a new block for v46.0 with a patch, do you know will f-droid automatically realize it should also use that patch for v46.1, v47.0 and newer when it tries to add them? Or what would be the procedure for the future versions?

Well, right now update checking is disabled. But if all is good it can be re-enabled. With Auto update checking enabled, F-Droid copies the build instructions of the previous version so it'll automatically try the exact same patches for new releases too.

Talking about new releases, is there any reason v46.0 is being added to F-Droid instead of v46.1, given v46.1 seems to be the latest stable?

mnalis commented 1 year ago

I see linsui responded to the MR already, they are a lot more active in fdroiddata than I am so I'd recommend listening to them over me :)

Yup, I'll have to do more research there where to put extra commnads. Your method of just providing patch seemed simpler to me than patch + rm + sed proposed in MR, but I can see how the later would be more robust...

Well, right now update checking is disabled. But if all is good it can be re-enabled. With Auto update checking enabled, F-Droid copies the build instructions of the previous version so it'll automatically try the exact same patches for new releases too.

That is good news! Is that auto checking something I myself can enable in a patch (I see there is field called AutoUpdateMode: None, but it has that value in your example too, so I suspect that is not it), or is it more of fdroiddata admin thing?

Talking about new releases, is there any reason v46.0 is being added to F-Droid instead of v46.1, given v46.1 seems to be the latest stable?

well, v46.0 was the first where the patch applied cleanly (it fails on v45.x and lower), so I though to go with that, and then see if autoupdate code picks it up for v46.1 automatically.

TheLastProject commented 1 year ago

That is good news! Is that auto checking something I myself can enable in a patch (I see there is field called AutoUpdateMode: None, but it has that value in your example too, so I suspect that is not it), or is it more of fdroiddata admin thing?

I've added a suggestion on the MR, let's see what linsui thinks :)

well, v46.0 was the first where the patch applied cleanly (it fails on v45.x and lower), so I though to go with that, and then see if autoupdate code picks it up for v46.1 automatically.

I'm mostly wondering because it seems a bit weird to me to go this route unless there's a reason someone would want to explicitly run v46.0 and not v46.1.

Worst case scenario, the next build cycle starts after the merge but before the next checkupdates, which means users will first get an update to v46.0 and then a few days later an update to v46.1, which would mean users would first update to a version that has some known bugs already fixed in v46.1 and they'd be bothered with a new update again real quickly. Given each update is 56MB (looking at historic data), that could be a bit harsh on users with bandwidth caps with no real gain as far as I can tell.

mnalis commented 1 year ago

I've added a suggestion on the MR, let's see what linsui thinks :)

thanks!

I'm mostly wondering because it seems a bit weird to me to go this route unless there's a reason someone would want to explicitly run v46.0 and not v46.1.

People might want to try to pinpoint in what version something broke... But yeah, it is a little thin for a reason :smiley: So I've updated MR to reference v46.1 now.

IzzySoft commented 1 year ago

@mnalis apologies I didn't respond to your ping in time (I was on vacation) – luckily Silvia (@TheLastProject) jumped in with the same track I'd have proposed. Glad a way was found to get SC updated at F-Droid again, looking forward to the update! I became quite addicted to the app I have to admit and am quite proud of my ranking :see_no_evil: Cyprus needed a lot of SC-love, and it received it during the past 2 weeks :star_struck:

riQQ commented 1 year ago

StreetComplete 46.1 without AR core is available on F-Droid as of yesterday, September 29th. Thanks @mnalis

opk12 commented 1 year ago

licaon-kter: so that makes this a question of policy rather than a question of necessity. You do not want to have any apps in F-Droid whose dependencies you could not theoretically build from source regardless of whether they are licensed with an open source license or not.

No, the source is necessary to ensure that the binary is malware-free, by public scrutiny and reproducible builds (see https://f-droid.org/docs/Reproducible_Builds/ ): when independent builds give a byte-per-byte-identical result, you do not have to trust the build machine, kids, roommates and random guests of whoever submitted each lib binary. That these security hygiene concepts do not exist in Google Play, Windows and generally compulsory education / public legislation is a shame, not something that should be encouraged. This can't be done through decompiling, as it won't help with native code and R8-minified stuff, and is illegal unless allowed in the license.

FloEdelmann commented 1 year ago

Version 47.0 is now available on F-Droid, so everything is working as expected again. Thanks to everyone involved fixing this!

IzzySoft commented 1 year ago

Thanks a lot for all the efforts all of you have put into this. Very much appreciated, and makes me love (and recommend) the app even more! :heart_eyes:

mnalis commented 1 year ago

@FloEdelmann although, if we closed this issue, we should probably open another issue linked to this one - while F-droid builds now work, the issue with ARcore is not fixed - SC is still in copyright breach on Google Play (and GitHub Releases page) and a final solution (either Google actually opensourcing the lib, or the ARcode being moved to separate companion app + f-droid patch being removed) is still pending.

See this comment for example: https://github.com/streetcomplete/StreetComplete/issues/4289#issuecomment-1225586563

westnordost commented 1 year ago

The Google ARCore team seems to have finally done something about the open license issue by clarifying in the LICENSE.md that indeed the library is not open source (but only some related files in that repo are open source):

https://github.com/google-ar/arcore-android-sdk/commit/6f887e14366c5737621f91e3d8e29f520016e25d#diff-c693279643b8cd5d248172d9c22cb7cf4ed163a3c98c8a3f69c2717edd3eacb7R11-R13

So, with the certainty now here, it is time to remove ARCore from StreetComplete.

mnalis commented 1 year ago

Just a short note: before StreetComplete version with removed ARCore is released, we should also undo f-droid MR 11785, so F-droid builds can continue to build new version normally too (as it will take some time for new MR to be accepted there).

I can handle that f-droid part again (if you prefer), when the patch removing ARcore lands in StreetComplete master branch.

westnordost commented 1 year ago

Done now. For v51.0 onwards, the f-droid MR 11785 can be undone. (Note: I do not feel responsible for this because I amm also not the author of MR 11785, someone else will have to do that)

IzzySoft commented 1 year ago

Thanks Tobias! I've forwarded your message to the corresponding MR, asking my two colleagues who were working on it to take care (and linking them here).

IzzySoft commented 1 year ago

It's done. Thanks again for all your efforts, Tobias!