Skip to content
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

reproducible build #272

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jenniferberger
Copy link
Contributor

@jenniferberger jenniferberger commented May 1, 2018

inspired by doc/BUILDING-external-libs.md

  • create a container definition for the android npk and cross-compile build chain
  • bash script to download and check remote resources (the cpp part)
  • compile artifacts for monero, openssl and boost for four architectures
  • apk: link static-a files for four architectures and produce debug and release builds, instruct gradle to remove timestamps from the result (more network requests)

The apk files currently differ against the verification because the gradle.properties are taken from the mr2049r/xmrwallet that does android.keepTimestampsInApk = true. On a modest hardware (i3/hdd) the build all takes about 4 hours, verification 4 hours again. When only java code changes this can be shaved to about 10 minutes (make artifacts/apk). Building with a patched xmrwallet repository the apk-files do not differ.

@m2049r please comment if this toolchain is applicable in the current release workflow to make one of the next versions verifiable and if you need something to drop your current way of building the binaries. I think it is still required to do some work on signing the builds with the correct keys. The debug packages are signed with the debug.keystore added by this PR and the release packages do not contain a CERT.RSA file - not sure which is the one that will be released.

The current release (monerujo-1x5x3_armeabi-v7a.apk) produces different binaries (so-files) than uploaded to the https://github.com/m2049r/xmrwallet/releases page. This is expected and not meant as an offence.

Reviewers: please check the sha256 sums in the Dockerfile (android toolchain) and build-artifacts.sh (OpenSSL/Boost) so i really used the right versions for the container. There is still reviewing required in the monero fork of m2049r, but this is out of scope of this PR.

The verification can still be improved. Signal for example records all dependencies with their checksums. They are a bigger project with more resources to that direction. Even if I think those checks are important for a payment application, there are currently more urgent places that require attention.

references #232 (makes the artifacts reproducible and verifiable) and #57 (because it is still a goal to get into the official store some day as they do the verify as well)

@0140454
Copy link
Contributor

0140454 commented May 2, 2018

Nice work!

But I have some idea:

  • Find latest monero branch automatically
    We can use the following command to find latest branch.

    git branch -a --sort=-committerdate | head -1 | tr -d '[:space:]'
    

    Therefore, the command to checkout branch can be

    git checkout $(git branch -a --sort=-committerdate | head -1 | tr -d '[:space:]')
    
  • Prevent from compilation error
    Changing OPENSSL_INCLUDE_DIR to ${ARTIFACT_ROOT}/openssl/${arch} can prevent from random.o compilation error without modifying source code.
    Reference: Build OpenSSL error  #246

@jenniferberger
Copy link
Contributor Author

jenniferberger commented May 3, 2018

@0140454 thanks for your hint to the closed ticket - it contained a additional magic -Werror hint
the OPENSSL_INCLUDE_DIR should only be required with nonstandard installs, i reduced the -D 's for monero

  • added the 'lastest' branch finding for monero
  • reworked the openssl build (obsoleting build-all-archs in android-openssl)
  • added report_checksum task
  • added apk task (that is not working jet)
    now the notebook sounds like it is web-mining but it produces the binary artifacts after ~1h / 3.6ghz

please comment on your opinion on

  • one large script build-artifacts vs many small (build-openssl, build-monero)?
  • how would we verify the shasums?
  • any reason not to make install (as the description was doing)?
  • is openssl 1.0.1j or 1.1 the real goal?
  • does the build really require to compile all archs (arm, arm64, x86, x86_64)?

i am giving up with the gradle/androidstudio stuff for now, input on that would be appreciated - its not clear to me where it expects the built binaries and where it puts the apk - it just fails with java.security.InvalidAlgorithmParameterException when it tries to download from the repositories,

@jenniferberger
Copy link
Contributor Author

another round after the results did not match:

  • run (arch) objdump --enable-deterministic-archives on all a-files
  • collect case $arch in.... blocks to a single exporting method
  • correct monero artifact copy (absolute OUTPUTDIR)

now two runs produce the same result:

cd external-libs/build
make docker_image
time make artifacts
time make artifacts-verification

should output "verify ok"

this means boost, openssl and monero artifacts can now be verified by a third party

@0140454
Copy link
Contributor

0140454 commented May 4, 2018

@jenniferberger

  • one large script build-artifacts vs many small (build-openssl, build-monero)?
    If the script is written in a clear way, I think its size is not a problem.
  • any reason not to make install (as the description was doing)?
    Using Makefile to provide a simple way to build libraries is a good idea.
  • does the build really require to compile all archs (arm, arm64, x86, x86_64)?
    Maybe we can provide a parameter to setup the architecture we want to build.
    For example, make ARCH=x86.
  • About others and new commit, I may try it tomorrow and give you some feedback.

@jenniferberger
Copy link
Contributor Author

There is a --arch commandline switch, from what i see in the release apk's all four arch's are required

The make install question was meant the other way around: the build description explicitly copies the *.a files to the destinations instead of running make install. This is the case for OpenSSL. boost uses the install target. monero does not have one. Was there a reason that should be considered when writing the copy-lines as i migrated them to make install

@0140454
Copy link
Contributor

0140454 commented May 7, 2018

Oh, I misunderstood your text.
In my opinion, the reason for copying *.a directly rather than using make install is just to prevent install useless files.

In addition, I modified the script according to the issues I encountered at compiling apk and created a pull request in your repository. I fix some errors and complete building apk part.

@jenniferberger
Copy link
Contributor Author

