Repository navigation
rosidl generators: stale type descriptions, installed sources, egg-info - #179
Open
baszalmstra wants to merge 5 commits into
Open
baszalmstra wants to merge 5 commits into
baszalmstra wants to merge 5 commits into
Conversation
…ng the code ament_python_install_package() copies the generated Python package into the build directory and runs `setup.py egg_info` on it, but nothing orders that copy after the code generation. The copy runs as soon as the build starts, when the directory only holds the __init__.py written at configure time, so the published egg-info lists only that file: SOURCES.txt of ros2-action-tutorials-interfaces has no action/__init__.py or action/_fibonacci.py, and importlib.metadata.files() misses every generated module. Make the copy (or symlink) target depend on the generation target. Rolling fixed this with ament/ament_cmake#587 and ros2/rosidl_python#254, which lyrical and rolling already ship; neither was backported to humble or jazzy.
Every interface package ships the C/C++ sources rosidl generates for its libraries in include/, next to the headers: - rosidl_generator_c, rosidl_typesupport_introspection_c and rosidl_typesupport_introspection_cpp install their output directory with a bare `PATTERN "*.h"` (or "*.hpp"). Without FILES_MATCHING or EXCLUDE a pattern filters nothing, so the .c/.cpp files are installed too, and generator_c's detail/<type>__type_support.c is overwritten by introspection_c's file of the same name. Exclude the sources instead, as rosidl_typesupport_fastrtps already does. EXCLUDE (rather than FILES_MATCHING) also keeps ament's symlink install working. - rosidl_generator_py (jazzy and later) writes its extension modules into the generated Python package, which is installed as a whole. With the Visual Studio generator used on Windows the modules land in a Release/ subdirectory, so every win-64 package ships them twice. A generator expression in the output directory keeps CMake from adding that subdirectory. Humble sets per-configuration output directories and doesn't have this problem. In ros2-action-tutorials-interfaces (jazzy, win-64) this removes 347 KB of sources from include/ and 74 KB of Release/*.pyd, 21% of the unpacked package; headers, type descriptions and the remaining files are unchanged. Both changes belong upstream in ros2/rosidl and ros2/rosidl_python; the patches can go once a release has them.
… incremental builds rosidl_generator_type_description reads the installed .json type descriptions of the dependency packages and copies them, with their type hashes, into this package's descriptions. Its custom command doesn't depend on those files, though: `_dependency_files` is initialized and never filled. When a message in a dependency changes and the workspace is rebuilt incrementally, the C/C++/Python code is regenerated but the type descriptions are not, so the package keeps the dependency's old description and RIHS01 hash, and the regenerated C code embeds them. Reproduced with two packages built by colcon in an environment with the published jazzy generators: top_msgs/Outer has a dep_msgs/Inner field; adding a field to Inner and running `colcon build` again leaves Outer.json with Inner's old hash (RIHS01_1cf77f... instead of RIHS01_f61610...) and fields, and outer__description.c expects the old hash. With these patches both are updated, and a no-op build regenerates nothing. - rosidl_generator_type_description: depend on the dependencies' <pkg>/<ns>/<Type>.json files. - rosidl_generator_c, rosidl_generator_cpp: depend on this package's own type description files, which the generated code embeds (until now only ordered by target). Clean builds, like the ones that produce the packages here, give the same output as before; this is for users who build their own interface packages incrementally against these generators. Humble has no type descriptions. The fix belongs upstream in ros2/rosidl, which has the same code in rolling.
New per-package build numbers (`rs rebuild`, above every published build) for the generator packages the previous commits patch, so that they are built and published again: - humble (24): rosidl_generator_c, rosidl_generator_py, rosidl_typesupport_introspection_c, rosidl_typesupport_introspection_cpp - jazzy (29), lyrical (32), rolling (34): the same plus rosidl_generator_cpp and rosidl_generator_type_description Interface packages that are already published keep their contents until they are rebuilt themselves; that is left to a separate decision.
Collaborator
|
Thanks @baszalmstra! Do we really need to wait for upstream PRs? |
Author
|
No! Happy to merge before the upstream ones are in! |
baszalmstra
marked this pull request as ready for review
October 28, 2026 06:16
Tobias-Fischer
approved these changes
Oct 28, 2026
Tobias-Fischer
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, but I'll let @traversaro take a look please too :)
Member
|
Thanks @baszalmstra , what would be main motivation for these fixes? Build speed? In particular I am a bit afraid of ros2/rosidl#1019, are we sure that there are no downstream consumers of those .c and .cpp files? I learned in the past that the idl machinery may be really confusing, for humans and agents alike. : ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes in the rosidl generators:
.c/.cppfiles are installed intoinclude/: Don't install generated C and C++ sources into include ros2/rosidl#1019site-packages/<pkg>/Release/: Don't put the extension modules in a per-config subdirectory ros2/rosidl_python#272ament/ament_cmake#587andros2/rosidl_python#254.The patched generators get new build numbers. Interface packages that are already published keep their files until they are rebuilt.
Draft until the upstream PRs are merged.