signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.51k stars 409 forks source link

Unable to compile `v0.36.1` (JNI) - boring-sys requires git to be installed #549

Closed marsavar closed 9 months ago

marsavar commented 9 months ago

Hi,

I'm building libsignal in a Docker container using the java/build_jni.sh script. This has worked before, up until v0.32.1. However, when trying to build v0.36.1, it fails with the error below, which appears to be related to boring-sys:

#28 85.01    Compiling boring-sys v3.1.0 (https://github.com/signalapp/boring?branch=libsignal#8245063a)
#28 85.08    Compiling toml_datetime v0.6.5
#28 85.09    Compiling tinyvec_macros v0.1.1
#28 85.23    Compiling tinyvec v1.6.0
#28 85.25    Compiling tokio v1.33.0
#28 86.04    Compiling darling v0.14.4
#28 86.09    Compiling rand v0.8.5
#28 86.76    Compiling toml_edit v0.19.15
#28 87.70    Compiling ctr v0.9.2
#28 87.83    Compiling aes v0.8.3
#28 88.63    Compiling foreign-types-macros v0.2.3
#28 88.79 error: failed to run custom build command for `boring-sys v3.1.0 (https://github.com/signalapp/boring?branch=libsignal#8245063a)`
#28 88.79 
#28 88.79 Caused by:
#28 88.79   process didn't exit successfully: `/usr/src/app/libsignal/libsignal-0.36.1/target/release/build/boring-sys-2dfa691a37eac005/build-script-build` (exit status: 101)
#28 88.79   --- stdout
#28 88.79   cargo:rerun-if-env-changed=BORING_BSSL_PATH
#28 88.79   cargo:rerun-if-env-changed=BORING_BSSL_INCLUDE_PATH
#28 88.79   cargo:rerun-if-env-changed=BORING_BSSL_SOURCE_PATH
#28 88.79   cargo:rerun-if-env-changed=BORING_SSL_PRECOMPILED_BCM_O
#28 88.79   cargo:rerun-if-env-changed=BORINGSSL_BUILD_DIR
#28 88.79 
#28 88.79   --- stderr
#28 88.79   thread 'main' panicked at /root/.cargo/git/checkouts/boring-b37daebd62069023/8245063/boring-sys/build.rs:523:34:
#28 88.79   called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
#28 88.79   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
#28 88.79 warning: build failed, waiting for other jobs to finish...
#28 ERROR: process "/bin/sh -c ./build_jni.sh desktop" did not complete successfully: exit code: 101
------
 > [libsignal-build 12/15] RUN ./build_jni.sh desktop:
88.79   cargo:rerun-if-env-changed=BORING_BSSL_INCLUDE_PATH
88.79   cargo:rerun-if-env-changed=BORING_BSSL_SOURCE_PATH
88.79   cargo:rerun-if-env-changed=BORING_SSL_PRECOMPILED_BCM_O
88.79   cargo:rerun-if-env-changed=BORINGSSL_BUILD_DIR
88.79 
88.79   --- stderr
88.79   thread 'main' panicked at /root/.cargo/git/checkouts/boring-b37daebd62069023/8245063/boring-sys/build.rs:523:34:
88.79   called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }
88.79   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
88.79 warning: build failed, waiting for other jobs to finish...

It loos like the build script can't find a required file. Any help would be greatly appreciated!

jrose-signal commented 9 months ago

Ugh, I believe this is boring-sys spuriously wanting git to be installed (https://github.com/signalapp/libsignal/commit/90bbc3b15698119662ff5c01b603dcf6fdfeb6df). I think there's a feature flag we can use now to turn that off, but v0.36 doesn't have it. Can you add git to your Docker image and see if it helps?

marsavar commented 9 months ago

Thanks @jrose-signal, that seems to have done the trick. boring-sys now compiles fine, but I'm running into an issue where libsignal-protocol fails to compile. Looking at some previous commits, it looks like prost was also recently bumped from v0.9 to v0.12, but as of v11, protoc is no longer bundled in the library, so protobuf-compiler also needs to be installed in the Dockerfile. I'm using the amazoncorretto:21 image, but unfortunately its package manager has a fairly old version of protobuf-compiler which doesn't support the experimental flag, so the build script fails with the following error:

#26 285.0 error: failed to run custom build command for `libsignal-protocol v0.1.0 (/usr/src/app/libsignal/libsignal-0.36.1/rust/protocol)`
#26 285.0
#26 285.0 Caused by:
#26 285.0   process didn't exit successfully: `/usr/src/app/libsignal/libsignal-0.36.1/target/release/build/libsignal-protocol-718afc7d65a4d7bd/build-script-build` (exit status: 101)
#26 285.0   --- stderr
#26 285.0   thread 'main' panicked at rust/protocol/build.rs:18:10:
#26 285.0   Protobufs in src are valid: Custom { kind: Other, error: "protoc failed: Unknown flag: --experimental_allow_proto3_optional\n" }

This seems solvable though, I believe I just need to get a more recent version of protobuf-compiler from another repository or download a pre-compiled release (I'd like to avoid compiling from source if possible). Thanks again for your help so far!

jrose-signal commented 9 months ago

We probably should have called these updated build requirements out in the release notes. Let us know if you run into further issues after the protoc update!

marsavar commented 9 months ago

I edited the Dockerfile to download a pre-built protoc binary, and libsignal-protocol compiled just fine. All is well that ends well! I'm happy to submit a PR to update the readme file in this repo if that's ok with you :-)

jrose-signal commented 9 months ago

I want to look into boring-sys a little more to see if I'm right about the purpose of that feature flag. I'll keep this issue open for that. Glad it's working now, though!

jrose-signal commented 9 months ago

Okay, so the feature flag would avoid the git dependency, but because it's a feature flag, it's not necessarily appropriate to apply it if someone wants to test out any of the other features that require patches. A more polite build script would check if there are any patches to apply, and not try to use git otherwise, but in the mean time I think editing the readme would be fine. (Which I can take care of, since it's easier to get into a subsequent release that way.)

jrose-signal commented 9 months ago

Readme updated in v0.38.0. Thanks for pointing this out!