Update Firefox RBM config and build for Android
Update Firefox RBM config and build for Android
Activity
commit [patch for stretch]: https://github.com/sisbell/tor-browser-build/commit/757e922ee369e3c244e71c72ef0a11ffe8add794
commit [config/build] : https://github.com/sisbell/tor-browser-build/commit/1b9052d6f98d1ffe8bc3c9108302750ca036a075
Replying to sisbell:
commit [config/build] : https://github.com/sisbell/tor-browser-build/commit/1b9052d6f98d1ffe8bc3c9108302750ca036a075
Some comments about this commit:
- setup of android-toolchain should be done in
var/setupdefined inprojects/android-toolchain/config. - it should not be needed to list
android-toolchainas dependency if it is set asvar/compiler. - ndk.zip should be downloaded as part of
android-toolchaininstead of inprojects/firefox/config - why is there an
exit 0after the./mach packageline?
Trac:
Keywords: TorBrowserTeam201809R deleted, TorBrowserTeam201809 added
Status: needs_review to needs_revision- setup of android-toolchain should be done in
android-toolchain now handles the ndk.zip download. I still need to remove the ndk file entry from the firefox config. Getting firefox project working with the latest android-toolchain changes is next on my list.
The exit 0 is there since mach package builds the apk, which I believe is the final packaging we want. It didn't seem the Mozilla Archive that follows is relevant to Android.
-
Replying to sisbell:
The exit 0 is there since mach package builds the apk, which I believe is the final packaging we want. It didn't seem the Mozilla Archive that follows is relevant to Android.
I think that
./mach packagewill create the apk in the build directory. However we exit immediately after that, so it seems to me we are just losing the result of the build after it finished. I think we are missing something to move the apk file, and probably the martools, to the destination directory. I made some additional fixes.
-
Build packages the android dist directory.
-
No more exit 0 after build package.
-
Uses a tarball of android archives so there is no network connection needed during build.
-
Uses android-ndk packaged compilers.
Some additional minor cleanup items left.
-
What to include in dist package. Just apks?
-
Use a new var to indicate if mar tools should be built (rather than platform var)
-
-
Replying to sisbell:
I made some additional fixes.
- Could you push your new revisions to new branches instead of force-pushing on the same branch? This makes it harder to see what was changed since the previous revision.
- Could you also rebase it on current master? It is currently based on a commit that is more than 2 months old.
- I am not sure I understand what those lines are doing. This seems to be creating a
gcc -> gccsymlink?
+[% IF c('var/android') %] + ln -s gcc $ANDROID_NDK_HOME/arm/bin/gcc +[% END -%]- could you add a comment explaining why we need the
projects/firefox/gradle.patch? Maybe this is something we want to patch intor-browser.gitdirectly? - there is a trailing whitespace in
projects/firefox/mozconfig-android-armv7at the end of the--with-android-sdk=line - do we really need our own build of binutils for the android build of firefox?
- commit
6af47ae272b6098eac1c9ba886ff9ab04e3fc441seems to have the wrong bug number - could you explain why we need
projects/firefox/android.patch? It seems to be removing some code behind#if defined(GP_OS_linux)and forcing use of the code previously behind#elif defined(GP_OS_android). Shouldn'tdefined(GP_OS_linux)be false anddefined(GP_OS_android)be true in the android build?
Some additional minor cleanup items left.
- What to include in dist package. Just apks?
I think
projects/firefox/buildcan just copy the generated apk files, for now. Later we will probably need to repackage them inprojects/tor-browser/buildto include other components such as tor and Orbot, but this can be done later.- Use a new var to indicate if mar tools should be built (rather than platform var)
Don't we want to build mar files for android too? Or is the firefox updater based on mar files not working on android?
Replying to boklm:
Replying to sisbell:
- Use a new var to indicate if mar tools should be built (rather than platform var)
Don't we want to build mar files for android too? Or is the firefox updater based on mar files not working on android?
Using the Firefox updater for mobile is pretty much untested. I think we want to do that at some point for bundles we distribute on our website but it's okay to start without it.
-
I think those comments are still valid for commit
2276a44b14edf82f0f44ac98c50597a8637e5883:- could you add a comment explaining why we need the
projects/firefox/gradle.patch? Maybe this is something we want to patch intor-browser.gitdirectly? - could you explain why we need
projects/firefox/android.patch? It seems to be removing some code behind#if defined(GP_OS_linux)and forcing use of the code previously behind#elif defined(GP_OS_android). Shouldn'tdefined(GP_OS_linux)be false anddefined(GP_OS_android)be true in the android build?
And also:
- the patch is adding an
iafter the creation of the martools zip file. Those lines should be indented for theIF.
- could you add a comment explaining why we need the
-
Additionally:
-[% IF c("var/linux") %] +[% IF c("var/linux") %]I think there is no need to change that line.
+ ./mach package + cp -a obj-*/dist/* $distdir/Browser/Please make it
+ ./mach package + cp -a obj-*/dist/* $distdir/Browser/Could you remove
+#ac_add_options --with-android-version=23+#enable ccache to set amount of cache assigned for build.+#ac_add_options --with-ccachefrom the mozconfig file?
Changes (android-1024)
- Removed gradle and android patches. With the correct version of NDK/SDK we don't need the android.patch
- Use android-version 22 for the NDK. The target sdk is still 26. This is the highest version that correctly compiles the project w/o patches. Verified that the apk runs successfully,
- Removed ac_add_options --disable-install-strip from mozconfig. This causes a runtime error of apk. After researching forums, this is the suggested solution.
- Removed 'i' character at end of file
- Removed commented out options from mozconfig
- Fixed formatting issues in build file
After these fixes, we have a successful end-to-end build with the latest code and no patches needed. Apk runs on device. App branded with tor resources.
-
Looking at commit
370168b7b2b00dcd72e51e6a229d7720cfe69c5d, currently the output of the build is ator-browser.tar.gzfile containing two apk files in aBrowser/directory. Instead I think we could just copy the two apk files to the destination directory ([% dest_dir _ '/' _ c('filename') %]), after creating it. And after the apk files are copied, I think we can add a[% RETURN %]if none of the following lines apply to the android build (and in that case also remove all thevar/martoolschanges).Trac:
Status: needs_review to needs_revision -
Here are three things that I think that should get addressed:
-
mozconfig-android-armv7is not in sync with the.mozconfig-androidfile in thetor-browserrepo, which we use for building Tor Browser for Android right now. E.g. do you disable optimizations while we have that enabled in the .mozconfig file we use for official builds.MOZILLA_OFFICIALis note exported either etc. Please double-check the file in thetor-browserrepo and fix up the mozconfig file in thetor-browser-buildrepo. -
Where are all those Gradle dependencies coming from? How do we make sure we got all in the .txt fils (and only those we need)? Can we document the process of generating/updating the .txt file in that file itself (both to preserve knowledge and make the process transparent)?
-
It seems to me not all the packages in
arch_depsare needed (e.g.chrpathwas just added for selfrando on Linux). Can we clean that up accordingly?
Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201810R deleted, TorBrowserTeam201810 added -
-
Replying to gk:
- It seems to me not all the packages in
arch_depsare needed (e.g.chrpathwas just added for selfrando on Linux). Can we clean that up accordingly?
To be more specific here: building works for me just with the OpenJDK related package in
arch_deps. - It seems to me not all the packages in
Changes (android-1105)
- config: Cleaned up arch_deps. Added ccache to support --with-ccache option in mozconfig-android-armv7
- Added projects/firefox/how-to-create-dependencies-list.txt
- mozconfig-android-armv7: Enabled optimization and cache options. I added the MOZILLA_OFFICIAL export to align with what is in tor-browser git repo. Although I'm not convinced this is really needed, it won't do any harm.
I verified that apk runs on device.
-
I think we can disable the
ccacherelated things. We are building within containers from scratch andccacheshould not help much in that scenario. However, it might do so in local builds which is why it is there. Sorry, for not being more explicit here. :(Trac:
Status: needs_review to needs_revision I verified build was working this morning but within the last hour it stop building with errors:
- Could not find junit:junit:4.12
- Could not find !org.robolectric:robolectric:3.5.1
- org.mockito:mockito-core:1.10.19
These are easy to add but the question is why did these test dependencies show up? I'll look into the cause of the changes...
I'm excited for this!
In
projects/firefox/mozconfig-android-armv7+mk_add_options MOZILLA_OFFICIAL=1We don't need this automake directive anymore (the export replaces it)
+ac_add_options --with-android-version=22Do we need this? We've used the default value without a problem (the default is 9 for 32-bit CPU archs, and 21 for 64-bit).
+ac_add_options --with-ccacheI agree with gk in comment:28 - ccache won't help us here
In
projects/firefox/config+ android-armv7: + var: + arch_deps: + - openjdk-8-jdk + - ccachenit: ccache here, too
projects/firefox/how-to-create-dependencies-list.txtnit: can the lines be wrapped at some length? (72, 79, 80, 100?)In
projects/firefox/fetch-gradle-dependencies+ echo "[% artifact.sha256sum %] downloaded_file" | sha256sum -cDoes the script fail when this line returns non-zero? There's no error handling, so I'm guessing this is caught another way?
The MOZILLA_OFFICIAL export executes some tasks that handles dependencies in a different way. Basically the build just fails with missing artifacts, rather than trying to download them. Many of these artifacts are in different repositories than we were using on the first list of dependencies.
Its tedious tracking each dependency through manually so I'm looking for an easier way to handle this.
Replying to sysrqb:
I'm excited for this!
{{{ +ac_add_options --with-android-version=22 }}} Do we need this? We've used the default value without a problem (the default is 9 for 32-bit CPU archs, and 21 for 64-bit).
Under the current build configuration, it defaults to 9 in all cases. Is there someway to tell the build that its for 32 or 64 bit? From my testing, 22 is the highest version that works. This is the only way I'm aware of to configure the android version.
Changes (android-1105a)
- Remove MOZILLA_OFFICIAL ac_add_options
- Regenerated the gradle-dependencies-list to include test artifacts (around 150 additional entries)
- Renamed file to how-to-create-gradle-dependencies-list.txt, wrapped lines
- Updated how-to-create-gradle-dependencies-list.txt with new info
- Remove ccache references
-
Replying to sysrqb:
In
projects/firefox/fetch-gradle-dependencies{{{- echo "[% artifact.sha256sum %] downloaded_file" | sha256sum -c }}} Does the script fail when this line returns non-zero? There's no error handling, so I'm guessing this is caught another way?
There is a
set -eat the beginning of the script, so the script should fail if one of the commands fail. -
Replying to sisbell:
- Renamed file to how-to-create-gradle-dependencies-list.txt, wrapped lines
The file now contains many trailing white spaces, which should be removed.
And I think the commands should not be wrapped:
export GRADLE_MAVEN_REPOSITORIES="file://$rootdir/[% c('input_files_by_name/gradle-dependencies') %]"`cat logs/firefox-android-armv7.log | grep "Download http" | sed "s/^.*Download //g" > download-urls.txt`In the line
* sha256sum | urlI think the*should be removed. Changes (android-1106)
- Fixed how-to-create-gradle-dependencies-list.txt as specified in https://trac.torproject.org/projects/tor/ticket/27443#comment:38
-
Okay, I spent quite some time testing and reviewing the updated patch. I feel we are close. Here are three things that I found so far which should be fixed:
-
You have
--disable-stripin the mozconfig file. Please use--enable-stripinstead (even though the resulting binaries are indeed stripped), as this is less confusing. -
Could you add a comment in the mozconfig file above
ac_add_options --with-libclang-path=/var/tmp/dist/android-toolchain/android-ndk/arm/lib64 ac_add_options --with-clang-path=/var/tmp/dist/android-toolchain/android-ndk/arm/bin/clangexplaining that we need that for the Stylo build part and the preferred way, using
LLVM_CONFIG, is not available as there is nollvm-configin the ndk. (at least I could not find it)- The gradle deps download script mentions issues with setting
LC_ALL=C. Please add the bug number as reference (it is #28117 (moved)).
Finally, could you explain what breaks without setting
--with-android-version=22? You mentioned "22 is the highest version that works" but what does that mean?Trac:
Status: needs_review to needs_revision -
-
Oh, I forgot one and a half thing to fix actually:
- I followed the instructions on how to create the gradle dependency list. In particular I tested:
It may also be the case that you wish to cleanup old versions of the artifacts. For this you will need to run the build with an empty gradle-dependencies-list.txt file and proceed to reconstruct the list from scratch.assuming "empty" means still there but without contents. That actually breaks the build. So, I am not sure which step you had in mind for cleaning up old versions of the artifacts. Could you reformulate that part?
And s/cleanup/clean up/.
Changes (android-1107)
- [mozconfig:L mozconfig: enable-strip option set]
- mozconfig: added comment that llvm-config is not supported in android NDK
- fetch-gradle-dependencies: add bug number to comment
- how-to-create-gradle-dependencies-list.txt: clarified how to start dependency list from scatch
- build: copy apk with a fixed file name (to make it easier to process in tor-browser)
Trac:
Status: needs_revision to needs_review-
Okay, the errors I get are:
68:58.69 /var/tmp/build/firefox-d60cb3854f94/tools/profiler/core/shared-libraries-linux.cc:191: error: undefined reference to 'dl_iterate_phdr' 68:58.69 /var/tmp/build/firefox-d60cb3854f94/tools/profiler/core/shared-libraries-linux.cc:0: error: undefined reference to 'dl_iterate_phdr' 68:58.69 armv7-linux-androideabi/release/libgkrust.a(elf.o):elf.c:function backtrace_initialize: error: undefined reference to 'dl_iterate_phdr' 68:58.69 armv7-linux-androideabi/release/libgkrust.a(mmapio.o):mmapio.c:function backtrace_get_view: error: undefined reference to 'getpagesize' 68:58.69 clang++: error: linker command failed with exit code 1 (use -v to see invocation) 68:58.69 /var/tmp/build/firefox-d60cb3854f94/config/rules.mk:699: recipe for target 'libxul.so' failed 68:58.69 make[4]: *** [libxul.so] Error 1 Replying to gk:
Okay, the errors I get are: {{{ 68:58.69 /var/tmp/build/firefox-d60cb3854f94/tools/profiler/core/shared-libraries-linux.cc:191: error: undefined reference to 'dl_iterate_phdr' 68:58.69 /var/tmp/build/firefox-d60cb3854f94/tools/profiler/core/shared-libraries-linux.cc:0: error: undefined reference to 'dl_iterate_phdr' 68:58.69 armv7-linux-androideabi/release/libgkrust.a(elf.o):elf.c:function backtrace_initialize: error: undefined reference to 'dl_iterate_phdr' 68:58.69 armv7-linux-androideabi/release/libgkrust.a(mmapio.o):mmapio.c:function backtrace_get_view: error: undefined reference to 'getpagesize' 68:58.69 clang++: error: linker command failed with exit code 1 (use -v to see invocation) 68:58.69 /var/tmp/build/firefox-d60cb3854f94/config/rules.mk:699: recipe for target 'libxul.so' failed 68:58.69 make[4]: *** [libxul.so] Error 1 }}}
Yes, this is the error that shows up if we don't use 21 or 22 NDK. I previously had a patch that could make it run on other versions but I think it would be better to use a version of the NDK that doesn't require a patch. I'm ok with either 21 or 22, I just chose 22 since I default to the highest compile version is best, due to possible bug fixes.
I'll document this in the mozconfig file.