-
Notifications
You must be signed in to change notification settings - Fork 76
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
port to Android #90
port to Android #90
Conversation
Also, FYI, someone with admin access to this repo could allow GitLab CI to post the build results like Travis CI does. |
Thank you so much for this effort. Looks fantastic! @eighthave When I get some time I (or @arlolra ?) can look at this in more depth, but so far... 👍 💯 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should close out #32
No, this is just building on the work from #23. #32 is more about using the official binaries (as you were trying to do in https://trac.torproject.org/projects/tor/ticket/28205) but from a time before those really even existed.
And here is building the whole thing using the committed libwebrtc.a binaries
The binary is 96.9 MB, which is bigger than the others included here. Are symbols not being stripped? Any idea why that might be?
Also, FYI, someone with admin access to this repo could allow GitLab CI to post the build results like Travis CI does.
@keroserene is the only one with admin privileges because it's a personal repo. While we're admining, it would be great to get @uumaro push/pull rights here.
I don't know why the libwebrtc.a binary is so large. It could be because Android only provides libc, nothing else. So on Android, it has to link in a lot more libs. |
28ea767
to
bc5a8fe
Compare
One thing I do know is that the snowflake _client_ Android binaries that
include _libwebrtc.a_ are about 23MB.
Also, as far as I can tell, I addressed all the things in the GitHub
Review thing, but it seems to think something is still open.
|
23MB! Definitely need to package it as a standalone app/plugin for Orbot somehow. (Way to go @eighthave !) |
It looks like the difference is the presence of debugging symbols. Extract a sample object file:
The linux-arm one has no debugging while the android-arm one has:
Side note, neither one is stripped (nor are our other prebuilt libraries).
I would have thought that
|
I'd like the Android build to also be covered using the existing .travis.yml. But I don't know how to add it trivially, and I don't want it to block this pull request, so I created a new issue #91. |
Thanks @eighthave for providing all this. It overall looks good. The part I'm unsure of is the download of the Play Services SDK and automated acceptance of the licenses. My main concerns are:
https://bugs.chromium.org/p/webrtc/issues/detail?id=5578#c9
Like, maybe we can comment this out or something? |
What is here is very far from ideal, the Play Services issue is one of many shitty things. Play Services is definitely non-free. What I achieved here is close to two weeks of work. We're already over budget on this work, so this is where it is going to stand until someone else either does the work or finds the budget. Seems to me the ideal outcome would be to free the libwebrtc build process from the vast and horrible chromium build setup, e.g. go more towards the route that @infinity0 did in #32 My guess is that will require two weeks of work by itself. So for now, I think we need to consider the Android version non-free software, albeit with binaries that we are allowed distribute following the Play Services license. I haven't actually looked though, whether the libwebrtc binaries are free software. They might be, and it could just be the Chromium build system always pulls in Play Services since it also builds Chrome. FYI, I'm part of the Debian Android Tools Team. The Android SDK is a similar gianormous, ugly build like Chromium. We've build writing our own Makefiles for years now. It is a lot of work to maintain them, but after this experience, I think it might be less worth than wrestling the Google's chromium nightmare. |
Just for reference, here's what you get, including the Play Services licence agreement, when you run Click to expand (161 lines)
I cheekily tried responding
|
I had to answer
For me,
(I checked and the large size is due to it being full of debugging information as in #90 (comment), despite Anyway, my minor changes on top of eighthave's are at https://github.com/uumaro/go-webrtc/tree/android (currently uumaro@3bbd340). This is still a work in progress as I want to check out the license agreement thing some more. |
@uumaro : Invited you to have access to this repo. Thanks |
Right on, cheers 😄 |
@uumaro great that you got a working build! I'll incorporate your fixes in my merge request. As you can see in my flailing around the license prompt in .gitlab-ci.yml, the issue was about trying to figure out how to skip it in an automated way. If its possible to build libwebrtc without Play Services being installed, as it seems you did, then it seems feasible to upstream a patch to let that work with the upstream build system. |
Ok, this includes @uumaro's changes, with the binary for now, since it is huge. It seems that now it can at least be manually repeated, so it is not so important to have included in this pull request. Here are all the gitlab-ci runs, it is more or less the same results as before: |
I did two separate runs of $ diff -u <(cd webrtc-no-play && find . -name .git -prune -o -type f) <(cd webrtc-play && find . -name .git -prune -o -type f)
--- /dev/fd/63 2018-12-05 05:36:59.664083603 +0000
+++ /dev/fd/62 2018-12-05 05:36:59.664083603 +0000
@@ -60570,6 +60570,20 @@
./src/third_party/android_tools/sdk/extras/google/gcm/gcm-server/src/com/google/android/gcm/server/Message.java
./src/third_party/android_tools/sdk/extras/google/gcm/gcm-server/src/com/google/android/gcm/server/Constants.java
./src/third_party/android_tools/sdk/extras/google/gcm/gcm-server/src/com/google/android/gcm/server/InvalidRequestException.java
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-iid/11.2.0/play-services-iid-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-nearby/11.2.0/play-services-nearby-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-cast/11.2.0/play-services-cast-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-auth-base/11.2.0/play-services-auth-base-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-basement/11.2.0/play-services-basement-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-base/11.2.0/play-services-base-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-gcm/11.2.0/play-services-gcm-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-location/11.2.0/play-services-location-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-vision/11.2.0/play-services-vision-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-tasks/11.2.0/play-services-tasks-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-vision-common/11.2.0/play-services-vision-common-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms/play-services-auth/11.2.0/play-services-auth-11.2.0.aar
+./src/third_party/android_tools/sdk/extras/google/m2repository/google_play_services_library.zip.sha1
+./src/third_party/android_tools/sdk/extras/google/m2repository/LICENSE
./src/third_party/android_tools/sdk/platform-tools/etc1tool
./src/third_party/android_tools/sdk/platform-tools/NOTICE.txt
./src/third_party/android_tools/sdk/platform-tools/systrace/NOTICE So in uumaro@656dca2 in my android branch, I not only auto-decline the license agreement, but also delete the src/third_party/android_tools/sdk/extras/google/m2repository/com/google/android/gms for good measure, just in case it's been accidentally accepted in the same tree in the past. I added a best-effort attempt to check that the download directory looks how we expect it to look, so that the build will break in the case of a future reorganization that downloads the library into a different place.
I did some mild refactoring of build.sh to avoid the need for repeated |
My two issues with this port were (1) not to accept a proprietary license and (2) to reduce the size of the output library. Now that both of those are taken care of, is there anything else to do? @eighthave, does the library in uumaro@c7061bd work for your purposes (i.e. keroserene/snowflake#43)? I can take care of squashing eighthave's a my commits into a coherent history. |
@eighthave would I need to fork snowflake and go-webrtc to make all the necessary changes to point to the updated "master" repo with the magic binary? Or would this be better somehow built through your gitlab-ci work? |
well, I suck at Go, but I've good luck with symlinking for this kind of
thing. I would clone @uumaro's repo to your /go/src, then use symlinks
to make Go think that it is building github.com/keroserene/go-webrtc
|
I'm just looking for confirmation that uumaro@c7061bd works for @n8fr8 and @eighthave's purposes before merging this branch. Or @eighthave, did you want to edit .gitlab-ci.yml to match the procedure in build.sh? I can do it, but I can't test it unless there's an easy way for me to run a gitlab-ci job. |
It is very easy to set up your own fork on gitlab.com that will automatically run the gitlab CI jobs:
|
I rebased your uumaro/android branch on upstream's master and triggered a rebuild on gitlab-CI: Feel free to merge it as it is or rework it as needed |
Thanks, I started a fork at https://gitlab.com/uumaro/go-webrtc/tree/android and emailed you a token.
Thanks, this uncovered a bug in one of my changes that manifested when the third_party/webrtc directory did not exist yet. |
I happened to find a way to disable the license prompt. The environment variable In uumaro@9087925, I set |
@eighthave, did any of your gitlab CI runs complete successfully, all the way through the I'm just trying to figure out if |
I never got a full run of the libwebrtc build to complete successfully
in the CI, due to the license prompt and gclient issues. `gomobile
bind` here is just a test run anyway, the real `gomobile bind` happens
in snowflake.git, and that works provided there is a libwebrtc binary.
I think in theory, `gomobile bind` should work for go-webrtc also.
The issue in your last build seems to be this, which I have no idea about:
```
# github.com/keroserene/go-webrtc
In file included from ctestenums.cc:2:
./include/api/peerconnectioninterface.h:70:10: fatal error: 'memory'
file not found
rm -r -f "$WORK"
gomobile: go build -v -x -buildmode=c-shared
-o=/tmp/gomobile-work-646157724/android/src/main/jniLibs/armeabi-v7a/libgojni.so
gobind failed: exit status 2
```
|
No, ignore that error. That was me randomly trying to add Better look at this one instead: |
I don't know why it would fail to link, my guess it that it is some C++
namespace thing related to `__ndk1`. If I recall correctly, the Android
NDK offers 2 or 3 different C++ standard libs, with some options, e.g.
https://developer.android.com/ndk/guides/cpp-support
Then those config options need to be translated to `gomobile bind`
somehow. I guess it would make sense to double check if the config is
the same as the snowflake job, which works.
|
This omits timestamps and uids. It was the default on linux, but not the default using the arm-linux-androideabi-ar for Android, so my archive was changing every time I rebuilt.
This is to omit more debugging information and make the output library smaller on Android. It turns out that is_debug doesn't directly control the inclusion of debugging symbols, it just affects the logic for computing symbol_level. https://chromium.googlesource.com/chromium/src/build/+/99653454ee423cc5528426f9827dc24627f95c6a/config/compiler/compiler.gni#175 On Android, the default (with is_debug=false) is 1, while on linux and mac the default is 0. Here is where it is applied to actually set compiler flags: https://chromium.googlesource.com/chromium/src/build/+/99653454ee423cc5528426f9827dc24627f95c6a/config/compiler/BUILD.gn#2009 This change reduces the size of libwebrtc-android-arm-magic.a from 97M to 45M, and that of third_party/webrtc/src/out/Release/obj/libwebrtc.a from 77M to 38M. There is no change in libwebrtc-linux-amd64-magic.a nor libwebrtc-linux-arm-magic.a. Description from `gn --list=is_debug args out/Release`: symbol_level How many symbols to include in the build. This affects the performance of the build since the symbols are large and dealing with them is slow. 2 means regular build with symbols. 1 means minimal symbols, usually enough for backtraces only. Symbols with internal linkage (static functions or those in anonymous namespaces) may not appear when using this level. 0 means no symbols. -1 means auto-set according to debug/release and platform.
GOOS=android GOARCH=arm ./build.sh
I spent a few days trying to understand the failure of Anyway I've rebased and squashed my and @eighthave's changes that were in my android branch. It's at the point where I want it now. I'd appreciate any review before merging it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's at the point where I want it now. I'd appreciate any review before merging it.
The changes to build.sh
seem reasonable to me. Feel free to merge it if you're happy.
Super effort, all! |
This ports go-webrtc to Android. Most of that was getting a repeatable build process for the libwebrtc component and pointing the go code to that binary. You can find the details of this epic saga here:
https://trac.torproject.org/projects/tor/ticket/28205 #82
In the end, I also got the whole process running in GitLab CI, including building the libwebrtc binaries for GNU/Linux amd64 and ARM. This should close out #32 For example, here is building the libwebrtc.a binary part for Debian and Android (close but not quite working):
https://gitlab.com/eighthave/go-webrtc/pipelines/37556080
And here is building the whole thing using the committed libwebrtc.a binaries, this runs on Debian/stretch using the Go 1.10 and 1.11 binaries, then on Ubuntu/LTS, Ubuntu/cosmic, and Ubuntu/devel using the Go from Ubuntu:
https://gitlab.com/eighthave/go-webrtc/pipelines/37555959
Once this is merged, gitlab will automatically build all commits in this repo here:
https://gitlab.com/torproject/go-webrtc/pipelines
The process for building libwebrtc.a is pretty huge, so that runs just once a month via a schedule:
https://gitlab.com/torproject/go-webrtc/pipeline_schedules
@arlolra @n8fr8 @uniqx