Closed seanyen closed 5 years ago
Can you please compare the proposed changes with the code on the crystal-devel
branch which is used for ROS 2 and should already support Windows?
@dirk-thomas Thank for the feedback. Do you want me to test ROS on Windows melodic distro with the crystal-devel python_qt_binding? Or are you suggesting me to cherry-pick the necessary changes from ROS2? Currently https://aka.ms/ros is still building ROS1 (melodic distro) and focused on upstreaming the minimum changes for melodic.
Do you want me to test ROS on Windows melodic distro with the crystal-devel python_qt_binding?
No.
are you suggesting me to cherry-pick the necessary changes from ROS2?
Yes, at least you should check the changes applied for ROS 2 and either cherry-pick them if they apply 1-to-1 or use a custom patch like the one in this PR but with a description why it needs to be different.
Thanks for the clarification! I will be reworking on that.
@dirk-thomas I separated out cherry-picking the crystal-devel Windows port into #61. This PR will be focused on the build configuration keyword filtering fix.
Thanks for the patch and for updating it so quickly.
Thanks for the merge!
In Windows build, the generated CMake Config for package could contain the configuration keywords and the sip_configure.py doesn't expect that. So use catkin_filter_libraries_for_build_configuration to normalize the content before it is passed to SIP.
FYI, here is an example what we saw from ${${PROJECT_NAME}_LIBRARIES}
And without the fix, when it runs into the CMake config with configuration keywords, the linker could fail on the following errors:
This change is verified in https://aka.ms/ros project.