Skip to content
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

fix failed windows ci of PR1781 by removing unnecessary RCLCPP_COMPONENTS_PUBLIC #1843

Merged
merged 1 commit into from
Dec 9, 2021
Merged

fix failed windows ci of PR1781 by removing unnecessary RCLCPP_COMPONENTS_PUBLIC #1843

merged 1 commit into from
Dec 9, 2021

Conversation

gezp
Copy link
Contributor

@gezp gezp commented Dec 9, 2021

Signed-off-by: zhenpeng ge zhenpeng.ge@qq.com

PR #1781 make windows ci failed.
Acooding to suggestion from @SteveMacenski , try to fix it by instantiating template class ComponentManagerIsolated.

@aprotyas
Copy link
Member

aprotyas commented Dec 9, 2021

@gezp I would suggest updating the PR title to better (or also) reflect the proposed change.

Running CI:
Repos file: https://gist.githubusercontent.com/aprotyas/6fbc67583abc036ca479a88dee62ea8e/raw/f3c5a935a05dd7310653a0def8469246fedc1940/ros2.repos
Build args: --packages-above-and-dependencies rclcpp_components
Test args: --packages-above rclcpp_components
Job: https://ci.ros2.org/job/ci_launcher/9478/

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@SteveMacenski
Copy link
Collaborator

SteveMacenski commented Dec 9, 2021

I'm not surprised it didn't fix it, the lines

template class ComponentManagerIsolated<rclcpp::executors::SingleThreadedExecutor>;
template class ComponentManagerIsolated<rclcpp::executors::MultiThreadedExecutor>;

are only useful when the template implementations are in a separate cpp file. Since you had it all in the header definitions, these lines are not necessary.


03:33:16 component_container_isolated.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::MultiThreadedExecutor>::add_node_to_executor(unsigned __int64)" (?add_node_to_executor@?$ComponentManagerIsolated@VMultiThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\component_container_isolated.vcxproj]

03:33:16 component_container_isolated.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::SingleThreadedExecutor>::add_node_to_executor(unsigned __int64)" (?add_node_to_executor@?$ComponentManagerIsolated@VSingleThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\component_container_isolated.vcxproj]

03:33:16 component_container_isolated.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::MultiThreadedExecutor>::remove_node_from_executor(unsigned __int64)" (?remove_node_from_executor@?$ComponentManagerIsolated@VMultiThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\component_container_isolated.vcxproj]

03:33:16 component_container_isolated.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::SingleThreadedExecutor>::remove_node_from_executor(unsigned __int64)" (?remove_node_from_executor@?$ComponentManagerIsolated@VSingleThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\component_container_isolated.vcxproj]

03:33:16 C:\ci\ws\build\rclcpp_components\Release\component_container_isolated.exe : fatal error LNK1120: 4 unresolved externals [C:\ci\ws\build\rclcpp_components\component_container_isolated.vcxproj]

03:33:16 test_component_manager_api.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::MultiThreadedExecutor>::add_node_to_executor(unsigned __int64)" (?add_node_to_executor@?$ComponentManagerIsolated@VMultiThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\test_component_manager_api.vcxproj]

03:33:16 test_component_manager_api.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::SingleThreadedExecutor>::add_node_to_executor(unsigned __int64)" (?add_node_to_executor@?$ComponentManagerIsolated@VSingleThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\test_component_manager_api.vcxproj]

03:33:16 test_component_manager_api.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::MultiThreadedExecutor>::remove_node_from_executor(unsigned __int64)" (?remove_node_from_executor@?$ComponentManagerIsolated@VMultiThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\test_component_manager_api.vcxproj]

03:33:16 test_component_manager_api.obj : error LNK2001: unresolved external symbol "protected: virtual void __cdecl rclcpp_components::ComponentManagerIsolated<class rclcpp::executors::SingleThreadedExecutor>::remove_node_from_executor(unsigned __int64)" (?remove_node_from_executor@?$ComponentManagerIsolated@VSingleThreadedExecutor@executors@rclcpp@@@rclcpp_components@@MEAAX_K@Z) [C:\ci\ws\build\rclcpp_components\test_component_manager_api.vcxproj]

03:33:16 C:\ci\ws\build\rclcpp_components\Release\test_component_manager_api.exe : fatal error LNK1120: 4 unresolved externals [C:\ci\ws\build\rclcpp_components\test_component_manager_api.vcxproj]

Same bit. I honestly don't know. I don't work with Windows. It's hard to offer suggestions when I can't play around and reproduce it.

Edit: my suggestion would be to try to add those 2 lines before main() in the cpp file, that's what's not picking it up (and the test, see the second blob). If that works, the test executable would still fail, what I think you should actually do is add to your cmakelist the hpp file. It's not linking so it might be as simple as needing to explicitly add the file for compilation linking.

Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
@gezp
Copy link
Contributor Author

gezp commented Dec 9, 2021

i updated this PR (RCLCPP_COMPONENTS_PUBLIC in ComponentManagerIsolated was removed), and tested on my local win10 environment, it could be built on my local win10 successfully.

ComponentManagerIsolated is template class ( just in header file component_manager_isolated.hpp), which isn't add to library, so it needn't RCLCPP_COMPONENTS_PUBLIC.

could you help me retrigger CI? @aprotyas

@gezp gezp changed the title fix failed windows ci of PR1781 fix failed windows ci of PR1781 by removing unnecessary RCLCPP_COMPONENTS_PUBLIC Dec 9, 2021
@aprotyas
Copy link
Member

aprotyas commented Dec 9, 2021

Re-running CI:
Repos file: https://gist.githubusercontent.com/aprotyas/6fbc67583abc036ca479a88dee62ea8e/raw/f3c5a935a05dd7310653a0def8469246fedc1940/ros2.repos
Build args: --packages-above-and-dependencies rclcpp_components
Test args: --packages-above rclcpp_components
Job: https://ci.ros2.org/job/ci_launcher/9479/

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me with green CI.

@clalancette clalancette merged commit 0775e2f into ros2:master Dec 9, 2021
Bi0T1N pushed a commit to Bi0T1N/rclcpp that referenced this pull request Dec 15, 2021
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.

None yet

4 participants