Skip to content

Rename package to behaviortree_cpp_picknik with versioned soname - #26

Merged
griswaldbrooks merged 1 commit into
mainfrom
rename/behaviortree-cpp-picknik-20928
Jul 31, 2026
Merged

Rename package to behaviortree_cpp_picknik with versioned soname#26
griswaldbrooks merged 1 commit into
mainfrom
rename/behaviortree-cpp-picknik-20928

Conversation

@griswaldbrooks

Copy link
Copy Markdown

[written by AI]

Implements the rename decided for moveit_pro#20928 (decision record there; targets MoveIt Pro 10.0.0): this fork becomes behaviortree_cpp_picknik so stock upstream behaviortree_cpp can serve nav2 on the same system, ending the shared-name ABI collision that crashes bt_navigator on Jazzy.

What changes

  • project()/package.xmlbehaviortree_cpp_picknik; library, ament package, CMake config, and exported targets all follow.
  • Versioned soname (approved by Josh Whitley): VERSION 4.7.2, SOVERSION 4.7libbehaviortree_cpp_picknik.so.4.7. Consumers stamp the versioned name into DT_NEEDED, so a future ABI-breaking rebase (e.g. the 4.9 merge, #17640 in moveit_pro) bumps the soname and stale binaries fail loudly at load instead of silently corrupting — the exact failure mode of moveit_pro#20928. Minor version bumps of this fork are treated as ABI breaks.
  • Headers install under a package-scoped root (include/behaviortree_cpp_picknik/behaviortree_cpp/...), with the scoped root exported via ament_export_include_directories and INSTALL_INTERFACE. Consumers keep #include "behaviortree_cpp/..." unchanged, and the install no longer collides with stock's include/behaviortree_cpp/.
  • tests/CMakeLists.txt: the shared tail hardcoded behaviortree_cpp_test while the ament branch derives ${BTCPP_LIBRARY}_test — the names only coincided before the rename; unified on the variable.
  • Non-ament (conan) build path is behavior-identical: it defines the same BTCPP_INCLUDE_DESTINATION (plain include), which the INSTALL_INTERFACE now references.

Validation (ROS Jazzy container, colcon)

  • Package builds clean under the new name; full test suite: 208 tests, 0 failures.
  • Install layout verified: libbehaviortree_cpp_picknik.so → .so.4.7 → .so.4.7.2, SONAME libbehaviortree_cpp_picknik.so.4.7 (readelf), scoped headers, behaviortree_cpp_picknikConfig.cmake.
  • Consumer smoke test: separate CMake project, find_package(behaviortree_cpp_picknik) + ament_target_dependencies, source includes behaviortree_cpp/bt_factory.h unchanged — compiles, links, ticks a tree successfully, and readelf -d shows NEEDED: libbehaviortree_cpp_picknik.so.4.7.

Notes / follow-ups

  • The vendored lexy headers still install to unscoped include/lexy — no conflict with stock 4.9 (upstream removed lexy in 4.9), so left untouched to keep the diff minimal.
  • After merge: apt_build_farm PR pins packages/behaviortree_cpp/package.json to the merged SHA with the renamed package (tracked under moveit_pro#20928), then the moveit_pro consumption switch (find_package in ~6 packages, Dockerfile unpin of stock, mixed-loading CI guard).

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Compatibility

    • Added a build-time marker so downstream projects can distinguish the PickNik fork from upstream headers and detect potential ABI mismatches.
  • Packaging

    • Renamed the package to behaviortree_cpp_picknik.
    • Improved installation layout for headers and exported include paths.
    • Added version information to the installed library for clearer compatibility tracking.
  • Maintenance

    • Updated test target naming to align consistently with the configured library name.

Walkthrough

Summary

The project and package are renamed for the PickNik fork, installed headers use package-scoped paths, library versioning is configured, fork detection is exposed through a macro, and test targets use the library name.

Changes

PickNik packaging and build integration

Layer / File(s) Summary
Package identity and fork detection
CMakeLists.txt, package.xml, include/behaviortree_cpp/basic_types.h
The project and package names use behaviortree_cpp_picknik, and the public header defines BTCPP_PICKNIK_FORK.
Scoped include installation and export
cmake/ament_build.cmake, CMakeLists.txt
Installed and exported include directories use include/${PROJECT_NAME} through BTCPP_INCLUDE_DESTINATION.
Library versioning and test target wiring
CMakeLists.txt, tests/CMakeLists.txt
The library receives project version and soname properties, while non-ament test operations use ${BTCPP_LIBRARY}_test.

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore (reviewers only)

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Human Review Check ❌ Error The PR changes the public SDK surface: package/project and library names, exported CMake targets/config, installed include root, and shared-library SONAME. This PR requires review by a requested human reviewer. After review, a non-author requested reviewer should override this pre-merge check.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the rename, rationale, implementation details, validation results, and follow-up work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks

Comment @coderabbitai help to get the list of available commands.

Renames the fork so it can coexist with upstream behaviortree_cpp on the
same system (PickNikRobotics/moveit_pro#20928): package/library become
behaviortree_cpp_picknik, headers install under a package-scoped root
while source keeps including behaviortree_cpp/..., and the library gets
VERSION/SOVERSION so ABI mismatches fail loudly at load time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@griswaldbrooks
griswaldbrooks force-pushed the rename/behaviortree-cpp-picknik-20928 branch from 06940cc to 8972919 Compare July 31, 2026 00:32
@griswaldbrooks
griswaldbrooks marked this pull request as ready for review July 31, 2026 01:42
@griswaldbrooks
griswaldbrooks added this pull request to the merge queue Jul 31, 2026
Merged via the queue into main with commit 353c2fb Jul 31, 2026
5 of 9 checks passed
dv-picknik added a commit that referenced this pull request Aug 4, 2026
The fork rename in #26 changed PROJECT_NAME to behaviortree_cpp_picknik, so
tests/CMakeLists.txt now builds ${BTCPP_LIBRARY}_test as
behaviortree_cpp_picknik_test. Three call sites still hardcoded the old
behaviortree_cpp_test name and failed at the "run tests" step:
pixi.toml (both win-64 and linux-64 tasks) and cmake_windows.yml.

This was masked on the pixi workflow: fail-fast cancelled the ubuntu job
partway through whenever the windows job died first, so its failure never
surfaced. Set fail-fast: false so the two report independently.

cmake_ubuntu.yml is unaffected — it runs ctest, not the binary by name.

Refs PickNikRobotics/moveit_pro#21028

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFjW9VieunSxySXNKSFEmX
dv-picknik added a commit that referenced this pull request Aug 5, 2026
…st binary)

Three jobs were red. Two are external drift; the third had been masked by
fail-fast and is our own fork-rename fallout.

cmake Windows and Pixi (conda) both broke on the same runner-image change --
windows-latest is now windows-2025-vs2026, with Visual Studio 2026 (MSVC
19.51, tools 14.51) as the only Visual Studio installed -- but they broke on
different things, so they get different fixes. Both stay on windows-latest.

cmake Windows is not a generator problem. Conan already selects the
"Visual Studio 18 2026" generator and gtest configures and builds clean
under it. The break is libsodium, the one package in the graph built by
msbuild rather than cmake: its hand-written .vcxproj gets PlatformToolset
v145 from compiler.version=195 while conanvcvars lands on VCToolsVersion
14.44.35207, and msbuild rejects the pair (MSB8052). Passing
-o "zeromq/*:encryption=tweetnacl" uses the copy of tweetnacl bundled in
libzmq and drops the libsodium dependency entirely. CURVE support is
unchanged, and nothing here touches it -- the ZMQ_CURVE_* references in
src/loggers/zmq.hpp are cppzmq's own ifdef'd option wrappers.

Pixi is a generator problem. cmake picks its default generator from a
hardcoded list, and the conda-forge cmake 3.31.5 in the lock predates VS
2026 entirely, so it asked for "Visual Studio 17 2022" and found nothing.
That generator exists only in cmake >= 4.2, hence the dependency floor and
the win-64 build task that names it explicitly. Relocking to cmake 4.4.2
moves pixi.lock to format v7, which the pinned pixi v0.40.3 cannot read, so
setup-pixi and pixi move up with it.

ros2-rolling is unrelated: ROS 2 Rolling migrated to Ubuntu 26.04
(resolute), and packages.ros.org publishes no ros-rolling-* debs for
resolute yet, so ros-rolling-ros-environment isn't there to install. Pin
OS_CODE_NAME to noble until it is.

Pixi (conda) on ubuntu was never actually passing -- fail-fast cancelled it
mid-build once windows died, hiding its own failure. The rename in #26
changed PROJECT_NAME to behaviortree_cpp_picknik, so tests build
behaviortree_cpp_picknik_test while pixi.toml and cmake_windows.yml still
ran behaviortree_cpp_test. Fixed, and fail-fast disabled so one job's
failure stops hiding its sibling's.

cmake_ubuntu.yml needed nothing -- it runs ctest, not the binary by name.

Verified locally where possible: industrial_ci in Docker with ROS_DISTRO=
rolling ROS_REPO=main OS_CODE_NAME=noble builds and passes 207 tests, and
pixi run build && pixi run test under cmake 4.4.2 builds clean and passes
207 tests, so the CMake 4 policy bump costs nothing (the one pre-4
declaration left is lexy's cmake_minimum_required(VERSION 3.8), which warns
rather than errors). The windows halves are CI-testable only; all six jobs
are green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants