Skip to content

ament_vendor: Add IS_VENDORED_OUTPUT_VARIABLE_NAME argument and AMENT_VENDOR_POLICY CMake option #592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

traversaro
Copy link
Contributor

Evolution of #552 .

This PR aim is based on our experience at RoboStack with the ament_vendor, and aims to improve the experience of downstream packagers of ROS on non-officially supported platform (being them conda/robostack, nix or similar), reducing the need of patching the vendor package to obtain the required behaviour.

In particular, a common pattern that we are experiencing is that compare to stable linux distributions, rolling releases as conda-forge have much more recent packages, so in many case we want to disable completely the vendoring in _vendor packages.

A few examples:

At the moment, we handle that with a lot of patches (see for example https://github.com/RoboStack/ros-kilted/blob/main/patch/ros-kilted-zenoh-cpp-vendor.patch, https://github.com/RoboStack/ros-kilted/blob/main/patch/ros-kilted-gz-cmake-vendor.patch, https://github.com/RoboStack/ros-kilted/blob/main/patch/ros-kilted-gz-rendering-vendor.patch and all similar patch in that directory). However, patches are typically cumbersome to maintain, as you need to frequently update them if patched project changes.

To deal with this, this PR adds the AMENT_VENDOR_POLICY CMake option, whose documentation is included in the code, and is reported in the following:

#   AMENT_VENDOR_POLICY: String option that specifies how ament_vendor behaves,
#                        the allowed values are listed in the following.
#     DEFAULT: Vendor if ``SATISFIED`` argument is not supplied or false,
#              do not vendor otherwise.
#     FORCE_BUILD_VENDOR: Always vendor, independently of the value of the
#                   ``SATISFIED`` argument.
#     NEVER_VENDOR: Never vendor, and raise an error if ``SATISFIED`` argument
#                   is not supplied or false.
#     NEVER_VENDOR_IGNORE_SATISFIED_CHECK: Never vendor, and do not raise
#                   an error even if ``SATISFIED`` argument is not supplied
#                   or false. This option is in unsupported by most packages,
#                   so use at your own risk, as it could break the buid.

One downside of having this option, is that now it is not anymore trivial to know if ament_vendor actually vendored something or not, while before it was sufficient to check if the variable passed to SATISFIED was TRUE or not (see for example https://github.com/gazebo-release/gz_sim_vendor/blob/423f5eaf34d1c69735c99704c610e434e902886e/CMakeLists.txt#L90). For this reason, the PR also adds a IS_VENDORED_OUTPUT_VARIABLE_NAME argument that pass the variable name that is populated by ament_vendor reporting if the package is actually vendored or not.

For example, the gz-vendor packages could be modified as in the following:

ament_vendor(${LIB_NAME_UNDERSCORE}_vendor
  SATISFIED ${${LIB_NAME_FULL}_FOUND}
  VCS_URL https://github.com/gazebosim/${GITHUB_NAME}.git
  VCS_VERSION ${GITHUB_NAME}${LIB_VER_MAJOR}_${LIB_VER}${LIB_VER_SUFFIX}
  CMAKE_ARGS
    -DSKIP_PYBIND11:BOOL=ON
  GLOBAL_HOOK
  IS_VENDORED_OUTPUT_VARIABLE_NAME ${LIB_NAME_UNDERSCORE}_is_vendored
)

# [...]

# Code that only should done if the package is actually vendored
if(${LIB_NAME_UNDERSCORE}_is_vendored)
  ament_environment_hooks("${CMAKE_CURRENT_SOURCE_DIR}/${PROJECT_NAME}.dsv.in")
  # Create a dummy .sh file needed for ament_package to source the .dsv file.
  # See https://github.com/ament/ament_package/issues/145
  file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}.sh "# Dummy .sh file needed for .dsv file to be sourced.")
  ament_environment_hooks("${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}.sh")
endif()

@YuanYuYuan
Copy link

As one of the ros-nix-overlay users, I'm excited about this solution 😄

@traversaro
Copy link
Contributor Author

As one of the ros-nix-overlay users, I'm excited about this solution 😄

Great, feel free to comment if there is anything that could be improved from the nix point of view, as I a not so familiar with nix.

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