Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Tweaks and add a Dockerfile to compile for Android #8036

Merged
merged 1 commit into from
Apr 5, 2018

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Mar 2, 2018

Close #7954

About the PR

This PR does some tweaks to the code for it to compile for Android, and adds a Dockerfile that sets up the environment for building the Parity client for Android.

This PR is not totally final, because it replaces ring with my local fork while waiting for this PR to be merged. Unfortunately the PR is probably controversial, so I'm not sure what is going to happen there.

Also I don't think I'm putting the Dockerfile (and the files that come with it) in the right repository. Let me know where I should put it instead.

About the changes

The minimum Android version required to run Parity compiled with this PR is version 8.0, which is very recent. It should be possible to go down to version 7.0 or even 5.1 by applying manual patches to the C code, but applying patches is obviously not a great thing to do.

If you want to run the compiled client on an actual hardware device, you have to pass --base-path=/data/local/tmp --no-ipc. Android doesn't support IPC, and has very few writable directories.
The plan in the future is that Parity should be used as a library by various Android apps, and therefore the path where the data needs to be written will have to be provided by the app.

@tomaka tomaka added A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. labels Mar 2, 2018
@parity-cla-bot
Copy link

It looks like @tomaka signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@5chdn 5chdn added this to the 1.11 milestone Mar 2, 2018
@tomaka
Copy link
Contributor Author

tomaka commented Mar 4, 2018

Another thing is that we probably want to add Android builds to the CI.
All that needs to be done is to run cargo build --target=armv7-unknown-androideabi from the docker image that this PR defines.

Since I'm not familiar with Parity's CI process, I wouldn't be against some help to do that.

@kirushik
Copy link
Collaborator

kirushik commented Mar 5, 2018

@tomaka Let me finalize a huge CI pipeline refactoring (and maybe @twittner's work on #7829), and then let's talk about CI for Android -- it would become trivial then

@tomaka
Copy link
Contributor Author

tomaka commented Mar 14, 2018

Now that #8099 is merged and OpenSSL is no longer a dependency of Parity, should I remove the bit about compiling OpenSSL from the Dockerfile?

One problem that we need to anticipate is that the only crate in the Rust ecosystem that can generate RSA key pairs (which are needed for libp2p) is OpenSSL. Therefore we might have to pull it back in the future.

@tomaka
Copy link
Contributor Author

tomaka commented Mar 21, 2018

Please review paritytech/ring#1
Then I'll update this PR to point to the paritytech fork of ring and it will be good to go.

By the way, I think it would be a good idea to run something like arm-linux-androideabi-objdump -X ./target/.../parity | grep libc++_shared and ensure that the output is empty. This makes sure that we don't accidentally depend on libc++_shared.so.

Theoretically the Docker image is supposed to be tweaked so that we use the static stdc++, but since this is a bit clumsy I think it's better to have a CI check.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 1, 2018

The Android build that this PR adds is now broken because of https://github.com/paritytech/wasmi/blob/6253dd6fdf1cc523bb42caeaab29cbc4b2156435/src/lib.rs#L105-L110

cc wasmi-labs/wasmi#43
cc @pepyakin

I do own a tablet that has a 32bits CPU, but I don't know how prevalent 32bits devices are today, therefore I don't know whether it's a good idea to drop support for 32bits altogether.

@pepyakin
Copy link
Contributor

pepyakin commented Apr 2, 2018

I do own a tablet that has a 32bits CPU, but I don't know how prevalent 32bits devices are today

AFAIK about a year ago, ARMv7 was the most used architecture in Android world (like more than 80%, but I can't find sources of that info), and it's 32-bit. I think it's highly desirable for us to support 32bits.

cc wasmi-labs/wasmi#43

That issue was created only because there was no testing on x32 bit platforms so far. I've expected some problems in edge cases. That said, I think all these cases should be reachable only with enourmous amounts of gas.

There is a special opt-in-32bit feature that allows to skip this warning, with it parity builds successfully. Will it work for you in the meantime?

Android doesn't support IPC, and has very few writable directories.

There is actually IBinder mechanism for IPC. However, I'm not sure how easy it would be to use it from Rust.

@tomaka
Copy link
Contributor Author

tomaka commented Apr 5, 2018

I don't understand why, but the final binary depends on libc++_shared.so, which means that we cannot run it on a regular Android device.
Other than that, the PR should be good.

We could either merge this PR and open an issue, or wait until I figure that out (which could take some time).

@5chdn
Copy link
Contributor

5chdn commented Apr 5, 2018

I'd say merge and open a ticket.

@tomaka tomaka merged commit 27c32d3 into openethereum:master Apr 5, 2018
@tomaka tomaka deleted the android-build branch April 5, 2018 12:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants