cginternals / libzeug

deprecated: C++ sanctuary for small but powerful and frequently required, stand alone features.
MIT License
16 stars 13 forks source link

fix decl specs for static build #68

Closed kateyy closed 10 years ago

kateyy commented 10 years ago

This uses the generate_export_header function introduced in CMake 2.8.6, simplifying the dll export/import/static build switch.

It's only tested on MSVC.

sbusch42 commented 10 years ago

Crucial points missing in this pull request:

So, any thoughts on this one?

kateyy commented 10 years ago

Yep, I could have been more verbose about the PR, sorry for that.

Currently, static build does not work with Visual Studio (OPTION_BUILD_STATIC checked in CMake). The *ZEUG_EXPORTS are defined to __declspec(dllimport) when compiling or including the libraries. This breaks the build (dllimport not allowed for static data member etc), and is not needed when linking static libraries.

The obvious solution would be adding another condition to the *zeug_api.h files. CMake generated export headers will do that for us. This way we can build on a working implementation, removing redundant code, which is, I think, less error-prone.

The api import/export headers in cmake-init also contain definitions for debugging and exception handling. We can't include that in the CMake-generated files, but it seems not to be needed in libzeug. To either we fix our own api headers and use them for all projects, or use the CMake generated headers at least where possible. Yep, I could have been more verbose about the PR, sorry for that.

Currently, static build does not work with Visual Studio (OPTION_BUILD_STATIC checked in CMake). The *ZEUG_EXPORTS are defined to __declspec(dllimport) when compiling or including the libraries. This breaks the build (dllimport not allowed for static data member etc), and is not needed when linking static libraries.

The obvious solution would be adding another condition to the *zeug_api.h files. CMake generated export headers will do that for us. This way we can build on a working implementation, removing redundant code, which is, I think, less error-prone.

The api import/export headers in cmake-init also contain definitions for debugging and exception handling. We can't include that in the CMake-generated files, but it seems not to be needed in libzeug. -fvisibility=hidden for gcc is also supported by the CMake command, but is not included in my proposed solution.

So either we fix our own api headers and use them for all projects, or use the CMake generated headers where possible.

cgcostume commented 10 years ago

api headers of cmake init were already fixed for this (it was just not applied to libzeug - sorry) - furthermore, cmake-init just got refined concerning static linking.

design decision for now is to not use generate_export_header based on the following remarks:

to conclude:

The only nice solution would be to let the config-cmake know what kind of linking is specified and provide the appropriate definition used in the include. This issue however, is known to the cmake community and afaik not solved/apporached yet.

I would suggest to close the pull request without merge.

cgcostume commented 10 years ago

ah, forgot: instead apply cmake-init way to libzeug

kateyy commented 10 years ago

yep okay, so let's wait for the fixes from cmake-init