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

MSVC: Fix generation of clsocket.lib #17

Merged
merged 2 commits into from
Aug 8, 2020

Conversation

Palakis
Copy link
Contributor

@Palakis Palakis commented Aug 8, 2020

When building the library on Windows with the Visual Studio compiler, clsocket.dll is generated correctly but clsocket.lib (the Import Library for use by programs linking with it) isn't generated at all because nothing is exported from the library.

This PR fixes this by adding an __declspec(dllexport) clause to the library's classes. This clause is abstracted behind an EXPORT conditional define, so that different platforms can use different export behaviors.

@Palakis Palakis changed the title Bugfix/windows export lib MSVC: Fix generation of clsocket.lib Aug 8, 2020
Copy link
Member

@lethosor lethosor left a comment

Choose a reason for hiding this comment

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

Thanks!

@lethosor lethosor merged commit e516220 into DFHack:master Aug 8, 2020
@Palakis Palakis deleted the bugfix/windows-export-lib branch August 10, 2020 01:12
@stgatilov
Copy link

Should exporting be optional?
For instance, if someone wants to embed the code directly or build a static lib, he might be unhappy by this change.

@lethosor
Copy link
Member

That's a good point. This might actually break DFHack because it uses a static library - I haven't checked yet. I've been too busy to work on this recently, but it could probably be addressed along with #19.

@lethosor
Copy link
Member

lethosor commented Sep 9, 2020

@hayguen's fix in 9cbb2e7 should address the above. (It turns out that this PR didn't break DFHack, at least the build, according to CI run on DFHack/dfhack#1642 - not entirely sure if it'll break at runtime, though, and this is certainly worth fixing for other clients.)

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.

3 participants