Skip to content

Update INSTALL.md with Android instructions - #1106

Merged
cdecker merged 2 commits into
ElementsProject:masterfrom
icota:patch-1
Feb 27, 2018
Merged

Update INSTALL.md with Android instructions#1106
cdecker merged 2 commits into
ElementsProject:masterfrom
icota:patch-1

Conversation

@icota

@icota icota commented Feb 26, 2018

Copy link
Copy Markdown
Contributor

Add Android build instructions

Add Android build instructions
@ZmnSCPxj

Copy link
Copy Markdown
Contributor

Looks OK, would ACK, will wait a little to see if anyone objects.

@cdecker

cdecker commented Feb 26, 2018

Copy link
Copy Markdown
Member

Looks good to me, great work @icota, did you try to run it on android as well? I'd be pleasantly surprised if it runs out of the box :-)

@icota

icota commented Feb 26, 2018

Copy link
Copy Markdown
Contributor Author

Just realised I've jumped the gun with this one. Instructions will be valid after #1102

@icota

icota commented Feb 26, 2018

Copy link
Copy Markdown
Contributor Author

@cdecker It works as expected. I had to jump through some hoops since Android expects all binaries to adhere to a lib_something_.so naming convention, but surprisingly after that everything went swimmingly. I run it as a mobile LN wallet, it connects to my full node via RPC thanks to the --bitcoin-rpc* options.

Comment thread doc/INSTALL.md Outdated
valgrind asciidoc --run make
```

To Cross-compile for Android

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: "To cross-compile for Android" – lower-case "cross-compile" :-)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed :)

Comment thread doc/INSTALL.md Outdated

Make a standalone toolchain as per https://developer.android.com/ndk/guides/standalone_toolchain.html. For c-lightning you must target an API level of 24 or higher.

Depending on your toolchain location and target arch source env variables such as:

@practicalswift practicalswift Feb 26, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Is this sentence complete? Feels like it is missing a word. s/source/set/?

@icota icota Feb 26, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was definitely missing a comma. I've used the word source since I feel people will be using the source command on a file rather than setting all these manually.

@ZmnSCPxj

Copy link
Copy Markdown
Contributor

ACK 875d374

We can ignore the CI error, that is fixed by #1111

@cdecker
cdecker merged commit 694fb41 into ElementsProject:master Feb 27, 2018
@cdecker

cdecker commented Feb 27, 2018

Copy link
Copy Markdown
Member

Thanks @icota, getting it to work on android is huge news 👍

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.

4 participants