Opened 8 months ago
Closed 7 months ago
#29981 closed task (fixed)
Add option to build without using containers
Reported by: | boklm | Owned by: | tbb-team |
---|---|---|---|
Priority: | Medium | Milestone: | |
Component: | Applications/Tor Browser | Version: | |
Severity: | Normal | Keywords: | tbb-rbm, TorBrowserTeam201904R, tbb-8.5 |
Cc: | hans@… | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
By default the Tor Browser build is done inside containers, which we run using runc, which require root access.
In some cases, such as F-Droid builds (#27539), this can be a problem. I think we should be able to add an option to do the builds without using containers.
Child Tickets
Ticket | Status | Owner | Summary | Component |
---|---|---|---|---|
#30089 | closed | tbb-team | Use apksigner instead of jarsigner | Applications/Tor Browser |
Attachments (4)
Change History (33)
comment:1 follow-up: 2 Changed 8 months ago by
comment:2 Changed 8 months ago by
Replying to gk:
Replying to boklm:
By default the Tor Browser build is done inside containers, which we run using runc, which require root access.
In some cases, such as F-Droid builds (#27539), this can be a problem. I think we should be able to add an option to do the builds without using containers.
How would that help the F-Droid case given that F-Droid is taking just the signature for an app and applying that one to the built they made themselves? I mean the non-container build would then need to match our build done in a containerized setup so that F-Droid would get a bit-for-bit identical output. Maybe I am too pessimistic here but I'd be surprised if that worked out-of-the-box...
If the F-Droid build VM is using Debian Strech too, then I think the main differences between our build environment and the F-Droid build environment would be:
- the username of the build user. In the rbm build we use an
rbm
user, but I think we can add an option to change it if that is an issue. - the list of packages installed. In our case we build each component with only the minimum set of dependencies required to build this component, while in the F-Droid case they would always have the dependencies required to build all of the components. I think in most cases the additional packages should not change the output of the build. Maybe in some cases it will requires some changes to avoid the effect of some additional package.
Unless I'm forgetting an other important difference, I think it should not be very difficult to get matching builds. Although I did not try it, so maybe I'm too optimistic.
comment:3 Changed 8 months ago by
I started working on a patch for that, and after comparing a build done without containers, and a normal build with containers, I get an almost matching apk. The only difference is:
Binary files 1/META-INF/ANDROIDQ.RSA and 2/META-INF/ANDROIDQ.RSA differ diff -r 1/META-INF/ANDROIDQ.SF 2/META-INF/ANDROIDQ.SF 2,4c2,4 < SHA1-Digest-Manifest-Main-Attributes: nJdsdwNyiHipRN3sUz498qUG+L0= < SHA1-Digest-Manifest: vF7lnm0ro+G4diQqON1e3X8+snc= < Created-By: 1.8.0_171 (Oracle Corporation) --- > SHA1-Digest-Manifest-Main-Attributes: E8IbsJM7v8R6d8dy6OqA+g4Q9EE= > SHA1-Digest-Manifest: cBYG++7ArsIP93o3h2mGE0OM4WI= > Created-By: 1.8.0_212 (Oracle Corporation) diff -r 1/META-INF/MANIFEST.MF 2/META-INF/MANIFEST.MF 2c2 < Created-By: 1.8.0_171 (Oracle Corporation) --- > Created-By: 1.8.0_212 (Oracle Corporation)
It seems to be because the tor-browser
step is done in a buster container. The debug signature is also causing the build without containers to stall.
So I think if we find an other workaround for the debug signature that doesn't require using buster, we should be able to get matching build without and without containers.
comment:4 Changed 8 months ago by
Wow, you have achieved reproducible builds then! The oracle version number diff can be solved by using apksigner rather than jarsigner to make the APK Signatures. apksigner is what the Android SDK has been using by default for about 2 years now, and is reproducibly built in Debian stretch and buster:
comment:5 Changed 8 months ago by
If you need the newer apksigner in Debian/stretch, I can backport it. As far as I remember, the changes are only to support APK v2 and v3 signatures better, and were not security fixes. But it might be worth double checking my memory.
comment:6 Changed 8 months ago by
Cc: | hans@… added |
---|
comment:7 follow-up: 8 Changed 8 months ago by
Technically, you don't need to update to apksigner although there is nothing wrong with it.
You can override the default manifest Oracle. First create the a manifest.txt to use to override entries
Created-By: Tor Build
Then use the 'm' flag
jar -cvfm tor-browser.jar manifest.txt
The generated manifest will be
Manifest-Version: 1.0 Created-By: Tor Build
comment:8 Changed 8 months ago by
Replying to sisbell:
Technically, you don't need to update to apksigner although there is nothing wrong with it.
You can override the default manifest Oracle. First create the a manifest.txt to use to override entries
Created-By: Tor BuildThen use the 'm' flag
jar -cvfm tor-browser.jar manifest.txtThe generated manifest will be
Manifest-Version: 1.0 Created-By: Tor Build
The main issue is not that some entries in the manifest are different, but that the signing is stalling when we do it with faketime on stretch, which is the reason we currently have to do it in a buster container.
If apksigner does not stall when used with faketime on stretch (or doesn't need faketime to produce reproducible output), then that would solve the issue.
comment:9 Changed 8 months ago by
While you are at it, you should also drop faketime for signing. The Android tools have been zeroing out the dates in the ZIP header for a while now, like 2+ years. If TBA doesn't use a version of the Android tools that does that, it is not hard to do it with a tool or with Python or something as a post-build, pre-signing process.
comment:10 Changed 8 months ago by
Using the patch from branch bug_29981_v3
I have been able to do a build without containers:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_29981_v3&id=ab3c5d96be66d09d4dc98be385e495244bef48af
The apk I got does not match the one built using container, but the only difference is:
--- tor-browser-8.5a10-android-armv7-multi-qa.apk +++ ../../testbuild/tor-browser-8.5a10-android-armv7-multi-qa.apk ├── zipinfo {} │ @@ -1502,13 +1502,13 @@ │ -rwxr-xr-x 2.0 unx 5596 b- defX 10-Jan-01 00:00 lib/armeabi-v7a/libplugin-container.so │ -rw-r--r-- 2.0 unx 382 b- defX 10-Jan-01 00:00 application.ini │ -rw-r--r-- 2.0 unx 32 b- defX 10-Jan-01 00:00 package-name.txt │ -rw-r--r-- 2.0 unx 795 b- defX 10-Jan-01 00:00 ua-update.json │ -rw-r--r-- 2.0 unx 48 b- defX 10-Jan-01 00:00 platform.ini │ -rw-r--r-- 2.0 unx 97 b- defX 10-Jan-01 00:00 removed-files │ -rw-r--r-- 2.0 unx 10641927 b- stor 10-Jan-01 00:00 assets/omni.ja │ --rw-r--r-- 3.0 unx 502590 b- defN 00-Jan-01 00:00 assets/distribution/extensions/{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi │ -rw-r--r-- 3.0 unx 1765652 b- defN 00-Jan-01 00:00 assets/distribution/extensions/https-everywhere-eff@eff.org.xpi │ +-rw-r--r-- 3.0 unx 502590 b- defN 00-Jan-01 00:00 assets/distribution/extensions/{73a6fe31-595d-460b-a920-fcc0f8843232}.xpi │ -rw---- 2.0 fat 144465 b- defN 10-Jan-01 00:00 META-INF/ANDROIDQ.SF │ -rw---- 2.0 fat 1194 b- defN 10-Jan-01 00:00 META-INF/ANDROIDQ.RSA │ -rw---- 2.0 fat 144360 b- defN 10-Jan-01 00:00 META-INF/MANIFEST.MF │ 1512 files, 58292874 bytes uncompressed, 44899152 bytes compressed: 23.0%
I think the cause of this is this line in projects/tor-browser/build.android
:
zip -r -X $apk $ext_dir
Which is not adding the extensions to the .apk in a sorted order.
comment:11 Changed 8 months ago by
Wow, great progress! That sort order issue is frustrating. My experience is that doing post-processing of APKs with zip
or other tools usually leads to random repro issues like that. The Android tools have been getting better, but they still have repro quirks. It seems that the zip
commands are used for adding and removing static assets. I can help move that to the pre-packaging phase of the Android build, if gradle is in use.
comment:12 Changed 8 months ago by
Keywords: | tbb-8.5-must added |
---|
comment:13 Changed 8 months ago by
Keywords: | TorBrowserTeam201904R added; TorBrowserTeam201904 removed |
---|---|
Status: | new → needs_review |
There are two patches for review in branch bug_29981_v5
:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_29981_v5&id=069c880961e93fe2f5052ae6d428b2f9efdd1d4e
I have been able to get matching builds from this branch for the android-x86 and android-armv7 builds.
The packages that I installed on a stretch machine to be able to do the build are:
build-essential python bison automake libtool zip unzip autoconf2.13 yasm openjdk-8-jdk gettext-base autotools-dev automake autoconf libtool autopoint libssl-dev pkg-config zlib1g-dev libparallel-forkmanager-perl libfile-slurp-perl bzip2 xz-utils apksigner
comment:14 follow-up: 21 Changed 8 months ago by
Keywords: | TorBrowserTeam201904 added; TorBrowserTeam201904R removed |
---|---|
Status: | needs_review → needs_revision |
While still testing the changes here come some preliminary comments:
commit e38b5d94a19caff488382d92968dbd8cd85b18eb
The change to the README
says "Adding the no_containers target will disable the use of containers", but the first thing that happens is actually building a container (called "empty"). So, we might want to be a bit more verbose explaining what is happening here to avoid confusion.
"and install build dependencies for all the components that are built" We are listing build dependencies in the README
file. I think we should list all the additional build dependencies needed for building without containers as well while we are talking about them. There should be no need for builders that want to build without containers to try figuring that out themselves.
commit 069c880961e93fe2f5052ae6d428b2f9efdd1d4e
"Add the extensions in sorted order" it's not only extensions it seems? So, please adjust the commit message accordingly.
I wonder why we need container/use_container
and container/disable
as two separate options. If use_container
is false then cleary I build without containers, no? In other words: why can't we just use/amend the already existing use_container
option to avoid creating another one that seems to do the same thing but just (slightly) differently?
comment:15 follow-up: 16 Changed 8 months ago by
Alright, I started a build on two different machines, both on commit 069c880961e93fe2f5052ae6d428b2f9efdd1d4e (boklm/bug_29981_v5). One machine is building with containers and one without. While this setup builds reproducibly on each machine the *.apks on one machine differ from those on the second one.
The differing parts are essentially all the Firefox .so files in assets/ or lib/. Both machines built from the same tor-browser
commit (c722d57604db58695140d95565a78433989fe9ca).
I attached two .so files showing the issue.
FWIW: I built on the bare stretch machine without installing bison, yasm, and openjdk-8-jdk. (However, openjdk-8-jre is installed it seems)
Changed 8 months ago by
Attachment: | liblgpllibs_1.so added |
---|
Changed 8 months ago by
Attachment: | liblgpllibs_2.so added |
---|
Changed 8 months ago by
Attachment: | pkg_list.txt added |
---|
List of packages installed on my build machine.
comment:16 follow-up: 17 Changed 8 months ago by
Replying to gk:
FWIW: I built on the bare stretch machine without installing bison, yasm, and openjdk-8-jdk. (However, openjdk-8-jre is installed it seems)
I'm wondering if the difference in the build could be caused by some different versions of packages installed, or maybe additional or missing packages.
In my case, the build that I did without containers is matching the one done with containers. I attached the list of packages installed on this machine (the output from dpkg -l
). Is there any difference with the list of packages installed on your build machine, other than bison, yasm and openjdk-8-jdk?
comment:17 follow-up: 19 Changed 8 months ago by
Replying to boklm:
Replying to gk:
FWIW: I built on the bare stretch machine without installing bison, yasm, and openjdk-8-jdk. (However, openjdk-8-jre is installed it seems)
I'm wondering if the difference in the build could be caused by some different versions of packages installed, or maybe additional or missing packages.
In my case, the build that I did without containers is matching the one done with containers. I attached the list of packages installed on this machine (the output from
dpkg -l
). Is there any difference with the list of packages installed on your build machine, other than bison, yasm and openjdk-8-jdk?
Okay, installing openjdk-8-jdk does not make a difference. I attached the package list of the stretch machine I used. In fact, you have access to it, too, it's the TPO build machine. In case you want to make more elaborate queries to figure out what the important difference here is.
Changed 8 months ago by
Attachment: | pkg_list_tpo added |
---|
comment:18 Changed 8 months ago by
Keywords: | tbb-8.5 added; tbb-8.5-must removed |
---|
Given the reproducibility issues and the work to get this properly going on F-Droid itself we might need to resort to hc helping us again for 8.5 given that we don't want to postpone it further and test the native F-Droid setup in an alpha build.
comment:19 follow-up: 20 Changed 8 months ago by
Replying to gk:
Okay, installing openjdk-8-jdk does not make a difference. I attached the package list of the stretch machine I used. In fact, you have access to it, too, it's the TPO build machine. In case you want to make more elaborate queries to figure out what the important difference here is.
I did a build without containers on build-sunet-a, with commit 069c880961e93fe2f5052ae6d428b2f9efdd1d4e
, after installing all the packages from comment 13, I have x86 and armv7 apk files matching the ones I built on an other machine with containers.
Can you try to do an other build on this machine?
Also, did you do alpha or nightly builds? In my case I did alpha builds, as it makes it easier to be sure both machines are using the same commits.
This is the apk files that I get:
e88fcd32929f86765e17ef0ccf1979c64a1fe9b2d6e2099590bac3b061f31487 testbuild/tor-browser-8.5a11-android-armv7-multi-qa.apk df0823edbc7aeb3c19a33bada6edde6743958d4b71629a24e6fed8d999ff4814 testbuild/tor-browser-8.5a11-android-x86-multi-qa.apk
comment:20 Changed 8 months ago by
Replying to boklm:
Replying to gk:
Okay, installing openjdk-8-jdk does not make a difference. I attached the package list of the stretch machine I used. In fact, you have access to it, too, it's the TPO build machine. In case you want to make more elaborate queries to figure out what the important difference here is.
I did a build without containers on build-sunet-a, with commit
069c880961e93fe2f5052ae6d428b2f9efdd1d4e
, after installing all the packages from comment 13, I have x86 and armv7 apk files matching the ones I built on an other machine with containers.
Can you try to do an other build on this machine?
Will do.
Also, did you do alpha or nightly builds? In my case I did alpha builds, as it makes it easier to be sure both machines are using the same commits.
Nightlies. But that should not matter much here as the Firefox libs where different and I used the same Firefox commit for both builds. But I can try doing an alpha one.
comment:21 Changed 8 months ago by
Keywords: | TorBrowserTeam201904R added; TorBrowserTeam201904 removed |
---|---|
Status: | needs_revision → needs_review |
Replying to gk:
While still testing the changes here come some preliminary comments:
commit e38b5d94a19caff488382d92968dbd8cd85b18eb
The change to the
README
says "Adding the no_containers target will disable the use of containers", but the first thing that happens is actually building a container (called "empty"). So, we might want to be a bit more verbose explaining what is happening here to avoid confusion.
I added something about that in the README, and renamed the file empty
to containers_disabled
.
"and install build dependencies for all the components that are built" We are listing build dependencies in the
README
file. I think we should list all the additional build dependencies needed for building without containers as well while we are talking about them. There should be no need for builders that want to build without containers to try figuring that out themselves.
commit 069c880961e93fe2f5052ae6d428b2f9efdd1d4e
"Add the extensions in sorted order" it's not only extensions it seems? So, please adjust the commit message accordingly.
I pushed a new revision of the commits fixing that in branch bug_29981_v6
:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_29981_v6&id=718d1ab83e497cc9492d3c8b1fc101095e984e81
I wonder why we need
container/use_container
andcontainer/disable
as two separate options. Ifuse_container
is false then cleary I build without containers, no? In other words: why can't we just use/amend the already existinguse_container
option to avoid creating another one that seems to do the same thing but just (slightly) differently?
container/use_container
is defined in projects configs, which have priority over definitions in rbm.conf
. So if we want to change the value of container/use_container
for the no_containers
target, we would have to do it in all projects/*/config
files where it is defined. I have renamed var/container/disable
to var/container/global_disable
to try to make it less confusing.
comment:22 follow-up: 23 Changed 8 months ago by
Thanks for the update. How did you get the list of packages to install? E.g. I did not need to install bison
nor yasm
to get the mobile browser built, so it seems those are not actually needed.
comment:23 Changed 8 months ago by
Replying to gk:
Thanks for the update. How did you get the list of packages to install? E.g. I did not need to install
bison
noryasm
to get the mobile browser built, so it seems those are not actually needed.
I looked at the list of packages we list as dependencies for android in the various projects/*/config
files, and rbm.conf
.
For bison
, we have it in rbm.conf
. We added it to the default list of packages for the other platforms with commit 78a5009fa4e0fc3e7e4ccd854df7dd3833b6b174
, as it seems it is needed to build binutils
, and adding it to the default list of packages avoids creating and storing a specific container for the binutils build. Then we probably copied it to the android list of dependencies without checking that it was really required. We could probably remove it from the osx-x86_64 build too as we don't need to build binutils there. I opened #30325 for that.
For yasm, we have it in the list of packages needed to build firefox on all platforms, although it seems it's not needed for the android build. I opened #30326 to fix that.
In branch bug_29981_v7
I updated the README to remove bison and yasm from the list of packages to install:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_29981_v7&id=0fc15a39f2adcf029676810c2ec37c566886879e
comment:24 Changed 8 months ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Thanks for the detailed analysis! The new branch looks good to me. I've merged it to master
(commits 9072578261fc2e4198e39b81313683bf2674c5b4 and 0fc15a39f2adcf029676810c2ec37c566886879e).
comment:25 follow-up: 27 Changed 8 months ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
So it seems yasm is not needed for the android-armv7 build, but still needed for the android-x86 one.
In branch bug_29981_v8
I added a new commit mentioning that in the README:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_29981_v8&id=1959d1c9dbe993b8fb4732b333892cb04f7ccfae
comment:26 Changed 8 months ago by
Status: | reopened → needs_review |
---|
comment:27 follow-up: 28 Changed 8 months ago by
Keywords: | TorBrowserTeam201904 added; TorBrowserTeam201904R removed |
---|---|
Status: | needs_review → needs_revision |
Replying to boklm:
So it seems yasm is not needed for the android-armv7 build, but still needed for the android-x86 one.
In branch
bug_29981_v8
I added a new commit mentioning that in the README:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_29981_v8&id=1959d1c9dbe993b8fb4732b333892cb04f7ccfae
The previous apt-get install
comment does not talk about android-armv7
specifically, thus starting now talking of android-x86
seems a bit weird to me. I think *if* we go with just installing yasm
everywhere to not create an extra container (I agree with that idea) then we should treat it as a first-class citizen here and just add it to the other packages. Otherwise we are starting to divert regarding installed packages between container/non-container builds (think of folks thinking "I only want to build android-armv7
packages, thus no yasm for me`) which might fall onto our feet at some point.
Maybe a note in the commit message would be enough to help us remembering in case we stumble over that again in the future? Otherwise I could think of "This can be done
with the following command (note: yasm is only needed for android-x86 builds):" to make our point clear while minimizing the risk for deviating from the packages installed in our container builds.
comment:28 Changed 7 months ago by
Keywords: | TorBrowserTeam201904R added; TorBrowserTeam201904 removed |
---|---|
Status: | needs_revision → needs_review |
Replying to gk:
Maybe a note in the commit message would be enough to help us remembering in case we stumble over that again in the future?
Yes, I think it's probably enough. I did that in branch bug_29981_v9
:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_29981_v9&id=f7ae040da39ac0009776802371e088f15531b9e7
comment:29 Changed 7 months ago by
Resolution: | → fixed |
---|---|
Status: | needs_review → closed |
Thanks. Merged to master
(commit f7ae040da39ac0009776802371e088f15531b9e7) and maint-8.5
(commit 7e784c57f614b97a9b2f614bc0550fd5ade1e735).
Replying to boklm:
How would that help the F-Droid case given that F-Droid is taking just the signature for an app and applying that one to the built they made themselves? I mean the non-container build would then need to match our build done in a containerized setup so that F-Droid would get a bit-for-bit identical output. Maybe I am too pessimistic here but I'd be surprised if that worked out-of-the-box...