Thanks @0140454 the apk is now being built.
I tried to tune the download of the gradle files but no success - the apk build will always download dependencies (and sometimes fail in the process) instead of downloading them with the distfiles (and allowing to cache/checksum them too)
As the container closes to 10gb, there is a issue that can only be mitigated by cleaning up in the process. The /var/src/[product] have to be removed after success, otherwise the gradle build (that adds eta 1.5gb) fails with no space left on device. For now this works, but soon^tm the android build chain will cause this error again. The only way to fix this then is to split the container (eg no cpp build chain for the apk build).
The contents of the apk are reproducible (dex/so files do not change between builds) the apk file contains timestamps what makes the final file changing every time.
I will look for a simple solution (like with the a-files) to that.

@jenniferberger jenniferberger changed the title [wip][rfc] reproducible build of binary artifacts reproducible build May 12, 2018
@jenniferberger
Copy link
Contributor Author

Verify build on random docker host successful

sha256sum -c "${TMP_CHECKFILE}" \
|| die "checksum for ${FILENAME} not expected:" \
" is $(sha256sum "${FILENAME}") instead of ${SHA256SUM}"
# XX todo: should also be removed on die, irrelevant in container
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid leaving temporary file, we can use pipe instead of file.
For instance, echo -n "${SHA256SUM} ${FILENAME}" | sha256sum -c -

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok



BOOST_VERSION=${BOOST_VERSION:-1.58.0}
# j!!!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's 1.0.2o currently

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, i misunderstood your comment...

@@ -17,4 +17,4 @@ org.gradle.jvmargs=-Xmx1536m
# org.gradle.parallel=true

# keep build timestamps in APK
android.keepTimestampsInApk = true
android.keepTimestampsInApk = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can extract apk to verify artifacts instead of adding debug.keystore or droping timestamp.
Just like f-droid verification server does [GitLab].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

timestamp dropping is a good thing imho, but the debug.keystore is annoying.

So i rewrote the artifacts-verification build and the checksum method to create checksums of the contents of the apk and verify them with a second build.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason for the timestampsInApk is that I use that option when deploying on f-droid to generate the repo metadata (i.e. to have the correct apk release date). it comes in very handy when recreating the whole repo from scratch.

a reproducible build, of course, may not have this set to true.

@jenniferberger
Copy link
Contributor Author

root@docker-host:/xmrwallet/external-libs/build/artifacts-verification# time make artifacts-verification
[...]
checksums verified ok

real 225m56.830s
user 0m4.948s
sys 0m5.384s

package.checksums.txt

- create a container definition for the android npk
- install java and gradle

- bash script to download, check, compile and verify artifacts
 - build boost
 - build ssl (using the logic from build-all-arch) so that the
   openssl version can be increased
   correct OpenSSL checksum for 1.0.2o
 - build monero using the m2049r/monero repository with the latest
   tag
 - build apk using the m2049r/xmrwallet repository with the latest
   tag
 - compile checksums from non-volitale files, skipping META-INF
   directories from apk and check them with a artifacts-verification
   version
@jenniferberger
Copy link
Contributor Author

rebased for clean history

@m2049r
Copy link
Owner

m2049r commented Sep 25, 2018

@jenniferberger running make all builds a lot of stuff and dies building monero:

CMake Error at /usr/share/cmake-3.10/Modules/FindBoost.cmake:1947 (message):
  Unable to find the requested Boost libraries.

  Boost version: 1.58.0

  Boost include path: /var/src/artifacts/boost/arm/include

  Could not find the following static Boost libraries:

          boost_locale

  Some (but not all) of the required Boost libraries were found.  You may
  need to install these additional Boost libraries.  Alternatively, set
  BOOST_LIBRARYDIR to the directory containing Boost libraries or BOOST_ROOT
  to the location of Boost.
Call Stack (most recent call first):
  CMakeLists.txt:845 (find_package)


CMake Error at CMakeLists.txt:56 (message):
  Could not find Boost libraries, please make sure you have installed
  Boost or libboost-all-dev (1.58) or the equivalent
Call Stack (most recent call first):
  CMakeLists.txt:849 (die)


-- Configuring incomplete, errors occurred!
See also "/var/src/monero/build/release.arm/CMakeFiles/CMakeOutput.log".
See also "/var/src/monero/build/release.arm/CMakeFiles/CMakeError.log".
+ die 'could not configure monero'
+ echo 'could not configure monero'
could not configure monero
+ exit 1
Makefile:36: recipe for target 'artifacts/monero' failed
make: *** [artifacts/monero] Error 1

@kingdom998
Copy link

@m2049r

@jenniferberger running make all builds a lot of stuff and dies building monero:

CMake Error at /usr/share/cmake-3.10/Modules/FindBoost.cmake:1947 (message):
  Unable to find the requested Boost libraries.

  Boost version: 1.58.0

  Boost include path: /var/src/artifacts/boost/arm/include

  Could not find the following static Boost libraries:

          boost_locale

  Some (but not all) of the required Boost libraries were found.  You may
  need to install these additional Boost libraries.  Alternatively, set
  BOOST_LIBRARYDIR to the directory containing Boost libraries or BOOST_ROOT
  to the location of Boost.
Call Stack (most recent call first):
  CMakeLists.txt:845 (find_package)


CMake Error at CMakeLists.txt:56 (message):
  Could not find Boost libraries, please make sure you have installed
  Boost or libboost-all-dev (1.58) or the equivalent
Call Stack (most recent call first):
  CMakeLists.txt:849 (die)


-- Configuring incomplete, errors occurred!
See also "/var/src/monero/build/release.arm/CMakeFiles/CMakeOutput.log".
See also "/var/src/monero/build/release.arm/CMakeFiles/CMakeError.log".
+ die 'could not configure monero'
+ echo 'could not configure monero'
could not configure monero
+ exit 1
Makefile:36: recipe for target 'artifacts/monero' failed
make: *** [artifacts/monero] Error 1

Hi, have you fix this error? i encountered the same problem recently on my unbuntu-16.04-LTS system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants