Skip to content

Remove rosidl generator py patch - #33

Open
mini-1235 wants to merge 1 commit into
RoboStack:mainfrom
mini-1235:patch-5
Open

Remove rosidl generator py patch#33
mini-1235 wants to merge 1 commit into
RoboStack:mainfrom
mini-1235:patch-5

Conversation

@mini-1235

Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: Maurice <mauricepurnawan@gmail.com>
@mini-1235

Copy link
Copy Markdown
Contributor Author

@wep21 I am running into a similar issue to RoboStack/ros-lyrical#5 while testing Nav2. I asked my AI to dig into the root cause, and this seems to be the issue (I haven't fully validated it yet, but it looks reasonable to me)

Do you remember what issues you were running into that led us to introduce this patch in the first place?

@mini-1235
mini-1235 marked this pull request as ready for review August 12, 2026 13:54
@wep21

wep21 commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

This comes from upstream PR.
cc @traversaro

@mini-1235

Copy link
Copy Markdown
Contributor Author

Thanks for pointing me to the PR. That seems like the issue, perhaps I should update it similar to what we have in the kilted repo?

@traversaro

Copy link
Copy Markdown
Member

@wep21 I am running into a similar issue to RoboStack/ros-lyrical#5 while testing Nav2. I asked my AI to dig into the root cause, and this seems to be the issue (I haven't fully validated it yet, but it looks reasonable to me)

Can you provide the exact error you are getting, and why do you think this is related to the patch in question? Feel free to use any AI tool, but please explain their reasoning to us. : )

@mini-1235

mini-1235 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@wep21 I am running into a similar issue to RoboStack/ros-lyrical#5 while testing Nav2. I asked my AI to dig into the root cause, and this seems to be the issue (I haven't fully validated it yet, but it looks reasonable to me)

Can you provide the exact error you are getting, and why do you think this is related to the patch in question? Feel free to use any AI tool, but please explain their reasoning to us. : )

@traversaro I will provide the detailed errors I saw over the weekend. Coincidentally, when I asked AI to verify whether this was actually the root cause yesterday, it came up with a repro example very similar to the one in RoboStack/ros-kilted#76 (comment).

The workaround I used yesterday was to preload the library as mentioned in RoboStack/ros-lyrical#5, and that resolved the issue for me.

@traversaro

Copy link
Copy Markdown
Member

Ah ok, RoboStack/ros-kilted#76 (comment) make more sense as an issue.

@traversaro

Copy link
Copy Markdown
Member

I think rather then removing the patch, we need to go back to https://github.com/RoboStack/ros-kilted/blob/main/patch/ros-kilted-rosidl-generator-py.osx.patch . To avoid confusion, I should probably update the upstream PR : ros2/rosidl_python#253 .

@mini-1235

Copy link
Copy Markdown
Contributor Author

I think rather then removing the patch, we need to go back to https://github.com/RoboStack/ros-kilted/blob/main/patch/ros-kilted-rosidl-generator-py.osx.patch . To avoid confusion, I should probably update the upstream PR : ros2/rosidl_python#253 .

Okay! One question I have: if I split this change into a separate PR, will rosidl_python and the relevant dependent packages be rebuilt automatically, or would they only get rebuilt as part of a full rebuild?

ps: I suspect ros-lyrical has the same issue, since it uses the same patch as well

@traversaro

Copy link
Copy Markdown
Member

I think rather then removing the patch, we need to go back to https://github.com/RoboStack/ros-kilted/blob/main/patch/ros-kilted-rosidl-generator-py.osx.patch . To avoid confusion, I should probably update the upstream PR : ros2/rosidl_python#253 .

Okay! One question I have: if I split this change into a separate PR, will rosidl_python and the relevant dependent packages be rebuilt automatically, or would they only get rebuilt as part of a full rebuild?

ps: I suspect ros-lyrical has the same issue, since it uses the same patch as well

If you do not change the build number, the package will not be rebuilt. The build number can be bumped as part of a full rebuild or also for a single package without a full rebuild.

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