Page MenuHomePhabricator

[Services] Use shared services CMake project from tunnelbroker
ClosedPublic

Authored by jon on Aug 5 2022, 12:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 11:13 AM
Unknown Object (File)
Fri, Nov 22, 11:05 AM
Unknown Object (File)
Fri, Nov 22, 10:57 AM
Unknown Object (File)
Fri, Nov 22, 10:56 AM
Unknown Object (File)
Fri, Nov 22, 5:34 AM
Unknown Object (File)
Sat, Nov 16, 8:16 PM
Unknown Object (File)
Tue, Nov 12, 12:35 AM
Unknown Object (File)
Thu, Nov 7, 8:04 PM

Details

Summary

Use shared services library now that it's exported.

Test Plan

If you're running this on mac, you will also need later fixes for Clang + MacOS.

Please checkout D4766 then run the below workflow:

Docker workflow:

docker build . -f services/tunnelbroker/Dockerfile

Nix worfklow

nix develop
cd services/tunnelbroker
mkdir build && cd build && cmake .. && make

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Aug 5 2022, 12:18 PM
Harbormaster failed remote builds in B11171: Diff 15377!

Remove unneeded include path

Make docker environment more reflective of code base structure

docker build . -f services/tunnelbroker/Dockerfile

Running into the following:

 => ERROR [12/21] RUN ./install_corrosion.sh                              22.7s
------
 > [12/21] RUN ./install_corrosion.sh:
#16 0.266 Cloning into 'corrosion'...
#16 1.035 Note: switching to '28fa50ca44f68313e7345dd59d79599a91df57a7'.
#16 1.035 
#16 1.035 You are in 'detached HEAD' state. You can look around, make experimental
#16 1.035 changes and commit them, and you can discard any commits you make in this
#16 1.035 state without impacting any branches by switching back to a branch.
#16 1.035 
#16 1.035 If you want to create a new branch to retain commits you create, you may
#16 1.035 do so (now or later) by using -c with the switch command. Example:
#16 1.035 
#16 1.035   git switch -c <new-branch-name>
#16 1.035 
#16 1.035 Or undo this operation with:
#16 1.035 
#16 1.035   git switch -
#16 1.035 
#16 1.035 Turn off this advice by setting config variable advice.detachedHead to false
#16 1.035 
#16 1.529 /tmp/corrosion /tmp
#16 2.248 -- The CXX compiler identification is GNU 9.4.0
#16 2.275 -- Check for working CXX compiler: /usr/bin/c++
#16 3.056 -- Check for working CXX compiler: /usr/bin/c++ -- works
#16 3.062 -- Detecting CXX compiler ABI info
#16 3.916 -- Detecting CXX compiler ABI info - done
#16 4.015 -- Detecting CXX compile features
#16 4.019 -- Detecting CXX compile features - done
#16 4.137 info: This is the version for the rustup toolchain manager, not the rustc compiler.
#16 4.493 info: The currently active `rustc` version is `rustc 1.62.1 (e092d0b6b 2022-07-16)`
#16 4.589 -- Rust Toolchain: stable-x86_64-unknown-linux-gnu
#16 4.837 -- Rust Target: x86_64-unknown-linux-gnu
#16 4.840 -- Found Rust: /root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc (found version "1.62.1") 
#16 4.844 -- Using Corrosion as a subdirectory
#16 22.42 Child killed
#16 22.48 CMake Error at cmake/Corrosion.cmake:427 (message):
#16 22.48   corrosion-generator failed: 1
#16 22.48 Call Stack (most recent call first):
#16 22.48   test/cpp2rust/CMakeLists.txt:1 (corrosion_import_crate)
#16 22.48 
#16 22.48 
#16 22.51 -- Configuring incomplete, errors occurred!
#16 22.51 See also "/tmp/corrosion/build/CMakeFiles/CMakeOutput.log".
------
executor failed running [/bin/sh -c ./install_corrosion.sh]: exit code: 1
atul requested changes to this revision.Aug 8 2022, 10:28 AM

For Nix workflow I'm getting the following:

In file included from /Users/atul/comm/services/tunnelbroker/src/Amqp/AmqpManager.cpp:1:
In file included from /Users/atul/comm/services/tunnelbroker/src/Amqp/AmqpManager.h:6:
/nix/store/3vkd0v2jnpihwakl088fjj4zwv9ylcxn-amqp-cpp-4.3.16/include/amqpcpp/libuv.h:22:10: fatal error: 'uv.h' file not found
#include <uv.h>
         ^~~~~~
