Skip to content

No need to have generate_parameter_library at runtime #354

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.

Sign up for GitHub

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
Timple wants to merge 5 commits into
base: main
Choose a base branch
from
Open

No need to have generate_parameter_library at runtime #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d195135
No need to have generate_parameter_library at runtime
Timple May 11, 2026
e1ed640
Actually the generated headers need their dependencies
Timple May 11, 2026
07afd2b
No we don't
Timple May 11, 2026
a346f40
This is also not used runtime
Timple May 11, 2026
37ce1e3
Implement suggestions of christophfroehlich
Timple Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion example/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ find_package(ament_cmake_python REQUIRED)
find_package(generate_parameter_library REQUIRED)
find_package(rclcpp REQUIRED)
find_package(rclcpp_components REQUIRED)
find_package(rclpy REQUIRED)

generate_parameter_library(admittance_controller_parameters
src/parameters.yaml
Expand Down
3 changes: 2 additions & 1 deletion example_cmake_python/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
<author email="paul.gesel@picknik.ai">Paul Gesel</author>

<depend>generate_parameter_library</depend>
<depend>rclpy</depend>

<exec_depend>rclpy</exec_depend>

<buildtool_depend>ament_cmake_python</buildtool_depend>

Expand Down
4 changes: 2 additions & 2 deletions example_python/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@
<maintainer email="paulgesel@gmail.com">paul</maintainer>
<license>BSD-3-Clause</license>

<depend>generate_parameter_library</depend>
<depend>generate_parameter_library_py</depend>
<depend>rclpy</depend>

<exec_depend>rclpy</exec_depend>

<test_depend>python3-pytest</test_depend>

Expand Down
22 changes: 9 additions & 13 deletions generate_parameter_library/package.xml
View file
Open in desktop

@christophfroehlich christophfroehlich May 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if we change libexpected-dev to build_export_depend, then almost every dependency is of type build_export_depend here?

@Timple Timple May 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one is not in the generated header files, the other ones are.

So the other ones are actually required to be installed for the dependend package.

libexpected-dev doesn't show up in the generated header files, that's the difference.

@christophfroehlich christophfroehlich May 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

as this package is not "building" any code but only creating cmake modules for downstream packages, isn't basically anything a build_export_depend? Or do I misunderstand the package manifest?

From a different perspective:
If it is not included in generated files, do we need libexpected-dev here at all?
Or should we add it as an include explicitly? Because generated code creates something like this, with validation_result being of type tl::expected being pulled in from rsl/parameter_validators.hpp (and listed as <depend> there).

            if(auto validation_result = size_gt<std::string>(param, 0);
              !validation_result) {
                return rsl::to_parameter_result_msg(validation_result);
            }

@Timple Timple May 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well now I am in over my head 😄

Being a bit pragmatic here, most users will just install everything in a package.xml (via tooling).

Slighly more advanced users will install everything (perhaps not exec_depend) and build their binaries.
Then they have a robot (or container o.a.). where they need only those binaries and all depends and exec_depends.

For that latter group I'm trying to prevent libexpected-dev to end up on their robot.

Initially this PR started with making generate_parameter_library itself a build_depend in the examples, but that was wrong (and I forgot why).

@christophfroehlich christophfroehlich May 13, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For that latter group I'm trying to prevent libexpected-dev to end up on their robot.

ok, currently this is still the case because it is a <depend> in RSL. So maybe it is cleaner to add it as an include to the jinja template and leave it as a build_depend as you suggested, maybe changing it also in RSL.

btw: I appreciate any cleanup in this direction, but want to understand it myself ;)

@Timple Timple May 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this package is a bit of an odd one.

I remember now why it could not be just a build_depend of the examples. And that's because it generates a header file with dependencies. Otherwise the example package would technically need to list these and that would be impractical in the README.md I guess.
Or we need a generate_parameter_library_runtime package just like the message generation has a build time and a runtime part.

OK, that last part sounded like a big refactor, but actually the runtime package only needs to contain a package.xml with exec_depends.

README would say:

<build_depend>generate_parameter_library</build_depend>
<exec_depend>generate_parameter_library_runtime</exec_depend>

This would all be fully backwards compatible with folks having

<depend>generate_parameter_library</depend>

@christophfroehlich christophfroehlich May 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Slighly more advanced users will install everything (perhaps not exec_depend) and build their binaries.
Then they have a robot (or container o.a.). where they need only those binaries and all depends and exec_depends.

Can't we have the same without adding a new package?

In my ideal world (which might be incorrect, and I might have forgotten something) it could work as follows:

for python setup.py packages: my_python_package has on generate_parameter_library_py.

  • generate_parameter_library_py declares as <build_export_depend> everything what is needed for generating the code at build time (for me this is the state of running colcon build) 
    <build_export_depend>python3</build_export_depend>
