-
Notifications
You must be signed in to change notification settings - Fork 54
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
Required fixes to build libzt on Windows #263
Comments
As for |
What about the others? I'd open a PR myself but I have no permissions on your repo. |
No idea sorry. (I'm not associated with ZeroTier to be clear, I've been working on nodejs bindings so know some parts of the project well.)
I'm a bit confused by what you mean? You haven't forked the repo, how are you trying to open a PR? |
Apparently it's me who was confused here. So far I hadn't used GitHub to contribute to projects which weren't my own, so I wasn't aware that I had to fork the repo first. The whole time I was trying to set up my own branch in this repo. |
I'm maintaining a homemade Conan package for libzt and I'm constantly having to apply patches to make it build properly on Windows. (note that I'm building it purely using CMake and not using the provided powershell scripts, though I doubt it would change things)
I will lay out here the recurring needed changes:
Linking declarations
[Link to relevant code]
This should be as follows:
We need these dllimport/dllexport declarations only when building a .dll, and NOT when building a static library. Currently in my Conan package I optionally provide this preprocessor define
ZTS_STATIC
based on the conan build configuration (static/shared). I see that the root CMakeLists.txt has multiple configurations at the same time so I wasn't sure how it would handle this - maybe per (cmake) target?Side note: could we rename ADD_EXPORTS -> ZTS_ADD_EXPORTS? If you mixed this with another library which defined the generic ADD_EXPORTS you'd be stuck with either having both shared or both static (not always a problem but who knows?)
zts_errno linking
[Link to relevant code]
This should be as follows:
Otherwise you can't use this crucial variable at all when libzt is built as a DLL.
Side note: is this variable thread-local?
nlohmann/json include paths
[Link to relevant code]
The following line needs to be added:
Otherwise certain headers from ZeroTierOne will try to
#include <nlohmann/json.hpp>
and won't be able to find it so the build will fail. I'm surprised that it even builds without this on other platforms (system package maybe?).The text was updated successfully, but these errors were encountered: