Skip to content

Fix: function definition is marked dllimport#210

Merged
MiguelCompany merged 3 commits intoeProsima:masterfrom
felixf4xu:master
Nov 11, 2024
Merged

Fix: function definition is marked dllimport#210
MiguelCompany merged 3 commits intoeProsima:masterfrom
felixf4xu:master

Conversation

@felixf4xu
Copy link
Copy Markdown
Contributor

Fix #209

Copy link
Copy Markdown
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 felixf4xu requested a review from JesusPoderoso June 6, 2024 14:17
@felixf4xu
Copy link
Copy Markdown
Contributor 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
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
Contributor

Build status:

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

@richiprosima
Copy link
Copy Markdown
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
Copy Markdown
Member

@Mergifyio rebase

@mergify
Copy link
Copy Markdown

mergify Bot commented Oct 1, 2024

rebase

✅ Branch has been successfully rebased

@MiguelCompany MiguelCompany self-requested a review October 1, 2024 13:31
@rsanchez15 rsanchez15 modified the milestones: v2.2.5, v2.2.6 Oct 3, 2024
@MiguelCompany
Copy link
Copy Markdown
Member

CI:

@MiguelCompany
Copy link
Copy Markdown
Member

CI run successfully on #247

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