1 error generated.
make[2]: *** [CMakeFiles/tunnelbroker.dir/src/Amqp/AmqpManager.cpp.o] Error 1
make[1]: *** [CMakeFiles/tunnelbroker.dir/all] Error 2
make: *** [all] Error 2
This revision now requires changes to proceed.Aug 8 2022, 10:28 AM

Ensure macOS changes have been rebased

atul requested changes to this revision.Aug 8 2022, 11:29 AM

Running into the following:

atuls-MacBook-Pro:comm atul$ cd services/tunnelbroker
atuls-MacBook-Pro:tunnelbroker atul$ mkdir build && cd build && cmake .. && make
-- The C compiler identification is AppleClang 13.1.6.13160021
-- The CXX compiler identification is AppleClang 13.1.6.13160021
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PkgConfig: /nix/store/wf9rhgv9fl49s7cmck0x3gly318f0awz-pkg-config-wrapper-0.29.2/bin/pkg-config (found version "0.29.2") 
-- Found Boost: /nix/store/7rrgbk62knkpf4jngbrmzpjc002j2q65-boost-1.79.0-dev/lib/cmake/Boost-1.79.0/BoostConfig.cmake (found suitable version "1.79.0", minimum required is "1.51.0") found components: context filesystem program_options regex system thread 
-- Found folly: /nix/store/27vgfzvc113qcc165fviqbf5crfvj8rz-folly-2022.07.04.00
CMake Warning (dev) at /nix/store/cg3f9hs6zdag48yi8zdpg3kwq9ffcpbj-cmake-3.23.2/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
  The package name passed to `find_package_handle_standard_args` (Protobuf)
  does not match the name of the calling package (protobuf).  This can lead
  to problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  /nix/store/cg3f9hs6zdag48yi8zdpg3kwq9ffcpbj-cmake-3.23.2/share/cmake-3.23/Modules/Findprotobuf.cmake:650 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  CMakeLists.txt:17 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Found Protobuf: /nix/store/r5nv2l4hiraggzawpjhxhbqgnc93p0a9-protobuf-3.15.8/lib/libprotobuf.dylib (found version "3.15.8") 
-- Found ZLIB: /nix/store/lkgrb3lfingdcxr9fzc847c5wihk1w73-zlib-1.2.12/lib/libz.dylib (found version "1.2.12") 
-- Found OpenSSL: /nix/store/1yhkn4rmhxpxdpvf10m10x5z60sxwhkr-openssl-1.1.1q/lib/libcrypto.dylib (found version "1.1.1q")  
-- Found c-ares: /Users/atul/.local/lib/cmake/c-ares/c-ares-config.cmake (found version "1.14.0") 
-- Found Threads: TRUE  
-- Found RE2 via CMake.
-- Checking for module 'libcryptopp=8.6'
--   Found libcryptopp, version 8.6
-- Checking for module 'libuv>=1.43'
--   Found libuv, version 1.44.1
CMake Warning (dev) at /nix/store/cg3f9hs6zdag48yi8zdpg3kwq9ffcpbj-cmake-3.23.2/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
  The package name passed to `find_package_handle_standard_args` (Protobuf)
  does not match the name of the calling package (protobuf).  This can lead
  to problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  /nix/store/cg3f9hs6zdag48yi8zdpg3kwq9ffcpbj-cmake-3.23.2/share/cmake-3.23/Modules/Findprotobuf.cmake:650 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  /Users/atul/comm/native/cpp/CommonCpp/grpc/CMakeLists.txt:5 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Found AWS SDK for C++, Version: 1.9.238, Install Root:, Platform Prefix:, Platform Dependent Libraries: pthread;curl
