Skip to content

Commit a58856c

Browse files
facontidavideclaude
andcommitted
Add USE_VENDORED_JSON option to allow using system nlohmann::json (#959)
Add a CMake option USE_VENDORED_JSON (default ON) so users can opt out of the bundled nlohmann::json and link against their own system-installed version, avoiding ODR violations when both versions coexist. - Add USE_VENDORED_JSON CMake option with find_package when OFF - Conditionally include vendored vs system json.hpp in 4 public headers - Add BTCPP_VENDORED_JSON compile definition to control #ifdef - Update install rules to exclude vendored json.hpp when OFF - Add find_dependency in Config.cmake.in for downstream consumers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 53040fa commit a58856c

File tree

7 files changed

+211
-4
lines changed

7 files changed

+211
-4
lines changed

CMakeLists.txt

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ option(USE_VENDORED_FLATBUFFERS "Use the bundled version of flatbuffers" ON)
2828
option(USE_VENDORED_LEXY "Use the bundled version of lexy" ON)
2929
option(USE_VENDORED_MINICORO "Use the bundled version of minicoro" ON)
3030
option(USE_VENDORED_MINITRACE "Use the bundled version of minitrace" ON)
31+
option(USE_VENDORED_JSON "Use the bundled version of nlohmann/json" ON)
3132

3233
set(BTCPP_LIB_DESTINATION lib)
3334
set(BTCPP_INCLUDE_DESTINATION include)
@@ -150,6 +151,10 @@ else()
150151
find_package(minitrace REQUIRED)
151152
endif()
152153

154+
if(NOT USE_VENDORED_JSON)
155+
find_package(nlohmann_json 3.10 REQUIRED)
156+
endif()
157+
153158
list(APPEND BT_SOURCE
154159
src/action_node.cpp
155160
src/basic_types.cpp
@@ -263,6 +268,12 @@ target_include_directories(${BTCPP_LIBRARY}
263268
target_compile_definitions(${BTCPP_LIBRARY} PRIVATE $<$<CONFIG:Debug>:TINYXML2_DEBUG>)
264269
target_compile_definitions(${BTCPP_LIBRARY} PUBLIC BTCPP_LIBRARY_VERSION="${CMAKE_PROJECT_VERSION}")
265270

271+
if(USE_VENDORED_JSON)
272+
target_compile_definitions(${BTCPP_LIBRARY} PUBLIC BTCPP_VENDORED_JSON)
273+
else()
274+
target_link_libraries(${BTCPP_LIBRARY} PUBLIC nlohmann_json::nlohmann_json)
275+
endif()
276+
266277
target_compile_features(${BTCPP_LIBRARY} PUBLIC cxx_std_17)
267278

268279
if(MSVC)
@@ -332,8 +343,15 @@ INSTALL(TARGETS ${BTCPP_LIBRARY}
332343
INCLUDES DESTINATION ${BTCPP_INCLUDE_DESTINATION}
333344
)
334345

335-
INSTALL( DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/
336-
DESTINATION ${BTCPP_INCLUDE_DESTINATION}
337-
FILES_MATCHING PATTERN "*.h*")
346+
if(USE_VENDORED_JSON)
347+
INSTALL( DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/
348+
DESTINATION ${BTCPP_INCLUDE_DESTINATION}
349+
FILES_MATCHING PATTERN "*.h*")
350+
else()
351+
INSTALL( DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/include/
352+
DESTINATION ${BTCPP_INCLUDE_DESTINATION}
353+
FILES_MATCHING PATTERN "*.h*"
354+
REGEX "contrib/json\\.hpp$" EXCLUDE)
355+
endif()
338356

339357
export_btcpp_package()

FIXME.md

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
# FIXME - Bug Tracker
2+
3+
This file tracks reported bugs from GitHub issues. For each bug, the workflow is:
4+
1. Write a unit test that reproduces the issue
5+
2. Evaluate whether the behavior diverges from intended library semantics
6+
3. Fix the issue (if confirmed as a bug)
7+
8+
**IMPORTANT:** NEVER modify existing tests that were not created in this branch. Only add new tests.
9+
10+
---
11+
12+
## Control Nodes (Parallel)
13+
14+
### [ ] #1045 - Test PauseWithRetry is flaky
15+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1045
16+
- **Summary:** The `Parallel.PauseWithRetry` test fails intermittently due to tight timing assertions. Measured time drifts beyond the allowed margin.
17+
- **Component:** `tests/gtest_parallel.cpp`
18+
- **Test file:** `tests/gtest_parallel.cpp`
19+
20+
### [ ] #1041 - Performance degradation after prolonged operation in parallel nodes
21+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1041
22+
- **Summary:** After prolonged operation, when a parallel node's child detects an exception and exits, the interval before other nodes wait for execution becomes increasingly longer. Likely related to resource accumulation or thread management.
23+
- **Component:** `src/controls/parallel_node.cpp`
24+
- **Test file:** `tests/gtest_parallel.cpp` (or new)
25+
26+
### [ ] #966 - ParallelAll max_failures off-by-one semantics
27+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/966
28+
- **Summary:** `ParallelAll` documents `max_failures` as "if the number of children returning FAILURE *exceeds* this value" but implements `>=` (greater-than-or-equal). Documentation and implementation disagree.
29+
- **Component:** `src/controls/parallel_all_node.cpp`
30+
- **Test file:** `tests/gtest_parallel.cpp`
31+
32+
---
33+
34+
## Control Nodes (Reactive)
35+
36+
### [ ] #1031 - Race condition in ReactiveSequence/ReactiveFallback with async children
37+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1031
38+
- **Summary:** With backchaining patterns, a previously `RUNNING` child is halted only *after* another child's `onStart` is executed. Two children are in `RUNNING` state simultaneously briefly, causing race conditions.
39+
- **Component:** `src/controls/reactive_sequence.cpp`, `src/controls/reactive_fallback.cpp`
40+
- **Test file:** `tests/gtest_reactive.cpp` (or new)
41+
42+
### [ ] #917 - Precondition not re-checked in ReactiveSequence
43+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/917
44+
- **Summary:** When a node has a `_successIf` precondition inside a `ReactiveSequence`, the precondition is never re-evaluated after the node enters `RUNNING` state. The node continues returning `RUNNING` instead of `SUCCESS`.
45+
- **Component:** `src/controls/reactive_sequence.cpp`, `src/tree_node.cpp`
46+
- **Test file:** `tests/gtest_reactive.cpp`
47+
48+
---
49+
50+
## XML Parsing
51+
52+
### [ ] #979 - Recursive behavior trees cause stack overflow
53+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/979
54+
- **Summary:** A behavior tree can reference its own ID as a SubTree, causing infinite recursion and stack overflow during XML parsing. The parser does not detect or prevent cyclic subtree references.
55+
- **Component:** `src/xml_parsing.cpp`
56+
- **Test file:** `tests/gtest_factory.cpp` (or new)
57+
58+
### [ ] #883 - JSON string in port value interpreted as blackboard reference
59+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/883
60+
- **Summary:** Setting a port value to a JSON string in XML (e.g., `settings='{{"a": 0.8}}'`) causes `{}` content to be interpreted as blackboard variable references instead of literal JSON.
61+
- **Component:** `src/xml_parsing.cpp`, `src/tree_node.cpp`
62+
- **Test file:** `tests/gtest_factory.cpp` (or new)
63+
64+
### [ ] #880 - External subtree from file not found when loaded via inline XML
65+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/880
66+
- **Summary:** When a subtree is registered via `registerBehaviorTreeFromFile()` and then referenced from inline XML via `createTreeFromText()`, the factory throws `"Can't find a tree with name: MyTree"`.
67+
- **Component:** `src/xml_parsing.cpp`, `src/bt_factory.cpp`
68+
- **Test file:** `tests/gtest_factory.cpp`
69+
70+
### [ ] #672 - Stack buffer overflow in xml_parsing.cpp
71+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/672
72+
- **Summary:** AddressSanitizer detects a stack-buffer-overflow in `xml_parsing.cpp` when parsing certain XML tree definitions. Memory safety issue in the XML parser.
73+
- **Component:** `src/xml_parsing.cpp`
74+
- **Test file:** `tests/gtest_factory.cpp` (or new)
75+
76+
---
77+
78+
## BehaviorTreeFactory
79+
80+
### [ ] #1046 - Heap use-after-free when factory is destroyed before ticking
81+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1046
82+
- **Summary:** If `BehaviorTreeFactory` is destroyed before its created tree is ticked, `NodeConfig` holds a dangling pointer to `TreeNodeManifest` owned by the factory. Accessing it causes heap-use-after-free.
83+
- **Component:** `src/bt_factory.cpp`, `include/behaviortree_cpp/tree_node.h`
84+
- **Test file:** `tests/gtest_factory.cpp`
85+
86+
### [ ] #937 - BehaviorTreeFactory cannot be returned by value
87+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/937
88+
- **Summary:** Making `BehaviorTreeFactory` non-copyable also inadvertently prevents returning it by value from functions. Some compilers do not apply guaranteed copy elision in all contexts.
89+
- **Component:** `include/behaviortree_cpp/bt_factory.h`
90+
- **Test file:** `tests/gtest_factory.cpp` (or new)
91+
92+
### [ ] #934 - Segfault when substituting subtree
93+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/934
94+
- **Summary:** `factory.loadSubstitutionRuleFromJSON()` followed by `factory.createTree()` causes a segmentation fault when the substitution target is a subtree node.
95+
- **Component:** `src/bt_factory.cpp`
96+
- **Test file:** `tests/gtest_factory.cpp`
97+
98+
### [ ] #930 - Mock substitution with registerSimpleAction causes tree to hang
99+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/930
100+
- **Summary:** When using mock node substitution rules with `registerSimpleAction()`, the behavior tree gets stuck after an async delay finishes. The substituted node does not behave like the original.
101+
- **Component:** `src/bt_factory.cpp`
102+
- **Test file:** `tests/gtest_factory.cpp` (or new)
103+
104+
---
105+
106+
## Loggers
107+
108+
### [ ] #1057 - Groot2Publisher enters infinite loop on exception
109+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/1057
110+
- **Summary:** `Groot2Publisher` can enter an infinite loop during destruction. When `active_server` is set to `false`, it can be reset to `true` depending on timing, causing `server_thread.join()` to hang indefinitely.
111+
- **Component:** `src/loggers/groot2_publisher.cpp`
112+
- **Test file:** New test needed
113+
114+
---
115+
116+
## Tree Execution
117+
118+
### [ ] #686 - haltTree doesn't effectively halt tickWhileRunning
119+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/686
120+
- **Summary:** `haltTree()` resets tree status to `IDLE`, which only breaks the inner loop of `tickRoot`. The outer loop then restarts execution because `IDLE` satisfies its condition, so the tree is never truly halted with `tickWhileRunning()`.
121+
- **Component:** `src/bt_factory.cpp`, `include/behaviortree_cpp/bt_factory.h`
122+
- **Test file:** `tests/gtest_tree.cpp` (or new)
123+
124+
---
125+
126+
## TreeNode
127+
128+
### [ ] #861 - Tick time monitoring can measure 0 due to instruction reordering
129+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/861
130+
- **Summary:** The `std::chrono` calls around `executeTick()` in `tree_node.cpp` can be reordered by the compiler, resulting in a measured tick duration of 0.
131+
- **Component:** `src/tree_node.cpp`
132+
- **Test file:** `tests/gtest_tree.cpp` (or new)
133+
134+
---
135+
136+
## JSON Exporter
137+
138+
### [ ] #989 - JsonExporter throws during vector conversion
139+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/989
140+
- **Summary:** A `bad_function_call` exception is thrown when converting `std::vector` types to JSON via `JsonExporter`. The conversion function is not properly registered for vector types.
141+
- **Component:** `include/behaviortree_cpp/json_export.h`
142+
- **Test file:** New test needed
143+
144+
---
145+
146+
## Platform-Specific
147+
148+
### [ ] #869 - Read access violation on Windows debug build
149+
- **URL:** https://github.com/BehaviorTree/BehaviorTree.CPP/issues/869
150+
- **Summary:** Running on VS 2022 in Debug mode causes a read access violation in string hashing within the library DLL. Release mode works fine, suggesting a debug-mode-specific memory issue.
151+
- **Component:** Core library (platform-specific)
152+
- **Test file:** Requires Windows environment
153+
154+
---
155+
156+
## Summary
157+
158+
| Category | Count | Issue Numbers |
159+
|----------|-------|---------------|
160+
| Control Nodes (Parallel) | 3 | #1045, #1041, #966 |
161+
| Control Nodes (Reactive) | 2 | #1031, #917 |
162+
| XML Parsing | 4 | #979, #883, #880, #672 |
163+
| BehaviorTreeFactory | 4 | #1046, #937, #934, #930 |
164+
| Loggers | 1 | #1057 |
165+
| Tree Execution | 1 | #686 |
166+
| TreeNode | 1 | #861 |
167+
| JSON Exporter | 1 | #989 |
168+
| Platform-Specific | 1 | #869 |
169+
| **Total** | **18** | |

cmake/Config.cmake.in

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
@PACKAGE_INIT@
22

3+
if(NOT @USE_VENDORED_JSON@)
4+
include(CMakeFindDependencyMacro)
5+
find_dependency(nlohmann_json 3.10)
6+
endif()
7+
38
include("${CMAKE_CURRENT_LIST_DIR}/@PROJECT_NAME@Targets.cmake")
49

510
set(@PROJECT_NAME@_TARGETS "BT::@PROJECT_NAME@")

include/behaviortree_cpp/blackboard.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
#pragma once
22

33
#include "behaviortree_cpp/basic_types.h"
4+
#ifdef BTCPP_VENDORED_JSON
45
#include "behaviortree_cpp/contrib/json.hpp"
6+
#else
7+
#include <nlohmann/json.hpp>
8+
#endif
59
#include "behaviortree_cpp/exceptions.h"
610
#include "behaviortree_cpp/utils/locked_reference.hpp"
711
#include "behaviortree_cpp/utils/safe_any.hpp"

include/behaviortree_cpp/bt_factory.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515
#define BT_FACTORY_H
1616

1717
#include "behaviortree_cpp/behavior_tree.h"
18+
#ifdef BTCPP_VENDORED_JSON
1819
#include "behaviortree_cpp/contrib/json.hpp"
20+
#else
21+
#include <nlohmann/json.hpp>
22+
#endif
1923
#include "behaviortree_cpp/contrib/magic_enum.hpp"
2024

2125
#include <filesystem>

include/behaviortree_cpp/json_export.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
#include "behaviortree_cpp/basic_types.h"
44
#include "behaviortree_cpp/utils/safe_any.hpp"
55

6-
// Use the version nlohmann::json embedded in BT.CPP
6+
#ifdef BTCPP_VENDORED_JSON
77
#include "behaviortree_cpp/contrib/json.hpp"
8+
#else
9+
#include <nlohmann/json.hpp>
10+
#endif
811

912
namespace BT
1013
{

include/behaviortree_cpp/loggers/groot2_protocol.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
#pragma once
22

33
#include "behaviortree_cpp/basic_types.h"
4+
#ifdef BTCPP_VENDORED_JSON
45
#include "behaviortree_cpp/contrib/json.hpp"
6+
#else
7+
#include <nlohmann/json.hpp>
8+
#endif
59

610
#include <array>
711
#include <condition_variable>

0 commit comments

Comments
 (0)