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

Declarations for shared library in Windows. #245

Merged
merged 1 commit into from
Dec 10, 2015
Merged

Declarations for shared library in Windows. #245

merged 1 commit into from
Dec 10, 2015

Conversation

macdems
Copy link
Contributor

@macdems macdems commented Dec 9, 2015

When trying to use cppformat in a shared library I have found that it does not compile in Windows (tested with MinGW32-w64). The reason for this is a lack fo __declspec(dllimport) and __declspec(dllexport) declaration. This PR solves this issue: when compiling shared library, one needs to add definition FMT_EXPORTS and skip it when using the library.

I hope this small improvement can help to make this great library even better.

@@ -115,7 +115,7 @@ if (CPP11_FLAG AND FMT_PEDANTIC)
endif ()

set_target_properties(cppformat
PROPERTIES COMPILE_FLAGS "${FMT_EXTRA_COMPILE_FLAGS}")
PROPERTIES COMPILE_FLAGS "-DFMT_EXPORTS ${FMT_EXTRA_COMPILE_FLAGS}")
Copy link
Contributor

Choose a reason for hiding this comment

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

FMT_EXPORTS shouldn't be defined unconditionally, only when the BUILD_SHARED_LIBS CMake variable is set, because it breaks building the static library.

@vitaut
Copy link
Contributor

vitaut commented Dec 9, 2015

Thanks for the PR. I think this could be useful, but I have some comments/questions (above).

Added __declspec(dllimport) and __declspec(dllexport) declarations
when compiled in Windows.
@macdems
Copy link
Contributor Author

macdems commented Dec 10, 2015

Concerning your comments, I have made several improvements to the patch. I hope it will now allow to use cppformat as dynamic library in Windows.

vitaut added a commit that referenced this pull request Dec 10, 2015
Declarations for shared library in Windows.
@vitaut vitaut merged commit a9d2e82 into fmtlib:master Dec 10, 2015
@vitaut
Copy link
Contributor

vitaut commented Dec 10, 2015

Merged with very minor adjustments in 1cba0ae. Thanks for contributing to the library!

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.

2 participants