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

runtime: cargo test fails with modern GCC #3947

Closed
Yawning opened this issue May 14, 2021 · 6 comments · Fixed by #3968
Closed

runtime: cargo test fails with modern GCC #3947

Yawning opened this issue May 14, 2021 · 6 comments · Fixed by #3968

Comments

@Yawning
Copy link
Contributor

Yawning commented May 14, 2021

cargo test fails to build with newer gcc (11.1.1) due to C dependencies pulled in by our ancient version of grpcio-sys.

  /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.4.7/grpc/third_party/boringssl/third_party/fiat/curve25519.c:1905:57: error: argument 2 of type ‘const uint8_t *’ {aka ‘const unsigned char *’} declared as a pointer [-Werror=array-parameter=]
   1905 | void x25519_ge_scalarmult_base(ge_p3 *h, const uint8_t *a) {
        |                                          ~~~~~~~~~~~~~~~^
  In file included from /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.4.7/grpc/third_party/boringssl/third_party/fiat/curve25519.c:41:
  /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.4.7/grpc/third_party/boringssl/third_party/fiat/internal.h:125:56: note: previously declared as an array ‘const uint8_t[32]’ {aka ‘const unsigned char[32]’}
    125 | void x25519_ge_scalarmult_base(ge_p3 *h, const uint8_t a[32]);
        |                                          ~~~~~~~~~~~~~~^~~~~
  cc1: all warnings being treated as errors
  gmake[3]: *** [third_party/boringssl/third_party/fiat/CMakeFiles/fiat.dir/build.make:82: third_party/boringssl/third_party/fiat/CMakeFiles/fiat.dir/curve25519.c.o] Error 1
  gmake[2]: *** [CMakeFiles/Makefile2:3544: third_party/boringssl/third_party/fiat/CMakeFiles/fiat.dir/all] Error 2
  gmake[2]: *** Waiting for unfinished jobs....
  gmake[1]: *** [CMakeFiles/Makefile2:1727: CMakeFiles/grpc.dir/rule] Error 2
  gmake: *** [Makefile:300: grpc] Error 2
  thread 'main' panicked at '
  command did not execute successfully, got: exit status: 2
@kostko
Copy link
Member

kostko commented May 18, 2021

Can you check if it works with #3950?

@Yawning
Copy link
Contributor Author

Yawning commented May 18, 2021

Can you check if it works with #3950?

  /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.8.1/grpc/third_party/abseil-cpp/absl/synchronization/internal/graphcycles.cc: In member function ‘void absl::lts_2020_09_23::synchronization_internal::GraphCycles::RemoveNode(void*)’:
  /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.8.1/grpc/third_party/abseil-cpp/absl/synchronization/internal/graphcycles.cc:451:26: error: ‘numeric_limits’ is not a member of ‘std’
    451 |   if (x->version == std::numeric_limits<uint32_t>::max()) {
        |                          ^~~~~~~~~~~~~~
  /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.8.1/grpc/third_party/abseil-cpp/absl/synchronization/internal/graphcycles.cc:451:49: error: expected primary-expression before ‘>’ token
    451 |   if (x->version == std::numeric_limits<uint32_t>::max()) {
        |                                                 ^
  /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.8.1/grpc/third_party/abseil-cpp/absl/synchronization/internal/graphcycles.cc:451:52: error: ‘::max’ has not been declared; did you mean ‘std::max’?
    451 |   if (x->version == std::numeric_limits<uint32_t>::max()) {
        |                                                    ^~~
        |                                                    std::max
  In file included from /usr/include/c++/11/algorithm:62,
                   from /home/yawning/.cargo/registry/src/github.com-1ecc6299db9ec823/grpcio-sys-0.8.1/grpc/third_party/abseil-cpp/absl/synchronization/internal/graphcycles.cc:38:
  /usr/include/c++/11/bits/stl_algo.h:3467:5: note: ‘std::max’ declared here
   3467 |     max(initializer_list<_Tp> __l, _Compare __comp)
        |     ^~~
  gmake[3]: *** [third_party/abseil-cpp/absl/synchronization/CMakeFiles/absl_graphcycles_internal.dir/build.make:82: third_party/abseil-cpp/absl/synchronization/CMakeFiles/absl_graphcycles_internal.dir/internal/graphcycles.cc.o] Error 1
  gmake[2]: *** [CMakeFiles/Makefile2:3340: third_party/abseil-cpp/absl/synchronization/CMakeFiles/absl_graphcycles_internal.dir/all] Error 2
  gmake[2]: *** Waiting for unfinished jobs....
  gmake[1]: *** [CMakeFiles/Makefile2:1009: CMakeFiles/grpc.dir/rule] Error 2
  gmake: *** [Makefile:261: grpc] Error 2
  thread 'main' panicked at '
  command did not execute successfully, got: exit status: 2

It managed to move the error around at least.

@kostko
Copy link
Member

kostko commented May 18, 2021

This is an upstream issue, see tikv/grpc-rs#524.

@kostko
Copy link
Member

kostko commented May 25, 2021

@Yawning can you check if #3968 fixes this issue for you?

@Yawning
Copy link
Contributor Author

Yawning commented May 25, 2021

Yeah it does. While I'm complaining about this test battery, can we do something about

error: environment variable `OASIS_STORAGE_PROTOCOL_SERVER_BINARY` not defined
  --> runtime/src/storage/mkvs/interop/protocol_server.rs:24:46
   |
24 | const PROTOCOL_SERVER_BINARY: &'static str = env!("OASIS_STORAGE_PROTOCOL_SERVER_BINARY");
   |                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info)

As well? (Eg: auto-skip/fail the tests if it isn't defined)

@kostko
Copy link
Member

kostko commented May 25, 2021

You could run make test-unit-rust to set these up for you. But I agree that we should fix the above to not fail compilation in case the env variable is unset.

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 a pull request may close this issue.

2 participants