fix: make bt4_picknik tools and vendored lexy disjoint from stock

[griswaldbrooks]

[written by AI]

Motivation

Closes the remaining gap in issue #20928. #26 gave the fork's library, headers, and CMake config distinct names from stock upstream behaviortree_cpp (used by nav2 in the same image), but two file sets were still shared or shareable between the two packages:

• bin/bt4_plugin_manifest (and bin/bt4_recorder, gated on ZMQ_FOUND) used stock's unscoped tool names. Verified live: installing this fork's deb followed by stock's deb hits a real dpkg fatal error — trying to overwrite '/opt/ros/jazzy/bin/bt4_plugin_manifest', which is also in package ros-jazzy-behaviortree-cpp-picknik. dpkg's path-exclude mechanism (used elsewhere in the moveit_pro image to keep stock's dev files out) does not prevent this class of conflict — it only suppresses extraction for files that aren't already claimed by another installed package; the overwrite check runs regardless. Proven with a minimal synthetic two-package dpkg test before touching this repo.
• Vendored lexy installed its own headers, CMake config, and static lib unscoped (include/lexy/, include/lexy_ext/, lib/<arch>/cmake/lexy/, lib/<arch>/liblexy_file.a). The currently pinned stock snapshot doesn't vendor lexy, but a future one plausibly could, recreating the same collision class.

Renaming/excluding at the source (this repo) is more robust than trying to path-exclude around a collision after the fact, since exclude-based mitigation is proven insufficient for genuinely double-owned files.

Changes

• tools/CMakeLists.txt: renamed bt4_plugin_manifest → bt4_picknik_plugin_manifest, bt4_recorder → bt4_picknik_recorder. No other file in the repo references the old names (confirmed via full-repo grep before renaming).
• CMakeLists.txt: set LEXY_ENABLE_INSTALL OFF before add_subdirectory(3rdparty/lexy), using lexy's own existing install-guard option rather than hand-patching each install() call in its CMakeLists. Verified safe to drop: lexy is linked PRIVATE/BUILD_INTERFACE-only, and its only consumer, src/script_parser.cpp, is compiled into this library itself. No installed public header transitively includes lexy — only the internal scripting/operators.hpp → scripting/any_types.hpp chain does, and neither behavior_tree.h, bt_factory.h, nor basic_types.h reaches that chain (confirmed by grep).

Tests

207/207 pass (ament build, behaviortree_cpp_picknik_test, zero failures). Test count is unaffected by these edits by construction — renaming CMake executable targets and toggling an unrelated 3rdparty subproject's install option cannot change which gtest cases are discovered from tests/CMakeLists.txt.

Empirical disjointness proof (acceptance criterion)

Built this branch's deb the same way apt_build_farm does — bloom-generate rosdebian --ros-distro jazzy --debian-inc 1 then fakeroot debian/rules binary — and compared its dpkg-deb -c file list against the pinned stock snapshot (ros-jazzy-behaviortree-cpp 4.9.0-1noble.20260412.042652, the exact package apt_build_farm's build environment resolves today).

$ comm -12 <(dpkg-deb -c ros-jazzy-behaviortree-cpp-picknik_4.7.2-1noble_amd64.deb | ...) \
           <(dpkg-deb -c ros-jazzy-behaviortree-cpp_4.9.0-1noble.20260412.042652_amd64.deb | ...)
# directory entries only (/, /opt/, /opt/ros/jazzy/bin/, /usr/share/doc/, ...) — 16 shared, all directories
# regular FILES only, after filtering out directory entries (paths ending in /):
#   fork:  108 files
#   stock: 110 files
#   intersection: 0

Fork's bin/ now contains only bt4_picknik_plugin_manifest (bt4_picknik_recorder doesn't build in this ament configuration — pre-existing, unrelated: tools/CMakeLists.txt checks ZMQ_FOUND, but cmake/FindZeroMQ.cmake sets ZeroMQ_FOUND, so the if(ZMQ_FOUND) guard has never been true here). Fork's include/ and lib/ ship zero lexy paths. Stock's file list is unchanged from before this PR.

Release notes

N/A — this repo doesn't use moveit_pro's release-note convention.

Agent review

This repo has no ## Claude agent checks template, so recording results here instead of a checklist. Reviewers ran sequentially per moveit_pro's .claude/rules/agent-delegation.md (code-reviewer always; platform-architect-bot added for packaging/CMake scope; security-auditor gated on findings).

• code-reviewer: 1 P2 — the LEXY_ENABLE_INSTALL CACHE BOOL help-string was empty (""), giving no context to anyone inspecting cmake-gui/ccmake. Applied: help-string now reads "Disabled by behaviortree_cpp_picknik, see comment above".
• platform-architect-bot: no P0/P1. Empirically verified the CACHE ... FORCE idiom is load-bearing, not just sufficient — a plain set(LEXY_ENABLE_INSTALL OFF) gets silently overridden by lexy's own option(LEXY_ENABLE_INSTALL ...) because 3rdparty/lexy/CMakeLists.txt's cmake_minimum_required(VERSION 3.8) puts CMP0077 in OLD behavior; confirmed by a counterfactual build. Also verified the $<BUILD_INTERFACE:>/$<INSTALL_INTERFACE:> split correctly keeps lexy out of this library's exported INTERFACE_LINK_LIBRARIES. 4 P2s:
  • Comment at CMakeLists.txt:97-99 was self-contradictory — it claimed "No installed public header transitively includes it" while its own parenthetical named scripting/operators.hpp/any_types.hpp, which are installed. Applied: reworded to "No header reachable from the public entry points … includes lexy; the … chain does, but it is parser-internal."
  • tools/CMakeLists.txt's bt4_picknik_recorder rename is currently unreachable/unexercised by any build in this repo, since the guard checks ZMQ_FOUND (never set — only cmake/FindZeroMQ.cmake's ZeroMQ_FOUND is) — a pre-existing, unrelated bug. Deferred: out of scope for this PR (already called out in the disjointness-proof section above); fixing the ZMQ_FOUND/ZeroMQ_FOUND mismatch is a separate change.
  • cmake/conan_build.cmake:18 sets BTCPP_INCLUDE_DESTINATION to bare include (unscoped), unlike ament_build.cmake:31's include/${PROJECT_NAME} — so the conan build path would install headers to include/behaviortree_cpp/**, colliding wholesale with stock. Deferred: moveit_pro only ever builds this fork via the ament path (confirmed disjoint, proof above); the conan path is pre-existing and out of scope for a packaging-disjointness PR scoped to what moveit_pro actually ships.
  • Optionally exclude scripting/operators.hpp/any_types.hpp from the installed headers glob, since they're lexy-only and parser-internal. Deferred: the bot's own framing was "arguably its own PR" — it would change the installed file set (requiring the disjointness proof to be re-run) for a header-hygiene improvement unrelated to this PR's actual collision fix.
• security-auditor: not run. No finding from code-reviewer or platform-architect-bot was security-flavored (all were comment accuracy, CMake variable scoping, and unreachable-build-path observations); nothing in this PR's diff touches parsing untrusted input, credentials, or network-facing code.

Tests re-confirmed green after the P2 fixes: cmake --configure succeeds cleanly (verified in-container); both applied changes are comment/help-string text only, touching no install() rule or target name, so the file set — and therefore the empirical disjointness proof above — is unchanged and was not re-run.

CodeRabbit review

• CMakeLists.txt:95-104 (Minor): LEXY_ENABLE_INSTALL OFF CACHE ... FORCE is configure-wide; if this project is embedded via add_subdirectory() alongside another, unrelated lexy instance, the forced OFF would silently suppress that instance's install rules too. Applied (b8d70d6e): save the caller's prior cache state (absent, or its previous value) before forcing OFF, restore it immediately after add_subdirectory(3rdparty/lexy) returns — safe because lexy's own option(LEXY_ENABLE_INSTALL ...) has already read the forced value by then (the same CMP0077-OLD mechanism platform-architect-bot verified makes CACHE FORCE necessary in the first place only applies to that one read). Re-verified after the fix: clean configure, 207/207 tests pass, install tree still ships zero lexy paths and only bt4_picknik_plugin_manifest in bin/ — install rules unchanged, so the empirical disjointness proof above was not re-run.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 81d9848d-2538-415f-8a74-1594636b70a4

📥 Commits

Reviewing files that changed from the base of the PR and between 408bc8c and b8d70d6.

📒 Files selected for processing (2)

• CMakeLists.txt
• tools/CMakeLists.txt

🚧 Files skipped from review as they are similar to previous changes (2)

• CMakeLists.txt
• tools/CMakeLists.txt

---

📝 Walkthrough

Summary by CodeRabbit

• Build and Installation
  • Prevented bundled library installation files from being included in project installations without changing existing configuration settings.
  • Updated recorder and plugin manifest tool names to use the bt4_picknik_* naming convention.
  • Preserved existing build, linking, installation, and optional ZeroMQ behavior.

Walkthrough

Changes

CMake updates

Layer / File(s)	Summary
Disable vendored lexy installation CMakeLists.txt	CMake preserves the existing LEXY_ENABLE_INSTALL cache state, disables lexy installation while configuring the vendored directory, and restores the prior state afterward.
Rename CMake tool targets tools/CMakeLists.txt	The recorder, plugin manifest, and commented log-cat targets use the bt4_picknik prefix. Build, linking, installation, and ZeroMQ conditions remain unchanged.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The pull request description is detailed, relevant, and documents motivation, changes, tests, review findings, and deferred 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.
Human Review Check	✅ Passed	The PR changes only two CMake build/install files; it adds no auth, CI, scripts, public headers, SDK/API declarations, tutorials, or moveit behavior code.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CMakeLists.txt`:
- Around line 95-104: Preserve the caller’s configure-wide LEXY_ENABLE_INSTALL
cache state around add_subdirectory(3rdparty/lexy): capture whether the cache
entry existed and its original value before the forced OFF assignment, then
restore that value afterward or unset the cache variable when it was previously
absent. Keep lexy installation disabled only while adding the vendored lexy
instance.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ef6970e1-51dc-4662-b905-e877fb3f73e2

📥 Commits

Reviewing files that changed from the base of the PR and between 353c2fb and a175461.

📒 Files selected for processing (2)

• CMakeLists.txt
• tools/CMakeLists.txt