Skip to content

RSDK-9740: Make google libraries a private dependency of the SDK #357

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.

Sign up for GitHub

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

Merged
merged 18 commits into from
Jan 16, 2025
Merged

RSDK-9740: Make google libraries a private dependency of the SDK #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
cf3256a
make viamapi a private dep of sdk
lia-viam Jan 14, 2025
7ece1c8
re-group requires
lia-viam Jan 14, 2025
3f32c12
use helper function to link viam api
lia-viam Jan 14, 2025
e9bbc32
remove duplicate link
lia-viam Jan 14, 2025
d940b1a
include utils unconditionally
lia-viam Jan 14, 2025
1448de3
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into goog…
lia-viam Jan 15, 2025
8fe505f
make google a private dep of the sdk and link grpc to examples where …
lia-viam Jan 15, 2025
334b528
update conan for private google
lia-viam Jan 15, 2025
bac8c8a
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into goog…
lia-viam Jan 15, 2025
92af179
remove unused grpc includes and link
lia-viam Jan 16, 2025
bfe6e21
remove direct mention of abseil
lia-viam Jan 16, 2025
413e162
remove grpc link from example modules
lia-viam Jan 16, 2025
c88e685
remove grpc from tflite
lia-viam Jan 16, 2025
3577cbc
remove unused find_package
lia-viam Jan 16, 2025
c600fdd
remove additional mentions of absl
lia-viam Jan 16, 2025
f223455
find package no longer automatically pulls in api
lia-viam Jan 16, 2025
c193784
only api requires absl
lia-viam Jan 16, 2025
cbcfd23
return viamapi to find components
lia-viam Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/workflows/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:

- name: Install dependencies
run: |
brew install abseil cmake boost grpc protobuf xtensor
brew install cmake boost grpc protobuf xtensor
brew install pkg-config ninja buf

