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

Embedded Filecoin: Enable cross-compiling #114

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

eigendude
Copy link
Contributor

@eigendude eigendude commented Feb 25, 2020

Description of the Change

Because what would be cooler than filecoin for embedded systems?

Porting something as complex as filecoin and all dependencies to embedded is not a small task, but I was able to make progress and even runtime test some features. This PR outlines the work I've done across various repos and the remaining steps to achieve embedded filecoin.

Structure of the PR

The PR is organized as follows:

  1. Unmerged dependent PRs (in this case there's a simple git conflict)
  2. Fixes needed for cross-compiling
  3. Changes needed for cross-compiling
  4. Additions needed for cross-compiling
  5. Changes needed for linking filecoin from another process

Next, links to commits needed for cross-compiling libp2p. Depending on feedback, I can break these out and PR to the libp2p repo.

Finally, links to commits in the repos of two different embedded build systems, one of which I runtime tested.

Dependent PRs

Commit:

The PR diff is adjacent to a later change, so it's more convenient to rebase the PR out when it's merged to avoid the small git conflict.

Fixes for cross-compiling

Commits:

These commits come first, because they address what could be considered defects. Inclusion would be valuable even if the rest of the PR is omitted. Further description is in the commit message.

Changes for cross-compiling

Commits:

These changes enable more control over the build process when embedded in another build system.

CMake is controlled by user variables; the first one we need is -DHUNTER_ENABLED=OFF so that we can have control over dependencies.

Another useful variable is -DTESTING=OFF, because embedding test infra drastically increases the up-front effort. The first two commits above allow this variable to exclude test infra from the dependency discovery in addition to the build.

Finally, I introduced a new variable: -DBUILD_INTERNAL_DEPS=OFF. This causes the in-tree dependencies to be skipped. I needed this for a build system that doesn't support git submodules. Instead, the dependencies are provided by the build sytsem.

Additions for cross-compiling

Commits:

When building without Hunter, CMake needs some help discovering dependencies that aren't known to CMake. I took the standard approach of including CMake modules for these missing dependencies.

Findleveldb.cmake is definitely needed for the nested CMakeLists.txt file that links to leveldb. Not sure about Findcppcodec.cmake or FindMicrosoft.GSL.cmake...

Changes for linking from another process

Commits:

At this point, make succeeds. The next step of a build system is to run make install so that the other process can link to the necessary libraries and headers. Here, we get the following error:

make: *** No rule to make target 'install'.  Stop.

The second commit above adds this missing step. However, unlike libp2p where the library names are prefixed, this results in an explosion of generically-named libraries polluting the system root. To solve this, I preceded the commit with a functional change that prefixes libraries and test cases to mirror libp2p's approach.

Finally, headers need to be installed too. The third commit takes an approach similar to typical cmake projects, where headers are in the source tree and they are referenced from the CMakeLists.txt file. This was less work for me to get to the runtime testing, as I didn't have to touch code, but I don't think it's the right approach here. Instead, we should mirror libp2p and move headers to a top-level include/ folder. All sources will need to be adjusted to include files from inside a filecoin/ top level folder, similar to how libp2p headers are included, e.g.:

#include <libp2p/protocol/echo/echo.hpp>

This is a requirement to avoid clashes when mixing headers from a generic directory like common/ with external code.

Fixes for libp2p

I made the following fixes for libp2p. Not all are probably necessary, so extra testing is needed. I'll break these out, do the extra work and upstream to libp2p depending on feedback.

Commits:

I also got link errors about spdlog so I needed to comment a bunch of logging, not sure why but I'm not including the commit here.

Additions for libp2p

I also authored an addition to libp2p:

This is because filecoin uses blake2b-256 for CIDs, which shows up as "unknown" in libp2p's CID stringification.

Before:

identity - cidv1 - dag-cbor - unknown-256-d36a2619a672494604e11bb447cbcf5231e9f2ba25c2169177edc941bd50ad6c

After:

identity - cidv1 - dag-cbor - blake2b-256-256-d36a2619a672494604e11bb447cbcf5231e9f2ba25c2169177edc941bd50ad6c

Additions for OpenEmbedded (meta-cryptocurrency layer)

With these changes in place, I'm able to compile and link filecoin and dependencies using two build systems: OpenEmbedded, and Kodi's depends system. I almost succeeded in getting OpenEmbedded to cross-compile the Rust code, but ended up stubbing BLS signatures and proofs for Kodi's build system so that I could get to some runtime testing.

Here's my work on these two build systems. Commits are structured so that filecoin inclusion can be upstreamed.

Commits:

This first commits enables libp2p support in the entire OpenEmbedded ecosystem. The second commit joins it with filecoin, but cargo fails a long way into the build, so there's a little work left to do.

Additions for Kodi

Commits for C++17 support:

Kodi currently uses C++14. These 3 commits can be PR'd to enable C++17 support, needed for libp2p and filecoin. Merging is probably a long way off, as Kodi supports a long tail of devices with embedded build systems that often use old compilers.

Commits for the depends build system:

With the commits in this PR, in addition to stubbing proofs and BLS signatures, I was able to cross-compile filecoin for x86_64 Linux and macOS for runtime testing. Next is to test compilation for Raspberry Pi and Android NDK.

Commits for runtime testing:

I didn't spend a lot of time on kademlia, but the ipfs data store, echo and gossip test code produces promising output. Tested on Linux x86_64 and macOS.

Benefits

IoT is the obvious goal, but Kodi opens up another little-known area of embedded: OTT (over-the-top), stretching across both set-top boxes and almost all mobile

Possible Drawbacks

When I do PRs like this people sometimes freak out. Sorry if it causes any negative feelings, but I figured what the hell, might as well share the work and see what happens.

@igor-egorov igor-egorov mentioned this pull request Feb 25, 2020
@igor-egorov igor-egorov self-requested a review February 25, 2020 10:32
@eigendude
Copy link
Contributor Author

I see two 🚀! I have experience with big PRs like this and think I can get it merged. I'll break out commits into separate PRs roughly as they're grouped above to facilitate review. It's gonna take a lot of work but I think I'll be able to find the time over the next few weeks to see embedded filecoin happen.

@igor-egorov
Copy link
Member

Hi @eigendude
First of all - thank you for your efforts! We much appreciate your contribution.

The most debatable change for us is the headers' disclosure. Initially, Fuhon was not supposed to be a library.

Here is the question (1) - what are the benefits for you to have a library rather than an executable?

If we decide to make it as a library - what should be an interface then (2)? The spec itself is roughly defined, and it seems hard to design a reliable interface now.

Anyway, it seems that most of your changes are going to be merged 🎉 The thing that is not going to be merged - filecoin_add_headers. The approach with a common /include directory for headers is more preferable. The project comes more structured and less error-prone - a developer could forget to expose a particular header in CMakeLists.txt.

Feel free to join our newly created community chat for faster communications https://t.me/fuhon_filecoin :)

@eigendude
Copy link
Contributor Author

I've upstreamed the libp2p fixes: libp2p/cpp-libp2p#59

Build fixes broken out into new PR: #128

Error was:

| CMake Error in core/storage/ipld/CMakeLists.txt:
|   Target "ipld_node_protobuf"
|   INTERFACE_INCLUDE_DIRECTORIES property contains path:
|
|     "build/core/storage/ipld"
|
|   which is prefixed in the build directory.
When protobuf is provided by other build systems, it may require discovery
using a Findprotobuf.cmake module instead of the config file. By removing
'CONFIG', we allow CMake to choose.

Error was:

| CMake Error at cmake/dependencies.cmake:18 (find_package):
|   Could not find a package configuration file provided by "Protobuf" with any
|   of the following names:
|
|     ProtobufConfig.cmake
|     protobuf-config.cmake
Error was:

| CMake Error at cmake/dependencies.cmake:22 (find_package):
|   By not providing "FindMicrosoft.GSL.cmake" in CMAKE_MODULE_PATH this
|   project has asked CMake to find a package configuration file provided by
|   "Microsoft.GSL", but CMake did not find one.
|
|   Could not find a package configuration file provided by "Microsoft.GSL"
|   with any of the following names:
|
|     Microsoft.GSLConfig.cmake
|     microsoft.gsl-config.cmake
Instead, allow the build system to provide Cargo, and rely on the build
system to set the PATH appropriately.

We could possibly keep the error message if cargo is not found, but I
believe the error output of add_custom_target() will be useful enough if
cargo is missing.
When tinycbor is provided by an external build system (such as required when
git submodules are not supported), the tinycbor headers are placed in a
'tinycbor/' directory:

| #include <tinycbor/cbor.h>

We need to adjust the cbor sources for this directory, which means our
internal dependency needs to be built slightly differently. We copy the
headers to a folder in the build directory, and point the top-level CMake
to this folder.

Fixes build error:

| In file included from cbor_decode_stream.cpp:6:
| cbor_decode_stream.hpp:13:10: fatal error: cbor.h: No such file or directory
|  #include <cbor.h>
|           ^~~~~~~~
This change shuffles the order of several CMake steps to expose CMake
options to included files.

By defining options before inclusion, included files are able to access
the input provided by the build system.
When building without tests, GTest and GMock are not needed. This change
allows build systems to continue with filecoin compilation without devoting
resources to its testing infrastructure.
Not every build system can handle git submodules, so expose an option to
exclude these dependencies from the build
@eigendude eigendude force-pushed the cross-compile branch 2 times, most recently from 0246b86 to bdf013a Compare March 9, 2020 05:34
@eigendude
Copy link
Contributor Author

Two years later, we've merged #128! And I split out the rest of the CMake changes to #601.

My last question is about installing libraries in addition to the built executables (fuhon-node and fuhon-miner). When embedding Fuhon into a single process (needed for e.g. Android TV), the executables aren't needed, but the compiled libraries are.

I would propose we add two CMake options:

option(INSTALL_EXECUTABLES "Enable installing Fuhon executables" ON)
option(INSTALL_LIBRARIES "Enable installing Fuhon's libraries and headers" OFF)

The first option is standard for any kind of cross-compiling. You can't run executables compiled for another system. But there's no harm in installing files that aren't shipped.

If one or both of these options are desired, I'll open a PR with the changes and close this one.

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.

2 participants