-- Components specified for AWSSDK: core;dynamodb, application will be depending on libs: aws-cpp-sdk-dynamodb;aws-cpp-sdk-core
-- Try finding aws-cpp-sdk-core
-- Found aws-cpp-sdk-core
-- Try finding aws-cpp-sdk-dynamodb
-- Found aws-cpp-sdk-dynamodb
-- Found Boost: /nix/store/7rrgbk62knkpf4jngbrmzpjc002j2q65-boost-1.79.0-dev/lib/cmake/Boost-1.79.0/BoostConfig.cmake (found suitable version "1.79.0", minimum required is "1.40") found components: program_options 
CMake Warning (dev) at /nix/store/cg3f9hs6zdag48yi8zdpg3kwq9ffcpbj-cmake-3.23.2/share/cmake-3.23/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
  The package name passed to `find_package_handle_standard_args` (Protobuf)
  does not match the name of the calling package (protobuf).  This can lead
  to problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  /nix/store/cg3f9hs6zdag48yi8zdpg3kwq9ffcpbj-cmake-3.23.2/share/cmake-3.23/Modules/Findprotobuf.cmake:650 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  /Users/atul/comm/services/lib/src/CMakeLists.txt:26 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Found AWS SDK for C++, Version: 1.9.238, Install Root:, Platform Prefix:, Platform Dependent Libraries: pthread;curl
