Missing exports

[Zylann]

There are no export macros on JsonBox classes (something like JB_EXPORT?), it would be useful if the library is compiled as a dynamic library, or statically linked to a dynamic library. (Currently, I have to statically compile the library with every module of my game engine, multiplying the binary size).

[madbranch]

I'll look into it.

[mwoehlke-kitware]

CMake's GenerateExportHeader module makes this pretty easy; three commands in the CMakeLists.txt (include the module, call the function, add the binary directory to include directories), then just include the export header in the relevant headers and add the ABI export decoration symbol in front of class names (and free functions, if any).

[Zylann]

Sounds great... but I thought it would be even easier if there was an empty macro in front of exported classes (maybe not all have to be exported), and this macro could be user-defined. This way, anyone can just set this macro in a header or build commands to have needed exports, without having to rely on anything but the C++ compiler.

[mwoehlke-kitware]

I thought it would be even easier if there was an empty macro in front of exported classes (maybe not all have to be exported), and this macro could be user-defined.

No, really, it won't be. If you went this way, every Windows user would need to define that symbol twice; once to build JsonBox, and again in their own projects to be able to use it. With GenerateExportHeader, JsonBox adds a few lines of CMake, and users don't need to do anything. There's a reason that GenerateExportHeader is the accepted standard way to define export symbols.

[Zylann]

Strange... I use libraries defining exports this way, doing this myself too and I never had any issues on Windows. I'm curious to learn the reason why a second definition of symbols would be needed (also, I use another build system, and compile JsonBox as static lib).

[mwoehlke-kitware]

When you build a shared library on Windows, you MUST decorate anything exported with declspec(dllexport). Then, when you use the library, you MUST decorate anything imported with declspec(dllimport). Or, if it's static, you must omit any such decorations. If you just have 'an empty symbol that the user defines', then everyone using the library must define it to declspec(dllimport).

Good libraries will provide a header that correctly defines the ABI export symbol to the appropriate declspec based on a preprocessor symbol that is defined when the library itself is being built, so that everyone includes the One True Header, and everything Just Works™. GenerateExportHeader creates this header for you. (It's possible to create such a header "by hand"; I just wouldn't bother in any project that doesn't already have one, because using GenerateExportHeader is just much, much easier and makes it much easier to update the headers when things changes, e.g. adding or renaming libraries.)

Frankly, if I must add a compile definition when compiling my code in order to use your library, then your library is poorly designed. Unless I am misunderstanding something, this would be the case with your proposed "solution".

(If you ignore the build system that is provided with JsonBox, then, frankly, you have taken it on your own head to take care of all of this yourself.)

[Zylann]

The part of my project that is used as a library has such header (maybe not as complete as what CMake would generate, I have to admit), and one of the libraries I use even has this "define your exports yourself as [some_macro]" thing. Not very convenient, but I set that macro as mine easily.

I try to unify the build system in my project as much as possible so it works out of the box. Unless I make JsonBox an "external" and introduce a dependency to CMake, I guess i'll have to take care of this myself then.

Thank you for explaining :)

[mwoehlke-kitware]

Why is this closed? It's not fixed, is it?

I might look into this... how would folks feel about bumping the minimum CMake requirement to 2.8.12?

[madbranch]

I am not good with CMake, thank your for the help. I just noticed the pull requests... My bad.

[madbranch]

I don't mind bumping the minimum CMake requirement to 2.8.12, it was released a year and a half ago.

[madbranch]

Since the default Makefile included, the default Xcode projects and the default Visual Studio 2010 are now broken because they cannot find the Export.h file, I guess I'll remove them.

[mwoehlke-kitware]

Since the default Makefile included, the default Xcode projects and the default Visual Studio 2010 are now broken because they cannot find the Export.h file, I guess I'll remove them.

Heh :smile:... I am admittedly biased, but I've long been in favor of just using CMake rather than trying to maintain multiple build systems.

That said, if you wanted to keep them, you could add platform-specific Exports.h (or possibly a generic one; you'd have to write it yourself, however, as GenerateExportHeader generates non-generic ones that are only suitable for the build platform) and stick them in the source somewhere, and (as is done by #32) install the appropriate one alongside the other headers.

If you don't object to dropping the non-CMake builds, would you like for me to do that in #32?

[mwoehlke-kitware]

Ah, never mind, I see you did this already :grin:.

[Misttgun]

First of all, thank you for your hard work.

I don't know why the issue is still open, but when I used CMake to build the project to VS2015, it generated the Export.h file which I added to my project.

Everything is working perfectly now !