Skip to content

Conversation

@LucienMorey
Copy link

option vars have caching that I don't fully understand but this seems to do the trick. I ran the following build command sets to verify it gets set the right way in and out of ROS:

# run with respect to a normal workspace
colcon build --packages-up-to behaviortree_cpp --cmake-args -DUSE_VENDORED_TINYXML2=ON
colcon build --packages-up-to behaviortree_cpp --cmake-args -DUSE_VENDORED_TINYXML2=OFF

# run from within a build dir inside the main repo
cmake .. -DUSE_VENDORED_TINYXML2=OFF
cmake .. -DUSE_VENDORED_TINYXML2=ON

also quick check of the dependencies:

rosdep keys --from-paths src/BehaviorTree.CPP --ignore-src                                                                                                                ✔
/usr/bin/rosdep:6: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
  from pkg_resources import load_entry_point
git
libzmq3-dev
libsqlite3-dev
tinyxml2

I think this is the correct one to depend on -> https://index.ros.org/d/tinyxml2/

@LucienMorey
Copy link
Author

Looks like the pinned version of the library in an ubuntu 22.04 distro doesn't contain the cmake lookup macros. I am trying to add the vendor dep as well under the assumption that the installation of both will provide the required cmake macro as a fallback on that system and the default will be used otherwise...

@LucienMorey LucienMorey force-pushed the prefer_ros_dep_in_ament branch from 82ae4e5 to 4a78096 Compare October 29, 2025 00:32
message(STATUS "------------------------------------------")
include(cmake/conan_build.cmake)
endif()
option(USE_VENDORED_TINYXML2 "Use the bundled version of tinyxml2" ${_default_use_vendored_tiny_xml})
Copy link
Contributor

@ericriff ericriff Oct 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could leave the option declaration above with the rest and on line 97 use a combination of set + force to disable it.
That way all related options are still on the same place.

Another possible approach is to move up the find ament call so it happens before we declare the options and then do something like `set(AMENT_MODE ament_cmake_FOUND)
And use that as the default value for the tinyxml option

option(USE_VENDORED_TINYXML2 "Use the bundled version of tinyxml2" ${AMENT_MODE})

The rationale is the same, keep all the options to opt out of vendored dependencies together

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried playing around with something like the first option, but the cached option didn't behave the way I expected. I am only a ROS user and I wasn't sure how to make the change without potentially breaking someone else's workflow by always force overriding a cached thing.

Open to doing the second thing. It is the way it is to minimise the line diff. If the logic is acceptable I will squash the commits and reorder to move the option back to the same spot as the others.

@facontidavide
Copy link
Collaborator

fixed my way. Thanks

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.

3 participants