From f78507f4e497ad84e800d31371daf8e7e745cb36 Mon Sep 17 00:00:00 2001 From: lia <167905060+lia-viam@users.noreply.github.com> Date: Fri, 18 Oct 2024 13:29:02 -0400 Subject: [PATCH] RSDK-8822: Whole archive linking (#306) --- .github/workflows/test.yml | 7 ++++- CMakeLists.txt | 14 +++------ conanfile.py | 17 +++++++++- etc/docker/tests/run_test.sh | 31 ++++++++++++++----- .../examples/modules/complex/CMakeLists.txt | 18 ++++++----- src/viam/sdk/CMakeLists.txt | 7 ++++- test_package/conanfile.py | 26 ++++++++++++---- 7 files changed, 86 insertions(+), 34 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 19cd55917..7cdcc4410 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -10,6 +10,11 @@ jobs: runs-on: ubuntu-latest container: image: ghcr.io/viamrobotics/canon:amd64 + strategy: + matrix: + include: + - BUILD_SHARED: ON + - BUILD_SHARED: OFF steps: - uses: actions/checkout@v4 ########################################### @@ -18,4 +23,4 @@ jobs: - name: build-docker-test run: | docker build -t cpp . -f etc/docker/tests/Dockerfile.debian.bullseye - docker run --rm -i -w /tmp cpp /bin/bash + docker run -e BUILD_SHARED=${{ matrix.BUILD_SHARED }} --rm -i -w /tmp cpp /bin/bash diff --git a/CMakeLists.txt b/CMakeLists.txt index bfe9585cc..8067dac91 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -51,7 +51,9 @@ project(viam-cpp-sdk # # Enabled by default so that we produce a # modern SDK, this option can be set to `OFF` to build a static -# library. Note that this may result in non-functional examples. +# library. +# Note that the pkg-config files installed by the project do not work +# for a static build; please use Conan for more robust pkg-config support. # option(BUILD_SHARED_LIBS "If enabled, build shared libraries" ON) @@ -129,14 +131,8 @@ option(VIAMCPPSDK_CLANG_TIDY "Run the clang-tidy linter" OFF) # The following options are only defined if this project is not being included as a subproject if (CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR) - include(CMakeDependentOption) - option(VIAMCPPSDK_BUILD_EXAMPLES "Build the example executables (requires building tests too)" ON) - - # Note that the complex module example requires the testing library and adds its tests to the unit - # test suite. Thus you can only disable tests if examples are also disabled. - # The call below says don't give the user the option to disable tests unless examples are already - # disabled, and default to building the tests in either case. - cmake_dependent_option(VIAMCPPSDK_BUILD_TESTS "Build the unit test suite" ON "NOT VIAMCPPSDK_BUILD_EXAMPLES" ON) + option(VIAMCPPSDK_BUILD_EXAMPLES "Build the example executables" ON) + option(VIAMCPPSDK_BUILD_TESTS "Build the example executables" ON) endif() diff --git a/conanfile.py b/conanfile.py index 0ac8f875c..98fe462cc 100644 --- a/conanfile.py +++ b/conanfile.py @@ -1,6 +1,8 @@ from conan import ConanFile from conan.tools.cmake import CMake, CMakeDeps, CMakeToolchain, cmake_layout from conan.tools.files import load +from conan.tools.apple import is_apple_os +import os import re class ViamCppSdkRecipe(ConanFile): @@ -81,9 +83,10 @@ def package(self): def package_info(self): self.cpp_info.components["viam_rust_utils"].libs = ["viam_rust_utils"] + + self.cpp_info.components["viamsdk"].libs = ["viamsdk"] for component in ["viamsdk", "viamapi"]: - self.cpp_info.components[component].libs = [component] self.cpp_info.components[component].set_property("cmake_target_name", "viam-cpp-sdk::{}".format(component)) self.cpp_info.components[component].set_property("pkg_config_name", "viam-cpp-sdk-lib{}".format(component)) self.cpp_info.components[component].requires = ["grpc::grpc++", "protobuf::libprotobuf"] @@ -95,6 +98,18 @@ def package_info(self): self.cpp_info.components["viamapi"].includedirs.append("include/viam/api") + if self.options.shared: + self.cpp_info.components["viamapi"].libs = ["viamapi"] + else: + lib_folder = os.path.join(self.package_folder, "lib") + lib_fullpath = os.path.join(lib_folder, "libviamapi.a") + if is_apple_os(self): + whole_archive = f"-Wl,-force_load,{lib_fullpath}" + else: + whole_archive = f"-Wl,--push-state,--whole-archive,{lib_fullpath},--pop-state" + self.cpp_info.components["viamapi"].exelinkflags.append(whole_archive) + self.cpp_info.components["viamapi"].sharedlinkflags.append(whole_archive) + self.cpp_info.components["viamsdk"].requires.extend([ "viamapi", "boost::headers", diff --git a/etc/docker/tests/run_test.sh b/etc/docker/tests/run_test.sh index 83fbc51a6..2f61050ec 100755 --- a/etc/docker/tests/run_test.sh +++ b/etc/docker/tests/run_test.sh @@ -5,8 +5,10 @@ mkdir build cd build cmake .. -G Ninja -DVIAMCPPSDK_USE_DYNAMIC_PROTOS=ON \ -DVIAMCPPSDK_OFFLINE_PROTO_GENERATION=ON \ - -DVIAMCPPSDK_SANITIZED_BUILD=ON \ - -DVIAMCPPSDK_CLANG_TIDY=ON + -DVIAMCPPSDK_SANITIZED_BUILD=$BUILD_SHARED \ + -DVIAMCPPSDK_CLANG_TIDY=ON \ + -DBUILD_SHARED_LIBS=$BUILD_SHARED + ninja all ninja install INSTALL_DIR="$(pwd)/install" @@ -20,15 +22,28 @@ popd # Test that example_module builds and runs with the SDK install from above. # Check with both CMake and make/pkg-config that we can build the example -# and have it exit with the expected error message. +# and have it start listening. + +run_module() { + ./example_module fake-socket-path > module-out-temp.txt & + MODULE_PID=$! + sleep 2 + kill ${MODULE_PID} + grep 'Module listening' module-out-temp.txt + return $? +} cd ../src/viam/examples/project pushd cmake cmake . -G Ninja # Just do an in-source build to save path fiddling ninja all -[ $(./example_module 2>&1 | grep 'main failed with exception:' -c) = 1 ] -popd -pushd pkg-config -PKG_CONFIG_PATH=${INSTALL_DIR}/lib/pkgconfig make all -[ $(./example_module 2>&1 | grep 'main failed with exception:' -c) = 1 ] +run_module popd + +if [ ${BUILD_SHARED} = "ON" ] +then + pushd pkg-config + PKG_CONFIG_PATH=${INSTALL_DIR}/lib/pkgconfig make all + run_module + popd +fi diff --git a/src/viam/examples/modules/complex/CMakeLists.txt b/src/viam/examples/modules/complex/CMakeLists.txt index 51f63c044..6ac850332 100644 --- a/src/viam/examples/modules/complex/CMakeLists.txt +++ b/src/viam/examples/modules/complex/CMakeLists.txt @@ -140,11 +140,13 @@ install( # writing their own modules. We are simply integrating tests from # `test_complex_module.cpp` into the Viam C++ SDK testing suite. -enable_testing() -viamcppsdk_add_boost_test(test_complex_module.cpp) -target_sources(test_complex_module - PRIVATE - gizmo/impl.cpp - summation/impl.cpp -) -target_link_libraries(test_complex_module complex_module_sources) +if (VIAMCPPSDK_BUILD_TESTS) + enable_testing() + viamcppsdk_add_boost_test(test_complex_module.cpp) + target_sources(test_complex_module + PRIVATE + gizmo/impl.cpp + summation/impl.cpp + ) + target_link_libraries(test_complex_module complex_module_sources) +endif() diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt index 04077c991..3ed83a7d5 100644 --- a/src/viam/sdk/CMakeLists.txt +++ b/src/viam/sdk/CMakeLists.txt @@ -227,8 +227,13 @@ target_link_directories(viamsdk "$" ) +if (BUILD_SHARED_LIBS) + target_link_libraries(viamsdk PUBLIC viam-cpp-sdk::viamapi) +else() + target_link_libraries(viamsdk PUBLIC "$") +endif() + target_link_libraries(viamsdk - PUBLIC viam-cpp-sdk::viamapi PUBLIC ${VIAMCPPSDK_GRPCXX_LIBRARIES} PUBLIC ${VIAMCPPSDK_GRPC_LIBRARIES} PUBLIC ${VIAMCPPSDK_PROTOBUF_LIBRARIES} diff --git a/test_package/conanfile.py b/test_package/conanfile.py index 408f6b4b5..d3627813a 100644 --- a/test_package/conanfile.py +++ b/test_package/conanfile.py @@ -1,5 +1,5 @@ +import subprocess import os -from io import StringIO from conan import ConanFile from conan.errors import ConanException @@ -23,8 +23,22 @@ def layout(self): def test(self): if can_run(self): - cmd = os.path.join(self.cpp.build.bindir, "example_module") - stderr = StringIO() - self.run(cmd, env='conanrun', stderr=stderr, ignore_errors=True) - if "main failed with exception:" not in stderr.getvalue(): - raise ConanException("Unexpected error output from test") + sock = "fake-socket-path" + + cmd = os.path.join(self.cpp.build.bindir, f"example_module {sock}") + + # the ConanFile run method is a wrapper around Popen, but it only returns the retcode. + # A properly intialized module waits indefinitely on a signal, so we have to use Popen manually. + proc = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, text=True) + + out = None + + try: + out = proc.communicate(timeout=2)[0] + except subprocess.TimeoutExpired: + proc.terminate() + out = proc.communicate()[0] + pass + + if f"Module listening on {sock}" not in out: + raise ConanException(f"Simple example failed to start module listening")