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

[#15] Fix build with GCC #20

Merged
merged 4 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 2 deletions .github/actions/install-clang/action.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
name: 'Install clang toolchain'
name: 'Install clang'
runs:
using: "composite"
steps:
- name: Install iceoryx dependencies and clang-tidy
- name: Install clang
shell: bash
run: |
sudo wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
Expand Down
12 changes: 12 additions & 0 deletions .github/actions/install-gcc/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: 'Install GCC'
runs:
using: "composite"
steps:
- name: Install GCC
shell: bash
run: |
apt-get update
apt-get install -y software-properties-common
add-apt-repository -y ppa:ubuntu-toolchain-r/test
apt-get update
apt-get install -y gcc-13 g++-13
203 changes: 113 additions & 90 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
source-code: ${{ steps.filter.outputs.source-code }}
markdown: ${{ steps.filter.outputs.markdown }}
steps:
- name: Checkout sources
- name: Check out sources
uses: actions/checkout@v4

- name: Check for changed file types
Expand All @@ -37,7 +37,7 @@ jobs:
timeout-minutes: 10
runs-on: ubuntu-latest
steps:
- name: Checkout sources
- name: Check out sources
uses: actions/checkout@v4

- name: Check format of all commit messages
Expand All @@ -52,7 +52,7 @@ jobs:
timeout-minutes: 10
runs-on: ubuntu-latest
steps:
- name: Checkout sources
- name: Check out sources
uses: actions/checkout@v4

- name: Install clang toolchain
Expand All @@ -71,95 +71,118 @@ jobs:
# git fetch origin main
# ./scripts/check-clang-tidy.sh warning-as-error diff-to-main

colcon-build-test-clang:
colcon-build-test:
needs: [preflight-check, static-code-analysis]
strategy:
fail-fast: false
matrix:
compiler: [clang, gcc]
include:
- compiler: clang
cc: clang
cxx: clang++
- compiler: gcc
cc: gcc
cxx: g++
runs-on: ubuntu-latest
container:
image: ros:rolling
steps:
- name: Install ros build dependencies
run: |
apt-get update
apt-get install -y \
python3-colcon-common-extensions \
python3-colcon-mixin \
python3-vcstool \
python3-rosdep \
libacl1-dev \
software-properties-common \
curl \
wget

- name: Install rust toolchain
uses: dtolnay/rust-toolchain@stable

- name: Create workspace
run: |
mkdir -p $WORKSPACE_DIR/src/rmw_iceoryx2
mkdir -p $WORKSPACE_DIR/src/test_interface_files

- name: Checkout test_interface_files
uses: actions/checkout@v4
with:
repository: ros2/test_interface_files
path: src/test_interface_files
ref: rolling

- name: Checkout current ref
uses: actions/checkout@v4
with:
path: src/rmw_iceoryx2

- name: Install clang toolchain
uses: ./src/rmw_iceoryx2/.github/actions/install-clang

- name: Import dependencies with VCS
run: |
cd $WORKSPACE_DIR
sed 's|[email protected]:|https://github.com/|g' src/rmw_iceoryx2/iceoryx.repos > _deps.repos
vcs import src < _deps.repos

- name: Build with colcon
run: |
cd $WORKSPACE_DIR
. /opt/ros/rolling/setup.sh
colcon build \
--cmake-args \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DBUILD_TESTING=On \
--symlink-install \
--packages-select \
test_interface_files \
iceoryx_platform\
iceoryx_hoofs \
iceoryx2_cxx \
rmw_iceoryx2_cxx_test_msgs \
rmw_iceoryx2_cxx

- name: Run tests
run: |
cd $WORKSPACE_DIR
. /opt/ros/rolling/setup.sh
. install/setup.sh
colcon test \
--packages-select \
rmw_iceoryx2_cxx \
--event-handlers console_direct+ \
--return-code-on-test-failure

- name: Generate test reports
if: always()
run: |
cd $WORKSPACE_DIR
colcon test-result --verbose

- name: Upload test results
if: always()
uses: actions/upload-artifact@v4
with:
name: test-results
path: |
${{ env.WORKSPACE_DIR }}/build/*/test_results/*/*.xml
${{ env.WORKSPACE_DIR }}/log/**/test*.log
- name: Install common build dependencies
run: |
apt-get update
apt-get install -y \
python3-colcon-common-extensions \
python3-colcon-mixin \
python3-vcstool \
python3-rosdep \
libacl1-dev \
software-properties-common \
curl \
wget

- name: Install rust toolchain
uses: dtolnay/rust-toolchain@stable

- name: Setup workspace
run: |
mkdir -p $WORKSPACE_DIR/src/rmw_iceoryx2
mkdir -p $WORKSPACE_DIR/src/test_interface_files

- name: Check out test message interfaces
uses: actions/checkout@v4
with:
repository: ros2/test_interface_files
path: src/test_interface_files
ref: rolling

- name: Check out current ref
uses: actions/checkout@v4
with:
path: src/rmw_iceoryx2

# this is also required for all builds as iceoryx2 depends on libclang
- name: Install clang toolchain
uses: ./src/rmw_iceoryx2/.github/actions/install-clang

- name: Install gcc toolchain
if: matrix.compiler == 'gcc'
uses: ./src/rmw_iceoryx2/.github/actions/install-gcc

- name: Import iceoryx
run: |
cd $WORKSPACE_DIR

# need to switch to https in ci
sed 's|[email protected]:|https://github.com/|g' src/rmw_iceoryx2/iceoryx.repos > _deps.repos
vcs import src < _deps.repos

- name: Check compiler versions
run: |
echo "C compiler version:"
${{ matrix.cc }} --version
echo "C++ compiler version:"
${{ matrix.cxx }} --version

- name: Build workspace
run: |
cd $WORKSPACE_DIR
. /opt/ros/rolling/setup.sh
colcon build \
--cmake-args \
-DCMAKE_C_COMPILER=${{ matrix.cc }} \
-DCMAKE_CXX_COMPILER=${{ matrix.cxx }} \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DBUILD_TESTING=On \
--symlink-install \
--packages-select \
test_interface_files \
iceoryx_platform \
iceoryx_hoofs \
iceoryx2_cxx \
rmw_iceoryx2_cxx_test_msgs \
rmw_iceoryx2_cxx

- name: Run tests
run: |
cd $WORKSPACE_DIR
. /opt/ros/rolling/setup.sh
. install/setup.sh
colcon test \
--packages-select rmw_iceoryx2_cxx \
--return-code-on-test-failure

- name: Generate test reports
if: always()
run: |
cd $WORKSPACE_DIR
colcon test-result --verbose

- name: Upload test results
if: always()
uses: actions/upload-artifact@v4
with:
name: test-results-${{ matrix.compiler }}
path: |
${{ env.WORKSPACE_DIR }}/build/*/test_results/*/*.xml
${{ env.WORKSPACE_DIR }}/log/**/test*.log
11 changes: 4 additions & 7 deletions doc/release-notes/unreleased.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@
conflicts when merging.
-->

* Example text [#1](https://github.com/ekxide/rmw_iceoryx2/issues/1)

### Bugfixes

<!--
NOTE: Add new entries sorted by issue number to minimize the possibility of
conflicts when merging.
-->

* Example text [#1](https://github.com/ekxide/rmw_iceoryx2/issues/1)
* Fix failing `gcc` build [#15](https://github.com/ekxide/rmw_iceoryx2/issues/15)

### Refactoring

Expand All @@ -29,17 +27,16 @@
conflicts when merging.
-->

* Example text [#1](https://github.com/ekxide/rmw_iceoryx2/issues/1)

### Workflow

<!--
NOTE: Add new entries sorted by issue number to minimize the possibility of
conflicts when merging.
-->

* Use VCS to manage dependencies [#13](https://github.com/ekxide/rmw_iceoryx2/issues/13)
* Add CI workflow for building and testing [#12](https://github.com/ekxide/rmw_iceoryx2/issues/12)
* Use `vcs` to manage dependencies [#13](https://github.com/ekxide/rmw_iceoryx2/issues/13)
* Add CI for building and testing with `clang` [#12](https://github.com/ekxide/rmw_iceoryx2/issues/12)
* Add CI for building and testing with `gcc` [#15](https://github.com/ekxide/rmw_iceoryx2/issues/15)

### New API features

Expand Down
8 changes: 4 additions & 4 deletions rmw_iceoryx2_cxx/include/rmw_iceoryx2_cxx/iox2/iceoryx2.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,15 @@ class RMW_PUBLIC Iceoryx2

// ===================================================================================================================

template <::iox2::ServiceType ServiceType>
auto Iceoryx2::service_builder(const std::string& service_name) -> ::iox2::ServiceBuilder<ServiceType> {
template <::iox2::ServiceType S>
auto Iceoryx2::service_builder(const std::string& service_name) -> ::iox2::ServiceBuilder<S> {
auto name = ::iox2::ServiceName::create(service_name.c_str());
if (name.has_error()) {
// TODO: propagate error
}
if constexpr (ServiceType == ::iox2::ServiceType::Local) {
if constexpr (S == ::iox2::ServiceType::Local) {
return local().service_builder(name.value());
} else if constexpr (ServiceType == ::iox2::ServiceType::Ipc) {
} else if constexpr (S == ::iox2::ServiceType::Ipc) {
return ipc().service_builder(name.value());
} else {
static_assert(std::false_type::value, "Attempted to build a service of unknown type");
Expand Down
4 changes: 2 additions & 2 deletions rmw_iceoryx2_cxx/src/rmw/graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ rmw_ret_t rmw_get_publishers_info_by_topic(const rmw_node_t* node,
if (!rcutils_allocator_is_valid(allocator)) {
return RMW_RET_INVALID_ARGUMENT;
}
if (auto result = rmw_topic_endpoint_info_array_check_zero(publishers_info) != RMW_RET_OK) {
if (rmw_topic_endpoint_info_array_check_zero(publishers_info) != RMW_RET_OK) {
return RMW_RET_INVALID_ARGUMENT;
}

Expand Down Expand Up @@ -443,7 +443,7 @@ rmw_ret_t rmw_get_subscriptions_info_by_topic(const rmw_node_t* node,
if (!rcutils_allocator_is_valid(allocator)) {
return RMW_RET_INVALID_ARGUMENT;
}
if (auto result = rmw_topic_endpoint_info_array_check_zero(subscriptions_info) != RMW_RET_OK) {
if (rmw_topic_endpoint_info_array_check_zero(subscriptions_info) != RMW_RET_OK) {
return RMW_RET_INVALID_ARGUMENT;
}

Expand Down
42 changes: 23 additions & 19 deletions rmw_iceoryx2_cxx/test/test_rmw_allocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@ namespace

using namespace rmw::iox2::testing;

class DummyClass
{
public:
DummyClass()
: value(42) {
}
~DummyClass() {
value = 0;
}
int value;
};

class AllocatorHelpersTest : public TestBase
{
protected:
Expand Down Expand Up @@ -113,21 +101,37 @@ TEST_F(AllocatorHelpersTest, deallocate_nullptr) {
ASSERT_NO_THROW(rmw::iox2::deallocate(ptr));
}

class DummyClass
{
public:
DummyClass() {
++constructor_calls;
}
~DummyClass() {
++destructor_calls;
}

static size_t constructor_calls;
static size_t destructor_calls;
};
size_t DummyClass::constructor_calls = 0;
size_t DummyClass::destructor_calls = 0;

TEST_F(AllocatorHelpersTest, construct_and_destruct) {
auto allocation = rmw::iox2::allocate<DummyClass>();
ASSERT_TRUE(allocation.has_value());

auto ptr = allocation.value();
auto constructed = rmw::iox2::construct(ptr);
ASSERT_TRUE(constructed.has_value());
ASSERT_FALSE(constructed.has_error());
ASSERT_EQ(DummyClass::constructor_calls, 1u);
ASSERT_EQ(DummyClass::destructor_calls, 0u);

auto obj = constructed.value();
ASSERT_EQ(obj->value, 42);
rmw::iox2::destruct<DummyClass>(ptr);
ASSERT_EQ(DummyClass::constructor_calls, 1u);
ASSERT_EQ(DummyClass::destructor_calls, 1u);

rmw::iox2::destruct<DummyClass>(obj);
ASSERT_EQ(obj->value, 0);

rmw::iox2::deallocate(obj);
rmw::iox2::deallocate(ptr);
}

TEST_F(AllocatorHelpersTest, destruct_nullptr) {
Expand Down
Loading
Loading