- name: Setup build directory
Expand Down Expand Up @@ -128,7 +128,6 @@ jobs:
gnupg \
gpg \
less \
libabsl-dev \
libboost-all-dev \
libgrpc++-dev \
libprotobuf-dev \
Expand Down Expand Up @@ -221,7 +220,6 @@ jobs:
gnupg \
gpg \
less \
libabsl-dev \
libboost-all-dev \
libc-ares-dev \
libgrpc++-dev \
Expand Down
8 changes: 4 additions & 4 deletions BUILDING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ guide, before continuing further.
The project depends on [CMake](https://cmake.org/) >= 3.25,
[Boost](https://www.boost.org/) >= 1.74, [gRPC](https://grpc.io/) >=
1.30.2, [protobuf](https://protobuf.dev/overview/) >= 3.12.4,
[xtensor](https://github.com/xtensor-stack/xtensor) >= 0.24.3,
[abseil](https://abseil.io/) and on the transitive dependencies of
[xtensor](https://github.com/xtensor-stack/xtensor) >= 0.24.3
and on the transitive dependencies of
those projects.

You will need to install these required dependencies before building
the Viam C++ SDK, preferably by way of your system package manager:

- Debian: `apt-get install cmake build-essential libabsl-dev libboost-all-dev libgrpc++-dev libprotobuf-dev libxtensor-dev`
- MacOS with Homebrew: `brew install abseil cmake boost grpc protobuf xtensor`
- Debian: `apt-get install cmake build-essential libboost-all-dev libgrpc++-dev libprotobuf-dev libxtensor-dev`
- MacOS with Homebrew: `brew install cmake boost grpc protobuf xtensor`

There are also several optional or conditionally required dependencies:

Expand Down
5 changes: 0 additions & 5 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,17 +317,12 @@ add_custom_target(
)


set(VIAMCPPSDK_ABSL_VERSION_MINIMUM) # Someday?
set(VIAMCPPSDK_BOOST_VERSION_MINIMUM 1.71)
set(VIAMCPPSDK_GRPC_VERSION_MINIMUM 1.30.2)
set(VIAMCPPSDK_PROTOBUF_VERSION_MINIMUM 3.12.4)
set(VIAMCPPSDK_XTL_VERSION_MINIMUM 0.7.2)
set(VIAMCPPSDK_XTENSOR_VERSION_MINIMUM 0.24.3)

# Time to find absl. Note that the version min value is currently empty,
# because absl doesn't use a versioning scheme that works here.
find_package(absl ${VIAMCPPSDK_ABSL_VERSION_MINIMUM} REQUIRED)

# Time to find `BOOST`.
find_package(Boost ${VIAMCPPSDK_BOOST_VERSION_MINIMUM} REQUIRED COMPONENTS headers log program_options)

Expand Down
11 changes: 6 additions & 5 deletions conanfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ def requirements(self):

# The SDK supports older grpc and protobuf, but these are the oldest
# maintained conan packages.
self.requires('grpc/[>=1.48.4]', transitive_headers=True)
self.requires('protobuf/[>=3.17.1]', transitive_headers=True)
self.requires('grpc/[>=1.48.4]')
self.requires('protobuf/[>=3.17.1]')

self.requires('xtensor/[>=0.24.3]')
self.requires('abseil/[>=20230125.3]', transitive_libs=True)
self.requires('abseil/[>=20230125.3]')
Copy link
Member

@acmorrow acmorrow Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually still needed? I'd expect absl to be a transitive dep of proto/grpc so not needing to be mentioned here?

Copy link
Collaborator Author

@lia-viam lia-viam Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised by this too but yes--my working theory is that it's because the API gencode does include some abseil headers/need some of their symbols, this was making the build fail with undefined symbol errors when I took it out


def build_requirements(self):
if self.options.offline_proto_generation:
Expand Down Expand Up @@ -94,6 +94,8 @@ def package_info(self):
if self.settings.os in ["Linux", "FreeBSD"]:
self.cpp_info.components[component].system_libs = ["pthread"]

self.cpp_info.components["viamapi"].requires.append("abseil::absl_strings")
Copy link
Member

@acmorrow acmorrow Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guessing this is related to above inclusion.

Copy link
Collaborator Author

@lia-viam lia-viam Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly!


if self.settings.os in ["Linux", "FreeBSD"]:
self.cpp_info.components["viamsdk"].system_libs.extend(["dl", "rt"])

Expand All @@ -119,8 +121,7 @@ def package_info(self):
"xtensor::xtensor",

"viamapi",
"viam_rust_utils",
"abseil::absl_strings",
"viam_rust_utils"
])

self.cpp_info.components["viamsdk"].frameworks = ["Security"]
1 change: 0 additions & 1 deletion etc/docker/base-images/Dockerfile.debian.bookworm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ RUN apt-get -y --no-install-recommends install \
gnupg \
gpg \
less \
libabsl-dev \
libboost-all-dev \
libgrpc++-dev \
libprotobuf-dev \
Expand Down
1 change: 0 additions & 1 deletion etc/docker/base-images/Dockerfile.debian.bullseye
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ RUN apt-get -y --no-install-recommends install \
gnupg \
gpg \
less \
libabsl-dev \
libboost-all-dev \
libgrpc++-dev \
libprotobuf-dev \
Expand Down
1 change: 0 additions & 1 deletion etc/docker/base-images/Dockerfile.debian.sid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ RUN apt-get -y --no-install-recommends install \
gnupg \
gpg \
less \
libabsl-dev \
libboost-all-dev \
libgrpc++-dev \
libprotobuf-dev \
Expand Down
1 change: 0 additions & 1 deletion etc/docker/base-images/Dockerfile.ubuntu.jammy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ RUN apt-get -y --no-install-recommends install \
gnupg \
gpg \
less \
libabsl-dev \
libboost-all-dev \
libgrpc++-dev \
libprotobuf-dev \
Expand Down
2 changes: 0 additions & 2 deletions src/viam/config/viam-cpp-sdkConfig.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

include(CMakeFindDependencyMacro)

find_dependency(absl)

find_dependency(Boost @Boost_VERSION_MAJOR@.@Boost_VERSION_MINOR@ COMPONENTS headers log)

if (@gRPC_FOUND@)
Expand Down
4 changes: 0 additions & 4 deletions src/viam/examples/dial_api_key/example_dial_api_key.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@

#include <boost/optional.hpp>
#include <boost/program_options.hpp>
#include <grpcpp/channel.h>
#include <grpcpp/client_context.h>
#include <grpcpp/grpcpp.h>
#include <grpcpp/support/status.h>

#include <viam/sdk/robot/client.hpp>
#include <viam/sdk/rpc/dial.hpp>
Expand Down
4 changes: 2 additions & 2 deletions src/viam/examples/modules/complex/proto/buf.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ deps:
- remote: buf.build
owner: googleapis
repository: googleapis
commit: c0913f24652a4cfc95f77d97443a5005
digest: shake256:0ef3248c6235d420fe61f373154adcde6b94e3297f82472b1d8d8c3747240b61b4a10405e2a6f8ac1c98816ac6e690ea7871024aa5ae0e035cd540214667ceed
commit: e93e34f48be043dab55be31b4b47f458
digest: shake256:93dbe51c27606999eef918360df509485a4d272e79aaed6d0016940379a9b06d316fc5228b7b50cca94bb310f34c5fc5955ce7474f655f0d0a224c4121dda3c1
2 changes: 0 additions & 2 deletions src/viam/examples/modules/simple/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
#include <sstream>

#include <boost/log/trivial.hpp>
#include <grpcpp/grpcpp.h>
#include <grpcpp/server_context.h>

#include <viam/sdk/common/exception.hpp>
#include <viam/sdk/common/proto_value.hpp>
Expand Down
3 changes: 0 additions & 3 deletions src/viam/examples/modules/tflite/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@
#include <sstream>
#include <stdexcept>

#include <grpcpp/channel.h>
#include <grpcpp/create_channel.h>
#include <grpcpp/security/credentials.h>
#include <tensorflow/lite/c/c_api.h>

#include <viam/sdk/common/proto_value.hpp>
Expand Down
7 changes: 3 additions & 4 deletions src/viam/sdk/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,14 +236,13 @@ target_link_directories(viamsdk
viamcppsdk_link_viam_api(viamsdk PRIVATE)

target_link_libraries(viamsdk
PUBLIC ${VIAMCPPSDK_GRPCXX_LIBRARIES}
PUBLIC ${VIAMCPPSDK_GRPC_LIBRARIES}
PUBLIC ${VIAMCPPSDK_PROTOBUF_LIBRARIES}
PUBLIC Boost::headers
PUBLIC Boost::log
PUBLIC xtensor
PRIVATE ${VIAMCPPSDK_GRPCXX_REFLECTION_LIBRARIES}
PRIVATE absl::strings
PRIVATE ${VIAMCPPSDK_GRPCXX_LIBRARIES}
Copy link
Member

@acmorrow acmorrow Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

PRIVATE ${VIAMCPPSDK_GRPC_LIBRARIES}
PRIVATE ${VIAMCPPSDK_PROTOBUF_LIBRARIES}
Copy link
Member

@acmorrow acmorrow Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⬇️ - I have a suspicion there may no longer be any actual direct usage of abseil in viamsdk. Can you please check?

Copy link
Collaborator Author

@lia-viam lia-viam Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

happily you're right!

PRIVATE viam_rust_utils
PRIVATE Threads::Threads
)
Expand Down
1 change: 0 additions & 1 deletion src/viam/sdk/services/private/mlmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <type_traits>
#include <utility>

#include <absl/strings/escaping.h>
#include <boost/variant/get.hpp>

#include <viam/sdk/common/exception.hpp>
Expand Down
Loading