Skip to content

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

Merged
clalancette merged 1 commit intoros2:masterfrom
gezp:fix_pr1781_windows_ci
Dec 9, 2021
Merged

fix failed windows ci of PR1781 by removing unnecessary RCLCPP_COMPONENTS_PUBLIC#1843
clalancette merged 1 commit intoros2:masterfrom
gezp:fix_pr1781_windows_ci

Conversation

@gezp
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

SteveMacenski commented Dec 9, 2021 •
edited
Loading

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.

@gezp
remove RCLCPP_COMPONENTS_PUBLIC in class ComponentManagerIsolated
8c3e8d0 
Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
@gezp
Copy link
Copy Markdown
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
@gezp gezp mentioned this pull request Dec 9, 2021
Closed
@aprotyas
Copy link
Copy Markdown
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

clalancette
clalancette approved these changes Dec 9, 2021
View reviewed changes
Copy link
Copy Markdown
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
@clalancette clalancette mentioned this pull request Dec 10, 2021
Merged
Bi0T1N pushed a commit to Bi0T1N/rclcpp that referenced this pull request Dec 15, 2021
@gezp
remove RCLCPP_COMPONENTS_PUBLIC in class ComponentManagerIsolated (ro…
4cc16df 
…s2#1843)

Signed-off-by: zhenpeng ge <zhenpeng.ge@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clalancette clalancette clalancette approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gezp @aprotyas @SteveMacenski @clalancette