-- Components specified for AWSSDK: core;dynamodb, application will be depending on libs: aws-cpp-sdk-dynamodb;aws-cpp-sdk-core
-- Try finding aws-cpp-sdk-core
-- Found aws-cpp-sdk-core
-- Try finding aws-cpp-sdk-dynamodb
-- Found aws-cpp-sdk-dynamodb
-- Found Boost: /nix/store/7rrgbk62knkpf4jngbrmzpjc002j2q65-boost-1.79.0-dev/lib/cmake/Boost-1.79.0/BoostConfig.cmake (found suitable version "1.79.0", minimum required is "1.40") found components: program_options thread system context filesystem regex 
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.62.1 (e092d0b6b 2022-07-16)`
-- Rust Toolchain: stable-aarch64-apple-darwin
-- Rust Target: aarch64-apple-darwin
-- Found Rust: /Users/atul/.rustup/toolchains/stable-aarch64-apple-darwin/bin/rustc (found version "1.62.1") 
-- Defaulting Cargo to build debug
-- Using Corrosion as a subdirectory
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/atul/comm/services/tunnelbroker/build
   Compiling proc-macro2 v1.0.40
   Compiling quote v1.0.20
   Compiling unicode-ident v1.0.1
   Compiling syn v1.0.98
   Compiling autocfg v1.1.0
   Compiling cc v1.0.73
   Compiling libc v0.2.126
   Compiling cfg-if v1.0.0
   Compiling pin-project-lite v0.2.9
   Compiling memchr v2.5.0
   Compiling log v0.4.17
   Compiling futures-core v0.3.21
   Compiling once_cell v1.13.0
   Compiling futures-task v0.3.21
   Compiling bytes v1.1.0
   Compiling futures-sink v0.3.21
   Compiling futures-channel v0.3.21
   Compiling futures-util v0.3.21
   Compiling untrusted v0.7.1
   Compiling slab v0.4.6
   Compiling itoa v1.0.2
   Compiling futures-io v0.3.21
   Compiling pin-utils v0.1.0
   Compiling fnv v1.0.7
   Compiling pkg-config v0.3.25
   Compiling hashbrown v0.12.2
   Compiling httparse v1.7.1
   Compiling serde v1.0.139
   Compiling try-lock v0.2.3
   Compiling base64 v0.13.0
   Compiling serde_derive v1.0.139
   Compiling erased-serde v0.3.21
   Compiling foreign-types-shared v0.1.1
   Compiling serde_json v1.0.82
   Compiling tower-service v0.3.2
   Compiling httpdate v1.0.2
   Compiling openssl v0.10.41
   Compiling ryu v1.0.10
   Compiling bitflags v1.3.2
   Compiling lazy_static v1.4.0
   Compiling tracing-core v0.1.28
   Compiling tokio v1.20.0
   Compiling indexmap v1.9.1
   Compiling http v0.2.8
   Compiling foreign-types v0.3.2
   Compiling ring v0.16.20
   Compiling openssl-sys v0.9.75
   Compiling http-body v0.4.5
   Compiling want v0.3.0
   Compiling mio v0.8.4
   Compiling socket2 v0.4.4
   Compiling num_cpus v1.13.1
   Compiling webpki v0.21.4
   Compiling sct v0.6.1
   Compiling rustls v0.19.1
   Compiling webpki-roots v0.21.1
   Compiling tokio-rustls v0.22.0
   Compiling futures-macro v0.3.21
   Compiling tracing-attributes v0.1.22
   Compiling openssl-macros v0.1.0
   Compiling thiserror-impl v1.0.31
   Compiling thiserror v1.0.31
   Compiling tracing v0.1.35
   Compiling tokio-util v0.7.3
   Compiling h2 v0.3.13
   Compiling futures-executor v0.3.21
   Compiling futures v0.3.21
   Compiling hyper v0.14.20
   Compiling hyper-alpn v0.3.0
   Compiling a2 v0.6.2
   Compiling rust-notifications v0.1.0 (/Users/atul/comm/services/tunnelbroker/rust-notifications)
    Finished dev [unoptimized + debuginfo] target(s) in 10.16s
[  0%] Built target cargo-build_rust-notifications
[  4%] Building CXX object common/CMakeFiles/comm-services-common.dir/DatabaseManagerBase.cpp.o
[  9%] Building CXX object common/CMakeFiles/comm-services-common.dir/DynamoDBTools.cpp.o
[ 14%] Building CXX object common/CMakeFiles/comm-services-common.dir/GlobalTools.cpp.o
[ 19%] Linking CXX static library libcomm-services-common.a
[ 19%] Built target comm-services-common
[ 23%] Building CXX object protos/CMakeFiles/comm-tunnelbroker-grpc.dir/_generated/tunnelbroker.pb.cc.o
[ 28%] Building CXX object protos/CMakeFiles/comm-tunnelbroker-grpc.dir/_generated/tunnelbroker.grpc.pb.cc.o
[ 33%] Linking CXX static library libcomm-tunnelbroker-grpc.a
[ 33%] Built target comm-tunnelbroker-grpc
[ 38%] Building CXX object CMakeFiles/tunnelbroker.dir/src/Amqp/AmqpManager.cpp.o
In file included from /Users/atul/comm/services/tunnelbroker/src/Amqp/AmqpManager.cpp:1:
In file included from /Users/atul/comm/services/tunnelbroker/src/Amqp/AmqpManager.h:6:
/nix/store/3vkd0v2jnpihwakl088fjj4zwv9ylcxn-amqp-cpp-4.3.16/include/amqpcpp/libuv.h:22:10: fatal error: 'uv.h' file not found
#include <uv.h>
         ^~~~~~
1 error generated.
make[2]: *** [CMakeFiles/tunnelbroker.dir/src/Amqp/AmqpManager.cpp.o] Error 1
make[1]: *** [CMakeFiles/tunnelbroker.dir/all] Error 2
make: *** [all] Error 2
This revision now requires changes to proceed.Aug 8 2022, 11:29 AM

@atul I split the diffs out to be smaller in scope, but for running this natively on macOS (without docker), you will need the other diff leading up to and including D4766

If you checkout D4766 then re-run the steps, that should work

atul requested changes to this revision.Aug 8 2022, 1:10 PM

Confirmed that everything works in D4766.

Wasn't able to get the Docker or Nix instructions to work when I patch this diff.

Do you think it's okay to land based on everything in D4766 working as expected?

This revision now requires changes to proceed.Aug 8 2022, 1:10 PM

Wasn't able to get the Docker or Nix instructions to work when I patch this diff.

Do you think it's okay to land based on everything in D4766 working as expected?

The smaller diffs are to help with review, but yes, the individual diffs might not be correct on MacOS+Nix until all of them are applied.

If you were running this on a x86_64-linux machine, then all of this would be green to green (which I know is not the desired end goal, but we started from only a linux-docker container).

If you want me to, I can squash everything so that they are easier to do test plans.

@ashoat, what would you say is best practice? I've been optimizing for "small diffs, easier to read; but they require another diff to work", should I instead do, "larger diffs, easier to test; but hard to read"?

Having MacOS, linux, clang, and docker all happy at the same time when changing workflows is pretty hard.

Do you think it's okay to land based on everything in D4766 working as expected?

The goal with these small diffs were supposed to make be easier to say, "The diff is trying to just do X, and the changes look like they are just trying to do X". Even if the workflow end-to-end requires me to do Y and Z before getting to the desired end state.

That makes sense, can see how getting the build to work across so many platforms and environments (x86 Darwin, arm Darwin, nix, docker, etc etc) can get gnarly/hard to break into stages where everything works.

Agree with your assessment that we should probably lean towards reviewability here.

I think if all of the changes are landed atomically, this should be good to go!

This revision is now accepted and ready to land.Aug 8 2022, 1:26 PM