From 9985ed410e05061743d4b6555d296123566e5e4d Mon Sep 17 00:00:00 2001 From: Murilo Marinho Date: Wed, 23 Sep 2026 04:52:43 +0100 Subject: [PATCH 1/2] fix: make sas::Clock a real member so it shadows rclcpp::Clock The thin wrapper's shims re-export the core via a using-directive ('using namespace marinholab::sas::core;'), which makes core names *findable* in namespace sas but does not *declare* them there. A downstream package that also does 'using namespace rclcpp;' (e.g. sas_robot_driver) then sees a bare 'Clock' as ambiguous between marinholab::sas::core::Clock and rclcpp::Clock: sas_robot_driver_ros.hpp:91:5: error: reference to 'Clock' is ambiguous This breaks the SmartArmStack PPA deb builder (run https://github.com/SmartArmStack/smart_arm_stack_ROS2/actions/runs/35760730121, job 'deb builder multiversion multiarch (ubuntu-24.04-arm, jazzy)'). Declare 'using Clock = marinholab::sas::core::Clock;' so Clock is a real member of namespace sas, shadowing rclcpp::Clock exactly like the pre-refactor monolithic package (which had 'class Clock' in namespace sas). Only Clock collides with rclcpp; the other core types (Object, RobotDriver, ShutdownSignaler, ThreadManager) and the core free functions keep working through the using-directive unchanged. Adds a [4/4] regression step to the docker smoke test: a downstream-style consumer with 'using namespace rclcpp;' + bare 'Clock' in namespace sas must compile. Verified locally against the real MarinhoLab/sas_cpp headers: original shim reproduces the ambiguity, fixed shim compiles. --- docker/smoke_test.sh | 56 ++++++++++++++++++++++++++++++---- include/sas_core/sas_clock.hpp | 8 +++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/docker/smoke_test.sh b/docker/smoke_test.sh index c6f09a0..1b3a203 100755 --- a/docker/smoke_test.sh +++ b/docker/smoke_test.sh @@ -2,22 +2,24 @@ # Integration smoke test for the sas_core thin wrapper. # # Runs inside the docker environment provided by docker/compose.yml: -# [1/3] colcon build of the wrapper package -# [2/3] Python shim smoke test (sas_core -> marinholab.sas.core) -# [3/3] C++ compatibility-header consumer compiled against the installed +# [1/4] colcon build of the wrapper package +# [2/4] Python shim smoke test (sas_core -> marinholab.sas.core) +# [3/4] C++ compatibility-header consumer compiled against the installed # libmarinholab_sas_core (legacy include path + namespace sas) +# [4/4] C++ namespace-ambiguity regression test (downstream-style consumer +# with `using namespace rclcpp;` + bare `Clock` in `namespace sas`) set -e cd /root/sas_core_devel/src/ -echo '=== [1/3] colcon build (thin wrapper) ===' +echo '=== [1/4] colcon build (thin wrapper) ===' colcon build -echo '=== [2/3] Python shim smoke test ===' +echo '=== [2/4] Python shim smoke test ===' source install/setup.bash python3 sas_core/scripts/sas_core_smoke_test.py -echo '=== [3/3] C++ compatibility header + installed library test ===' +echo '=== [3/4] C++ compatibility header + installed library test ===' # Wrapper-installed compat headers and the .deb-installed shared library. SAS_INC=/root/sas_core_devel/src/install/sas_core/include LIBDIR="$(dirname "$(ldconfig -p | awk '/libmarinholab_sas_core\.so/ {print $NF; exit}')")" @@ -43,4 +45,46 @@ g++ /tmp/sas_core_compat_test.cpp -o /tmp/sas_core_compat_test \ -Wl,-rpath,"${LIBDIR}" /tmp/sas_core_compat_test +echo '=== [4/4] C++ namespace-ambiguity regression test (downstream-style) ===' +# Reproduces SmartArmStack/smart_arm_stack_ROS2 PPA build failure: +# a downstream package does `using namespace rclcpp;` and declares a bare +# `Clock` inside `namespace sas`. With the thin wrapper's using-directive +# shim, `Clock` was ambiguous between rclcpp::Clock and +# marinholab::sas::core::Clock. The `using Clock = ...` declaration in the +# shim makes it a real member of `namespace sas`, shadowing rclcpp::Clock. +# Compile-only: linking the full rclcpp runtime here is not required for +# the name-lookup check. +cat > /tmp/sas_core_ambig_test.cpp <<'CPP' +#include +#include +#include + +using namespace rclcpp; + +namespace sas +{ +class Consumer +{ +private: + std::shared_ptr node_; // must resolve to rclcpp::Node + Clock clock_; // must resolve to marinholab::sas::core::Clock +public: + void use() + { + clock_.init(); // core Clock::init() + (void)node_; + } +}; +} + +int main() +{ + return 0; +} +CPP + +g++ /tmp/sas_core_ambig_test.cpp -c -o /tmp/sas_core_ambig_test.o \ + -I"${SAS_INC}" +echo 'Ambiguity regression test compiled (no ambiguous Clock).' + echo '=== ALL CHECKS PASSED ===' diff --git a/include/sas_core/sas_clock.hpp b/include/sas_core/sas_clock.hpp index af12b98..fe00023 100644 --- a/include/sas_core/sas_clock.hpp +++ b/include/sas_core/sas_clock.hpp @@ -7,4 +7,12 @@ namespace sas { using namespace marinholab::sas::core; + // `Clock` must be a *real member* of namespace sas (a using-declaration), + // not merely findable via the using-directive above. Otherwise a downstream + // package that also does `using namespace rclcpp;` sees a bare `Clock` + // reference as ambiguous between marinholab::sas::core::Clock and + // rclcpp::Clock (name lookup through the directive finds both). Declaring + // it here shadows rclcpp::Clock, matching the old monolithic sas_core where + // `namespace sas` had its own `class Clock`. + using Clock = marinholab::sas::core::Clock; } From b6996483f4606befd9c8ccfaab9337c8deff368f Mon Sep 17 00:00:00 2001 From: Murilo Marinho Date: Wed, 23 Sep 2026 05:52:49 +0100 Subject: [PATCH 2/2] test: make [4/4] ambiguity regression hermetic (rclcpp stub) The [4/4] smoke-test step used '#include ' but the raw g++ was not given the ROS include path, so it failed in CI with 'fatal error: rclcpp/rclcpp.hpp: No such file or directory'. The bug under test is purely a name-lookup issue, so a minimal rclcpp stub (class Clock; class Node) is name-lookup-identical to the real rclcpp::Clock and rclcpp::Node and lets the test run hermetically without ROS include plumbing. Verified: the consumer compiles with the fixed shim and fails ('reference to Clock is ambiguous') with the pre-fix shim. --- docker/smoke_test.sh | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/docker/smoke_test.sh b/docker/smoke_test.sh index 1b3a203..06ad0db 100755 --- a/docker/smoke_test.sh +++ b/docker/smoke_test.sh @@ -8,6 +8,11 @@ # libmarinholab_sas_core (legacy include path + namespace sas) # [4/4] C++ namespace-ambiguity regression test (downstream-style consumer # with `using namespace rclcpp;` + bare `Clock` in `namespace sas`) +# +# Note on CI gating: the `run` step uses `docker compose up`, which does not +# propagate the container's exit code. The checks here are therefore +# informational in CI today; run this script manually (or via +# `docker compose run --rm sas_core`) to get a real pass/fail signal. set -e cd /root/sas_core_devel/src/ @@ -52,8 +57,23 @@ echo '=== [4/4] C++ namespace-ambiguity regression test (downstream-style) ===' # shim, `Clock` was ambiguous between rclcpp::Clock and # marinholab::sas::core::Clock. The `using Clock = ...` declaration in the # shim makes it a real member of `namespace sas`, shadowing rclcpp::Clock. -# Compile-only: linking the full rclcpp runtime here is not required for -# the name-lookup check. +# +# A minimal `rclcpp` stub (Clock + Node) is used instead of the full rclcpp +# runtime: the bug is purely about *name lookup*, and a stub rclcpp::Clock / +# rclcpp::Node is name-lookup-identical to the real ones. This keeps the test +# hermetic and independent of the ROS include layout. +STUB=/tmp/sas_ambig_stub/rclcpp +mkdir -p "${STUB}" +cat > "${STUB}/rclcpp.hpp" <<'CPP' +#pragma once +#include +namespace rclcpp +{ + class Clock {}; + class Node { public: std::shared_ptr get_clock() { return nullptr; } }; +} +CPP + cat > /tmp/sas_core_ambig_test.cpp <<'CPP' #include #include @@ -84,7 +104,7 @@ int main() CPP g++ /tmp/sas_core_ambig_test.cpp -c -o /tmp/sas_core_ambig_test.o \ - -I"${SAS_INC}" + -I"${STUB%/*}" -I"${SAS_INC}" echo 'Ambiguity regression test compiled (no ambiguous Clock).' echo '=== ALL CHECKS PASSED ==='