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

Fixing Windows builds when using target_link_libraries() #10

Open
nickelpro opened this issue Apr 27, 2022 · 3 comments
Open

Fixing Windows builds when using target_link_libraries() #10

nickelpro opened this issue Apr 27, 2022 · 3 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@nickelpro
Copy link
Contributor

The current cmake configuration for ztd.idk is not header-only, it produces a library from the files in the source directory.

This is a problem, because the API linkage flag that api.h expects to see (ZTD_DLL) isn't set by the build system, so the library is built without exporting any symbols. This makes the library unlinkable on Windows.

This is all fixable, but I need to know the intent. The docs refer to idk as header only and it appears to work entirely in header-only mode. So should we just not compile these files? Should we create two targets, one for header only and one that compiles and links the library correctly?

Once I know what the intent is I can handle fixing up the cmake stuff.

@ThePhD
Copy link
Contributor

ThePhD commented Apr 27, 2022

It's not meant to be header-only anymore. I've got to fix that in the docs.

It should only turn on ZTD_DLL and other build-related macros if it detects the CMake property for building a shared library is on the target.

@ThePhD
Copy link
Contributor

ThePhD commented Apr 27, 2022

Pushed a fix, should work? Maybe. I should probably add tests for each platform that use BUILD_SHARED_LIBS=ON and one with it off, to see what happens...

@ThePhD ThePhD self-assigned this Apr 27, 2022
@ThePhD ThePhD added bug Something isn't working enhancement New feature or request labels Apr 27, 2022
@nickelpro
Copy link
Contributor Author

I'll check it out in a bit, but at a glance this should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants