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.

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

Jump to bottom

produce .deb with cpack #165

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft

produce .deb with cpack #165

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
da72d8b
successful .deb if you remove examples
abe-winter Oct 17, 2023
1d6bba7
debian packaging workflow
abe-winter Oct 20, 2023
40be3cb
hardcode prefix, lowercase grpc
abe-winter Oct 20, 2023
2bbd119
dev deps for dpkg, pkg-config fixes
abe-winter Oct 20, 2023
e36a88d
remove Requires from pc.in -- it takes 56 seconds
abe-winter Oct 20, 2023
82c5fb2
use smaller runner to hopefully not get stuck in build queue
abe-winter Oct 20, 2023
e7aa793
hardcode /usr prefix
abe-winter Oct 20, 2023
8f501a5
don't hardcode /usr prefix
abe-winter Oct 23, 2023
60550da
on dispatch instead of on push; this is manual for now, we are not re…
abe-winter Oct 23, 2023
b6b1da2
todo to remove sed command
abe-winter Oct 23, 2023
c6a3cf7
use cmake commands instead of ninja
abe-winter Oct 23, 2023
6c73a95
temporarily restore `on: push`
abe-winter Oct 23, 2023
70bacb1
cpack in build dir
abe-winter Oct 23, 2023
82b6883
add boost-log dep
abe-winter Oct 23, 2023
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
55 changes: 55 additions & 0 deletions .github/workflows/debian.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
on:
push:

env:
CMAKE_VERSION: 3.27.4

jobs:
build-deb:
strategy:
matrix:
include:
- container: debian:bookworm
label: bookworm-aarch64
runs-on: buildjet-4vcpu-ubuntu-2204-arm
- container: debian:bookworm
label: bookworm-x86_64
runs-on: buildjet-4vcpu-ubuntu-2204
container: ${{ matrix.container }}
runs-on: ${{ matrix.runs-on }}
steps:
- uses: actions/checkout@v3
- name: depends
run: |
apt-get update
apt-get install -qqy \
build-essential \
libabsl-dev \
libboost-all-dev \
libgrpc++-dev \
libprotobuf-dev \
pkg-config \
ninja-build \
protobuf-compiler-grpc \
git \
wget
- name: download cmake
run: |
ARCH=$(uname -m)
wget --quiet https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-linux-$ARCH.sh
chmod +x cmake-${CMAKE_VERSION}-linux-$ARCH.sh
./cmake-${CMAKE_VERSION}-linux-$ARCH.sh --skip-license --prefix=/usr
- name: remove examples
run: sed -i '/add_subdirectory.examples/d' src/viam/CMakeLists.txt
abe-winter marked this conversation as resolved.
Show resolved Hide resolved
- name: create build
run: mkdir build
- name: cmake
working-directory: ./build
run: cmake .. -G Ninja
- name: build
working-directory: ./build
run: ninja libviamsdk.so && cpack
Copy link
Member

Choose a reason for hiding this comment

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

Maybe better to use cmake as the driver, like:

cmake -S . -B ./build -G Ninja
cmake --build ./build --target <targets> -- <ninja-options>

Also, do you need to install before you cpack? I'd sort of expect so, but I could be wrong. If so, thats a cmake --install

Copy link
Member Author

Choose a reason for hiding this comment

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

cmake command changes done in c6a3cf7 and 70bacb1

it seems to work without --install, but it's possible some of my workarounds are caused by not doing an install

- uses: actions/upload-artifact@v3
with:
name: debian-${{ matrix.label }}
path: build/viam-cpp-sdk-*.deb
Copy link
Member

Choose a reason for hiding this comment

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

What does the complete list of debs that gets made for a given matrix entry look like right now?

Copy link
Member Author

Choose a reason for hiding this comment

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

single .deb for viam-cpp-sdk
https://github.com/viamrobotics/viam-cpp-sdk/actions/runs/6593088962/job/17914991253#step:10:15

Uploaded /__w/viam-cpp-sdk/viam-cpp-sdk/build/viam-cpp-sdk-0.0.0-Linux.deb

3 changes: 1 addition & 2 deletions src/viam/api/config/viam-cpp-sdk-libviamapi.pc.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
prefix=@CMAKE_INSTALL_PREFIX@
prefix=/usr
abe-winter marked this conversation as resolved.
Show resolved Hide resolved
libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@
base_includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@
deep_includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@/viam/api
Expand All @@ -7,7 +7,6 @@ Name: @PROJECT_NAME@-libapi
Description: @PROJECT_DESCRIPTION@
URL: @PROJECT_HOMEPAGE_URL@
Version: @PROJECT_VERSION@
Requires: gRPC++ >= @VIAMCPPSDK_GRPCXX_VERSION@ protobuf >= @VIAMCPPSDK_PROTOBUF_VERSION@
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, these need to remain. Otherwise, projects using pkg-config to consume the SDK will fail to link.

Copy link
Member

Choose a reason for hiding this comment

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

I just read your comment that said "remove Requires line -- it adds like a minute of CPU-bound processing inside the consumer's cmake config". I don't see how that could be true. These files aren't consumed by cmake, only by pkg-config.

Copy link
Member Author

Choose a reason for hiding this comment

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

example run with Requires in pkg-config (last line has time):

# cmake .. -G Ninja
-- The CXX compiler identification is GNU 12.2.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found PkgConfig: /usr/bin/pkg-config (found version "1.8.1") 
-- Checking for one of the modules 'viam-cpp-sdk-libviamsdk'
-- Checking for one of the modules 'viam-cpp-sdk-libviamapi'
-- Configuring done (31.1s)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. But why are you using pkg-config to obtain the SDK rather than using find_pacakge, which will use the cmake infra that the SDK provides?

Libs: -L${libdir} -L@Boost_LIBRARY_DIRS@ -lviamapi
Cflags: -I${base_includedir} -I${deep_includedir}

7 changes: 7 additions & 0 deletions src/viam/sdk/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,10 @@ install(FILES


add_subdirectory(tests)

set(CPACK_GENERATOR "DEB")
set(CPACK_ARCHIVE_COMPONENT_INSTALL ON)
set(CPACK_COMPONENTS_ALL viam-cpp-sdk_dev)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be using CPACK_COMPONENTS_GROUPING to get separate debs for different roles?

Copy link
Member Author

Choose a reason for hiding this comment

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

separate debs for different roles

roles = development vs runtime?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, typically on a deb based platform, a library ships at least three packages:

  • A libfoo package which contains the runtime role components (typically the explicitly versioned library file libfoo.so.x.y.z and its soname symlink libfoo.so.x).
  • A libfoo-dev development role package which contains the headers and the dev time lib symlink libfoo.so so that -lfoo finds the library.
  • A libfoo-dbg debug role package which contains debug info.

set(CPACK_PACKAGE_CONTACT "[email protected]")
set(CPACK_DEBIAN_PACKAGE_DEPENDS "libprotobuf-dev, libgrpc++-dev")
Copy link
Member

Choose a reason for hiding this comment

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

This list looks pretty incomplete. We definitely have dependencies on other development libraries. Also, I'd be hoping to see a split between dependencies required for build, for runtime, and for development. Does cpack let us express those? I'd also be interested to know if cpack has any ability to auto-detect these package dependencies rather than requiring us to explicitly enumerate them.

Copy link
Member Author

@abe-winter abe-winter Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

the viam-cpp-sdk .deb is a development library; we'll presumably give it a -dev suffix if we release. we're not creating a runtime library yet I think. given that, are -dev deps okay for now?

This change would generate deps automatically ... in theory:

-set(CPACK_DEBIAN_PACKAGE_DEPENDS "libprotobuf-dev, libgrpc++-dev")
+set(CPACK_DEBIAN_PACKAGE_GENERATE_SHLIBS ON)
+set(CPACK_DEBIAN_PACKAGE_SHLIBDEPS ON)

But it detects runtime deps instead of -dev libraries:

Depends: libabsl20220623 (>= 0~20220623.0-1), libboost-log1.74.0 (>= 1.74.0+ds1), libc6 (>= 2.34), libgcc-s1 (>= 4.2), libgrpc++1.51 (>= 1.51.1), libgrpc29 (>= 1.51.1), libprotobuf32 (>= 3.21.12), libstdc++6 (>= 11)

My downstream module fails with:

/usr/include/viam/sdk/components/generic/generic.hpp:6:10: fatal error: google/protobuf/descriptor.h: No such file or directory
    6 | #include <google/protobuf/descriptor.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

/usr/include/viam/sdk/common/proto_type.hpp:5:10: fatal error: boost/variant/get.hpp: No such file or directory
    5 | #include <boost/variant/get.hpp>

added libboost-log-dev in 82b6883

include(CPack)
5 changes: 2 additions & 3 deletions src/viam/sdk/config/viam-cpp-sdk-libviamsdk.pc.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
prefix=@CMAKE_INSTALL_PREFIX@
prefix=/usr
abe-winter marked this conversation as resolved.
Show resolved Hide resolved
libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@
includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@
includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@/viam/sdk
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure why this was changed. The viam/api tacked onto the include path for the libviamapi.pc.in is peculiar to the viamapi library, and is a side effect of some unfortunate naming issues in our api repository. But the sdk library doesn't have those problems, and the intended include root for the sdk library is definitely the INSTALL_INCLUDEDIR.


Name: @PROJECT_NAME@-libviamsdk
Description: @PROJECT_DESCRIPTION@
URL: @PROJECT_HOMEPAGE_URL@
Version: @PROJECT_VERSION@
Requires: gRPC++ >= @VIAMCPPSDK_GRPCXX_VERSION@ protobuf >= @VIAMCPPSDK_PROTOBUF_VERSION@ @PROJECT_NAME@-libviamapi >= @PROJECT_VERSION@
Copy link
Member

Choose a reason for hiding this comment

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

  • As above, I don't think these Requires can or should be removed.
  • I think this constraint on the libviamapi version should remain as well, but looking more closely it looks slightly wrong, in that it doesn't exactly match the name at src/viam/api/config/viam-cpp-sdk-libviamapi.pc.in. I think maybe that .pc.in should have Name: @PROJECT_NAME@-lib**viam**api instead.

Libs: -L${libdir} -L@Boost_LIBRARY_DIRS@ -lviamsdk -lviam_rust_utils
Libs.private: -lboost_log-mt
Cflags: -I${includedir} -I@Boost_INCLUDE_DIR@