librustzcash
librustzcash copied to clipboard
`zcash_client_sqlite`: Figure out how to build SQLite in a way that is compatible with Rust, or require downstreams to do so
The ECC wallet team opened https://github.com/mozilla/rust-android-gradle/issues/105 because they were having issues using rust-android-gradle
with the Android emulator on x86_64
machines. My subsequent investigation (https://github.com/mozilla/rust-android-gradle/issues/105#issuecomment-1481879021 and below) indicates that this is actually an incompatibility between SQLite and Rust:
- SQLite uses
long double
to preserve as much precision as possible in various places, primarily ASCII-to-float andprintf
. This causes it to depend on compiler builtins using 80-bit or 128-bit floats where support is available. - Rust does not support floats larger than
double
, so does not implement a bunch of "standard" compiler builtins related to them.
This problem went unnoticed on Android for a long time because:
- In NDK 22 and below,
libgcc
is linked for unwinding purposes, and it happens to provides these compiler builtin symbols, so there were no linker problems. - In NDK 23 and above,
libgcc
is absent, but Rust for some reason provides the needed symbols foraarch64-linux-android
targets.
We currently use SQLite via the rusqlite
crate's bundled
flag, which automatically compiles and bundles SQLite using whatever C compiler the cc
toolchain picks up. Technically we shouldn't do this here; it should be up to the user of zcash_client_sqlite
to choose how it links SQLite. In any case, the current setup means that the C compiler assumes these symbols will be available, but Rust doesn't provide them. On desktop I assume we just get the symbols from the C compiler itself, but on Android they came from the NDK which no longer provides them.
We need to figure out how to set up rusqlite
to build SQLite in such a way that the necessary symbols are always provided. This may mean removing the bundled
feature flag and pushing the problem onto downstream users, that have the necessary context to better solve the issue.
On desktop I assume we just get the symbols from the C compiler itself, but on Android they came from the NDK which no longer provides them.
On a desktop, libgcc (strictly, libgcc_s.so.1) is typically dynamically linked. It is one of the few libraries we dynamically link to in zcashd, despite statically linking most of the dependencies.
We currently use SQLite via the
rusqlite
crate'sbundled
flag, which automatically compiles and bundles SQLite using whatever C compiler thecc
toolchain picks up. Technically we shouldn't do this here; it should be up to the user of zcash_client_sqlite to choose how it links SQLite.
I agree, we shouldn't do that. The system cc
could be broken in various ways, and is not pinned. Even though the SQLite build can't/shouldn't affect consensus compatibility, the app that uses zcash_client_sqlite
might want to ensure a deterministic build or otherwise control build options.
On the other hand, the description of the bundled
option says:
This is a good option for cases where linking to SQLite is complicated, such as Windows.
The plight of a beleaguered dev who upgrades zcash_client_sqlite
and is suddenly faced with having to figure out how to link SQLite on Windows, fills me with sympathy and solidarity — so I propose that we add a non-default bundled
feature of zcash_client_sqlite
that enables the bundled
feature of rusqlite
, and document its limitations.
Ah, the cc
crate used to build the bundled SQLite will pay attention to CC
and CFLAGS
environment variables (which can be configured per-platform): https://github.com/rust-lang/cc-rs#external-configuration-via-environment-variables
I verified that cargo clean; CC=clang cargo check
works for zcash_client_sqlite
, and that cargo clean; CC=this-compiler-does-not-exist cargo check
fails as you'd expect.
If we figure out what CFLAGS
are needed, this approach could solve both the missing compiler builtins and the nondeterministic build issues, while still delegating to rusqlite / libsqlite3-sys to build the correct version of SQLite for its bindings.
@str4d @daira some new idea or solution for this problem? We just started to have this problem building zingo-mobile for Android...
cc: @zancas @AloeareV
We ended up solving this in the Android SDK with a build script, much the same way as others have: https://github.com/Electric-Coin-Company/zcash-android-wallet-sdk/blob/88058c63461f2808efc953af70db726b9f36f9b9/backend-lib/build.rs
Reopening because even with the above fix, we might still want to enable downstreams to control whether SQLite is bundled or not.