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

Overhaul static build mechanism #1173

Merged
merged 25 commits into from
Jun 16, 2020
Merged

Overhaul static build mechanism #1173

merged 25 commits into from
Jun 16, 2020

Conversation

jagerman
Copy link
Member

This PR collects together various scripts that we were using from custom docker scripts, ci scripts, and contrib/depends and re-synthesizes it into a cmake-based static build script that builds the necessary dependencies (boost, readline, unbound, etc.) and then builds loki using those dependencies.

The short takeaway is that you can use one of these to build a static binary by making some build directory and then running one of:

    # Build a static binary on the current system; works on Linux, Mac:
    cmake -DSTATIC_BUILD_DEPS=ON ..
    # For building a mac you can target an older macOS release using:
    cmake -DSTATIC_BUILD_DEPS=ON -DCMAKE_OSX_DEPLOYMENT_TARGET=10.13 ..
    # To build for Windows from a linux box:
    cmake -DCMAKE_TOOLCHAIN_FILE=../cmake/64-bit-toolchain.cmake -DBUILD_STATIC_DEPS=ON ..

followed by

    make
    make strip_binaries
    make create_archive

Which will build, strip, and create a lokid-x.y.z-dev-749ab75.tar.xz (or .zip for Windows) archive containing the static binaries.

All three builds are also added to drone and get uploaded to https://builds.lokinet.dev as part of the build (so that you can just push a branch and wait for drone to build and upload it). Sample from this PR at creation time: https://builds.lokinet.dev/jagerman/loki/static-builds/