<build_export_depend>python3-jinja2</build_export_depend>
<build_export_depend>python3-typeguard</build_export_depend>
<build_export_depend>python3-yaml</build_export_depend>
  • generate_parameter_library_py declares as <exec_depend> what is needed to run the generated code 
    <exec_depend>rcl_interfaces</exec_depend>
<exec_depend>rclpy</exec_depend>

for cmake/cmake_python: my_cmake_package has on generate_parameter_library.

  • generate_parameter_library declares as <build_export_depend> generate_parameter_library_py needed for generating the code at build time (transitive build dependency, which itself pulls stuff like python3-jinja2 from above)
  • generate_parameter_library declares as <build_export_depend> what is needed to build the generated c++ code (transitive build dependency) 
    <build_export_depend>fmt</build_export_depend>
<build_export_depend>rclcpp</build_export_depend>
<build_export_depend>rclcpp_lifecycle</build_export_depend>
<build_export_depend>rsl</build_export_depend>
  • rsl declares the header-only libraries as <build_export_depend> 
    <build_export_depend>libexpected-dev</build_export_depend>
<build_export_depend>tcb_span</build_export_depend>
  • generate_parameter_library declares as <exec_depend> what is needed to run the generated code and has to be shipped with the binaries 
    <exec_depend>fmt</exec_depend>
<exec_depend>rclcpp</exec_depend>
<exec_depend>rclcpp_lifecycle</exec_depend>
<exec_depend>rsl</exec_depend>

I see a drawback for cmake_python packages to pull rclcpp*, maybe we should ship the generate_parameter_module cmake function from a dedicated package to solve that.

Will resolve rosdep (and bloom) the packages as desired?

Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,16 @@
<buildtool_depend>ament_cmake</buildtool_depend>
<buildtool_depend>ament_cmake_python</buildtool_depend>

<buildtool_export_depend>generate_parameter_library_py</buildtool_export_depend>
<build_export_depend>fmt</build_export_depend>
<build_export_depend>generate_parameter_library_py</build_export_depend>
<build_export_depend>rclcpp</build_export_depend>
<build_export_depend>rclcpp_lifecycle</build_export_depend>
<build_export_depend>rsl</build_export_depend>

<depend>fmt</depend>
<depend>rclcpp</depend>
<depend>rclpy</depend>
<depend>rclcpp_lifecycle</depend>
<depend>rsl</depend>
<depend>tcb_span</depend>
<depend>libexpected-dev</depend>

<!-- for backward compatibility -->
<!-- remove once this redirection is removed -->
<!-- https://github.com/PickNikRobotics/cpp_polyfills/pull/12 -->
<depend>tl_expected</depend>
<exec_depend>fmt</exec_depend>
<exec_depend>rclcpp</exec_depend>
<exec_depend>rclcpp_lifecycle</exec_depend>
<exec_depend>rsl</exec_depend>

<export>
<build_type>ament_cmake</build_type>
Expand Down
11 changes: 7 additions & 4 deletions generate_parameter_library_py/package.xml
View file
Open in desktop

@christophfroehlich christophfroehlich May 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is a build dependency of downstream packages, right? is exec_depend then the right dependency type?
build_export_depend sounds more suitable

@Timple Timple May 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How can these be used in the build process of another package?
As far as I am aware only cmake package are build, and they use generate_parameter_library for that. As per example_cmake_python example.

@christophfroehlich christophfroehlich May 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you are right about using it in python using generate_parameter_module, but the actual code generator even for cpp code is part of generate_parameter_library_py

generate_parameter_library/generate_parameter_library/cmake/generate_parameter_library.cmake

Line 32 in d366f2d

find_program(generate_parameter_library_cpp_BIN NAMES "generate_parameter_library_cpp")

coming from

generate_parameter_library/generate_parameter_library_py/setup.py

Line 141 in d366f2d

'generate_parameter_library_cpp = generate_parameter_library_py.generate_cpp_header:main',

But because this will "execute" generate_parameter_library_cpp at build time, maybe exec_depend still might be the correct choice here?

Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
<license>BSD-3-Clause</license>
<author email="paul.gesel@picknik.ai">Paul Gesel</author>

<depend>python3</depend>
<depend>python3-jinja2</depend>
<depend>python3-typeguard</depend>
<depend>python3-yaml</depend>
<build_export_depend>python3</build_export_depend>
<build_export_depend>python3-jinja2</build_export_depend>
<build_export_depend>python3-typeguard</build_export_depend>
<build_export_depend>python3-yaml</build_export_depend>

<exec_depend>rcl_interfaces</exec_depend>
<exec_depend>rclpy</exec_depend>

<test_depend>python3-pytest</test_depend>

Expand Down