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: function definition is marked dllimport #210

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

felixf4xu
Copy link

Fix #209

Copy link
Contributor

@JesusPoderoso JesusPoderoso left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for your contribution @felixf4xu! 🚀

JesusPoderoso

This comment was marked as resolved.

@felixf4xu
Copy link
Author

Thanks for the review.

At the same time, I have a question about the compiling switches. The issue only happens when fastcdr is built as a shared dll (on Windows), in ros2, it's BUILD_SHARED_LIBS. I'm just curious, this project has all code ready (the difinition of Cdr_DllAPI and all the other stuff) but this issue show that maybe it's never compiled on Windows as shared dll.

ROS2 actually supports Windows, maybe it's build as a static lib on Windows?

The reason why I asked is that I actually have begun to work on fastrtps as well, it has even more issues like this one, but again, fastrtsp seems to be ready for windows shared dll because it already has all the code there, just not tested, maybe nobody wants to build it on Windows as shared dll?

@felixf4xu
Copy link
Author

And I see the github actions for this fastcdr project, Windows is also listed there. I'm curious if it's configured to compile as static lib?

I'm not familiar with github actions, is it possible to set some actions to build on windows as shared dll?

@MiguelCompany
Copy link
Member

@felixf4xu It might be something related with the compiler.

As stated in the supported platforms here, our tier 1 build system on Windows is Visual Studio 2019.

On that tier 1 platform we pass the CI with BUILD_SHARED_LIBS=ON, so the DLL system is always tested.

The error you mention in #209 is usually reported by MinGW. If you are building in MinGW then bear in mind that it is not an officially supported platform.

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status

@JesusPoderoso JesusPoderoso added this to the v2.2.5 milestone Sep 10, 2024
@MiguelCompany
Copy link
Member

@Mergifyio rebase

felixf4xu and others added 3 commits October 1, 2024 13:30
Co-authored-by: Jesús Poderoso <120394830+JesusPoderoso@users.noreply.github.com>
Co-authored-by: Jesús Poderoso <120394830+JesusPoderoso@users.noreply.github.com>
Copy link

mergify bot commented Oct 1, 2024

rebase

✅ Branch has been successfully rebased

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compiling error: function definition is marked dllimport
5 participants