-
Notifications
You must be signed in to change notification settings - Fork 608
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
[Windows][ROS2] Fixed Windows build issues #277
Conversation
It seemed to me that |
@gaoethan Hope I am pinging the correct owner. Can you help review/merge the change for fixing Windows build? |
The build is yellow because of the CMake warning w/r/t Boost. I think that's okay here. |
@@ -5,6 +5,7 @@ ament_target_dependencies(${PROJECT_NAME} | |||
"sensor_msgs" | |||
) | |||
target_link_libraries(${PROJECT_NAME} ${Boost_LIBRARIES} ${OpenCV_LIBRARIES}) | |||
set_target_properties(${PROJECT_NAME} PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS ON) |
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.
Should we be doing this or using visibility macros?
I think some of the I would like to get that landed, then I'll come back and look at this one. |
This would address #289, which I just opened, but I still don't understand why the Boost include only breaks the Windows build. Also, I was able to build without adding back the OpenCV include, so maybe that is no longer necessary since the PR was opened. |
Closed it and in favor of #301 |
This change is to address 2 issues found when I am building ROS2 Dashing from source.
Boost & OpenCV headers are not found at compile time because
${Boost_INCLUDE_DIRS}
&${OpenCV_INCLUDE_DIRS}
are not in the includes path.The imported library of
cv_bridge.lib
is not generated since no functions are dllexport'd fromcv_bridge
, consequentlycv_bridge-utest
fails to build when trying to link againstcv_bridge
. UseWINDOWS_EXPORT_ALL_SYMBOLS
to make every functions fromcv_bridge
dllexport'd to fix it.