Misc comments:

  • Enable ccache by default. Monero had a similar change that didn't get included in the monero merge because it was done poorly; did it properly here: enabled by default if we find ccache in the path, and respects the user passing a -DCMAKE_C[XX]_COMPILER_LAUNCHER to override or disable it.
  • USE_LTO now defaults to ON for a Release build. This is particularly useful for the static binaries as LTO reduces the overall archive size by nearly half. It's something to be aware of though for dev builds - it makes linking take a fair bit longer, so -DUSE_LTO=OFF might be recommended for Release builds.
  • Changed the way we set C++14 compilation mode to let CMake do it rather than hacking it into the global CXXFLAGS.
  • Similarly LTO enabling now also goes via built-in CMake handling.
  • There was some disgusting translation code using C code to produce C++ code. Aside from being nasty, this caused trouble when cross-compiling since the compiler isn't producing binaries that run on the host system (and appeared to require some unholy multi-stage build importing a generated cmake script into a separate, native system build). I threw it away and rewrote it using a cmake template instead which is simpler, works better, and is easier to understand.
  • Doing these builds uncovered a bunch of small compilation issues and warnings; I rewrote the PR to front-load them before the static build changes so that they can be reviewed easily one-by-one. (There's nothing really major here).

There's really no reason to submodule it - we work with pretty much any
libunbound version, and it's a very commonly available library
(comparable to sqlite3 or boost, which we don't submodule).

This removes the submodule and switches it to a hard dependency.
The monero version is just an improperly forked mirror of upstream that
tends to lag behind.  Switch to upstream miniupnp instead.
It can still be explicitly disabled with

    -DCMAKE_CXX_COMPILER_LAUNCHER= -DCMAKE_C_COMPILER_LAUNCHER=

if someone doesn't want it for some reason.
static_assert is required both by C++11 and C11; if we don't have a
standard compliant compiler then compilation should fail, not be hacked
like this.

The C++ version of this definition is particularly preposterous; the C
version is probably just covering up that the C code forget to include
the `<assert.h>` header where the `static_assert` macro is defined.
CMake has standard interfaces for this; hacking it into CXX_FLAGS is
just gross.
CMake 3.9+ has generic LTO enabling code, switch to it.

Update required cmake version to 3.10 (3.9 is probably sufficient, but
3.10 is bionic's version that we're actually testing).
Lower-case "release" is not a valid cmake build type (it's "Release"),
so fix it.
- libzmq required version was much older than we actually require

- libnorm and libpgm are transitive dependencies of libzmq3, it makes no
sense to list them here (rather the libzmq3-dev package should depend on
them if they are needed).

- gtest is now always build from the submodule copy

- the readline dev package was wrong for debian/ubuntu.
To be reverted with C++17 changes.
I have no idea where it comes from, but the compilation fails as a
result.  This is why macros are EVIL.
The mismatch in the extern declaration spews warnings under LTO (quite
rightly).  It's kind of surprising that the mismatched declaration and
implementation even worked.
Clang (rightly) warns about this.
The same struct `lazy_init` was being used in two different files linked
into the same binary, causing test failures depending on which one got
kept in the final binary.  (Enabling LTO builds noticed the violation,
warned about it, and caused the spent_outputs.not_found test to fail).
This makes three big changes to how translation files are generated:

- use Qt5 cmake built-in commands to do the translations rather than
calling lrelease directly.  lrelease is often not in the path, while Qt5
cmake knows how to find and invoke it.

- Slam the resulting files into a C++ file using a cmake script rather
than needing to compile a .c file to generate C++ file.  This is
simpler, but more importantly avoids the mess needed when cross
compiling of having to import a cmake script from an external native
build.

- In the actual generated files, use an unordered_map rather than a
massive list of static variable pointers.
This adds a static dependency script for libraries like boost, unbound,
etc. to cmake, invokable with:

    cmake .. -DBUILD_STATIC_DEPS=ON

which downloads and builds static versions of all our required
dependencies (boost, unbound, openssl, ncurses, etc.).  It also implies
-DSTATIC=ON to build other vendored deps (like miniupnpc, lokimq) as
static as well.

Unlike the contrib/depends system, this is easier to maintain (one
script using nicer cmake with functions instead of raw Makefile
spaghetti code), and isn't concerned with reproducible builds -- this
doesn't rebuild the compiler, for instance.  It also works with the
existing build system so that it is simply another way to invoke the
cmake build scripts but doesn't require any external tooling.

This works on Linux, Mac, and Windows.

Some random comments on this commit (for preserving history):

- Don't use target_link_libraries on imported targets.  Newer cmake is
fine with it, but Bionic's cmake doesn't like it but seems okay with
setting the properties directly.

- This rebuilds libzmq and libsodium, even though there is some
provision already within loki-core to do so: however, the existing
embedded libzmq fails with the static deps because it uses libzmq's
cmake build script, which relies on pkg-config to find libsodium which
ends up finding the system one (or not finding any), rather than the one
we build with DownloadLibSodium.  Since both libsodium and libzmq are
faily simple builds it seemed easiest to just add them to the cmake
static build rather than trying to shoehorn the current code into the
static build script.

- Half of the protobuf build system ignores CC/CXX just because Google,
and there's no documentation anywhere except for a random closed bug
report about needing to set these other variables (CC_FOR_BUILD,
CXX_FOR_BUILD) instead, but you need to.  Thanks Google.

- The boost build is set to output very little because even the minimum
-d1 output level spams ~15k lines of output just for the headers it
installs.
From a cmake build dir (`make` used for simple example; can also be
something else, such as ninja):

    make create_tarxz
    make create_zip

creater a loki-<OS>[-arch]-x.y.z[-dev]-<GITHASH>.tar.xz or .zip.

    make create_archive

decides what to do based on the build type: creates a .zip for a windows
build, a tar.xz for anything else.  (We have been distributing the macOS
binaries as a .zip but that seems unnecessary: I tested on our dev mac
and a .tar.xz offers exactly the same UX as a .zip, but is noticeably
smaller).

From the top-level makefile there is also a new `make
release-full-static-archive` that does a full static build (include all
deps) and builds the archive.
Adds static drone builds for linux (built on bionic), mac, and Windows
(built with mingw32).

The builds get uploaded to https://builds.lokinet.dev

The linux and mac builds use LTO (which takes longer, but significantly
reduces binary size).  The mingw32 build can probably also get there,
but currently fails with LTO when unbound tries linking against openssl
(it probably just needs a small patch to add magic -lwsock2 dep in the
right place in unbound).

The Mac binaries are built using a 10.13 deployment target.  (I'm not
100% sure that this is sufficient -- it's possible we might have to also
push the magic mac deployment flag to the built dependencies).
I've uploaded the various deps to https://builds.lokinet.dev/deps so as
not to hit the upstream mirrors so much (also reduces the reliance on
github, which is sometimes flakey).

This is prefixed onto the existing URLs so that if it isn't found we
fall back to the upstream URL.
This makes it a little clearer what failed when something fails.

Ideally we'd also be able to distinguish between build and test, but
currently that's not practical to do with drone: between each stage of
the build the docker image gets removed and replaced with the next
stage's image, which means we'd have to build up a new docker image with
dependencies before running the tests.

(Being able to have multiple stages in one docker image is a drone
upstream wishlist item).
@Doy-lee Doy-lee merged commit 998960e into oxen-io:dev Jun 16, 2020
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