pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
466 stars 134 forks source link

Win32: make other linking methods possible in addition to DLL #17

Closed attilabody closed 6 years ago

pboettch commented 6 years ago

Thank you for your contribution.

Where does the define "WIN32_DO_NOT_USE_DLL_API" come from? What are you trying to achieve?

attilabody commented 6 years ago

At the current state there is no way to link the validator statically or "embedd" it into a project as DLL usage is hardcoded into the source. The patch makes other linking methods this possible on Win32 for cases it is not possible (or inconvenient) to use de validator from a DLL. In this case will be enough to define WIN32_DO_NOT_USE_DLL_API symbol in the the build system of the project.

pboettch commented 6 years ago

CMake foresees the switch between dynamic and static libraries by using the BUILD_SHARED_LIBS variable. There is already a pull-request #15 where I suggested to use this variable to change the build-behavior - maybe we could also use it for your problem.

ianhbell commented 6 years ago

I would recommend that this PR not be accepted, rather it would be better to format the API definition like so (the export/import control of symbols should be explicit!):

#ifdef _WIN32
#    ifdef JSON_SCHEMA_VALIDATOR_EXPORTS
#        define JSON_SCHEMA_VALIDATOR_API __declspec(dllexport)
#    elif JSON_SCHEMA_VALIDATOR_IMPORTS
#        define JSON_SCHEMA_VALIDATOR_API __declspec(dllimport)
#    else
#        define JSON_SCHEMA_VALIDATOR_API 
#    endif
#else
#    define JSON_SCHEMA_VALIDATOR_API
#endif
pboettch commented 6 years ago

@ianhbell I don't know windows builds very well. When does one choose to use dllexport, dllimport or nothing.

ianhbell commented 6 years ago

dllexport is to compile a shared library and to export any functions that have the __declspec(dllexport) annotation.

dllimport is for the same thing, but for imports of symbols from a shared library.

The idea is that you have a single header that can be used to build the DLL, and then also to integrate into another project that would like to use your DLL.

You should, in my opinion, require an explicit selection of __declspec if you are going to modify it. You should also not assume you know how people are going to use your library :)

pboettch commented 6 years ago

Ok. So when does a Windows user would want to do

#        define JSON_SCHEMA_VALIDATOR_API 

a per your snippet? If she builds a static lib?

ianhbell commented 6 years ago

Yes, that is correct. That should be the default annotation on windows.

pboettch commented 6 years ago

Can you transform your snippet into a PR?

ianhbell commented 6 years ago

Sure, with pleasure. I’ll do so when I get home tonight.