-
Notifications
You must be signed in to change notification settings - Fork 240
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
Workaround pybind11 bug on Windows when CMAKE_BUILD_TYPE=RelWithDebInfo #538
Conversation
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
This patch is terrible bad but:
Without a proper fix in |
I'm not really in a position to evaluate the fitness here but I appreciate the head's up. Hopefully it's something we can push upstream in short order. |
I removed the |
It would be good to fix this upstream, @ivanpauno are you able open a PR and reference it in this change so that we can revert it after an upstream release? |
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this LGTM.
Looks like this will be fixed in the next pybind11 release (2.6.0). I'll keep my eyes open for it and update our vendor package when it's out.
…nfo (#538) Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copies windows debug fixes from rosbag2_py for using Pybind11 on Windows debug and RelWithDebInfo. For more info see the original PRs ros2/rosbag2#538 ros2/rosbag2#531 Signed-off-by: Shane Loretz <sloretz@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
…nfo (#538) Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copies windows debug fixes from rosbag2_py for using Pybind11 on Windows debug and RelWithDebInfo. For more info see the original PRs ros2/rosbag2#538 ros2/rosbag2#531 Signed-off-by: Shane Loretz <sloretz@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
* Copy windows debug fixes for pybind11 Copies windows debug fixes from rosbag2_py for using Pybind11 on Windows debug and RelWithDebInfo. For more info see the original PRs ros2/rosbag2#538 ros2/rosbag2#531 Signed-off-by: Shane Loretz <sloretz@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Workaround for consistent Py_DEBUG macro Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * fix argument order Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Does PUBLIC Py_DEBUG resolve differences? Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Don't include Python.h if including pybind11.h Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Remove Py_DEBUG if target doesn't use pybind11 Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Another workaround for a pybind11 bug ...
I should start sending patches upstream (for this and #504).
When that gets included in a new pybind11 release, we can update our vendor package and get rid of these workarounds (I will open an issue to track that).
Though it failed in a packaging build, only
-DCMAKE_BUILD_TYPE=RelWithDebInfo
is needed to reproduce the issue:TODO: This still doesn't pass, I have to check more closely in a Windows VM.CI is passing now.Fixes #537.